examples/ipsec-secgw: fix SPD no-match case
authorKonstantin Ananyev <konstantin.ananyev@intel.com>
Thu, 4 Apr 2019 12:13:26 +0000 (13:13 +0100)
committerAkhil Goyal <akhil.goyal@nxp.com>
Tue, 23 Apr 2019 12:44:01 +0000 (14:44 +0200)
acl_classify() returns zero value when no matching rule was found.
Currently ipsec-secgw treats it as a valid SPI value, though it has
to discard such packets.
Error could be easily observed by sending outbound unmatched packets,
user will see something like that in the log:
IPSEC: No cryptodev: core 7, cipher_algo 0, auth_algo 0, aead_algo 0

To fix it we need to treat packets with zero result from acl_classify()
as invalid ones. Also we can change DISCARD and BYPASS values to
simplify checks and save some extra space for valid SPI values.
To summarize the approach:
1. have special SPI values for DISCARD and BYPASS.
2. store in SPD full SPI value.
3. after acl_classify(), first check SPI value for DISCARD and BYPASS,
   then convert it in SA index.
4. add check at initilisation time that for each SPD rule there is a
   corresponding SA entry (with the same SPI).

Also marked few global variables as *static*.

Fixes: 906257e965b7 ("examples/ipsec-secgw: support IPv6")
Fixes: 2a5106af132b ("examples/ipsec-secgw: fix corner case for SPI value")
Cc: stable@dpdk.org
Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
Acked-by: Fan Zhang <roy.fan.zhang@intel.com>
Acked-by: Akhil Goyal <akhil.goyal@nxp.com>
examples/ipsec-secgw/ipsec-secgw.c
examples/ipsec-secgw/ipsec.h
examples/ipsec-secgw/sa.c
examples/ipsec-secgw/sp4.c
examples/ipsec-secgw/sp6.c

