Implementation of XAES-256-GCM with EVP_CIPHER #2750

ttungle96 · 2025-10-14T23:44:50Z

Issues:

Add XAES-256-GCM, which is extended AES-256-GCM with a derived key mode proposed by Filippo Valsorda in 2023, followed by a specification released in 2024.
This implementation supports EVP_CIPHER API, allows configuration to use either FIPS-compliant CMAC available in AWS-LC, or an optimized CMAC dedicated to the specific use case of XAES-256-GCM from XAES-256-GCM #2652.
Support varying nonce sizes: 20 ≤ b ≤ 24 based on the extension: https://eprint.iacr.org/2025/758.pdf#page=24

Description of Changes

Implementation for API EVP_CIPHER of XAES-256-GCM is appended to e_aes.c, and the tests are appended to cipher_test.cc.

Testing

We use the test vectors provided here:
https://github.com/C2SP/C2SP/blob/main/XAES-256-GCM.md
https://github.com/C2SP/C2SP/blob/main/XAES-256-GCM/openssl/openssl.c
https://github.com/C2SP/C2SP/blob/main/XAES-256-GCM/go/XAES-256-GCM_test.go

Evaluation

Performance evaluation with speedtool:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

github-actions

clang-tidy made some suggestions

crypto/obj/obj_dat.h

codecov-commenter · 2025-10-15T00:21:06Z

Codecov Report

❌ Patch coverage is 97.74011% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.03%. Comparing base (0fed4db) to head (7b0443e).

Files with missing lines	Patch %	Lines
crypto/fipsmodule/cipher/e_aes.c	96.05%	3 Missing ⚠️
crypto/cipher_extra/cipher_test.cc	99.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@               Coverage Diff                @@
##           xaes-256-gcm    #2750      +/-   ##
================================================
+ Coverage         78.94%   79.03%   +0.09%     
================================================
  Files               677      677              
  Lines            115525   114623     -902     
  Branches          16249    16261      +12     
================================================
- Hits              91198    90592     -606     
+ Misses            23530    23234     -296     
  Partials            797      797

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…to xaes-256-gcm

crypto/cipher_extra/cipher_test.cc

crypto/fipsmodule/cipher/e_aes.c

dkostic · 2025-11-05T17:48:57Z

crypto/fipsmodule/cipher/e_aes.c

+    out->ctx_size = sizeof(EVP_AES_GCM_CTX) + EVP_AES_GCM_CTX_PADDING 
+                + sizeof(XAES_256_GCM_CTX); 


is this correct? why are we not storing everything in XAES_256_GCM_CTX?

It is just style. Like I explained, I can declare a field of type EVP_AES_GCM_CTX inside XAES_256_GCM_CTX. Then, like in previous review, you require another function similar to aes_gcm_from_cipher_ctx to specify the start position of EVP_AES_GCM_CTX. Then I also have to see why a test case failed? Is that too cumbersome?

The way I understand it is that it differs from the AEAD implementation in #2652 in that, here, in the EVP API, we want to be able to do multiple cipher calls (complete with a tag, not streaming) using the same derived key but new (second half?) IV. In the AEAD implementation, every "seal" or "open" operation had a new IV that derived a new key so the gcm context was temporary. So here, the cipher call is pointing to aes_gcm_cipher and uses a gcm context.
I think it would be clearer if that context were part of the xaes context as dkostic suggests.
On another note, can the interface take a half IV for GCM and not re-derive the key? Or I'm wrong in assuming that that use-case is implemented?

@nebeid I have included EVP_AES_GCM_CTX inside XAES_256_GCM_CTX in the newest update. Please review again based on that. This will make the code longer since in EVP_AEAD implementation, XAES_256_GCM_CTX does not need EVP_AES_GCM_CTX inside. I will have to define another struct for AEAD case, then another aead_xaes_256_gcm_ctx_init(), which is totally the same as xaes_256_gcm_ctx_init(), but different by only one argument. Similarly for aead_xaes_256_gcm_CMAC_derive_key(), totally the same as xaes_256_gcm_CMAC_derive_key().

