Closed jvdsn closed 2 months ago
@livebe01 @jbrock24 gentle reminder about this PR. Is there anything you need from me to close this?
@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.
@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.
Thanks @jvdsn. You're right. The change is trivial. You can leave this PR open. We'll update https://github.com/usnistgov/ACVP-Server/issues/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.
This fix has been staged to be included in the next hotfix for deployment to ACVTS Demo.
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.