Skip to content
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

Server does not handle AES-GCM-SIV decryption properly #305

Closed
jvdsn opened this issue Jan 17, 2024 · 7 comments
Closed

Server does not handle AES-GCM-SIV decryption properly #305

jvdsn opened this issue Jan 17, 2024 · 7 comments
Assignees
Milestone

Comments

@jvdsn
Copy link

jvdsn commented Jan 17, 2024

environment
Demo

testSessionId
475095

vsId
2098825

Algorithm registration

{
        "revision":"1.0",
        "algorithm":"ACVP-AES-GCM-SIV",
        "direction":[
          "encrypt",
          "decrypt"
        ],
        "keyLen":[
          128,
          256
        ],
        "payloadLen":[
          128,
          256,
          120,
          248
        ],
        "ivLen":[
          96
        ],
        "aadLen":[
          128,
          256,
          120,
          0
        ]
      }

I am trying to test AES-GCM-SIV using BoringSSL. Encryption works great (no failures), however I'm having a strange issue with decryption. For decryption, the server sometimes gives me test vectors for which BoringSSL thinks the tag is invalid. This is quite expected to me and is in line with other tests (e.g. GCM and CCM). However, when I try to submit "testPassed":false to the server, the server complains that there should actually be a plaintext! So, somehow the server thinks that the tag is valid.

This issue doesn't happen all the time, it seems to happen about 1/4 vectors. For the remaining 3/4 vectors, both BoringSSL and the server agree that the tag is valid. Crucially, when BoringSSL and the server agree, the output plaintext is identical. This doesn't really make sense to me, because that would mean that the decryption routines for BoringSSL and the server function identically.

In fact, at the time of writing, the server does not provide any ciphertexts for which it thinks the tag is invalid. This can be confirmed by requesting vectors with expected values (testId and vsId above). Excerpt:

      {
        "tgId": 33,
        "testType": "AFT",
        "direction": "decrypt",
        "keyLen": 128,
        "payloadLen": 120,
        "aadLen": 0,
        "tests": [
          {
            "tcId": 481,
            "key": "26B45F1FB4CCD5F1F4BDF8E77BCCAC34",
            "aad": "",
            "iv": "C08D685A1EA33790892E5835",
            "ct": "7E21AE09FCB62CFC5B0A893BC8563894DCD7EF84A466DE98DFD12EBB66A38A"
          },
          {
            "tcId": 482,
            "key": "12533E875004D854958697F37166164C",
            "aad": "",
            "iv": "AAABA3196239C6051B17175D",
            "ct": "A86B41124E78DEC2797D3BAC55ECA85D0D29EEA808F832D1D2E565B65B8B2D"
          },
          {
            "tcId": 483,
            "key": "428D39AB7B36BC4BF09E6AB15EA41F95",
            "aad": "",
            "iv": "B8EB04095AA553D7912AF7AF",
            "ct": "65A98850F40AC1F67EA5FF7A6BAC3582D5EA04FFD5927D4B2D6ACA06AC204E"
          },
          {
            "tcId": 484,
            "key": "3CA7AEF34DE325A4F488F4079C43A04D",
            "aad": "",
            "iv": "A1B1E57C5E2D050A523F33CC",
            "ct": "759022B68C8A8CCCCF6AE7604A9E486CCA5053376011326B3E7049223C933F"
          },
          {
            "tcId": 485,
            "key": "9B69FBB2C5B41CD0A8012DEFB688FA0C",
            "aad": "",
            "iv": "9F51BC9ECB32370CE0DE2C01",
            "ct": "4002457CCD8FD97C58A983E0B0C1A891058A76F5520F34D44199E54539EF8F"
          },
          {
            "tcId": 486,
            "key": "B35C7CD35EF585AAB629FC3986B30D12",
            "aad": "",
            "iv": "913AE3FC0EAF4761180D8F24",
            "ct": "60EE82A3B6E618E2836381B2201B72A6079BBCC010737884C42F702D3DE3EB"
          },
          {
            "tcId": 487,
            "key": "3EC9B8DA641C372A23F83FEAF81CDBE9",
            "aad": "",
            "iv": "D43A26476CAFA8EF3E4ACB37",
            "ct": "C3B0AF2C4018B58569792A999FC549104DC94E7CB9BEFB1B0F0D7CE9FC3F69"
          },
          {
            "tcId": 488,
            "key": "E44DFB43B5610BF4BAFF2E68269CCDD2",
            "aad": "",
            "iv": "9A00631104B36A7C6B53A33F",
            "ct": "B4937A15E50D8A7BF2043CEE6153B5CEBCB600EC0CC6DBF16761B84412FA00"
          },
          {
            "tcId": 489,
            "key": "6642D50B87C61206C23B5A00D822E007",
            "aad": "",
            "iv": "A57B2330AC0FC70A36A62D81",
            "ct": "3E8D92DCD372011E7C4BB1E4B2DC7DBC9D71A29CEC0B85FA7F082E7501341B"
          },
          {
            "tcId": 490,
            "key": "09D007E42D7309BA9D698322084A7435",
            "aad": "",
            "iv": "D443593BD7F9A701C6B188B2",
            "ct": "4EC0E145ADC882731DB6FB95047DA99348E7223C803FA1AD0C9BFB85412E75"
          },
          {
            "tcId": 491,
            "key": "A9DFAA4AC6CB87BB38021ED478C8F54B",
            "aad": "",
            "iv": "235A6B4C27526250240EB78A",
            "ct": "B0D7C81130BF0D7E3201D73ED33BF89DFCFF07FD7029F9BA69ADFBF7060A9B"
          },
          {
            "tcId": 492,
            "key": "291D54E817743C1793A8CA03C54B530F",
            "aad": "",
            "iv": "E8EE880C1A5913309340D024",
            "ct": "4588912E224210C15F6342BFD256F2B350BA834039A5E7C78AC8C02A63B649"
          },
          {
            "tcId": 493,
            "key": "969C4A7CF908A0D3FFB228FB2E53B21F",
            "aad": "",
            "iv": "428BC5F11D3F9002B7388BD6",
            "ct": "1EE46855583320DFEAF68B9A449CA083076D7F874BD1AFDCB287151A1F5CE9"
          },
          {
            "tcId": 494,
            "key": "2EF13C245AA56D392703EC670D89B232",
            "aad": "",
            "iv": "8D0951CDBA893F71D1068E96",
            "ct": "FFB276BC8015FB0879997ED3BCE4416B1EBED087B0E59C1185CD3C37030A46"
          },
          {
            "tcId": 495,
            "key": "D7857027076C16C394CF06FD7AD38FD7",
            "aad": "",
            "iv": "25630CD8C4716FEDB22E242D",
            "ct": "BA58D6CE6B68F23470BDEA783822FAAC2778075114D1EC620EC249E2A63C74"
          }
        ]
      },

