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

in-toto records don't contain signatures #582

Closed
Tracked by #1005
mikhailswift opened this issue Jan 3, 2022 · 5 comments · Fixed by #973
Closed
Tracked by #1005

in-toto records don't contain signatures #582

mikhailswift opened this issue Jan 3, 2022 · 5 comments · Fixed by #973
Labels
bug Something isn't working ga_blocker

Comments

@mikhailswift
Copy link
Contributor

Currently the in-toto type does not contain any signatures. This prevents users of in-toto records from verifying attestations that are stored in rekor's attestation stores.

Additionally, the IntotoObj.content.hash refers to the hash of the entire DSSE envelope where the Attestation in the stores only contains the payload of the DSSE. This prevents us from ensuring the attestation returned to us from Rekor's stores is what the IntotoObj on the merkle tree is describing unless we can reassemble the entire DSSE envelope byte-for-byte, which includes the signature.

@SantiagoTorres proposes storing the entire DSSE envelope as the Attestation

@mikhailswift mikhailswift added the bug Something isn't working label Jan 3, 2022
@dlorenc
Copy link
Member

dlorenc commented Jan 4, 2022

Stepping back a bit, I think we might want to rename (or maybe alias for compatibility) the in-toto type to be a DSSE envelope, since that's more semantically correct. An envelope stores:

{
  "payload": "<Base64(SERIALIZED_BODY)>",
  "payloadType": "<PAYLOAD_TYPE>",
  "signatures": [{
    "keyid": "<KEYID>",
    "sig": "<Base64(SIGNATURE)>"
  }]
}

A rekor entry is meant to encapsulate everything needed to verify the signature on a signed object. So we'd want to store:

  • The hash of the "signed" content
  • The public key(s)
  • The signature(s)

Since envelopes can contain multiple signatures, we have a few options:

  1. Store one entry in rekor for one envelope, validating all the signatures (and then requiring multiple public keys)
  2. Store one entry in rekor for one envelope, validating at least one of the signatures (and only requiring one public key)
  3. Store multiple entries in rekor for one envelope, each with one valid signature and key

I think I prefer option 1 here, but I'm not strongly convinced.

Since DSSE uses PAE, we'll need to make sure the hash we store is in the PAE format, which includes the payloadType.

@asraa
Copy link
Contributor

asraa commented Jan 12, 2022

How often does someone append signatures on to an envelope and then trigger an upload without knowing the public key of the previous sig? If (1) is locked in, it would prevent someone from uploading to rekor in that situation.

(2) seems like a bad idea. The choice of the public key that would be in the entry isn't canonical, so someone reconstructing the entry wouldn't know ahead of time. I have a preference for (1) or (3)

Would you imagine (3) would deconstruct the envelope and throw away the sigs whose public keys aren't provided?

@mikhailswift
Copy link
Contributor Author

For what my 2 cents is worth,

How often does someone append signatures on to an envelope and then trigger an upload without knowing the public key of the previous sig?

This is a really good point. I personally can't imagine a case where I'd append a signature to something without verifying existing signatures but there's nothing preventing that use case from happening.

Would you imagine (3) would deconstruct the envelope and throw away the sigs whose public keys aren't provided?

I think throwing away unverified signatures may be the only real option there otherwise it would seem to have the same issue of option 2

@mikhailswift
Copy link
Contributor Author

mikhailswift commented Jan 13, 2022

I've only been able to work on this sporadically through the last week, but so far this is the rough code I have going down the path of option 1:

testifysec@14666fd

Haven't had a chance to test it yet

@bobcallaway
Copy link
Member

this should be fixed by #973

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ga_blocker
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants