-
Notifications
You must be signed in to change notification settings - Fork 173
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
WIP: add new version of intoto type #1279
Conversation
Signed-off-by: Bob Callaway <bcallaway@google.com>
Signed-off-by: Bob Callaway <bcallaway@google.com>
Signed-off-by: Bob Callaway <bcallaway@google.com>
Signed-off-by: Bob Callaway <bcallaway@google.com>
sorry, quick comment before I dive in: wdyt about renaming to a dsse type? i was talking with @laurentsimon and had a huge amount of confusion myself about what content/payloadHash referred to, and if it's a dsse type it could be better named as predicate/envelopeHash I think this came up in an issue before but I can't find a reference. |
In
I would presume that a more generically named |
Signed-off-by: Bob Callaway <bcallaway@google.com>
My two cents: I think it would be a bit cleaner to have the type referring to the outer envelope, e.g. rekor/pkg/types/cose/v0.0.1/entry.go Line 115 in fd8e054
The risk with this is of course that there can be a combinatorial explosion of "subtypes" in the To start with we may only recognize |
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.
Just some very minor comments, but overall this is so nicely organized, multiple signatures are covered, and so on.
return errors.New("either proposed content or envelope hash, payload hash, and signatures must be present") | ||
} | ||
v.IntotoObj = *intotoObj | ||
return nil |
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.
Because this is incoming proposed content with canonicalized fields, shouldn't verifyEnvelope
still apply here for robustness?
What if a user proposed a ProposedEntry
with pre-canonicalized fields, let's say, because it wants to hide the Envelope
content? It should still be verifiable, assuming verification over a hash.
But if it's clear that users MUST be publishing full envelope content to the log, fine with me.
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.
Only hesitation here is that I am wondering if it's possible for a client to be setting the EnvelopeHash
, PayloadHash
, and Signatures
of a bogus signed envelope and bypassing validation (not through the CLI, but through setting a proposed entry in the createentry handler)
Summary
This adds
v0.0.3
of the intoto type, aiming to address a few issues in v0.0.2:Release Note
Adds new default version of intoto type
FYI @pxp928 @bdehamer