In this test group, BoringSSL thinks that test case 481 and 487 should not validly decrypt:

      {
        "tgId":33,
        "tests":[
          {
            "tcId":481,
            "testPassed":false
          },
          {
            "tcId":482,
            "pt":"f7e231d8b36f1812fb550a5e84869a"
          },
          {
            "tcId":483,
            "pt":"d2b5f6d36a551ef99c971054006ea0"
          },
          {
            "tcId":484,
            "pt":"18c6dafe9fbfedf089123b9078e6bb"
          },
          {
            "tcId":485,
            "pt":"da98fb622c16dbf1634259ecc8190b"
          },
          {
            "tcId":486,
            "pt":"253709980b2f24a9b02fb0860ac042"
          },
          {
            "tcId":487,
            "testPassed":false
          },
          {
            "tcId":488,
            "pt":"2dec0bce00a501e61ffc50829a9637"
          },
          {
            "tcId":489,
            "pt":"958e0a453a1cabdd73398a95ebc576"
          },
          {
            "tcId":490,
            "pt":"e6da228bf371a7fd51ce694e7bf4dc"
          },
          {
            "tcId":491,
            "pt":"19e3adaa0dd3d4c4f127467d363d6e"
          },
          {
            "tcId":492,
            "pt":"2c9c3a690102a74ae3c14a567a76ac"
          },
          {
            "tcId":493,
            "pt":"bfe58e48478a4303e32334fdcc0267"
          },
          {
            "tcId":494,
            "pt":"ec4cd37738e91201c784a563eb44ea"
          },
          {
            "tcId":495,
            "pt":"d9c38e0a51d69383e918570b357e93"
          }
        ]
      },