index ffbd00b..2e20339 100644 (file)
@@ -438,11 +438,11 @@ inbound_sp_sa(struct sp_ctx *sp, struct sa_ctx *sa, struct traffic_type *ip,
        for (i = 0; i < ip->num; i++) {
                m = ip->pkts[i];
                res = ip->res[i];
-               if (res & BYPASS) {
+               if (res == BYPASS) {
                        ip->pkts[j++] = m;
                        continue;
                }
-               if (res & DISCARD) {
+               if (res == DISCARD) {
                        rte_pktmbuf_free(m);
                        continue;
                }
@@ -453,9 +453,8 @@ inbound_sp_sa(struct sp_ctx *sp, struct sa_ctx *sa, struct traffic_type *ip,
                        continue;
                }
 
-               sa_idx = ip->res[i] & PROTECT_MASK;
-               if (sa_idx >= IPSEC_SA_MAX_ENTRIES ||
-                               !inbound_sa_check(sa, m, sa_idx)) {
+               sa_idx = SPI2IDX(res);
+               if (!inbound_sa_check(sa, m, sa_idx)) {
                        rte_pktmbuf_free(m);
                        continue;
                }
@@ -541,16 +540,15 @@ outbound_sp(struct sp_ctx *sp, struct traffic_type *ip,
        j = 0;
        for (i = 0; i < ip->num; i++) {
                m = ip->pkts[i];
-               sa_idx = ip->res[i] & PROTECT_MASK;
-               if (ip->res[i] & DISCARD)
+               sa_idx = SPI2IDX(ip->res[i]);
+               if (ip->res[i] == DISCARD)
                        rte_pktmbuf_free(m);
-               else if (ip->res[i] & BYPASS)
+               else if (ip->res[i] == BYPASS)
                        ip->pkts[j++] = m;
-               else if (sa_idx < IPSEC_SA_MAX_ENTRIES) {
+               else {
                        ipsec->res[ipsec->num] = sa_idx;
                        ipsec->pkts[ipsec->num++] = m;
-               } else /* invalid SA idx */
-                       rte_pktmbuf_free(m);
+               }
        }
        ip->num = j;
 }
index 99f49d6..589398f 100644 (file)
 #define SPI2IDX(spi) (spi & (IPSEC_SA_MAX_ENTRIES - 1))
 #define INVALID_SPI (0)
 
-#define DISCARD (0x80000000)
-#define BYPASS (0x40000000)
-#define PROTECT_MASK (0x3fffffff)
-#define PROTECT(sa_idx) (SPI2IDX(sa_idx) & PROTECT_MASK) /* SA idx 30 bits */
+#define DISCARD        INVALID_SPI
+#define BYPASS UINT32_MAX
 
 #define IPSEC_XFORM_MAX 2
 
@@ -289,6 +287,14 @@ sp4_spi_present(uint32_t spi, int inbound);
 int
 sp6_spi_present(uint32_t spi, int inbound);
 
+/*
+ * Search through SA entries for given SPI.
+ * Returns first entry index if found(greater or equal then zero),
+ * or -ENOENT otherwise.
+ */
+int
+sa_spi_present(uint32_t spi, int inbound);
+
 void
 sa_init(struct socket_ctx *ctx, int32_t socket_id);
 
index a7298a3..b850e98 100644 (file)
@@ -126,11 +126,11 @@ const struct supported_aead_algo aead_algos[] = {
        }
 };
 
-struct ipsec_sa sa_out[IPSEC_SA_MAX_ENTRIES];
-uint32_t nb_sa_out;
+static struct ipsec_sa sa_out[IPSEC_SA_MAX_ENTRIES];
+static uint32_t nb_sa_out;
 
-struct ipsec_sa sa_in[IPSEC_SA_MAX_ENTRIES];
-uint32_t nb_sa_in;
+static struct ipsec_sa sa_in[IPSEC_SA_MAX_ENTRIES];
+static uint32_t nb_sa_in;
 
 static const struct supported_cipher_algo *
 find_match_cipher_algo(const char *cipher_keyword)
@@ -631,7 +631,7 @@ parse_sa_tokens(char **tokens, uint32_t n_tokens,
        *ri = *ri + 1;
 }
 
-static inline void
+static void
 print_one_sa_rule(const struct ipsec_sa *sa, int inbound)
 {
        uint32_t i;
@@ -1116,6 +1116,31 @@ ipsec_satbl_init(struct sa_ctx *ctx, const struct ipsec_sa *ent,
        return rc;
 }
 
+/*
+ * Walk through all SA rules to find an SA with given SPI
+ */
+int
+sa_spi_present(uint32_t spi, int inbound)
+{
+       uint32_t i, num;
+       const struct ipsec_sa *sar;
+
+       if (inbound != 0) {
+               sar = sa_in;
+               num = nb_sa_in;
+       } else {
+               sar = sa_out;
+               num = nb_sa_out;
+       }
+
+       for (i = 0; i != num; i++) {
+               if (sar[i].spi == spi)
+                       return i;
+       }
+
+       return -ENOENT;
+}
+
 void
 sa_init(struct socket_ctx *ctx, int32_t socket_id)
 {
index d1dc64b..ca9ee7f 100644 (file)
@@ -99,6 +99,7 @@ parse_sp4_tokens(char **tokens, uint32_t n_tokens,
 
        uint32_t *ri = NULL; /* rule index */
        uint32_t ti = 0; /* token index */
+       uint32_t tv;
 
        uint32_t esp_p = 0;
        uint32_t protect_p = 0;
@@ -169,8 +170,12 @@ parse_sp4_tokens(char **tokens, uint32_t n_tokens,
                        if (status->status < 0)
                                return;
 
-                       rule_ipv4->data.userdata =
-                               PROTECT(atoi(tokens[ti]));
+                       tv = atoi(tokens[ti]);
+                       APP_CHECK(tv != DISCARD && tv != BYPASS, status,
+                               "invalid SPI: %s", tokens[ti]);
+                       if (status->status < 0)
+                               return;
+                       rule_ipv4->data.userdata = tv;
 
                        protect_p = 1;
                        continue;
@@ -472,6 +477,36 @@ acl4_init(const char *name, int32_t socketid, const struct acl4_rules *rules,
        return ctx;
 }
 
+/*
+ * check that for each rule it's SPI has a correspondent entry in SAD
+ */
+static int
+check_spi_value(int inbound)
+{
+       uint32_t i, num, spi;
+       const struct acl4_rules *acr;
+
+       if (inbound != 0) {
+               acr = acl4_rules_in;
+               num = nb_acl4_rules_in;
+       } else {
+               acr = acl4_rules_out;
+               num = nb_acl4_rules_out;
+       }
+
+       for (i = 0; i != num; i++) {
+               spi = acr[i].data.userdata;
+               if (spi != DISCARD && spi != BYPASS &&
+                               sa_spi_present(spi, inbound) < 0) {
+                       RTE_LOG(ERR, IPSEC, "SPI %u is not present in SAD\n",
+                               spi);
+                       return -ENOENT;
+               }
+       }
+
+       return 0;
+}
+
 void
 sp4_init(struct socket_ctx *ctx, int32_t socket_id)
 {
@@ -488,6 +523,14 @@ sp4_init(struct socket_ctx *ctx, int32_t socket_id)
                rte_exit(EXIT_FAILURE, "Outbound SP DB for socket %u already "
                                "initialized\n", socket_id);
 
+       if (check_spi_value(1) < 0)
+               rte_exit(EXIT_FAILURE,
+                       "Inbound IPv4 SP DB has unmatched in SAD SPIs\n");
+
+       if (check_spi_value(0) < 0)
+               rte_exit(EXIT_FAILURE,
+                       "Outbound IPv4 SP DB has unmatched in SAD SPIs\n");
+
        if (nb_acl4_rules_in > 0) {
                name = "sp_ip4_in";
                ctx->sp_ip4_in = (struct sp_ctx *)acl4_init(name,
@@ -523,7 +566,7 @@ sp4_spi_present(uint32_t spi, int inbound)
        }
 
        for (i = 0; i != num; i++) {
-               if (acr[i].data.userdata == PROTECT(spi))
+               if (acr[i].data.userdata == spi)
                        return i;
        }
 
index e67d85a..76be3d3 100644 (file)
@@ -130,6 +130,7 @@ parse_sp6_tokens(char **tokens, uint32_t n_tokens,
 
        uint32_t *ri = NULL; /* rule index */
        uint32_t ti = 0; /* token index */
+       uint32_t tv;
 
        uint32_t esp_p = 0;
        uint32_t protect_p = 0;
@@ -202,8 +203,12 @@ parse_sp6_tokens(char **tokens, uint32_t n_tokens,
                        if (status->status < 0)
                                return;
 
-                       rule_ipv6->data.userdata =
-                               PROTECT(atoi(tokens[ti]));
+                       tv = atoi(tokens[ti]);
+                       APP_CHECK(tv != DISCARD && tv != BYPASS, status,
+                               "invalid SPI: %s", tokens[ti]);
+                       if (status->status < 0)
+                               return;
+                       rule_ipv6->data.userdata = tv;
 
                        protect_p = 1;
                        continue;
@@ -586,6 +591,36 @@ acl6_init(const char *name, int32_t socketid, const struct acl6_rules *rules,
        return ctx;
 }
 
+/*
+ * check that for each rule it's SPI has a correspondent entry in SAD
+ */
+static int
+check_spi_value(int inbound)
+{
+       uint32_t i, num, spi;
+       const struct acl6_rules *acr;
+
+       if (inbound != 0) {
+               acr = acl6_rules_in;
+               num = nb_acl6_rules_in;
+       } else {
+               acr = acl6_rules_out;
+               num = nb_acl6_rules_out;
+       }
+
+       for (i = 0; i != num; i++) {
+               spi = acr[i].data.userdata;
+               if (spi != DISCARD && spi != BYPASS &&
+                               sa_spi_present(spi, inbound) < 0) {
+                       RTE_LOG(ERR, IPSEC, "SPI %u is not present in SAD\n",
+                               spi);
+                       return -ENOENT;
+               }
+       }
+
+       return 0;
+}
+
 void
 sp6_init(struct socket_ctx *ctx, int32_t socket_id)
 {
@@ -602,6 +637,14 @@ sp6_init(struct socket_ctx *ctx, int32_t socket_id)
                rte_exit(EXIT_FAILURE, "Outbound IPv6 SP DB for socket %u "
                                "already initialized\n", socket_id);
 
+       if (check_spi_value(1) < 0)
+               rte_exit(EXIT_FAILURE,
+                       "Inbound IPv6 SP DB has unmatched in SAD SPIs\n");
+
+       if (check_spi_value(0) < 0)
+               rte_exit(EXIT_FAILURE,
+                       "Outbound IPv6 SP DB has unmatched in SAD SPIs\n");
+
        if (nb_acl6_rules_in > 0) {
                name = "sp_ip6_in";
                ctx->sp_ip6_in = (struct sp_ctx *)acl6_init(name,
@@ -637,7 +680,7 @@ sp6_spi_present(uint32_t spi, int inbound)
        }
 
        for (i = 0; i != num; i++) {
-               if (acr[i].data.userdata == PROTECT(spi))
+               if (acr[i].data.userdata == spi)
                        return i;
        }