Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions helper/ipsec.c
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ uint32_t odph_ipsec_auth_icv_len_default(odp_auth_alg_t auth_alg)
case ODP_AUTH_ALG_CHACHA20_POLY1305:
icv_len = 16;
break;
case ODP_AUTH_ALG_SM3_HMAC:
icv_len = 16;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where does this come from? This appears to be half the output length of HMAC-SM3. Without knowing better, I think other plausible lenghts would be 12 and 32 bytes. Do you have a normative reference for how SM4-CBC and HMAC-SM3 are to be used with IPsec or other relevant documentation you could point to?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default ICV length of 16 bytes was chosen by analogy with ODP's SHA-256 IPsec default; SM3 default ICV is not yet specified in ODP or in the referenced SM3 docs.
reference doc:
https://www.chinesestandard.net/PDF.aspx/GMT0022-2023
https://www.ietf.org/archive/id/draft-guo-ipsecme-ikev2-using-shangmi-02.html

break;
default:
ODPH_DBG("Unsupported authentication algorithm\n");
icv_len = 0;
Expand Down Expand Up @@ -100,6 +103,10 @@ int odph_ipsec_alg_check(const odp_ipsec_capability_t *capa,
if (!capa->ciphers.bit.chacha20_poly1305)
return -1;
break;
case ODP_CIPHER_ALG_SM4_CBC:
if (!capa->ciphers.bit.sm4_cbc)
return -1;
break;
default:
ODPH_DBG("Unsupported cipher algorithm\n");
return -1;
Expand Down Expand Up @@ -155,6 +162,10 @@ int odph_ipsec_alg_check(const odp_ipsec_capability_t *capa,
if (!capa->auths.bit.chacha20_poly1305)
return -1;
break;
case ODP_AUTH_ALG_SM3_HMAC:
if (!capa->auths.bit.sm3_hmac)
return -1;
break;
default:
ODPH_DBG("Unsupported authentication algorithm\n");
return -1;
Expand Down
11 changes: 11 additions & 0 deletions test/validation/api/ipsec/ipsec.c
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,17 @@ int ipsec_check_esp_aes_cbc_128_null(void)
ODP_AUTH_ALG_NULL, 0);
}

int ipsec_check_esp_sm4_cbc_sm3(void)
{
return ipsec_check_esp(ODP_CIPHER_ALG_SM4_CBC, 128,
ODP_AUTH_ALG_SM3_HMAC, 256);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here key length for HMAC-SM3 is 32 bytes but the key used in the test cases is 20 bytes long, so this is internally inconsistent. Which key length is correct? Do you have any source reference?

}

int ipsec_check_ah_sm3(void)
{
return ipsec_check_ah(ODP_AUTH_ALG_SM3_HMAC, 160);
}

