Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 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
4 changes: 3 additions & 1 deletion crypto/cipher_extra/cipher_extra.c
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ static const struct {
{NID_aes_256_ecb, "aes-256-ecb", EVP_aes_256_ecb},
{NID_aes_256_gcm, "aes-256-gcm", EVP_aes_256_gcm},
{NID_xaes_256_gcm, "xaes-256-gcm", EVP_xaes_256_gcm},
{NID_xaes_256_gcm_kc, "xaes-256-gcm-kc", EVP_xaes_256_gcm_kc},
{NID_aes_256_ofb128, "aes-256-ofb", EVP_aes_256_ofb},
{NID_aes_256_xts, "aes-256-xts", EVP_aes_256_xts},
{NID_chacha20_poly1305, "chacha20-poly1305", EVP_chacha20_poly1305},
Expand All @@ -116,7 +117,8 @@ static const struct {
{"id-aes128-gcm", "aes-128-gcm"},
{"id-aes192-gcm", "aes-192-gcm"},
{"id-aes256-gcm", "aes-256-gcm"},
{"id-xaes256-gcm", "xaes-256-gcm"}
{"id-xaes256-gcm", "xaes-256-gcm"},
{"id-xaes256-gcm-kc", "xaes-256-gcm-kc"}
};

const EVP_CIPHER *EVP_get_cipherbynid(int nid) {
Expand Down
39 changes: 32 additions & 7 deletions crypto/cipher_extra/cipher_test.cc
Copy link
Contributor

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?

Copy link
Author

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.cc to test negative cases with invalid key/nonce, and shorter nonce for both evp-xaes-256-gcm and evp-xaes-256-gcm-kc.

Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,8 @@ static const EVP_CIPHER *GetCipher(const std::string &name) {
return EVP_aes_256_ccm();
} else if (name == "XAES-256-GCM") {
return EVP_xaes_256_gcm();
} else if (name == "XAES-256-GCM-KC") {
return EVP_xaes_256_gcm_kc();
}
return nullptr;
}
Expand Down Expand Up @@ -186,14 +188,16 @@ static void TestCipherAPI(const EVP_CIPHER *cipher, Operation op, bool padding,
bssl::Span<const uint8_t> plaintext,
bssl::Span<const uint8_t> ciphertext,
bssl::Span<const uint8_t> aad,
bssl::Span<const uint8_t> tag) {
bssl::Span<const uint8_t> tag,
bssl::Span<const uint8_t> kc = {}) {
bool encrypt = op == Operation::kEncrypt;
bool is_custom_cipher =
EVP_CIPHER_flags(cipher) & EVP_CIPH_FLAG_CUSTOM_CIPHER;
bssl::Span<const uint8_t> in = encrypt ? plaintext : ciphertext;
bssl::Span<const uint8_t> expected = encrypt ? ciphertext : plaintext;
bool is_aead = EVP_CIPHER_flags(cipher) & EVP_CIPH_FLAG_AEAD_CIPHER;
bool is_ccm = EVP_CIPHER_mode(cipher) == EVP_CIPH_CCM_MODE;
bool is_kc = EVP_CIPHER_flags(cipher) & EVP_CIPH_FLAG_KC_CIPHER;

// Some |EVP_CIPHER|s take a variable-length key, and need to first be
// configured with the key length, which requires configuring the cipher.
Expand Down Expand Up @@ -236,6 +240,12 @@ static void TestCipherAPI(const EVP_CIPHER *cipher, Operation op, bool padding,
/*engine=*/nullptr,
/*key=*/nullptr, iv.data(), /*enc=*/-1));

// Verify key commitment
if(is_kc && !encrypt) {
ASSERT_TRUE(EVP_CIPHER_CTX_ctrl(ctx.get(), EVP_CTRL_AEAD_VERIFY_KC,
kc.size(), const_cast<uint8_t *>(kc.data())));
}

// CCM requires the full length of the plaintext to be known ahead of time.
if (is_ccm) {
int len;
Expand Down Expand Up @@ -358,8 +368,17 @@ static void TestCipherAPI(const EVP_CIPHER *cipher, Operation op, bool padding,
ASSERT_TRUE(EVP_CIPHER_CTX_ctrl(ctx.get(), EVP_CTRL_AEAD_GET_TAG,
tag.size(), rtag));
EXPECT_EQ(Bytes(tag), Bytes(rtag, tag.size()));
}

if(encrypt & is_kc) {
uint8_t rkc[32];
ASSERT_LE(kc.size(), sizeof(rkc));
ASSERT_TRUE(MaybeCopyCipherContext(copy, &ctx));
ASSERT_TRUE(EVP_CIPHER_CTX_ctrl(ctx.get(), EVP_CTRL_AEAD_GET_KC,
kc.size(), rkc));
EXPECT_EQ(Bytes(kc), Bytes(rkc, kc.size()));
}
}
}
}

