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.

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

Better docs on how to verify signedEntryTimestamp #1943

Open
ernoc opened this issue Jan 9, 2024 · 10 comments
Open

Better docs on how to verify signedEntryTimestamp #1943

ernoc opened this issue Jan 9, 2024 · 10 comments
Labels
enhancement New feature or request

Comments

@ernoc
Copy link

ernoc commented Jan 9, 2024

In order to verify signedEntryTimestamp, one needs to follow instructions that I could only find here in this yaml.

That's also the only specification I could find as to what's included in the signature (although I could be mistaken of course).

However, that's not easily discoverable in the docs, in particular, it doesn't show up on swagger docs.

Would be nice if these spec and instructions easily searchable in docs:

@ernoc ernoc added the enhancement New feature or request label Jan 9, 2024
@ernoc
Copy link
Author

ernoc commented Jan 9, 2024

@tiziano88

@ianhundere
Copy link
Contributor

@ernoc there's also info here:

@ernoc
Copy link
Author

ernoc commented Jan 11, 2024

One question that comes to mind from reading the comments at

rekor/openapi.yaml

Lines 433 to 476 in 4fcdcaa

LogEntry:
type: object
additionalProperties:
type: object
properties:
logID:
type: string
pattern: '^[0-9a-fA-F]{64}$'
description: This is the SHA256 hash of the DER-encoded public key for the log at the time the entry was included in the log
logIndex:
type: integer
minimum: 0
body:
type: object
additionalProperties: true
integratedTime:
type: integer
attestation:
type: object
properties:
data:
format: byte
mediaType:
format: string
format: byte
verification:
type: object
properties:
inclusionProof:
$ref: '#/definitions/InclusionProof'
signedEntryTimestamp:
type: string
format: byte
# To verify the signedEntryTimestamp:
# 1. Remove the Verification object from the JSON Document
# 2. Canonicalize the remaining JSON document by following RFC 8785 rules
# 3. Verify the canonicalized payload and signedEntryTimestamp against rekor's public key
description: Signature over the logID, logIndex, body and integratedTime.
required:
- "logID"
- "logIndex"
- "body"
- "integratedTime"
is Step 1 says "Remove the Verification object from the JSON Document".

The JSON document could be referring to:

  1. The entire JSON document, like the result of a curl as explained in https://docs.sigstore.dev/logging/verify-release/ , e.g.:
{
  "b6fdc91e6af5bdd8df133802b7966aa53c1e59365741ee56e287f11263e02c33": {
    "attestation": {},
    "body": "...",
    "integratedTime": 1627461494,
    "logID": "...",
    "logIndex": 25579,
    "verification": {...}
  }
}
  1. Just the LogEntry structure, e.g.:
{
  "attestation": {},
  "body": "...",
  "integratedTime": 1627461494,
  "logID": "...",
  "logIndex": 25579,
  "verification": {...}
}

I'm assuming the second is the right one?

@lkatalin
Copy link
Contributor

lkatalin commented Feb 20, 2024

I have this same problem. I'm currently trying to manually do this by following the logic in cosign and/or in rekor.

I got a dump of the canonicalized entry from rekor and it looks like

{"body":"eyJhcGlWZXJzaW9uIjoiMC4wLjEiLCJraW5kIjoicmVrb3JkIiwic3BlYyI6eyJkYXRhIjp7Imhhc2giOnsiYWxnb3JpdGhtIjoic2hhMjU2IiwidmFsdWUiOiJiNmQ2Y2E0MTAxOTNiYTQ0MDJiYjkwZjExYjgyM2UzNWM4NTc0MTQzMjhkN2M3YWM3YTgzMzQwZGMxYTZjODUyIn19LCJzaWduYXR1cmUiOnsiY29udGVudCI6IkxTMHRMUzFDUlVkSlRpQlRVMGdnVTBsSFRrRlVWVkpGTFMwdExTMEtWVEZPU1ZVd2JFaEJRVUZCUVZGQlFVRkVUVUZCUVVGTVl6Tk9iMHhYVm10TmFsVXhUVlJyUVVGQlFXZE5kREo2U0M5cUwwNUxaRUpxU2tSRVR6aG5id3AyTUVscVptNVFURkZITlhJeEsySXlNMGcwYm5vMlFVRkJRVUZGV20xc2MxcFJRVUZCUVVGQlFVRkJSMk15YUdoT1ZFVjVRVUZCUVZWM1FVRkJRWFI2Q21NeVozUmFWMUY1VGxSVmVFOVJRVUZCUlVGRlEzUk9ZamRWWmtSS1ZHaHNTblJZTVZSYVZrazJjWHBGU201Q1NUQTJTVFpWWTJsR1VVUjFNM3AyWjNVS1FXMTVLMFZzU1hKYVNXWlRTa3RTZG1KdU9HeFJOVGx2ZUhsdmVUUm5UVWxRSzFnemFHSnJSZ290TFMwdExVVk9SQ0JUVTBnZ1UwbEhUa0ZVVlZKRkxTMHRMUzBLIiwiZm9ybWF0Ijoic3NoIiwicHVibGljS2V5Ijp7ImNvbnRlbnQiOiJjM05vTFdWa01qVTFNVGtnUVVGQlFVTXpUbnBoUXpGc1drUkpNVTVVUlRWQlFVRkJTVVJNWkhONEx6UXZlbE51VVZsNVVYZDZka2xMVERsRFNUTTFlbmt3UW5WaE9XWnRPWFI0SzBvNEsyY0sifX19fQ==","integratedTime":1708462670,"logID":"c0d23d6ad406973f9559f3ba2d1ca01f84147d8ffc5b8445c224f98b9591801d","logIndex":72768902}

