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

Fix AES-GCM-SIV decryption #308

Closed
wants to merge 1 commit into from
Closed

Fix AES-GCM-SIV decryption #308

wants to merge 1 commit into from

Conversation

jvdsn
Copy link

@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
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
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
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
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
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