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.

Sign up for GitHub

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

Jump to bottom

Add Rekor OID info. #1390

Merged
merged 4 commits into from
Apr 28, 2023
Merged

Add Rekor OID info. #1390

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
543a271
Add Rekor OID info.
wlynch Mar 16, 2023
bd43407
oid.md: Add tag type.
wlynch Mar 16, 2023
e93bd1d
Remove individual OID fields for single TransparencyLogEntry OID.
wlynch Apr 7, 2023
565ed27
oid: Use UTF8STRING for TransparencyLogEntry.
wlynch Apr 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions oid-info.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# Rekor OID Information

## Description

This document defines Rekor
[OID values](https://github.com/sigstore/sigstore/blob/main/docs/oid-info.md).

Rekor reserves the `1.3.6.1.4.1.57264.3` OID root for all of its values.

## Directory

| OID | Name | Description |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

drive-by: would it make sense to use a single OID for a "sigstore bundle" that is using the spec at protobuf-specs?

(That would avoid a whole mess of comments that we've sorted out there: e.g. what encoding is the Log ID, is Verification required, what encoding is the root hash, is the log index the global, etc)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My only concern is there would be duplicated data that may be in other locations (body/attestation) - for gitsign body is the main concern, but as long as it's a hashed checksum I guess it wouldn't be that much more?

Let me try playing around with this in my draft PR and see how much of a pain it is.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To confirm, do you mean to turn this into the sigstore bundle OID instead of Rekor?

I think what's here should be transferable to the protobuf-specs spec? Though I think some of the naming isn't aligned.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but as long as it's a hashed checksum I guess it wouldn't be that much more?

Yes, if it's not an attestation it would just be the checksum of the body.

Yes! I was suggesting turning it into a sigstore bundle OID

Copy link
Member Author

@wlynch wlynch Mar 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, tried it out using https://pkg.go.dev/github.com/sigstore/cosign/v2/pkg/cosign/bundle#RekorBundle as a proxy (let me know if there are major differences between this and protobuf-specs). I don't think we want to do this.

There's a few fields that end up being duplicated across the signature with the bundle:

  • content hash
  • signature
  • cert chain
  • rfc3161 signed timestamp
  • canonicalized_body (which if it acts like the cosign RekorBundle, includes the base64 encoded HashedRekord entry which re-encodes the hash, signature, and cert chain again)

This ends up ~doubling (and if we used the full protobuf-spec bundle perhaps tripling) the size of the signature to store data that we already have elsewhere in the PKCS7 signature.

What would be better is if there was a modified version of TransparencyLogEntry, but let the client re-construct the canonicalized_body from other data it already has.

Maybe we could do this by nil-ing out the field and encoding that? Otherwise we can stick the original plan of just defining OIDs for the individual values. Open to ideas here - let me know what you think!

(aside - long term I'd love to offer a --- BEGIN SIGSTORE BUNDLE--- signature type and encode the proto bundle as is, but I don't think we have that luxury based on https://git-scm.com/docs/signature-format)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess if @haydentherapper is OK with adding Rekor specific OID, that's fine - if something changes with the semantics then I worry this won't be maintained, but if GitSign is one of the only consumers that's OK

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there's a way to do this for the protos, definitely on board.

Another option is to move these OIDs back to Gitsign and not have this in Rekor, if Gitsign really is the only client.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The more I think about it, maybe that is the best way to proceed. GitSign uses .3, defines its own child OIDs, we concurrently define OIDs for the bundle, and then at some point, GitSign adopts the bundle and these new OIDs.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the likelihood of detangling canonicalized_body from the rest of the TransparencyLogEntry in protobuf-specs? I think that would be my preference since that would probably be easiest to maintain long term. If you need me to open an issue elsewhere for discussion lmk!

For the OIDs, the initial idea was to put this in Rekor since we're describing Rekor types. Functionally it's fine if this lives in Gitsign, but I'd worry these may diverge over time.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened sigstore/protobuf-specs#72 for more discussion

| ------------------------- | ---------------------- | ------------------------------------------------------------------------------------------------------------- |
| 1.3.6.1.4.1.57264.3.1 | Integrated Time | When the data was added to the log. |
wlynch marked this conversation as resolved.
Show resolved Hide resolved
| 1.3.6.1.4.1.57264.3.2 | Log ID | This is a SHA256 hash of the DER-encoded public key for the log at the time the entry was included in the log |
| 1.3.6.1.4.1.57264.3.3 | Log Index | The index of the entry in the transparency log. |
| 1.3.6.1.4.1.57264.3.4 | Verification | Log Entry Verification data. |
wlynch marked this conversation as resolved.
Show resolved Hide resolved
| 1.3.6.1.4.1.57264.3.4.1 | Inclusion Proof | Proof of inclusion on the transparency log. |
wlynch marked this conversation as resolved.
Show resolved Hide resolved
| 1.3.6.1.4.1.57264.3.4.1.1 | Checkpoint | The checkpoint (signed tree head) that the inclusion proof is based on. |
| 1.3.6.1.4.1.57264.3.4.1.2 | Hashes | A list of hashes required to compute the inclusion proof, sorted in order from leaf to root. |
| 1.3.6.1.4.1.57264.3.4.1.3 | Root Hash | The hash value stored at the root of the merkle tree at the time the proof was generated. |
| 1.3.6.1.4.1.57264.3.4.1.4 | Tree Size | The size of the merkle tree at the time the inclusion proof was generated. |
| 1.3.6.1.4.1.57264.3.4.2 | Signed Entry Timestamp | Base64 encoded signature of the Inclusion Proof |
wlynch marked this conversation as resolved.
Show resolved Hide resolved

### Notes:

- This is not an exhaustive list of values included in a LogEntry.
- Log Index not included in Inclusion Proof since it's already defined in `1.3.6.1.4.1.57264.3.3`.