static void TestLowLevelAPI(
Expand Down Expand Up @@ -457,7 +476,8 @@ static void TestCipher(const EVP_CIPHER *cipher, Operation input_op,
bssl::Span<const uint8_t> plaintext,
bssl::Span<const uint8_t> ciphertext,
bssl::Span<const uint8_t> aad,
bssl::Span<const uint8_t> tag) {
bssl::Span<const uint8_t> tag,
bssl::Span<const uint8_t> kc = {}) {
size_t block_size = EVP_CIPHER_block_size(cipher);
bool is_ccm = EVP_CIPHER_mode(cipher) == EVP_CIPH_CCM_MODE;
std::vector<Operation> ops;
Expand Down Expand Up @@ -487,11 +507,11 @@ static void TestCipher(const EVP_CIPHER *cipher, Operation input_op,
SCOPED_TRACE(copy);
TestCipherAPI(cipher, op, padding, copy, in_place,
/*use_evp_cipher=*/false, chunk_size, key, iv,
plaintext, ciphertext, aad, tag);
plaintext, ciphertext, aad, tag, kc);
if (!padding && chunk_size % block_size == 0) {
TestCipherAPI(cipher, op, padding, copy, in_place,
/*use_evp_cipher=*/true, chunk_size, key, iv,
plaintext, ciphertext, aad, tag);
plaintext, ciphertext, aad, tag, kc);
}
}
if (!padding) {
Expand All @@ -509,7 +529,7 @@ static void CipherFileTest(FileTest *t) {
const EVP_CIPHER *cipher = GetCipher(cipher_str);
ASSERT_TRUE(cipher);

std::vector<uint8_t> key, iv, plaintext, ciphertext, aad, tag;
std::vector<uint8_t> key, iv, plaintext, ciphertext, aad, tag, kc;
// Force an allocation of the underlying data-store so that v.data() is
// non-NULL even for empty test vectors.
plaintext.reserve(1);
Expand All @@ -525,6 +545,10 @@ static void CipherFileTest(FileTest *t) {
ASSERT_TRUE(t->GetBytes(&tag, "Tag"));
}

if(EVP_CIPHER_flags(cipher) & EVP_CIPH_FLAG_KC_CIPHER) {
ASSERT_TRUE(t->GetBytes(&kc, "KC"));
}

Operation op = Operation::kBoth;
if (t->HasAttribute("Operation")) {
const std::string &str = t->GetAttributeOrDie("Operation");
Expand All @@ -540,7 +564,7 @@ static void CipherFileTest(FileTest *t) {
}

TestCipher(cipher, op, /*padding=*/false, key, iv, plaintext, ciphertext, aad,
tag);
tag, kc);
}

TEST(CipherTest, TestVectors) {
Expand Down Expand Up @@ -1084,6 +1108,7 @@ TEST(CipherTest, GetCipher) {
test_get_cipher(NID_aes_256_ecb, "aes-256-ecb");
test_get_cipher(NID_aes_256_gcm, "aes-256-gcm");
test_get_cipher(NID_xaes_256_gcm, "xaes-256-gcm");
test_get_cipher(NID_xaes_256_gcm_kc, "id-xaes256-gcm-kc");
test_get_cipher(NID_aes_256_ofb128, "aes-256-ofb");
test_get_cipher(NID_aes_256_xts, "aes-256-xts");
test_get_cipher(NID_chacha20_poly1305, "chacha20-poly1305");
Expand Down
28 changes: 28 additions & 0 deletions crypto/cipher_extra/test/cipher_tests.txt
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,34 @@ Ciphertext = 986ec1832593df5443a17943
AAD = 633273702e6f72672f584145532d3235362d47434d
Tag = 7fd083bf3fdb41abd740a21f71eb769d

# Note: KC are our own test values
Copy link
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Author

Choose a reason for hiding this comment

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

Does that mean I should remove all changes that were made to cipher_test.cc to support key commitment verification?

Copy link
Contributor

Choose a reason for hiding this comment

The 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 CipherFileTest because it needs its own "Cipher" string. So you can ignore the suggestion.

Copy link
Author

Choose a reason for hiding this comment

The 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
Expand Down
156 changes: 140 additions & 16 deletions crypto/fipsmodule/cipher/e_aes.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -381,13 +390,12 @@ 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

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

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;
return &((XAES_256_GCM_KC_CTX *)ptr)->aes_gcm_ctx;
default:
break;
}
Expand Down Expand Up @@ -1781,7 +1789,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)
/*
Expand Down Expand Up @@ -1834,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 @@ -2033,3 +2032,128 @@ 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) {
return (XAES_256_GCM_KC_CTX *)ctx->cipher_data;
}

static int xaes_256_gcm_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);
/* Since W1[i] and W2[i] are the same except at i = 15, where:
* W1[15] = x1[15] ^ k1[15] ^ 0x01, and W2[15] = x1[15] ^ k1[15] ^ 0x02, we have:
* W2[15] = W1[15] ^ 0x03 = (x1[15] ^ k1[15] ^ 0x01) ^ (0x01 ^ 0x02) = W2[15] */
W[AES_BLOCK_SIZE-1] ^= 0x03;
AES_encrypt(W, key_commitment + AES_BLOCK_SIZE, xaes_key);

return 1;
}

static int xaes_256_gcm_kc_init(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 ciphertext 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_extract_key_commitment(&xaes_ctx->xaes_key, xaes_ctx->k1, xaes_ctx->kc, iv, gctx->ivlen);
}

return 1;
}

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

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

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

Copy link
Author

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.

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;
}
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,
* 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(!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;
}
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_kc_init;
out->cipher = aes_gcm_cipher;
out->cleanup = aes_gcm_cleanup;
out->ctrl = xaes_256_gcm_kc_ctrl;
}
Loading
Loading