Define a new sigstore Bundle type to include additional verification data and materials #2422
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,86 @@ | ||
| // Copyright 2022 The Sigstore Authors. | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| package bundle | ||
|
|
||
| import ( | ||
| "github.com/sigstore/rekor/pkg/generated/models" | ||
| ) | ||
|
|
||
| type Bundle struct { | ||
| VerificationData | ||
|
There was a problem hiding this comment. What's the difference between There was a problem hiding this comment. It might be better to port that comment to the upstream bundle, but these are terms being used by those protos. There was a problem hiding this comment. I am gonna port these and other questions to the upstream bundle. Thanks. |
||
| VerificationMaterial | ||
| } | ||
|
|
||
| // VerificationData contains extra data that can be used to verify things | ||
| // such as transparency logs and timestamped verifications. | ||
| type VerificationData struct { | ||
| // RekorPayload holds metadata about recording a Signature's ephemeral key to | ||
| // a Rekor transparency log. | ||
| // Use Payload instead of TransparencyLogEntry to keep backwards compatibility. | ||
| Payload RekorPayload | ||
|
There was a problem hiding this comment. I kept the |
||
|
|
||
|
There was a problem hiding this comment. I am awaiting feedback before adding other fields such as the There was a problem hiding this comment. Those fields overlap with There was a problem hiding this comment. Good point! That could easily happen. |
||
| // TimestampVerificationData holds metadata about a timestamped verification. | ||
| TimestampVerificationData | ||
| } | ||
|
|
||
| // VerificationMaterial captures details on the materials used to verify | ||
| // signatures or any additional timestamped verifications. | ||
| type VerificationMaterial struct { | ||
| // A chain of X.509 certificates. | ||
| CertBytes []byte | ||
|
There was a problem hiding this comment. I'd expect this to be a I guess really...is this struct a data format or an object? My preference is always to move parsing as early as possible so we don't have to handle parse errors in the middle of the program logic. |
||
|
|
||
| // PublicKeyIdentifier optional unauthenticated hint on which key to use. | ||
| PublicKeyIdentifier string | ||
| } | ||
|
|
||
| // TimestampVerificationData contains various timestamped data following RFC3161. | ||
|
There was a problem hiding this comment. As we discussed in the protobuf-specs repo, SETs are not just timestamp verification data, hence why we went with this nested format - https://github.com/sigstore/protobuf-specs/blob/main/protos/sigstore_bundle.proto#L53-L59 There was a problem hiding this comment. I created one issue here sigstore/protobuf-specs#13. Hopefully we can get more ppl involved on this discussion. |
||
| type TimestampVerificationData struct { | ||
| // SignedEntryTimestamp holds metadata about a timestamped counter signature over the artifacts signature. | ||
| SignedEntryTimestamp []byte | ||
|
|
||
| // EntryTimestampAuthority contains the recorded entry from timestamp authority server. | ||
| EntryTimestampAuthority []byte | ||
|
There was a problem hiding this comment. I would prefer a name that includes RFC3161 rather than Timestamp Authority, as the former makes it clear what the format of the time record is There was a problem hiding this comment. Yes, it makes sense. I like it. |
||
| } | ||
|
|
||
| func EntryToBundle(tLogEntry *models.LogEntryAnon, signedEntryTimestamp, entryTimestampAuthority, certBytes []byte, pubKeyID string) *Bundle { | ||
|
There was a problem hiding this comment. Yes, we need tests but I wanted to firstly get some feedback, as I expected things might change. |
||
| b := &Bundle{} | ||
| // If none of the verification data is configured then return nil | ||
| if (tLogEntry == nil || tLogEntry.Verification == nil) && len(entryTimestampAuthority) == 0 { | ||
| return nil | ||
|
There was a problem hiding this comment. why return nil over returning an empty bundle? id be worried about accidental dereferences later There was a problem hiding this comment. I agree with you but |
||
| } | ||
| // Add Transparency log entry and a signed timestamp value | ||
| if tLogEntry != nil && tLogEntry.Verification != nil { | ||
| b.Payload = RekorPayload{ | ||
| Body: tLogEntry.Body, | ||
| IntegratedTime: *tLogEntry.IntegratedTime, | ||
| LogIndex: *tLogEntry.LogIndex, | ||
| LogID: *tLogEntry.LogID, | ||
| } | ||
| b.SignedEntryTimestamp = tLogEntry.Verification.SignedEntryTimestamp | ||
|
|
||
| if len(signedEntryTimestamp) > 0 { | ||
| b.SignedEntryTimestamp = signedEntryTimestamp | ||
| } | ||
| } | ||
| // Set the EntryTimestampAuthority from the timestamp authority server | ||
| if len(entryTimestampAuthority) > 0 { | ||
| b.EntryTimestampAuthority = entryTimestampAuthority | ||
| } | ||
| if len(certBytes) > 0 || pubKeyID != "" { | ||
| b.CertBytes = certBytes | ||
| b.PublicKeyIdentifier = pubKeyID | ||
| } | ||
| return b | ||
| } | ||
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.
nit: use nil instead of []byte{}?