However, the expected values returned by the server are:

      {
        "tgId": 33,
        "tests": [
          {
            "tcId": 481,
            "pt": "46B52AB340B0F02776ED60B2C8E442"
          },
          {
            "tcId": 482,
            "pt": "F7E231D8B36F1812FB550A5E84869A"
          },
          {
            "tcId": 483,
            "pt": "D2B5F6D36A551EF99C971054006EA0"
          },
          {
            "tcId": 484,
            "pt": "18C6DAFE9FBFEDF089123B9078E6BB"
          },
          {
            "tcId": 485,
            "pt": "DA98FB622C16DBF1634259ECC8190B"
          },
          {
            "tcId": 486,
            "pt": "253709980B2F24A9B02FB0860AC042"
          },
          {
            "tcId": 487,
            "pt": "07F4D034B06863579FAD2C85F47174"
          },
          {
            "tcId": 488,
            "pt": "2DEC0BCE00A501E61FFC50829A9637"
          },
          {
            "tcId": 489,
            "pt": "958E0A453A1CABDD73398A95EBC576"
          },
          {
            "tcId": 490,
            "pt": "E6DA228BF371A7FD51CE694E7BF4DC"
          },
          {
            "tcId": 491,
            "pt": "19E3ADAA0DD3D4C4F127467D363D6E"
          },
          {
            "tcId": 492,
            "pt": "2C9C3A690102A74AE3C14A567A76AC"
          },
          {
            "tcId": 493,
            "pt": "BFE58E48478A4303E32334FDCC0267"
          },
          {
            "tcId": 494,
            "pt": "EC4CD37738E91201C784A563EB44EA"
          },
          {
            "tcId": 495,
            "pt": "D9C38E0A51D69383E918570B357E93"
          }
        ]
      },

Then, I inspected BoringSSL using GDB to review what the plaintext would be, assuming BoringSSL didn't discard it because of the tag issue. This is the result: D20F630C13A5A939E7D09F552F2A0F. This does not match at all what the server thinks the plaintext would be. However, we know that for other test cases (e.g. 482), BoringSSL and the server fully agree on the plaintext.

Perhaps this is really a problem in BoringSSL? I tried to replicate this using OpenSSL. OpenSSL added AES-GCM-SIV in version 3.2.0 and as far as I can see, its implementation is independent from BoringSSL (which added AES-GCM-SIV a long time ago). This is the code I used:

#include <assert.h>
#include <openssl/core_names.h>
#include <openssl/evp.h>
#include <string.h>

unsigned char *hex_to_bin(char *hex, size_t *bin_len) {
	assert(strlen(hex) % 2 == 0);
	*bin_len = (size_t)strlen(hex) / 2;
	unsigned char *bin = malloc(*bin_len);
	for (size_t i = 0; i < *bin_len; i++) {
		sscanf(&hex[2 * i], "%2hhx", &bin[i]);
	}
	return bin;
}

void encrypt(char *key_hex, char *aad_hex, char *iv_hex, char *pt_hex) {
	size_t key_len;
	unsigned char *key = hex_to_bin(key_hex, &key_len);
	size_t aad_len;
	unsigned char *aad = hex_to_bin(aad_hex, &aad_len);
	size_t iv_len;
	unsigned char *iv = hex_to_bin(iv_hex, &iv_len);
	size_t pt_len;
	unsigned char *pt = hex_to_bin(pt_hex, &pt_len);
	size_t tag_len = 16;
	unsigned char *tag = malloc(tag_len);

	unsigned char *ct = malloc(pt_len);
	unsigned int tmp;

	EVP_CIPHER *cipher = EVP_CIPHER_fetch(NULL, "AES-128-GCM-SIV", NULL);
	assert(cipher != NULL);
	EVP_CIPHER_CTX *ctx = EVP_CIPHER_CTX_new();
	assert(ctx != NULL);
	assert(EVP_EncryptInit_ex2(ctx, cipher, NULL, NULL, NULL) == 1);
	assert(EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_AEAD_SET_IVLEN, iv_len, NULL) == 1);
	assert(EVP_EncryptInit_ex2(ctx, NULL, key, iv, NULL) == 1);
	assert(EVP_CIPHER_CTX_set_padding(ctx, 0) == 1);
	assert(EVP_EncryptUpdate(ctx, NULL, &tmp, aad, aad_len) == 1);
	assert(EVP_EncryptUpdate(ctx, ct, &tmp, pt, pt_len) == 1);
	assert(EVP_EncryptFinal_ex(ctx, ct + tmp, &tmp) == 1);
	assert(EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_AEAD_GET_TAG, tag_len, tag) == 1);

	printf("Ciphertext: ");
	for (size_t i = 0; i < pt_len; i++) {
		printf("%02x", ct[i]);
	}
	for (size_t i = 0; i < tag_len; i++) {
		printf("%02x", tag[i]);
	}
	printf("\n");
}