For your second question, currently it has not implemented yet since people have not agreed on that design. Furthermore, I tried to keep this PR as simple as possible for review. If that case is required, I'll fix and create another PR.

crypto/cipher_extra/cipher_test.cc

crypto/fipsmodule/cipher/e_aes.c

nebeid · 2025-11-07T20:50:01Z

crypto/cipher_extra/cipher_test.cc

+
+            // XAES-256-GCM Encryption
+            ASSERT_TRUE(EVP_CipherInit_ex(ctx.get(), nullptr, nullptr, key.data(), nonce.data(), -1));
+            ASSERT_EQ(aad_len, EVP_Cipher(ctx.get(), nullptr, aad.data(), aad_len));


This assert is questionable.; it compares the return value of a function with the length of the aad. ASSERT_TRUE on the EVP_Cipher call is what I believe you intended.

I follow this: https://github.com/aws/aws-lc/blob/main/crypto/cipher_extra/cipher_test.cc#L258
Should I need to change it to ASSERT_TRUE instead?

Thank you for the reference. I noticed this explanation

aws-lc/include/openssl/cipher.h

Lines 466 to 475 in 93f9f73

// If |ctx|'s cipher has the |EVP_CIPH_FLAG_CUSTOM_CIPHER| flag, it runs in one

// of two modes: If |in| is non-NULL, it behaves like |EVP_CipherUpdate|. If

// |in| is NULL, it behaves like |EVP_CipherFinal_ex|. In both cases, it returns

// |*out_len| on success and -1 on error.

//

// WARNING: The two possible calling conventions of this function signal errors

// incompatibly. In the first, zero indicates an error. In the second, zero

// indicates success with zero bytes of output.

OPENSSL_EXPORT int EVP_Cipher(EVP_CIPHER_CTX *ctx, uint8_t *out,

const uint8_t *in, size_t in_len);

. It's a confusing function, but we can leave it as is.

nebeid · 2025-11-07T20:56:14Z

crypto/test/test_util.h

 // hexdump writes |msg| to |fp| followed by the hex encoding of |len| bytes
 // from |in|.
 void hexdump(FILE *fp, const char *msg, const void *in, size_t len);
+bool ConvertToBytes(std::vector<uint8_t> *out, const std::string &value);


Sorry, I should have realised that you could probably use DecodeHex instead of exposing this function. It's used for example in

aws-lc/crypto/fipsmodule/evp/evp_ctx_test.cc

Line 413 in d826708

DecodeHex(&raw_key, hmac_hexkey);

Would you be able to try it?

nebeid · 2025-11-07T20:57:52Z

crypto/cipher_extra/cipher_test.cc

+            int len = 0;
+            ASSERT_TRUE(EVP_CipherFinal_ex(ctx.get(), ciphertext.data() + ciphertext_len, &len));
+            ciphertext_len += len;
+            ASSERT_TRUE(EVP_DigestUpdate(d.get(), ciphertext.data(), ciphertext_len));


This accumulation is necessary to match the final result in that linked file, right?

nebeid · 2025-11-07T21:02:29Z

crypto/cipher_extra/cipher_test.cc

+        test(plaintext, strlen((const char *)plaintext), ciphertext, tag);
+    }   
+
+    // Source of multi-loop tests: 


I don't see tests for the following usecases, unless you're planning on doing them in a subsequent PR (or point me to them if I missed them):

Are there KATs (known-answer tests) that test the streaming API, i.e. calling CipherUpdate multiple times?

are the shorter nonces tested at least with a self-consistency test, i.e. we can decrypt what we encrypt?

nebeid

(Pending CI)

xaes-256-gcm with evp_cipher

f5b1d82

ttungle96 requested a review from a team as a code owner October 14, 2025 23:44

github-actions bot reviewed Oct 14, 2025

View reviewed changes

crypto/obj/obj_dat.h Show resolved Hide resolved

crypto/obj/obj_dat.h Show resolved Hide resolved

ttungle96 added 17 commits October 14, 2025 18:09

xaes-256-gcm with evp_cipher

3b7d63a

xaes-256-gcm with evp_cipher

7b2af15

xaes-256-gcm with evp_cipher

