-
Notifications
You must be signed in to change notification settings - Fork 12
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
Add support for Notary claims #73
Conversation
Co-authored-by: Paul Liétar <lietarpaul@microsoft.com>
Signed-off-by: Maik Riechert <maik.riechert@microsoft.com>
|
||
// alg, crit, cty, io.cncf.notary.signingScheme are required. | ||
|
||
check_is_accepted_algorithm(phdr, configuration); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO this belongs to the generic verifier code path. All profiles are going to have an algorithm, and we should always enforce this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm open to be convinced about this, at the moment I think it's clearer for each profile
to contain all of the validation it requires (including an initial call to do generic validation). Though I don't think it makes much difference either way.
Implementing hangover review comments from #73 addressing #84 Featuring: - x5chain (de)serialization - QCBOR tweaks - Cleaning up redundant code - Notary spec allows x5chain in the protected header (but currently notation always puts it in the unprotected headers). We now check for the x5chain in the protected header of notary claims before the unprotected header. --------- Co-authored-by: Paul Liétar <lietarpaul@microsoft.com>
This PR allows claims generated by
notation
to be submitted to SCITT.Note:
crit
. This PR brings back a lot of code to work around that limitation so that we can verify Notary claims' signatures without using t_cose.crit
then we should remove the workaround code which is marked by comments stating the code is "Temporarily needed for notary_verify().".