So far though I can't quite get it to work

$ openssl dgst -sha256 -verify ~/.sigstore/root/targets/rekor.pub -signature tle_set_decoded tle_canon_entry 
Verification failure

I've also tried with a sha256sum of the tle_canon_entry.

@haydentherapper
Copy link
Contributor

See https://github.com/sigstore/cosign/blob/main/pkg/cosign/verify.go#L1311-L1329 - We marshal the body, integrated time, log ID and log index, and then use a json canonicalization library before verifying it. That last step might be what's missing?

@lkatalin
Copy link
Contributor

This works, but it was pretty involved:

$ TLE=$(rekor-cli get --uuid $ARTIFACT_UUID --format tle); \
> JSON=$(rekor-cli get --uuid $ARTIFACT_UUID --format json); \
> echo -n $TLE | jq .inclusionPromise.signedEntryTimestamp | tr -d "'\"" | base64 -d > set_decoded; \
> echo -n {\"body\":$(echo $TLE | jq .canonicalizedBody)\,\"integratedTime\":$(echo $TLE | jq .integratedTime | tr -d "'\"")\,\"logID\":$(echo $JSON | jq .LogID)\,\"logIndex\":$(echo $TLE | jq .logIndex | tr -d "'\"")} > rekor-bundle-70959618; \
> openssl dgst -sha256 -verify ~/.sigstore/root/targets/rekor.pub -signature set_decoded rekor-bundle-70959618
Verified OK

I know the new Sigstore protobuf bundle will include some of this info; but is it correct that for now there is no simpler way to get the entire Rekor bundle other than parsing and stitching together fields from the rekor-cli get output? If so, I will open an issue and take a look at getting it to emit this in a simpler way. One problem is that while the --format tle flag outputs most of the info needed for the SET verification, the logID displayed there is different from what is displayed with the --format json flag (which is what is needed for the Rekor bundle / SET verification). So you end up having to call the rekor-cli with both format flags to get all the output you need to create the Rekor bundle correctly.

--format tle output:

...
"logId": {
    "keyId": "wNI9atQGlz+VWfO6LRygH4QUfY/8W4RFwiT5i5WRgB0="
  },

--format json output:

...
"LogID": "c0d23d6ad406973f9559f3ba2d1ca01f84147d8ffc5b8445c224f98b9591801d"

@haydentherapper
Copy link
Contributor

A feature to output the canonicalized bundle seems reasonable.

@gaetanww
Copy link

The documentation referenced above does not accurately describe how the verification is actually performed.
The actual implementation in rekor does not canonicalize the JSON, but just deserialize the base64 body and verifies the output.

Moreover, the verification procedure above fails with some rekor entries, as it seems that old rekor entries (e.g., index 25579) have not been canonicalized properly. We noticed this issue in a sigstore-rs PR.

Is that a correct assessment? Should the documentation and/or verification procedure be updated?

@lkatalin
Copy link
Contributor

The actual implementation in rekor does not canonicalize the JSON, but just deserialize the base64 body and verifies the output

The SET verification (not inclusionProof verification) is doing canonicalization here. This is evident in the manual testing I did recently, where the payload for the SET always has the fields in alphabetical order: body, integratedTime, logID, logIndex.

The documentation should definitely be updated - I plan to work on that after adding the canonicalized Rekor bundle to rekor-cli output (draft PR here and I'm planning to make a Rekor PR based on the protobuf).

I'm not sure about the verification failure for older entries, maybe someone else knows about this.

@haydentherapper
Copy link
Contributor

Left a comment on the issue on protobuf-specs.

I'm not sure about the verification failure. That could be due to a change in requirements for the type? Early on there were a few changes that should have warranted a new type that we didn't do. We can relax those constraints if need be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants