-
Notifications
You must be signed in to change notification settings - Fork 153
Implementation of XAES-256-GCM with EVP_CIPHER #2750
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 48 commits
f5b1d82
3b7d63a
7b2af15
debc4f8
3d22930
f5e7eae
66a9190
aa801fe
7c58649
c33f280
4610415
f0c9e37
39e6b19
20feacc
88051b8
fc0ac84
98e525b
1d4b4da
84f7160
556612a
2a2f9b2
7cd2e47
79084fd
f0f66cd
7086831
c6e3cab
900ede2
569929a
6ea5729
49e14ec
adb0153
15e8f37
d39bcdd
6183644
3bcea85
be8331e
37e232b
9d3438c
9134a92
6275ef6
89d5521
eb6d0ca
2f06f29
a7b345e
b840a07
62fc5e3
687727f
bb45cae
f344725
0d9cfa0
f8c228a
51f65a8
335d49e
fc4b301
d42c4c1
a258173
a8bf03b
d2cbdd5
c2cd9aa
8dd784e
13f55b7
0a151a8
fc77e6c
7b0443e
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 |
|---|---|---|
|
|
@@ -68,6 +68,7 @@ | |
| #include <openssl/rand.h> | ||
| #include <openssl/sha.h> | ||
| #include <openssl/span.h> | ||
| #include <openssl/digest.h> | ||
|
|
||
| #include "../internal.h" | ||
| #include "../test/file_test.h" | ||
|
|
@@ -127,6 +128,8 @@ static const EVP_CIPHER *GetCipher(const std::string &name) { | |
| return EVP_aes_192_ccm(); | ||
| } else if (name == "AES-256-CCM") { | ||
| return EVP_aes_256_ccm(); | ||
| } else if (name == "XAES-256-GCM") { | ||
| return EVP_xaes_256_gcm(); | ||
| } | ||
| return nullptr; | ||
| } | ||
|
|
@@ -1080,6 +1083,7 @@ TEST(CipherTest, GetCipher) { | |
| test_get_cipher(NID_aes_256_ctr, "aes-256-ctr"); | ||
| 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_aes_256_ofb128, "aes-256-ofb"); | ||
| test_get_cipher(NID_aes_256_xts, "aes-256-xts"); | ||
| test_get_cipher(NID_chacha20_poly1305, "chacha20-poly1305"); | ||
|
|
@@ -1102,6 +1106,7 @@ TEST(CipherTest, GetCipher) { | |
| test_get_cipher(NID_aes_128_gcm, "id-aes128-gcm"); | ||
| test_get_cipher(NID_aes_192_gcm, "id-aes192-gcm"); | ||
| test_get_cipher(NID_aes_256_gcm, "id-aes256-gcm"); | ||
| test_get_cipher(NID_xaes_256_gcm, "id-xaes256-gcm"); | ||
|
|
||
| // error case | ||
| EXPECT_FALSE(EVP_get_cipherbyname(nullptr)); | ||
|
|
@@ -1454,3 +1459,98 @@ TEST(CipherTest, Empty_EVP_CIPHER_CTX_V1187459157) { | |
| CHECK_ERROR(EVP_DecryptUpdate(ctx.get(), out_vec.data(), &out_len, in_vec.data(), in_len), ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED); | ||
| CHECK_ERROR(EVP_DecryptFinal(ctx.get(), out_vec.data(), &out_len), ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED); | ||
| } | ||
|
|
||
| TEST(CipherTest, XAES_256_GCM_EVP_CIPHER) { | ||
dkostic marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
dkostic marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| // Test invalid nonce sizes and key length | ||
| { | ||
| std::vector<uint8_t> key(32), nonce(24); | ||
|
|
||
| // XAES-256-GCM Encryption | ||
| bssl::UniquePtr<EVP_CIPHER_CTX> ctx(EVP_CIPHER_CTX_new()); | ||
| ASSERT_TRUE(ctx); | ||
| ASSERT_TRUE(EVP_CipherInit_ex(ctx.get(), EVP_xaes_256_gcm(), NULL, NULL, NULL, 1)); | ||
|
|
||
| // Valid nonce size: 20 bytes <= |N| <= 24 bytes | ||
| // Test invalid nonce size | ||
| ASSERT_TRUE(EVP_CIPHER_CTX_ctrl(ctx.get(), EVP_CTRL_AEAD_SET_IVLEN, 19, NULL)); | ||
| ASSERT_FALSE(EVP_CipherInit_ex(ctx.get(), NULL, NULL, key.data(), nonce.data(), -1)); | ||
| ASSERT_TRUE(EVP_CIPHER_CTX_ctrl(ctx.get(), EVP_CTRL_AEAD_SET_IVLEN, 25, NULL)); | ||
| ASSERT_FALSE(EVP_CipherInit_ex(ctx.get(), NULL, NULL, key.data(), nonce.data(), -1)); | ||
| ASSERT_TRUE(EVP_CIPHER_CTX_ctrl(ctx.get(), EVP_CTRL_AEAD_SET_IVLEN, 24, NULL)); | ||
|
|
||
| // Valid key length: 32 bytes | ||
| // Test invalid key length | ||
| ctx.get()->key_len = 24; | ||
| ASSERT_FALSE(EVP_CipherInit_ex(ctx.get(), NULL, NULL, key.data(), nonce.data(), -1)); | ||
| } | ||
|
|
||
| // Source of multi-loop tests: | ||
|
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.
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.
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 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):
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. I added test cases.
|
||
| // https://github.com/C2SP/C2SP/blob/main/XAES-256-GCM/go/XAES-256-GCM_test.go | ||
| const auto test = [](int n, const char *output) { | ||
| bssl::ScopedEVP_MD_CTX s; | ||
| ASSERT_TRUE(EVP_DigestInit(s.get(), EVP_shake128())); | ||
| bssl::ScopedEVP_MD_CTX d; | ||
| ASSERT_TRUE(EVP_DigestInit(d.get(), EVP_shake128())); | ||
|
|
||
| bssl::UniquePtr<EVP_CIPHER_CTX> ctx(EVP_CIPHER_CTX_new()); | ||
| ASSERT_TRUE(ctx); | ||
| ASSERT_TRUE(EVP_CipherInit_ex(ctx.get(), EVP_xaes_256_gcm(), NULL, NULL, NULL, 1)); | ||
| ASSERT_TRUE(EVP_CIPHER_CTX_ctrl(ctx.get(), EVP_CTRL_AEAD_SET_IVLEN, 24, NULL)); | ||
|
|
||
| bssl::UniquePtr<EVP_CIPHER_CTX> dctx(EVP_CIPHER_CTX_new()); | ||
| ASSERT_TRUE(dctx); | ||
| ASSERT_TRUE(EVP_DecryptInit_ex(dctx.get(), EVP_xaes_256_gcm(), NULL, NULL, NULL)); | ||
| ASSERT_TRUE(EVP_CIPHER_CTX_ctrl(dctx.get(), EVP_CTRL_AEAD_SET_IVLEN, 24, NULL)); | ||
|
|
||
| std::vector<uint8_t> key(32), nonce(24), plaintext(256); | ||
| std::vector<uint8_t> aad(256), ciphertext(256), tag(16); | ||
| uint8_t plaintext_len = 0, aad_len = 0; | ||
|
|
||
| int tag_size = 16; | ||
|
|
||
| for(int i = 0; i < n; ++i) { | ||
| ASSERT_TRUE(EVP_DigestSqueeze(s.get(), key.data(), 32)); | ||
| ASSERT_TRUE(EVP_DigestSqueeze(s.get(), nonce.data(), 24)); | ||
| ASSERT_TRUE(EVP_DigestSqueeze(s.get(), &plaintext_len, 1)); | ||
| ASSERT_TRUE(EVP_DigestSqueeze(s.get(), plaintext.data(), plaintext_len)); | ||
| ASSERT_TRUE(EVP_DigestSqueeze(s.get(), &aad_len, 1)); | ||
| ASSERT_TRUE(EVP_DigestSqueeze(s.get(), aad.data(), aad_len)); | ||
|
|
||
| // XAES-256-GCM Encryption | ||
| ASSERT_TRUE(EVP_CipherInit_ex(ctx.get(), NULL, NULL, key.data(), nonce.data(), -1)); | ||
| ASSERT_EQ(aad_len, EVP_Cipher(ctx.get(), NULL, aad.data(), aad_len)); | ||
| int ciphertext_len = 0; | ||
| ASSERT_TRUE(EVP_CipherUpdate(ctx.get(), ciphertext.data(), &ciphertext_len, | ||
| plaintext.data(), plaintext_len)); | ||
|
|
||
| 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)); | ||
|
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. What is this line for as one as l.1532? It would be good if the flow here has comments as it is not very clear to me.
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. I already added the reference. The source of this test is at: I just rewrite it in C++. It uses hash function SHAKE128 to generate data. Also, verifying the ciphertext output is done by that hash function. It verifies the ciphertext output of each iteration, then compares the final digest output after 10K iterations, 1M iterations. EVP_DigestUpdate(d.get(), ciphertext.data(), ciphertext_len);
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. This accumulation is necessary to match the final result in that linked file, right?
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 is correct! The output of each iteration is accumulated into the final result of hash. |
||
|
|
||
| ASSERT_TRUE(EVP_CIPHER_CTX_ctrl(ctx.get(), EVP_CTRL_AEAD_GET_TAG, tag_size, tag.data())); | ||
| ASSERT_TRUE(EVP_DigestUpdate(d.get(), tag.data(), tag_size)); | ||
|
|
||
| // XAES-256-GCM Decryption | ||
| ASSERT_TRUE(EVP_DecryptInit_ex(dctx.get(), NULL, NULL, key.data(), nonce.data())); | ||
| ASSERT_TRUE(EVP_CIPHER_CTX_ctrl(dctx.get(), EVP_CTRL_AEAD_SET_TAG, tag_size, tag.data())); | ||
|
|
||
| std::vector<uint8_t> decrypted; | ||
| decrypted.resize(plaintext_len); | ||
| len = 0; | ||
| EVP_DecryptUpdate(dctx.get(), NULL, &len, aad.data(), aad_len); | ||
| ASSERT_TRUE(EVP_DecryptUpdate(dctx.get(), decrypted.data(), &len, ciphertext.data(), ciphertext_len)); | ||
| ASSERT_TRUE(EVP_DecryptFinal(dctx.get(), decrypted.data() + len, &len)); | ||
|
|
||
| ASSERT_EQ(Bytes(decrypted), Bytes(plaintext.data(), plaintext_len)); | ||
| } | ||
| std::vector<uint8_t> expected; | ||
| ASSERT_TRUE(ConvertToBytes(&expected, output)); | ||
| uint8_t got[32] = {0}; | ||
| ASSERT_TRUE(EVP_DigestFinalXOF(d.get(), got, 32)); | ||
| ASSERT_EQ(Bytes(got, 32), Bytes(expected)); | ||
| }; | ||
|
|
||
| test(10000, "e6b9edf2df6cec60c8cbd864e2211b597fb69a529160cd040d56c0c210081939"); | ||
| test(1000000, "2163ae1445985a30b60585ee67daa55674df06901b890593e824b8a7c885ab15"); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -506,6 +506,32 @@ AAD = feedfacedeadbeeffeedfacedeadbeefabaddad2 | |
| Tag = a44a8266ee1c8eb0c8b5d4cf5ae9f19b | ||
| Operation = InvalidDecrypt | ||
|
|
||
| # Source of test vectors: | ||
| # https://github.com/C2SP/C2SP/blob/main/XAES-256-GCM.md | ||
| Cipher = XAES-256-GCM | ||
| Key = 0101010101010101010101010101010101010101010101010101010101010101 | ||
|
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 assume you ensured these test vectors are being used, e.g. by inserting an error in one of 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. They are being used in cipher_test.cc. I verified like what you said. I edited the test case to make it different and when I run test, it shows errors and fails the test. |
||
| IV = 424242424242424242424242424242424242424242424242 | ||
| Plaintext = 48656c6c6f2c20584145532d3235362d47434d21 | ||
| Ciphertext = 01e5f78bc99de880bd2eeff2870d361f0eab5b2f | ||
| AAD = | ||
| Tag = c55268f34b14045878fe3668db980319 | ||
|
|
||
| Cipher = XAES-256-GCM | ||
| Key = 0101010101010101010101010101010101010101010101010101010101010101 | ||
| IV = 4142434445464748494a4b4c4d4e4f505152535455565758 | ||
| Plaintext = 584145532d3235362d47434d | ||
| Ciphertext = ce546ef63c9cc60765923609 | ||
| AAD = | ||
| Tag = b33a9a1974e96e52daf2fcf7075e2271 | ||
|
|
||
| Cipher = XAES-256-GCM | ||
| Key = 0303030303030303030303030303030303030303030303030303030303030303 | ||
| IV = 4142434445464748494a4b4c4d4e4f505152535455565758 | ||
| Plaintext = 584145532d3235362d47434d | ||
| Ciphertext = 986ec1832593df5443a17943 | ||
| AAD = 633273702e6f72672f584145532d3235362d47434d | ||
| Tag = 7fd083bf3fdb41abd740a21f71eb769d | ||
|
|
||
| # local add-ons, primarily streaming ghash tests | ||
| # 128 bytes aad | ||
| Cipher = AES-128-GCM | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -57,6 +57,7 @@ | |||||
| #include <openssl/mem.h> | ||||||
| #include <openssl/nid.h> | ||||||
| #include <openssl/rand.h> | ||||||
| #include <openssl/cmac.h> | ||||||
|
|
||||||
| #include <openssl/bytestring.h> | ||||||
| #include "../../internal.h" | ||||||
|
|
@@ -353,9 +354,19 @@ static EVP_AES_GCM_CTX *aes_gcm_from_cipher_ctx(EVP_CIPHER_CTX *ctx) { | |||||
|
|
||||||
| // |malloc| guarantees up to 4-byte alignment on 32-bit and 8-byte alignment | ||||||
| // on 64-bit systems, so we need to adjust to reach 16-byte alignment. | ||||||
| assert(ctx->cipher->ctx_size == | ||||||
| sizeof(EVP_AES_GCM_CTX) + EVP_AES_GCM_CTX_PADDING); | ||||||
|
|
||||||
| switch(ctx->cipher->nid) { | ||||||
| // only execute assert checking with aes-gcm | ||||||
| case NID_aes_128_gcm: | ||||||
| case NID_aes_192_gcm: | ||||||
| case NID_aes_256_gcm: | ||||||
| assert(ctx->cipher->ctx_size == | ||||||
| sizeof(EVP_AES_GCM_CTX) + EVP_AES_GCM_CTX_PADDING); | ||||||
| break; | ||||||
| // not execute assert checking with xaes-256-gcm | ||||||
dkostic marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
| default: | ||||||
| break; | ||||||
| } | ||||||
|
|
||||||
| char *ptr = ctx->cipher_data; | ||||||
| #if defined(OPENSSL_32_BIT) | ||||||
| assert((uintptr_t)ptr % 4 == 0); | ||||||
|
|
@@ -1745,3 +1756,153 @@ int EVP_has_aes_hardware(void) { | |||||
| } | ||||||
|
|
||||||
| OPENSSL_MSVC_PRAGMA(warning(pop)) | ||||||
|
|
||||||
| /* ---------------------------- XAES-256-GCM ---------------------------- | ||||||
| Specification: https://github.com/C2SP/C2SP/blob/main/XAES-256-GCM.md | ||||||
|
||||||
| -----------------------------------------------------------------------*/ | ||||||
| #define XAES_256_GCM_CTX_OFFSET (sizeof(EVP_AES_GCM_CTX) + EVP_AES_GCM_CTX_PADDING) | ||||||
| #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_CMAC_INPUT_SIZE (AES_BLOCK_SIZE * 2) | ||||||
|
||||||
| #define XAES_256_GCM_CMAC_INPUT_SIZE (AES_BLOCK_SIZE * 2) | |
| #define XAES_256_GCM_CMAC_INPUT_SIZE (AES_BLOCK_SIZE) |
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.
This const is no longer used as I removed the code using CMAC available in AWS-LC. I'll remove it.
dkostic marked this conversation as resolved.
Show resolved
Hide resolved
dkostic marked this conversation as resolved.
Show resolved
Hide resolved
dkostic marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
dkostic marked this conversation as resolved.
Show resolved
Hide resolved
dkostic marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
dkostic marked this conversation as resolved.
Show resolved
Hide resolved
dkostic marked this conversation as resolved.
Show resolved
Hide resolved
Outdated
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.
is this correct? why are we not storing everything in XAES_256_GCM_CTX?
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.
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?
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 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?
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.
@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.
Uh oh!
There was an error while loading. Please reload this page.