void decrypt(char *key_hex, char *aad_hex, char *iv_hex, char *ct_hex) {
	size_t key_len;
	unsigned char *key = hex_to_bin(key_hex, &key_len);
	size_t aad_len;
	unsigned char *aad = hex_to_bin(aad_hex, &aad_len);
	size_t iv_len;
	unsigned char *iv = hex_to_bin(iv_hex, &iv_len);
	size_t ct_len;
	unsigned char *ct = hex_to_bin(ct_hex, &ct_len);
	// Tag is the last 16 bytes of ct.
	size_t tag_len = 16;
	assert(ct_len >= tag_len);
	ct_len -= tag_len;
	unsigned char *tag = ct + ct_len;

	unsigned char *pt = malloc(ct_len);
	unsigned int tmp;

	EVP_CIPHER *cipher = EVP_CIPHER_fetch(NULL, "AES-128-GCM-SIV", NULL);
	assert(cipher != NULL);
	EVP_CIPHER_CTX *ctx = EVP_CIPHER_CTX_new();
	assert(ctx != NULL);
	assert(EVP_DecryptInit_ex2(ctx, cipher, NULL, NULL, NULL) == 1);
	assert(EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_AEAD_SET_IVLEN, iv_len, NULL) == 1);
	assert(EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_AEAD_SET_TAG, tag_len, tag) == 1);
	assert(EVP_DecryptInit_ex2(ctx, NULL, key, iv, NULL) == 1);
	assert(EVP_CIPHER_CTX_set_padding(ctx, 0) == 1);
	assert(EVP_DecryptUpdate(ctx, NULL, &tmp, aad, aad_len) == 1);
	assert(EVP_DecryptUpdate(ctx, pt, &tmp, ct, ct_len) == 1);
	if (EVP_DecryptFinal_ex(ctx, pt + tmp, &tmp) != 1) {
		printf("DECRYPTION FAILED!\n");
		return;
	}

	printf("Plaintext: ");
	for (size_t i = 0; i < ct_len; i++) {
		printf("%02x", pt[i]);
	}
	printf("\n");
}

int main() {
	// "tcId": 481.
	encrypt("26B45F1FB4CCD5F1F4BDF8E77BCCAC34", "", "C08D685A1EA33790892E5835", "D20F630C13A5A939E7D09F552F2A0F");
	encrypt("26B45F1FB4CCD5F1F4BDF8E77BCCAC34", "", "C08D685A1EA33790892E5835", "46B52AB340B0F02776ED60B2C8E442");
	decrypt("26B45F1FB4CCD5F1F4BDF8E77BCCAC34", "", "C08D685A1EA33790892E5835", "7E21AE09FCB62CFC5B0A893BC8563894DCD7EF84A466DE98DFD12EBB66A38A");
	// "tcId": 482.
	encrypt("12533E875004D854958697F37166164C", "", "AAABA3196239C6051B17175D", "F7E231D8B36F1812FB550A5E84869A");
	decrypt("12533E875004D854958697F37166164C", "", "AAABA3196239C6051B17175D", "A86B41124E78DEC2797D3BAC55ECA85D0D29EEA808F832D1D2E565B65B8B2D");
}

and this is the output:

$ ./tmp                                                                                  
Ciphertext: 39ff918ea2852ba87128664d0a1ea2e8c119c07c6d82f09e708e6f86f67f52
Ciphertext: 700cac784dcd21dd6c6051cdfd12b8d0f5abb67e58138a013ed4f7dbd14c1f
DECRYPTION FAILED!
Ciphertext: a86b41124e78dec2797d3bac55eca85d0d29eea808f832d1d2e565b65b8b2d
Plaintext: f7e231d8b36f1812fb550a5e84869a

The first statement tries to encrypt what BoringSSL thinks the plaintext would be. Note that the result doesn't match the provided ciphertext for test case 481. The second statement tries to encrypt what the server thinks the plaintext is. Again, the result also doesn't match the provided ciphertext for 481. The third statement tries to decrypt the provided ciphertext for 481, using OpenSSL's implementation. This decryption fails, as with BoringSSL, due to an error (presumably invalid tag).

