-
Notifications
You must be signed in to change notification settings - Fork 140
[PATCH v4] Sm3 sm4 ipsec support #2385
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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(¶m, | ||
| 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(¶m); | ||
|
|
||
| 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; | ||
|
|
@@ -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) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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(¶m, | ||
| 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(¶m); | ||
|
|
||
| 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; | ||
|
|
@@ -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, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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), | ||
|
|
@@ -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), | ||
| }; | ||
|
|
||
| /* | ||
|
|
@@ -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), | ||
| }; | ||
|
|
||
| /* | ||
|
|
@@ -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(¶m, | ||
| 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(¶m); | ||
|
|
||
| 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; | ||
|
|
@@ -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)) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
|
|
@@ -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, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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, | ||
|
|
@@ -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, | ||
|
|
@@ -770,6 +839,40 @@ static const ODP_UNUSED ipsec_test_packet | |
| }, | ||
| }; | ||
|
|
||
| static const ODP_UNUSED ipsec_test_packet pkt_ipv4_esp_sm4_cbc_sm3 = { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
| } | ||
| }; | ||
|
|
||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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: |
||
| static const ODP_UNUSED ipsec_test_packet | ||
| pkt_ipv4_icmp_0_esp_aes_cbc_sha256_1 = { | ||
| .len = 186, | ||
|
|
||
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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