int ipsec_check_esp_aes_cbc_128_sha1(void)
{
return ipsec_check_esp(ODP_CIPHER_ALG_AES_CBC, 128,
Expand Down
2 changes: 2 additions & 0 deletions test/validation/api/ipsec/ipsec.h
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,10 @@ int ipsec_check(odp_bool_t ah,
#define ipsec_check_esp(cipher, cipher_bits, auth, auth_bits) \
ipsec_check(false, cipher, cipher_bits, auth, auth_bits)
int ipsec_check_ah_sha256(void);
int ipsec_check_ah_sm3(void);
int ipsec_check_esp_null_sha256(void);
int ipsec_check_esp_aes_cbc_128_null(void);
int ipsec_check_esp_sm4_cbc_sm3(void);
int ipsec_check_esp_aes_cbc_128_sha1(void);
int ipsec_check_esp_aes_cbc_128_sha256(void);
int ipsec_check_esp_aes_cbc_128_sha384(void);
Expand Down
68 changes: 68 additions & 0 deletions test/validation/api/ipsec/ipsec_test_in.c
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,38 @@ static void test_in_ipv4_ah_sha256(void)
ipsec_sa_destroy(sa);
}

static void test_in_ipv4_ah_sm3(void)
{
odp_ipsec_sa_param_t param;
odp_ipsec_sa_t sa;

ipsec_sa_param_fill(&param,
ODP_IPSEC_DIR_INBOUND, ODP_IPSEC_AH, 123, NULL,
ODP_CIPHER_ALG_NULL, NULL,
ODP_AUTH_ALG_SM3_HMAC, &key_sm3,
NULL, NULL);

sa = odp_ipsec_sa_create(&param);

CU_ASSERT_FATAL(ODP_IPSEC_SA_INVALID != sa);

ipsec_test_part test = {
.pkt_in = &pkt_ipv4_icmp_0_ah_sm3_1,
.num_pkt = 1,
.out = {
{ .status.warn.all = 0,
.status.error.all = 0,
.l3_type = ODP_PROTO_L3_TYPE_IPV4,
.l4_type = ODP_PROTO_L4_TYPE_ICMPV4,
.pkt_res = &pkt_ipv4_icmp_0 },
},
};

ipsec_check_in_one(&test, sa);

ipsec_sa_destroy(sa);
}

static void test_in_ipv4_ah_sha256_tun_ipv4(void)
{
odp_ipsec_tunnel_param_t tunnel;
Expand Down Expand Up @@ -278,6 +310,38 @@ static void test_in_ipv4_esp_aes_cbc_sha1(void)
ipsec_sa_destroy(sa);
}

static void test_in_ipv4_esp_sm4_cbc_sm3(void)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is placed between different AES-CBC + SHA variants. It would be better to have this in a more logical place (maybe just before test_ipsec_print). And the same placement comment applies to other functions, test vectors, etc added by these comments. Please try not to place them randomly.

{
odp_ipsec_sa_param_t param;
odp_ipsec_sa_t sa;

ipsec_sa_param_fill(&param,
ODP_IPSEC_DIR_INBOUND, ODP_IPSEC_ESP, 1, NULL,
ODP_CIPHER_ALG_SM4_CBC, &key_sm4,
ODP_AUTH_ALG_SM3_HMAC, &key_sm3,
NULL, NULL);

sa = odp_ipsec_sa_create(&param);

CU_ASSERT_FATAL(ODP_IPSEC_SA_INVALID != sa);

ipsec_test_part test = {
.pkt_in = &pkt_ipv4_esp_sm4_cbc_sm3,
.num_pkt = 1,
.out = {
{ .status.warn.all = 0,
.status.error.all = 0,
.l3_type = ODP_PROTO_L3_TYPE_IPV4,
.l4_type = ODP_PROTO_L4_TYPE_UDP,
.pkt_res = &pkt_ipv4_udp_0 },
},
};

ipsec_check_in_one(&test, sa);

ipsec_sa_destroy(sa);
}

static void test_in_ipv4_esp_aes_cbc_sha256(void)
{
odp_ipsec_sa_param_t param;
Expand Down Expand Up @@ -2364,6 +2428,10 @@ odp_testinfo_t ipsec_in_suite[] = {
ipsec_check_esp_aes_gcm_128_reass_ipv6),
ODP_TEST_INFO_CONDITIONAL(test_in_ipv4_null_aes_xcbc_esp,
ipsec_check_esp_null_aes_xcbc),
ODP_TEST_INFO_CONDITIONAL(test_in_ipv4_esp_sm4_cbc_sm3,
ipsec_check_esp_sm4_cbc_sm3),
ODP_TEST_INFO_CONDITIONAL(test_in_ipv4_ah_sm3,
ipsec_check_ah_sm3),
ODP_TEST_INFO(test_ipsec_proto_err),
ODP_TEST_INFO(test_ipsec_auth_err),
ODP_TEST_INFO_NULL,
Expand Down
54 changes: 50 additions & 4 deletions test/validation/api/ipsec/ipsec_test_out.c
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,13 @@ struct auth_param {
#define ALG(alg, key, key_extra) { #alg, alg, key, key_extra }

/*
* Ciphers that can be used in ESP and combined with any integrity
* algorithm. This excludes combined mode algorithms such as AES-GCM.
* Ciphers that can be used in ESP. The test driver iterates over all
* cipher/auth integrity algorithm combinations and skips those that are
* not supported by the implementation. This means that, in practice,
* some implementations may only support specific pairings (for example,
* SM4-CBC with SM3-HMAC), but the test suite itself does not require
* or assume any fixed cipher/auth pairing. This excludes combined mode
* algorithms such as AES-GCM.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is overly verbose (at least to my taste) and perhaps not well placed here. One problem is that it makes it ambiguous what the last sentence ("This excludes...") refers to. If you really want to edit the comment, I think it would be sufficient to say that some combinations are not supported by all implementations (but there is a comment about that later too, so even that is not really necessary, IMHO).

*/
static struct cipher_param ciphers[] = {
ALG(ODP_CIPHER_ALG_NULL, NULL, NULL),
Expand All @@ -58,7 +63,8 @@ static struct cipher_param ciphers[] = {
ALG(ODP_CIPHER_ALG_AES_CBC, &key_a5_256, NULL),
ALG(ODP_CIPHER_ALG_AES_CTR, &key_a5_128, &key_mcgrew_gcm_salt_3),
ALG(ODP_CIPHER_ALG_AES_CTR, &key_a5_192, &key_mcgrew_gcm_salt_3),
ALG(ODP_CIPHER_ALG_AES_CTR, &key_a5_256, &key_mcgrew_gcm_salt_3)
ALG(ODP_CIPHER_ALG_AES_CTR, &key_a5_256, &key_mcgrew_gcm_salt_3),
ALG(ODP_CIPHER_ALG_SM4_CBC, &key_sm4, NULL),
};

/*
Expand All @@ -73,7 +79,8 @@ static struct auth_param auths[] = {
ALG(ODP_AUTH_ALG_SHA384_HMAC, &key_5a_384, NULL),
ALG(ODP_AUTH_ALG_SHA512_HMAC, &key_5a_512, NULL),
ALG(ODP_AUTH_ALG_AES_CMAC, &key_5a_128, NULL),
ALG(ODP_AUTH_ALG_AES_XCBC_MAC, &key_5a_128, NULL)
ALG(ODP_AUTH_ALG_AES_XCBC_MAC, &key_5a_128, NULL),
ALG(ODP_AUTH_ALG_SM3_HMAC, &key_sm3, NULL),
};

/*
Expand Down Expand Up @@ -186,6 +193,38 @@ static void test_out_ipv4_ah_sha256(void)
ipsec_sa_destroy(sa);
}

static void test_out_ipv4_ah_sm3(void)
{
odp_ipsec_sa_param_t param;
odp_ipsec_sa_t sa;

ipsec_sa_param_fill(&param,
ODP_IPSEC_DIR_OUTBOUND, ODP_IPSEC_AH, 123, NULL,
ODP_CIPHER_ALG_NULL, NULL,
ODP_AUTH_ALG_SM3_HMAC, &key_sm3,
NULL, NULL);

sa = odp_ipsec_sa_create(&param);

CU_ASSERT_FATAL(ODP_IPSEC_SA_INVALID != sa);

ipsec_test_part test = {
.pkt_in = &pkt_ipv4_icmp_0,
.num_pkt = 1,
.out = {
{ .status.warn.all = 0,
.status.error.all = 0,
.pkt_res = &pkt_ipv4_icmp_0_ah_sm3_1,
.seq_num = 1,
},
},
};

ipsec_check_out_one(&test, sa);

ipsec_sa_destroy(sa);
}

static void test_out_ipv4_ah_sha256_tun_ipv4(void)
{
odp_ipsec_tunnel_param_t tunnel;
Expand Down Expand Up @@ -581,6 +620,11 @@ static int sa_creation_failure_ok(const odp_ipsec_sa_param_t *param)
odp_cipher_alg_t cipher = param->crypto.cipher_alg;
odp_auth_alg_t auth = param->crypto.auth_alg;

/* If SM4 and SM3 are not properly paired, SA creation failure is acceptable. */
if ((cipher == ODP_CIPHER_ALG_SM4_CBC && auth != ODP_AUTH_ALG_SM3_HMAC) ||
(auth == ODP_AUTH_ALG_SM3_HMAC && cipher != ODP_CIPHER_ALG_SM4_CBC))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When an algorithm is used by itself wihtout combining it with any other algorithm (i.e. the other algorithm is the null algorithm), then SA creation should not fail, but this code would allow it for the new algorithms. I think think test should be moved after all the "must not fail" checks in this function.

return 1;

/* Single algorithm must not fail */
if (cipher == ODP_CIPHER_ALG_NULL || auth == ODP_AUTH_ALG_NULL)
return 0;
Expand Down Expand Up @@ -2143,6 +2187,8 @@ odp_testinfo_t ipsec_out_suite[] = {
ipsec_check_ah_sha256),
ODP_TEST_INFO_CONDITIONAL(test_out_ipv4_ah_sha256_tun_ipv6,
ipsec_check_ah_sha256),
ODP_TEST_INFO_CONDITIONAL(test_out_ipv4_ah_sm3,
ipsec_check_ah_sm3),
ODP_TEST_INFO_CONDITIONAL(test_out_ipv4_esp_null_sha256,
ipsec_check_esp_null_sha256),
ODP_TEST_INFO_CONDITIONAL(test_out_ipv4_esp_null_sha256_tun_ipv4,
Expand Down
103 changes: 103 additions & 0 deletions test/validation/api/ipsec/test_vectors.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,35 @@ KEY(key_des_192, 0xa4, 0xa4, 0xa4, 0xa4, 0xa4, 0xa4, 0xa4, 0xa4,
0xa4, 0xa4, 0xa4, 0xa4, 0xa4, 0xa4, 0xa4, 0xa4,
0xa4, 0xa4, 0xa4, 0xa4, 0xa4, 0xa4, 0xa4, 0xa4);

KEY(key_sm4, 0x0a, 0x00, 0x01, 0x00, 0x02, 0x04, 0x08, 0x09,
0x00, 0x0b, 0x00, 0x00, 0x0c, 0x00, 0x00, 0x0e);
KEY(key_sm3, 0x0d, 0x0a, 0x03, 0x00, 0x05, 0x00, 0x00, 0x02,
0x00, 0x00, 0x00, 0x0f, 0x0e, 0x0c, 0x00, 0x04,
0x00, 0x00, 0x0b, 0x02);

static const ODP_UNUSED ipsec_test_packet pkt_ipv4_udp_0 = {
.len = 64,
.l2_offset = 0,
.l3_offset = 14,
.l4_offset = 34,
.data = {
/* ETH */
0x02, 0x00, 0x00, 0x00, 0x00, 0x00,
0x02, 0x00, 0x00, 0x00, 0x01, 0x00, 0x08, 0x00,

/* IP */
0x45, 0x00, 0x00, 0x32, 0x00, 0x01, 0x00, 0x00,
0x01, 0x11, 0x31, 0x95, 0xc1, 0x12, 0x00, 0x01,
0xc6, 0x12, 0x01, 0x00,

/* UDP */
0x00, 0x01, 0x00, 0x09, 0x00, 0x1e, 0xae, 0x03,
0x0a, 0x53, 0x23, 0x30, 0x23, 0x49, 0x50, 0x76,
0x34, 0x55, 0x44, 0x50, 0x23, 0x23, 0x23, 0x23,
0x23, 0x23, 0x23, 0x23, 0x23, 0x0a,
},
};

static const ODP_UNUSED ipsec_test_packet pkt_ipv4_icmp_0 = {
.len = 142,
.l2_offset = 0,
Expand Down Expand Up @@ -407,6 +436,46 @@ static const ODP_UNUSED ipsec_test_packet pkt_ipv4_icmp_0_ah_sha256_1235 = {
},
};

static const ODP_UNUSED ipsec_test_packet pkt_ipv4_icmp_0_ah_sm3_1 = {
.len = 170,
.l2_offset = 0,
.l3_offset = 14,
.l4_offset = 34,
.data = {
/* ETH */
0xf1, 0xf1, 0xf1, 0xf1, 0xf1, 0xf1,
0xf2, 0xf2, 0xf2, 0xf2, 0xf2, 0xf2, 0x08, 0x00,

/* IP */
0x45, 0x00, 0x00, 0x9c, 0x00, 0x00, 0x00, 0x00,
0x40, 0x33, 0xab, 0xd9, 0xc0, 0xa8, 0x6f, 0x02,
0xc0, 0xa8, 0xde, 0x02,

/* AH */
0x01, 0x05, 0x00, 0x00, 0x00, 0x00, 0x00, 0x7b,
0x00, 0x00, 0x00, 0x01,
/* ICV */
0xd0, 0x9d, 0x62, 0xcf, 0x02, 0x6c, 0x48, 0xe6,
0x8b, 0x11, 0x6e, 0x5e, 0xdf, 0xe2, 0x67, 0xa6,

/* ICMP */
0x08, 0x00, 0xfb, 0x37, 0x12, 0x34, 0x00, 0x00,
0xba, 0xbe, 0x01, 0x23, 0x45, 0x67, 0xca, 0xfe,
0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f,
0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17,
0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f,
0x20, 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27,
0x28, 0x29, 0x2a, 0x2b, 0x2c, 0x2d, 0x2e, 0x2f,
0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37,
0x38, 0x39, 0x3a, 0x3b, 0x3c, 0x3d, 0x3e, 0x3f,
0x40, 0x41, 0x42, 0x43, 0x44, 0x45, 0x46, 0x47,
0x48, 0x49, 0x4a, 0x4b, 0x4c, 0x4d, 0x4e, 0x4f,
0x50, 0x51, 0x52, 0x53, 0x54, 0x55, 0x56, 0x57,
0x58, 0x59, 0x5a, 0x5b,
},
};

static const ODP_UNUSED ipsec_test_packet pkt_ipv4_icmp_0_esp_null_sha256_1 = {
.len = 170,
.l2_offset = 0,
Expand Down Expand Up @@ -770,6 +839,40 @@ static const ODP_UNUSED ipsec_test_packet
},
};

static const ODP_UNUSED ipsec_test_packet pkt_ipv4_esp_sm4_cbc_sm3 = {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where does this test vector come from? Have you generated it yourself or does it come from e.g. some standard? And the same question to the other test vectors in the PR.

.len = 106,
.l2_offset = 0,
.l3_offset = 14,
.l4_offset = 34,
.data = {
/* ETH */
0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00,
0x00, 0x00, 0x01, 0x00, 0x08, 0x00,

/* IP */
0x45, 0x00, 0x00, 0x5c, 0x00, 0x01, 0x00, 0x00,
0x01, 0x32, 0x31, 0x4a, 0xc1, 0x12, 0x00, 0x01,
0xc6, 0x12, 0x01, 0x00,

/* ESP */
0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01,

/* IV */
0x84, 0x38, 0x8e, 0xd8, 0xe5, 0x3c, 0x25, 0x11,
0x51, 0x58, 0x6d, 0xcb, 0xcb, 0x9c, 0xb9, 0x1a,

/* data */
0xb9, 0xfc, 0x6b, 0xa3, 0x62, 0x42, 0xcc, 0xb0,
0xd8, 0x72, 0x4f, 0xb0, 0x6c, 0x4f, 0x74, 0x1f,
0x50, 0x1f, 0xe0, 0x06, 0xb1, 0xc0, 0xc3, 0x67,
0x87, 0xbd, 0xa1, 0x92, 0x74, 0x16, 0x61, 0xfe,

/* ICV */
0x03, 0x49, 0x12, 0x00, 0x1f, 0x9c, 0xb7, 0xb6,
0x29, 0xb0, 0x30, 0x7b, 0x02, 0x7c, 0x8c, 0x4f
}
};

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The commit messages of all the commits could be improved to match ODP style better (imperative mood, subject line prefix). Here are some suggestions that you could take inspiration from:

helper: ipsec: add SM4-CBC and HMAC-SM3 support

Recognize SM4-CBC and HMAC-SM3 as known algorithms with IPsec.
Add the algorithms in the IPsec algorithm capability check helper
functions and return the default ICV length for SM3-HMAC too.
validation: ipsec: add SM4-CBC and HMAC-SM3 test cases

Add test cases for SM4-CBC and HMAC-SM3.
validation: ipsec: add AH test cases for HMAC-SM3

Add test cases for inbound and outbound AH transport mode
using the HMAC-SM3 algorithm.

static const ODP_UNUSED ipsec_test_packet
pkt_ipv4_icmp_0_esp_aes_cbc_sha256_1 = {
.len = 186,
Expand Down
Loading