-
Notifications
You must be signed in to change notification settings - Fork 155
Add EVP_CIPHER API for XAES-256-GCM with Key Commitment #2826
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
Changes from 1 commit
0a3768b
35defa9
28e379f
ff9b2ec
a7ccfc5
01a9681
d80c7d2
4e92f5d
e882576
c285850
417325b
cdfdb87
bfc193c
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 |
|---|---|---|
|
|
@@ -532,6 +532,34 @@ Ciphertext = 986ec1832593df5443a17943 | |
| AAD = 633273702e6f72672f584145532d3235362d47434d | ||
| Tag = 7fd083bf3fdb41abd740a21f71eb769d | ||
|
|
||
| # Note: KC are our own test values | ||
|
Contributor
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. Is it possible to use the same set of tests as the ones without KC and the test harness just ignores it when it's plain XAES?
Author
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. Does that mean I should remove all changes that were made to
Contributor
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. No, sorry, what I meant is if you were to unify the 2 sets of test vectors in this file (i.e. delete the ones above of XAES-256-GCM because they're superseded by the KC one), would they then work with both XAES and XAES-KC? It seems to me that XAES may not be tested in
Author
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. Yes. It needs to have "Cipher = XAES-256-GCM-KC" to specify the cipher. |
||
| Cipher = XAES-256-GCM-KC | ||
| Key = 0101010101010101010101010101010101010101010101010101010101010101 | ||
| IV = 424242424242424242424242424242424242424242424242 | ||
| Plaintext = 48656c6c6f2c20584145532d3235362d47434d21 | ||
| Ciphertext = 01e5f78bc99de880bd2eeff2870d361f0eab5b2f | ||
| AAD = | ||
| Tag = c55268f34b14045878fe3668db980319 | ||
| KC = bf37571be1b43aeca64a95d99a2f162e24f8bfd79bbb71fa7d943e6fc060a8ae | ||
|
|
||
| Cipher = XAES-256-GCM-KC | ||
| Key = 0101010101010101010101010101010101010101010101010101010101010101 | ||
| IV = 4142434445464748494a4b4c4d4e4f505152535455565758 | ||
| Plaintext = 584145532d3235362d47434d | ||
| Ciphertext = ce546ef63c9cc60765923609 | ||
| AAD = | ||
| Tag = b33a9a1974e96e52daf2fcf7075e2271 | ||
| KC = 04076b6085eebab138855fe57811c04112eff989d44120dfff662d5475a383c3 | ||
|
|
||
| Cipher = XAES-256-GCM-KC | ||
| Key = 0303030303030303030303030303030303030303030303030303030303030303 | ||
| IV = 4142434445464748494a4b4c4d4e4f505152535455565758 | ||
| Plaintext = 584145532d3235362d47434d | ||
| Ciphertext = 986ec1832593df5443a17943 | ||
| AAD = 633273702e6f72672f584145532d3235362d47434d | ||
| Tag = 7fd083bf3fdb41abd740a21f71eb769d | ||
| KC = 5553cd21d1592b422e3129632a3187eee8a658cdca5c5b32ce86308dcc18e9d1 | ||
|
|
||
| # local add-ons, primarily streaming ghash tests | ||
| # 128 bytes aad | ||
| Cipher = AES-128-GCM | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -353,6 +353,15 @@ typedef struct { | |||
| uint8_t k1[AES_BLOCK_SIZE]; | ||||
| } XAES_256_GCM_CTX; | ||||
|
|
||||
| #define XAES_256_GCM_KEY_COMMIT_SIZE (AES_BLOCK_SIZE * 2) | ||||
|
|
||||
| typedef struct { | ||||
| EVP_AES_GCM_CTX aes_gcm_ctx; | ||||
| AES_KEY xaes_key; | ||||
| uint8_t k1[AES_BLOCK_SIZE]; | ||||
| uint8_t kc[XAES_256_GCM_KEY_COMMIT_SIZE]; | ||||
| } XAES_256_GCM_KC_CTX; | ||||
|
|
||||
| static EVP_AES_GCM_CTX *aes_gcm_from_cipher_ctx(EVP_CIPHER_CTX *ctx) { | ||||
| OPENSSL_STATIC_ASSERT( | ||||
| alignof(EVP_AES_GCM_CTX) <= 16, | ||||
|
|
@@ -388,6 +397,17 @@ static EVP_AES_GCM_CTX *aes_gcm_from_cipher_ctx(EVP_CIPHER_CTX *ctx) { | |||
| assert((uintptr_t)ptr % 8 == 0); | ||||
| ptr += (uintptr_t)ptr & 8; | ||||
| return &((XAES_256_GCM_CTX *)ptr)->aes_gcm_ctx; | ||||
| // XAES-256-GCM-KC | ||||
| case NID_xaes_256_gcm_kc: | ||||
| assert(ctx->cipher->ctx_size == sizeof(XAES_256_GCM_KC_CTX)); | ||||
| ptr = ctx->cipher_data; | ||||
| #if defined(OPENSSL_32_BIT) | ||||
| assert((uintptr_t)ptr % 4 == 0); | ||||
| ptr += (uintptr_t)ptr & 4; | ||||
| #endif | ||||
| assert((uintptr_t)ptr % 8 == 0); | ||||
| ptr += (uintptr_t)ptr & 8; | ||||
| return &((XAES_256_GCM_KC_CTX *)ptr)->aes_gcm_ctx; | ||||
| default: | ||||
| break; | ||||
| } | ||||
|
|
@@ -1781,7 +1801,6 @@ Extension to support nonce size less than 24 bytes: | |||
| https://eprint.iacr.org/2025/758.pdf#page=24 | ||||
| -----------------------------------------------------------------------*/ | ||||
| #define XAES_256_GCM_KEY_LENGTH (AES_BLOCK_SIZE * 2) | ||||
| #define XAES_256_GCM_KEY_COMMIT_SIZE (AES_BLOCK_SIZE * 2) | ||||
| #define XAES_256_GCM_MAX_NONCE_SIZE (AES_GCM_NONCE_LENGTH * 2) | ||||
| #define XAES_256_GCM_MIN_NONCE_SIZE (20) | ||||
| /* | ||||
|
|
@@ -2033,3 +2052,125 @@ DEFINE_METHOD_FUNCTION(EVP_AEAD, EVP_aead_xaes_256_gcm) { | |||
| out->seal_scatter = aead_xaes_256_gcm_seal_scatter; | ||||
| out->open_gather = aead_xaes_256_gcm_open_gather; | ||||
| } | ||||
|
|
||||
| // ------------------------------------------------------------------------------ | ||||
| // ---------------- EVP_CIPHER XAES-256-GCM With Key Commitment ----------------- | ||||
| // ----------- Reference: https://eprint.iacr.org/2025/758.pdf#page=6 ----------- | ||||
| // ------------------------------------------------------------------------------ | ||||
| static XAES_256_GCM_KC_CTX *xaes_256_gcm_kc_from_cipher_ctx(EVP_CIPHER_CTX *ctx) { | ||||
| // alignment to be consistent with aes_gcm_from_cipher_ctx() | ||||
| char *ptr = ctx->cipher_data; | ||||
| #if defined(OPENSSL_32_BIT) | ||||
| assert((uintptr_t)ptr % 4 == 0); | ||||
| ptr += (uintptr_t)ptr & 4; | ||||
| #endif | ||||
| assert((uintptr_t)ptr % 8 == 0); | ||||
| ptr += (uintptr_t)ptr & 8; | ||||
|
Comment on lines
+2063
to
+2068
Contributor
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. I doubt we need these alignment requirements in addition to those of the gcm ctx. Can we try removing them here and on line 1858? Otherwise, do you know what would drive our need for them?
Author
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. It will lead to many CI tests failed. I tried that. You can see the failed tests below. However, when I created and run with a docker, all tests still passed. |
||||
| return (XAES_256_GCM_KC_CTX *)ptr; | ||||
| } | ||||
|
|
||||
| static int xaes_256_gcm_CMAC_extract_key_commitment(AES_KEY *xaes_key, uint8_t *k1, | ||||
| uint8_t *key_commitment, const uint8_t* nonce, const unsigned nonce_len) { | ||||
| uint8_t W[AES_BLOCK_SIZE]; | ||||
|
|
||||
| uint8_t kc_prefix[4] = {0x58, 0x43, 0x4D, 0x54}; | ||||
| uint8_t X1[AES_BLOCK_SIZE]; | ||||
| OPENSSL_memcpy(W, kc_prefix, 4); | ||||
| OPENSSL_memcpy(W + 4, nonce, AES_GCM_NONCE_LENGTH); | ||||
|
|
||||
| AES_encrypt(W, X1, xaes_key); | ||||
| // For nonce size < 24 | ||||
| // Reference: https://eprint.iacr.org/2025/758.pdf#page=24 | ||||
| OPENSSL_memcpy(W, nonce + nonce_len - AES_GCM_NONCE_LENGTH, AES_GCM_NONCE_LENGTH); | ||||
| W[AES_BLOCK_SIZE-4] = XAES_256_GCM_MAX_NONCE_SIZE - nonce_len; | ||||
| W[AES_BLOCK_SIZE-3] = 0x01; | ||||
| W[AES_BLOCK_SIZE-2] = 0x00; | ||||
| W[AES_BLOCK_SIZE-1] = 0x01; | ||||
|
|
||||
| for (size_t i = 0; i < AES_BLOCK_SIZE; i++) { | ||||
| W[i] ^= X1[i] ^ k1[i]; | ||||
| } | ||||
|
|
||||
| AES_encrypt(W, key_commitment, xaes_key); | ||||
| W[AES_BLOCK_SIZE-1] ^= 0x03; | ||||
| AES_encrypt(W, key_commitment + AES_BLOCK_SIZE, xaes_key); | ||||
|
|
||||
| return 1; | ||||
| } | ||||
|
|
||||
| static int xaes_256_gcm_init_key_commit(EVP_CIPHER_CTX *ctx, const uint8_t *key, | ||||
| const uint8_t *iv, int enc) { | ||||
| // Key length: 32 bytes | ||||
| if (ctx->key_len != XAES_256_GCM_KEY_LENGTH) { | ||||
| OPENSSL_PUT_ERROR(CIPHER, CIPHER_R_BAD_KEY_LENGTH); | ||||
| return 0; | ||||
| } | ||||
|
|
||||
| XAES_256_GCM_KC_CTX *xaes_ctx = xaes_256_gcm_kc_from_cipher_ctx(ctx); | ||||
|
|
||||
| // When main key is provided, initialize the xaes-256-gcm context | ||||
| if(key != NULL) { | ||||
| xaes_256_gcm_ctx_init(&xaes_ctx->xaes_key, xaes_ctx->k1, key); | ||||
| } | ||||
|
|
||||
| // If iv is provided, even if main key is not, derive a subkey and its key commitment | ||||
| /* Note: Unlike the MAC tag, key commitment only depends on the main key | ||||
| * and iv, not on plaintext and aad, therefore we extract it in init when | ||||
| * deriving subkey instead of at the end of encrytion like the MAC tag */ | ||||
| if(iv != NULL) { | ||||
| // Derive subkey | ||||
| xaes_256_gcm_set_gcm_key(ctx, iv, enc); | ||||
| // Extract key commitment | ||||
| EVP_AES_GCM_CTX *gctx = &xaes_ctx->aes_gcm_ctx; | ||||
| xaes_256_gcm_CMAC_extract_key_commitment(&xaes_ctx->xaes_key, xaes_ctx->k1, xaes_ctx->kc, iv, gctx->ivlen); | ||||
| } | ||||
|
|
||||
| return 1; | ||||
| } | ||||
|
|
||||
| static int xaes_256_gcm_key_commit_ctrl(EVP_CIPHER_CTX *ctx, int type, int arg, void *ptr) { | ||||
|
|
||||
| XAES_256_GCM_KC_CTX *xaes_ctx = xaes_256_gcm_kc_from_cipher_ctx(ctx); | ||||
|
|
||||
| switch(type) { | ||||
|
Contributor
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. I think it's more prudent to check that ptr is not NULL. aws-lc/crypto/fipsmodule/cipher/cipher.c Line 185 in 88aec89
Author
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. Yes. I'll add checking ptr for two new added ctrl commands. |
||||
| case EVP_CTRL_AEAD_GET_KC: | ||||
| if(arg < XAES_256_GCM_KEY_COMMIT_SIZE) { | ||||
| OPENSSL_PUT_ERROR(CIPHER, CIPHER_R_BUFFER_TOO_SMALL); | ||||
| return 0; | ||||
| } | ||||
| OPENSSL_memcpy(ptr, xaes_ctx->kc, arg); | ||||
| return 1; | ||||
| /* Since the verification of key commitment (KC) can happen at any time, | ||||
| * not only at the end of decryption like for MAC tag, we create a control | ||||
| * command for KC verification, rather than a command to set KC, then | ||||
| * verifying it at the end of decryption as for checking the MAC tag */ | ||||
| case EVP_CTRL_AEAD_VERIFY_KC: | ||||
| if(arg < XAES_256_GCM_KEY_COMMIT_SIZE) { | ||||
| OPENSSL_PUT_ERROR(CIPHER, CIPHER_R_BUFFER_TOO_SMALL); | ||||
| return 0; | ||||
| } | ||||
| if(OPENSSL_memcmp(xaes_ctx->kc, ptr, XAES_256_GCM_KEY_COMMIT_SIZE)) { | ||||
| OPENSSL_PUT_ERROR(CIPHER, CIPHER_R_KEY_COMMITMENT_INVALID); | ||||
| return 0; | ||||
| } | ||||
| return 1; | ||||
| default: | ||||
| return aes_gcm_ctrl(ctx, type, arg, ptr); | ||||
| } | ||||
| } | ||||
|
|
||||
| DEFINE_METHOD_FUNCTION(EVP_CIPHER, EVP_xaes_256_gcm_kc) { | ||||
| OPENSSL_memset(out, 0, sizeof(EVP_CIPHER)); | ||||
| out->nid = NID_xaes_256_gcm_kc; | ||||
| out->block_size = 1; | ||||
| out->key_len = XAES_256_GCM_KEY_LENGTH; | ||||
| out->iv_len = XAES_256_GCM_MAX_NONCE_SIZE; | ||||
| out->ctx_size = sizeof(XAES_256_GCM_KC_CTX); | ||||
| out->flags = EVP_CIPH_GCM_MODE | EVP_CIPH_CUSTOM_IV | EVP_CIPH_CUSTOM_COPY | | ||||
| EVP_CIPH_FLAG_CUSTOM_CIPHER | EVP_CIPH_ALWAYS_CALL_INIT | | ||||
| EVP_CIPH_CTRL_INIT | EVP_CIPH_FLAG_AEAD_CIPHER | EVP_CIPH_FLAG_KC_CIPHER; | ||||
| out->init = xaes_256_gcm_init_key_commit; | ||||
| out->cipher = aes_gcm_cipher; | ||||
| out->cleanup = aes_gcm_cleanup; | ||||
| out->ctrl = xaes_256_gcm_key_commit_ctrl; | ||||
| } | ||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -56,7 +56,7 @@ | |
|
|
||
| /* This file is generated by crypto/obj/objects.go. */ | ||
|
|
||
| #define NUM_NID 1000 | ||
| #define NUM_NID 1001 | ||
|
|
||
| static const uint8_t kObjectData[] = { | ||
|
Contributor
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. warning: unknown type name 'uint8_t' [clang-diagnostic-error] static const uint8_t kObjectData[] = {
^ |
||
| /* NID_rsadsi */ | ||
|
|
@@ -7340,6 +7340,16 @@ static const uint8_t kObjectData[] = { | |
| 0x04, | ||
| 0x01, | ||
| 0x31, | ||
| /* NID_xaes_256_gcm_kc */ | ||
| 0x60, | ||
| 0x86, | ||
| 0x48, | ||
| 0x01, | ||
| 0x65, | ||
| 0x03, | ||
| 0x04, | ||
| 0x01, | ||
| 0x32, | ||
| }; | ||
|
|
||
| static const ASN1_OBJECT kObjects[NUM_NID] = { | ||
|
Contributor
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. warning: unknown type name 'ASN1_OBJECT' [clang-diagnostic-error] static const ASN1_OBJECT kObjects[NUM_NID] = {
^ |
||
|
|
@@ -9029,6 +9039,8 @@ static const ASN1_OBJECT kObjects[NUM_NID] = { | |
| &kObjectData[6360], 0}, | ||
| {"id-xaes256-GCM", "xaes-256-gcm", NID_xaes_256_gcm, 9, &kObjectData[6369], | ||
| 0}, | ||
| {"id-xaes256-GCM-KC", "xaes-256-gcm-kc", NID_xaes_256_gcm_kc, 9, | ||
| &kObjectData[6378], 0}, | ||
| }; | ||
|
|
||
| static const uint16_t kNIDsInShortNameOrder[] = { | ||
|
|
@@ -9670,6 +9682,7 @@ static const uint16_t kNIDsInShortNameOrder[] = { | |
| 250 /* id-smime-spq-ets-sqt-unotice */, | ||
| 249 /* id-smime-spq-ets-sqt-uri */, | ||
| 999 /* id-xaes256-GCM */, | ||
| 1000 /* id-xaes256-GCM-KC */, | ||
| 676 /* identified-organization */, | ||
| 461 /* info */, | ||
| 748 /* inhibitAnyPolicy */, | ||
|
|
@@ -11010,6 +11023,7 @@ static const uint16_t kNIDsInLongNameOrder[] = { | |
| 158 /* x509Certificate */, | ||
| 160 /* x509Crl */, | ||
| 999 /* xaes-256-gcm */, | ||
| 1000 /* xaes-256-gcm-kc */, | ||
| 125 /* zlib compression */, | ||
| }; | ||
|
|
||
|
|
@@ -11726,6 +11740,7 @@ static const uint16_t kNIDsInOIDOrder[] = { | |
| 902 /* 2.16.840.1.101.3.4.1.47 (OBJ_aes_256_ccm) */, | ||
| 903 /* 2.16.840.1.101.3.4.1.48 (OBJ_id_aes256_wrap_pad) */, | ||
| 999 /* 2.16.840.1.101.3.4.1.49 (OBJ_xaes_256_gcm) */, | ||
| 1000 /* 2.16.840.1.101.3.4.1.50 (OBJ_xaes_256_gcm_kc) */, | ||
| 672 /* 2.16.840.1.101.3.4.2.1 (OBJ_sha256) */, | ||
| 673 /* 2.16.840.1.101.3.4.2.2 (OBJ_sha384) */, | ||
| 674 /* 2.16.840.1.101.3.4.2.3 (OBJ_sha512) */, | ||
|
|
||
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.
Can the negative tests that were added for XAES (e.g. shorter nonces) be modified to also test XAES-KC?
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.
I made changes in
cipher_test.ccto test negative cases with invalid key/nonce, and shorter nonce for bothevp-xaes-256-gcmandevp-xaes-256-gcm-kc.