debc4f8

xaes-256-gcm with evp_cipher

3d22930

full test coverage

f5e7eae

full test coverage

66a9190

increase test coverage

aa801fe

Merge branch 'xaes-256-gcm' into xaes-256-gcm

7c58649

improve test coverage

c33f280

Merge branch 'xaes-256-gcm' of https://github.com/ttungle96/aws-lc in…

4610415

…to xaes-256-gcm

increase test coverage

f0c9e37

improve test coverage

39e6b19

Final

20feacc

Final

88051b8

Final

fc0ac84

Final

98e525b

Final

1d4b4da

dkostic requested review from dkostic, nebeid and torben-hansen October 16, 2025 18:03

dkostic reviewed Oct 16, 2025

View reviewed changes

ttungle96 added 5 commits October 28, 2025 13:41

resolve issues

84f7160

update

556612a

update

2a2f9b2

update

7cd2e47

add flag

79084fd

ttungle96 requested review from dkostic and nebeid November 4, 2025 23:25

update

0d9cfa0

dkostic reviewed Nov 5, 2025

View reviewed changes

crypto/cipher_extra/cipher_test.cc Outdated Show resolved Hide resolved

dkostic reviewed Nov 5, 2025

View reviewed changes

crypto/cipher_extra/cipher_test.cc Show resolved Hide resolved

ttungle96 added 5 commits November 5, 2025 10:57

Fix

f8c228a

Fix

51f65a8

add comments

335d49e

fix tests

fc4b301

fix tests

d42c4c1

ttungle96 requested a review from dkostic November 5, 2025 20:15

ttungle96 added 6 commits November 5, 2025 13:05

final

a258173

test remove padding

a8bf03b

BE

d2cbdd5

BE

c2cd9aa

add alignment

8dd784e

BE

13f55b7

dkostic reviewed Nov 7, 2025

View reviewed changes

crypto/fipsmodule/cipher/e_aes.c Show resolved Hide resolved

add comment

0a151a8

dkostic previously approved these changes Nov 7, 2025

View reviewed changes

nebeid reviewed Nov 7, 2025

View reviewed changes

add tests

fc77e6c

ttungle96 dismissed dkostic’s stale review via fc77e6c November 7, 2025 22:41

Final

7b0443e

ttungle96 requested review from dkostic and nebeid November 7, 2025 22:44

dkostic approved these changes Nov 7, 2025

View reviewed changes

nebeid approved these changes Nov 7, 2025

View reviewed changes

torben-hansen merged commit 6d0c704 into aws:xaes-256-gcm Nov 8, 2025
175 of 374 checks passed

		out->ctx_size = sizeof(EVP_AES_GCM_CTX) + EVP_AES_GCM_CTX_PADDING
		+ sizeof(XAES_256_GCM_CTX);

	// If \|ctx\|'s cipher has the \|EVP_CIPH_FLAG_CUSTOM_CIPHER\| flag, it runs in one
	// of two modes: If \|in\| is non-NULL, it behaves like \|EVP_CipherUpdate\|. If
	// \|in\| is NULL, it behaves like \|EVP_CipherFinal_ex\|. In both cases, it returns
	// \|*out_len\| on success and -1 on error.
	//
	// WARNING: The two possible calling conventions of this function signal errors
	// incompatibly. In the first, zero indicates an error. In the second, zero
	// indicates success with zero bytes of output.
	OPENSSL_EXPORT int EVP_Cipher(EVP_CIPHER_CTX ctx, uint8_t out,
	const uint8_t *in, size_t in_len);

Implementation of XAES-256-GCM with EVP_CIPHER #2750

Implementation of XAES-256-GCM with EVP_CIPHER #2750

Uh oh!

Conversation

ttungle96 commented Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issues:

Description of Changes

Testing

Evaluation

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ttungle96 Nov 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nebeid Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nebeid left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ttungle96 commented Oct 14, 2025 •

edited

Loading

codecov-commenter commented Oct 15, 2025 •

edited

Loading

ttungle96 Nov 6, 2025 •

edited

Loading

nebeid Nov 7, 2025 •

edited

Loading