Skip to content

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.

Sign up for GitHub

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

Merged
merged 13 commits into from
Nov 20, 2025
+314 −72
Merged

Add EVP_CIPHER API for XAES-256-GCM with Key Commitment #2826

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
0a3768b
Add EVP_CIPHER API XAES-256-GCM with Key Commitment
ttunglee Nov 14, 2025
35defa9
Add comment optimized Key Commitment
ttunglee Nov 14, 2025
28e379f
Edit comments
ttunglee Nov 14, 2025
ff9b2ec
Naming
ttunglee Nov 14, 2025
a7ccfc5
re-run generate_build_files.py
ttunglee Nov 14, 2025
01a9681
Update
ttunglee Nov 14, 2025
d80c7d2
Update
ttunglee Nov 19, 2025
4e92f5d
Go back
ttunglee Nov 19, 2025
e882576
Add negative tests
ttunglee Nov 19, 2025
c285850
Reorder
ttunglee Nov 19, 2025
417325b
Delete CIPHER_R_BUFFER_INVALID
ttunglee Nov 19, 2025
cdfdb87
refresh ci
ttunglee Nov 19, 2025
bfc193c
Add EVP-XAES-256-GCM-KC to speedtool
ttunglee Nov 19, 2025
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
42 changes: 11 additions & 31 deletions crypto/fipsmodule/cipher/e_aes.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -390,23 +390,11 @@ static EVP_AES_GCM_CTX *aes_gcm_from_cipher_ctx(EVP_CIPHER_CTX *ctx) {
case NID_xaes_256_gcm:
assert(ctx->cipher->ctx_size == sizeof(XAES_256_GCM_CTX));
ptr = ctx->cipher_data;
#if defined(OPENSSL_32_BIT)
Copy link
Contributor

@nebeid nebeid Nov 19, 2025

Choose a reason for hiding this comment

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

In my previous comment, I meant keeping this alignment in aes_gcm_from_cipher_ctx and try and remove them from the ones below, but maybe it doesn't make sense because it's the same pointer to ctx->cipher_data and it would be different if you're not doing the same operations to get it.

Copy link
Author

@ttunglee ttunglee Nov 19, 2025

Choose a reason for hiding this comment

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

Yeah. Since aes_gcm_from_cipher_ctx() is called in other places like aes_gcm_cipher(), aes_gcm_ctrl(), and aes_gcm_init_key(), it needs to be consistent everywhere.

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_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;
Expand Down Expand Up @@ -1853,15 +1841,7 @@ static int xaes_256_gcm_CMAC_derive_key(AES_KEY *xaes_key, uint8_t *k1,
}

static XAES_256_GCM_CTX *xaes_256_gcm_from_cipher_ctx(EVP_CIPHER_CTX *ctx) {
// Handle alignment according to the way it is implemented for the AES-GCM context
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;
return (XAES_256_GCM_CTX *)ptr;
return (XAES_256_GCM_CTX *)ctx->cipher_data;
}

static int xaes_256_gcm_set_gcm_key(EVP_CIPHER_CTX *ctx, const uint8_t *nonce, int enc) {
Expand Down Expand Up @@ -2058,15 +2038,7 @@ DEFINE_METHOD_FUNCTION(EVP_AEAD, EVP_aead_xaes_256_gcm) {
// ----------- 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;
return (XAES_256_GCM_KC_CTX *)ptr;
return (XAES_256_GCM_KC_CTX *)ctx->cipher_data;
}

static int xaes_256_gcm_extract_key_commitment(AES_KEY *xaes_key, uint8_t *k1,
Expand Down Expand Up @@ -2132,7 +2104,7 @@ static int xaes_256_gcm_kc_init(EVP_CIPHER_CTX *ctx, const uint8_t *key,
}

static int xaes_256_gcm_kc_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) {
Copy link
Contributor

@nebeid nebeid Nov 18, 2025

Choose a reason for hiding this comment

The 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.
I see that there are no checks in aes_gcm_ctrl, but it's good to verify here and we should probably change there as well wherever it's used. It's sometimes intentionally NULL as in the INIT case in

aws-lc/crypto/fipsmodule/cipher/cipher.c

Line 185 in 88aec89

if (!EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_INIT, 0, NULL)) {

Copy link
Author

@ttunglee ttunglee Nov 19, 2025

Choose a reason for hiding this comment

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

Yes. I'll add checking ptr for two new added ctrl commands.

Expand All @@ -2141,6 +2113,10 @@ static int xaes_256_gcm_kc_ctrl(EVP_CIPHER_CTX *ctx, int type, int arg, void *pt
OPENSSL_PUT_ERROR(CIPHER, CIPHER_R_BUFFER_TOO_SMALL);
return 0;
}
if(!ptr) {
OPENSSL_PUT_ERROR(CIPHER, CIPHER_R_BUFFER_INVALID);
return 0;
}
OPENSSL_memcpy(ptr, xaes_ctx->kc, arg);
return 1;
/* Since the verification of key commitment (KC) can happen at any time,
Expand All @@ -2152,6 +2128,10 @@ static int xaes_256_gcm_kc_ctrl(EVP_CIPHER_CTX *ctx, int type, int arg, void *pt
OPENSSL_PUT_ERROR(CIPHER, CIPHER_R_BUFFER_TOO_SMALL);
return 0;
}
if(!ptr) {
OPENSSL_PUT_ERROR(CIPHER, CIPHER_R_BUFFER_INVALID);
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;
Expand Down
3 changes: 2 additions & 1 deletion include/openssl/cipher.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -744,7 +744,8 @@ BSSL_NAMESPACE_END
#define CIPHER_R_ALIGNMENT_CHANGED 142
#define CIPHER_R_SERIALIZATION_INVALID_SERDE_VERSION 143
#define CIPHER_R_SERIALIZATION_INVALID_CIPHER_ID 144
// We're introducing a new error code for invalid key commitment
// We're introducing a new error code for invalid key commitment and invalid buffer
#define CIPHER_R_KEY_COMMITMENT_INVALID 145
#define CIPHER_R_BUFFER_INVALID 146
Copy link
Contributor

@nebeid nebeid Nov 19, 2025

Choose a reason for hiding this comment

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

You can use

aws-lc/crypto/internal.h

Line 1447 in 88aec89

#define GUARD_PTR(ptr) __AWS_LC_ENSURE((ptr) != NULL, OPENSSL_PUT_ERROR(CRYPTO, ERR_R_PASSED_NULL_PARAMETER); \

and no need to introduce a new error.

Copy link
Author

@ttunglee ttunglee Nov 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Thank you! I replaced that directive in my update and deleted CIPHER_R_BUFFER_INVALID.


#endif // OPENSSL_HEADER_CIPHER_H
Loading