Then, the fourth statement encrypts what BoringSSL and the server agree the plaintext is for 482. The result of the encryption is indeed the provided ciphertext for 482. Finally, the fifth statement decrypts the provided ciphertext for 482. BoringSSL, OpenSSL, and the server agree that the result is f7e231d8b36f1812fb550a5e84869a.

I looked at the server source code for a bit but I couldn't find an obvious issue. Without access to a running server it is quite hard to debug why exactly the server returns 46B52AB340B0F02776ED60B2C8E442 for 481. Still, please let me know if I can be of any help. I will try to find another independent (perhaps Rust or Python) AES-GCM-SIV implementation to see if this issue exists there as well.

@jvdsn
Copy link
Author

jvdsn commented Jan 17, 2024

Seems like the server should in fact send failing test cases about 1/4 times: https://github.com/usnistgov/ACVP-Server/blob/master/gen-val/src/orleans/src/NIST.CVP.ACVTS.Libraries.Orleans.Grains/Aead/OracleObserverAesGcmSivCaseGrain.cs#L19. Why does the server later think those cases are valid?

@dspdon
Copy link

dspdon commented Jan 24, 2024

Joachim,

I hope you don't mind: I'm an interested third-party, not affiliated with NIST. I ran your set of test cases above on an independent implementation of GCM-SIV. I confirmed tcId 481 and 487 fail tag validation, while all other test cases pass both tag validation and plaintext match to expected results. If you have additional test cases that would benefit from independent confirmation, I'd be happy to provide further results.

Are you able to run YOUR implementation against the test cases defined in the "offline" ACVP-Server test file for GCM-SIV? It's in the folder:

ACVP-Server/tree/master/gen-val/json-files/ACVP-AES-GCM-SIV-1.0

I found that all of the GCM-SIV encrypt test cases in that file pass, while a number of decrypt test cases fail on tag validation for the cipher implementations I've tested. The test file indicates those failing test cases should pass however, so there's the issue I've been diagnosing. My study of these cases suggests there may be a tag computation issue by ACVP-Server. If you are able to run the test cases in that file, your results would be helpful / confirmatory.

FWIW, the GCM-SIV implementation I used on your test cases passes every test case in the specs and repositories I rely upon (e.g., RFC8452, BoringSSL, Wycheproof, metalnem, ...). It's only the handful of decrypt test cases in the NIST ACVP-Server "offline file" that are producing failures.

--Don

@jvdsn
Copy link
Author

jvdsn commented Jan 25, 2024

@dspdon yes, that file has the same issue. Specifically, for tcId 62, 66, 70, 72, 73, 80, 81, 82, 85, 91, 95, 98, 103, 107, 109, 111, 113, and 115, BoringSSL reports a tag validation failure. However, the expectedResults.json does not contain any of those values.

In fact, I believe I found the issue. Compare https://github.com/usnistgov/ACVP-Server/blob/master/gen-val/src/orleans/src/NIST.CVP.ACVTS.Libraries.Orleans.Grains/Aead/OracleObserverAesGcmCaseGrain.cs#L72-L76 (AES-GCM):

                if (shouldFail)
                {
                    result.Tag = _rand.GetDifferentBitStringOfSameSize(result.Tag);
                    result.TestPassed = false;
                }

with https://github.com/usnistgov/ACVP-Server/blob/master/gen-val/src/orleans/src/NIST.CVP.ACVTS.Libraries.Orleans.Grains/Aead/OracleObserverAesGcmSivCaseGrain.cs#L77-L80 (AES-GCM-SIV):

                if (shouldFail)
                {
                    result.CipherText = _rand.GetDifferentBitStringOfSameSize(result.CipherText);
                }

The server never sets result.TestPassed = false;!! I opened #308 in the hopes NIST can resolve this.

@dspdon
Copy link

dspdon commented Jan 25, 2024

Same set of 18 failed test cases that I see. Thanks for confirming this, and for the link to the PR you opened. Nice job on that.

@livebe01
Copy link
Collaborator

livebe01 commented May 2, 2024

We plan to address this as part of our next hotfix deployment. I'll update this ticket after the fix has been deployed to Demo.

@livebe01 livebe01 added this to the v1.1.0.34 milestone May 7, 2024
@livebe01
Copy link
Collaborator

The fix for this is now on Demo as part of the v1.1.0.34 release.

@livebe01
Copy link
Collaborator

livebe01 commented Jun 6, 2024

The fix for this is now on Prod as part of the v1.1.0.34 release.

@livebe01 livebe01 closed this as completed Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants