Skip to content

Fix AES-GCM-SIV decryption#308

Closed
jvdsn wants to merge 1 commit intousnistgov:masterfrom
jvdsn:patch-1
Closed

Fix AES-GCM-SIV decryption#308
jvdsn wants to merge 1 commit intousnistgov:masterfrom
jvdsn:patch-1

Conversation

@jvdsn
Copy link
Copy Markdown

@jvdsn jvdsn commented Jan 25, 2024

As shown in issue #305, the ACVP server doesn't handle AES-GCM-SIV decryption failures properly. The missing result.TestPassed = false; seems to be the cause (compared to GCM: https://github.com/usnistgov/ACVP-Server/blob/master/gen-val/src/orleans/src/NIST.CVP.ACVTS.Libraries.Orleans.Grains/Aead/OracleObserverAesGcmCaseGrain.cs#L72-L76)

Obviously this PR is untested on my end, considering I don't have an ACVP server :)

Note that this bug is also present in https://github.com/usnistgov/ACVP-Server/blob/master/gen-val/json-files/ACVP-AES-GCM-SIV-1.0/ as explained in the aforementioned issue. Those files would need to be updated.

@jvdsn
Copy link
Copy Markdown
Author

jvdsn commented Mar 12, 2024

@livebe01 @jbrock24 gentle reminder about this PR. Is there anything you need from me to close this?

@celic
Copy link
Copy Markdown
Collaborator

celic commented Mar 27, 2024

@jvdsn as a heads up, we cannot merge in PRs for this repository. We have an internal repository that we publish here when we have a release put together. We would need to incorporate these changes into that internal repository, and close out the PR.

@jvdsn
Copy link
Copy Markdown
Author

jvdsn commented Mar 27, 2024

@celic that is fair, you are free to apply this change internally and publish it in a new server release. I simply want to see the issue resolved. I believe this change is so trivial it wouldn't even be copyrightable, but if it is I hereby release it in the public domain to be used or reproduced without any restrictions.

@livebe01
Copy link
Copy Markdown
Collaborator

livebe01 commented Apr 3, 2024

Thanks @jvdsn. You're right. The change is trivial. You can leave this PR open. We'll update #305 and close this PR after we've incorporated the changes. We'll get this out as part of the v1.1.0.35 release.

@livebe01
Copy link
Copy Markdown
Collaborator

livebe01 commented May 7, 2024

This fix has been staged to be included in the next hotfix for deployment to ACVTS Demo.

@livebe01 livebe01 closed this May 7, 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

Successfully merging this pull request may close these issues.

3 participants