usnistgov / ACVP-Server

A repository tracking releases of NIST's ACVP server. See www.github.com/usnistgov/ACVP for the protocol.
52 stars 18 forks source link

AES GCM SIV only increments 8 bits of the counter instead of 32-bits specified by RFC8452 #209

Closed blackbird1999 closed 2 years ago

blackbird1999 commented 2 years ago

environment DEMO

testSessionId Unknown

vsId 991831

Algorithm registration AES-GCM-SIV

Endpoint in which the error is experienced Unknown

Expected behavior It should implement AES-GCM-SIV correctly

Additional context Test data obtained from the test server fails with our AES GCM code. If we adjust our counter increment function to only increment 8-bits, instead of the required 32-bits, we get matching output. However, we then fail the counter wrap tests in section C.3 of RFC 8452 indicating that the ACVP implementation is not quite right.

livebe01 commented 2 years ago

Hi @blackbird1999. Thanks for letting us know about this. We're looking into it and will get back to you.

celic commented 2 years ago

After only taking a quick look at our implementation (https://github.com/usnistgov/ACVP-Server/blob/master/gen-val/src/crypto/src/NIST.CVP.ACVTS.Libraries.Crypto/Symmetric/BlockModes/Aead/GcmSivBlockCipher.cs#L325), if I update the counter to increment with 32 bits, I end up failing other multi-block tests from the RFC, but do pass those CTR-Wrap tests. I'll work with it a bit more.

livebe01 commented 2 years ago

@blackbird1999 I'm not sure why your use of an 8-bit counter vs a 32-bit counter is getting you "better" results in relation to the SIV testing. Per the link Chris shared, you'll see that we are using a 32-bit counter. Curious, can you share what implementation you're testing?

blackbird1999 commented 2 years ago

I'm testing Information Security Corporation's Cryptographic Development Kit as part of a FIPS 140-3 level 1 evaluation. I can't say for sure that I've implemented AES-GCM-SIV correctly. If I read Chris' comment correctly "if I update the counter to increment with 32 bits", the code linked is only updating 8 bits which is consistent with my comment.

celic commented 2 years ago

AES-GCM-SIV is not approved for FIPS 140 devices, just to be clear. It is not available for testing on the production server to receive a validation certificate.

On Mon, Jun 20, 2022, 12:36 PM blackbird1999 @.***> wrote:

I'm testing Information Security Corporation's Cryptographic Development Kit as part of a FIPS 140-3 level 1 evaluation. I can't say for sure that I've implemented AES-GCM-SIV correctly. If I read Chris' comment correctly "if I update the counter to increment with 32 bits", the code linked is only updating 8 bits which is consistent with my comment.

— Reply to this email directly, view it on GitHub https://github.com/usnistgov/ACVP-Server/issues/209#issuecomment-1160652681, or unsubscribe https://github.com/notifications/unsubscribe-auth/AATQXEMOYEKTHUQVUTDYJM3VQCMYPANCNFSM5XEUUNBQ . You are receiving this because you commented.Message ID: @.***>

blackbird1999 commented 2 years ago

I am aware of that. I'm just trying to contribute feedback in the hope that if it is approved it will be possible to test against it and get a certificate. Thanks!

celic commented 2 years ago

I'm a bit curious why nobody has pointed out this issue previously... I've had other demo users test this algorithm and not see any issues (i.e., get through the testing fully agreeing with the server.) I do see the error when using the counter-wrap tests from the RFC. Were those in the original RFC or were they added at a later date? The algorithm hasn't seen any changes or modifications over the past couple of years correct?

blackbird1999 commented 2 years ago

I don't think anything relevant has changed since it became an RFC. Here's the history of it: https://datatracker.ietf.org/doc/rfc8452/history/

There are errata (https://www.rfc-editor.org/errata_search.php?rfc=8452&rec_status=0), but they don't seem relevant to me.

As for why other demo users didn't complain previously, my opinion is that there's an assumption that NIST is correct, and folks will often adjust their code to pass assuming that their code is wrong. Once they match NIST they're happy.

celic commented 2 years ago

This might be an edge case missed in other implementations, which tells me it would be a good candidate to add for a specific test in the vector sets too... once we fix our implementation to handle it.

celic commented 2 years ago

@blackbird1999 would you mind confirming a value from GCM-SIV here?

When running the following test from the RFC...

   Plaintext (32 bytes) =      02000000000000000000000000000000
                               03000000000000000000000000000000
   AAD (1 bytes) =             01
   Key =                       01000000000000000000000000000000
   Nonce =                     030000000000000000000000
   Record authentication key = d9b360279694941ac5dbc6987ada7377
   Record encryption key =     4004a0dcd862f2a57360219d2d44ef6c
   POLYVAL input =             01000000000000000000000000000000
                               02000000000000000000000000000000
                               03000000000000000000000000000000
                               08000000000000000001000000000000
   POLYVAL result =            2ce7daaf7c89490822051255b12eca6b
   POLYVAL result XOR nonce =  2fe7daaf7c89490822051255b12eca6b
   ... and masked =            2fe7daaf7c89490822051255b12eca6b
   Tag =                       e6af6a7f87287da059a71684ed3498e1
   Initial counter =           e6af6a7f87287da059a71684ed3498e1
   Result (48 bytes) =         620048ef3c1e73e57e02bb8562c416a3
                               19e73e4caac8e96a1ecb2933145a1d71
                               e6af6a7f87287da059a71684ed3498e1

I have the matching initial counter for the first block of the AES_CTR encrypt operation. Perhaps this is a misinterpretation of the line block[0:4] = little_endian_uint32(read_little_endian_uint32(block[0:4]) + 1) in the RFC. What do you get after incrementing the counter for the next block? My system is adding 1 and getting the counter as E6AF6A8087287DA059A71684ED3498E1 where the leading E6AF6A7F was incremented to E6AF6A80. This leads to an incorrect result on the encryption of the remaining blocks. The result I get is 620048EF3C1E73E57E02BB8562C416A3 B1FC0BF10B41826BC4738B20A5F3DE69 E6AF6A7F87287DA059A71684ED3498E1 where the only incorrect "block" is the middle, meaning the AES_CTR encrypt operation after the counter was incremented.

blackbird1999 commented 2 years ago

It goes: E6AF6A7F... E7AF6A7F... E8AF6A7F...

celic commented 2 years ago

Excellent. This will be fixed in the next release. Thank you for working with me here.

blackbird1999 commented 2 years ago

Here's some random test data that actually roles over the counter from the first byte to the second byte. It's from https://raw.githubusercontent.com/Metalnem/aes-gcm-siv/master/src/Cryptography.Tests/Vectors/encryption-1000.json

{ "plaintext": "77ea5cda72857c201396dee10d689ed04ebb44f36fc000d35bfe4486faedecf71532394c8efc5d841bd68fb261ab6cb3e84bcf7b83264b224210cd90931a74f5edf5c18f97ec3a8b9e374842372994224fc2b3963ebaf840ca3d391be19b766ea3849cc3a3c77f1f1301b776529bb424e2671c3c05b9e190bcaa78211f2d02a2338bb573dc53dfe0e2c0b1769b96587d67e920bd61f9a4addcd1151e3e43600c8b78c01af3583f82cc09b14c7ae7c185e9447f737cd729ea951912441ab513be82519295ac5bdbcb960b332f916497980b755c5295a11459ccd4a780190bd54a99bcf43ad9957551042630cdbffd04312259f76a29278db876235dc0d5092671399cc20d91a46db879bace4a258a6c699440b005b678e54f4deaf8f2912bb694375876861c3bbce2c1df274ecc8511916a01adb6628716a2675b17144f09da86681e567ddc8cf32fb2b2d158a4ffad7f5d4c6dd5e1dd45298b883533a0464f822e638daafa8a858a500dca36cf22def0126f2f1d6daad79b321dbe6e6892f1b2692a9aaa35a4ab48794a11586a8cd45f21c3419b3438f0142a0f90ec5cdd91b158a9635336b6f657f40e7e2310021bf08b39b3a85dc423122ba8cdf129647d67f3e17e35f04ef8509dc98851edfbf8592c37fe7737be45c52cb32e9866c68e60daf7bd30cc4bae001716713e91b0f2cc71c160b085e306b9595e787a99157bfe5575989bd241d4961b97d1bc2c959115f6e9d7dd624f40a73feb9853aec8582289fe012c03bf0e1cb311532cfa403eb7a1c48939c58abb2b02768c750b3f90879fa831cc1339e888ae3186a4ada204f322926ea13637c78dcf86e4f0f89a8c7d0f10e42fa80fe8b99a8c84541951d1eb4a712309f2d73712249992834d4cbd5cee03872c9e1de91b7b868c87c27046516f67b8c688ccea92219ad5c4ea56cdd7e4b16c09f5e60efe1dec4f3f267ae1899c8789d2ee5c82a633ae14db803df3e5cbf89ab3b504a79e3d9460a5895891e3f4e31c82345ac0222f07d5b9b8daa638e00fa37671cf5ccb036e866a5bd3647b50e6fa172a55cce2a0e66e4a082ec3047eb667c910dbf4fed215ababd7cc786e5fde85e59605985465b161a181ceee9dd1dbdd1244b94d6682fa5341dfe7067a912c8efaf23515159aa3907958019ebfcb86bc3c4db3544216b48b6337cde98df7696c6d4aba4b6847165e619a8d10c024f3dfda7f48630b87b6efb7a217a914a615c72bec3438a72af8932ae1be6a0df2d899f0906062c184e12069b276d9294833e2e6fc2dbe764f310f5f7689f629685493bea21ff92e58f189a075a1331605648cbaa998a7b206184efda807648deb6d5d20321abe7899eed3454567bf5697f5be5d802362fb22b960148cfe40dc", "aad": "", "key": "f8d690768e5e6b74da190743d9bf52b58c6c3b40acdb1ce3da9feca170424e6e", "nonce": "ad38f810d200d204f9710933", "result": "7ae4d689de96c8c3b2153313aaf86b5f713319ab668399070706957d169a035f01ae0bd6bf4f6ea3f9f13cc7e31188256f9e154e51e6d8030d4e0af06415e94f21575db3aca72fcbf737ba7b5f64ffafeb18973bee13ee70ab587d73c3f10403ff23f2fb9bf2ee36bcd8c162cb30ae93736b8db561a267c4aaa2001ac449a898cae98417cdb803abc62734dad7e3db2073ac0d00634de06b2d5f3e571eb24eaef11a526c4570be0143afddafb18d3dd0390f9705d56629b26671799a0776f4a51342cc87fb6f5463c06d3dbea45ce4cb42633d296472a527a70ed9ea2191f651c408edac359db1cac0e0bbf8bf424f000724cccecb7a53aa6c0f1f55bb5e7213db7389306ec67554b744eac7e586af5a6cdef55daad6cdf58de5483f2d4d99068833a55ca19fe799f5593326ef39d27c94fd260ffb889c02a1038a2dbf93c1aca0d679eb878ac535a9934622fa38fca6bd538f03e3cbbc10de23fc50531e103a960636a072416ba7c23e201efdd3df06af1201a5fb5c51e3ef1493ab7aec157039107f2a45dc3c514fa93bb197e2e0aee4101e56b21a8c0f23ad0e3c89a4948205c41f6c14f6ef6b25a6fd6956d72bdf0dde3192c4eb34464d733a90de2ebfcb8f03d2b9b228d5a1fe6e032ac854a4a0b8483ccf51404cdf204b9f818e60dc2754efab1f702863c533e1be51a1a2098a6d0003737a039d9f2eab4275b3973a1e8451583435d752b99db6d49f9dcc6a510e04906e9b6c7f415394c746e9086577bfe9cbdb4bae462be62b5140daae85f4c7ce7079233cd62b74f2f7583e10e2c7137e59ac1477be0afb816941b35ddc6a8fdfb9eb227f97b8dfd16e8121d3de520a953663e3b0604bd7d140e1598c84f4596475c9e7befcc97016b8fba7cfa3d43e0d2730486069047f43c2d2fbf42e5e6fc6a17de3e64046ffe3b900d95c1ac1b3b4add3e070448e58e5565efe8fb1293bd11617e5f601b835570ddc13ce67eda28de999f8eb0a8654198832ae5998137ff4bdd4cb0488148197a14abe373aa1729ff807be1d997f36638cd917ab9f4558e1f6fff5dde821daa6f2226854c3287041bd00f5cd520d3d2d539f6903cf98df4b314f65be1bc2bdd26494c140a28d51824678e928a84898f06787ab7eaed80cc99e54371e18eeb7a4fe66225359c949011d741a5ab7fbb431d3b7575c66943b5d68be0579121f00e8d0b14c4fca8a3094e969bcd9d5d6ab7b4684a306a167154bde71ba06b75d990d3346cfad8d759cf61cb8b98927ffb6f3e3524055bfd86c5887ba1020338d4765d508d7829edcc1bfb2cf647fb2803eb8ed3a0e9d276030bf2c1d5cab7ee0b2689184343fc393b28bc15adc35647a65b62b9c800d7d533b033e044a5f9870c69727e4150bea6cf2d5ae98494ada53b15c9c998a6bac4a" },

Our implementation matches the result.

celic commented 2 years ago

I get the same result.

livebe01 commented 2 years ago

The fix for this is now on Demo, v1.1.0.25.

livebe01 commented 2 years ago

The fix for this is now on Prod in release v1.1.0.25.