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

"other_headers" field in "signatures" field in reference implementation is not part of the spec #55

Closed
nmiyake opened this issue Oct 5, 2021 · 10 comments

Comments

@nmiyake
Copy link
Contributor

nmiyake commented Oct 5, 2021

The spec indicates that signed files should have the format:

{
  "signed" : "<ROLE>",
  "signatures" : [
      { "keyid" : "<KEYID>",
        "sig" : "<SIGNATURE>" }, 
  "..."
  ]
}

However, when I use the reference implementation to sign a link file using a GPG key, the signature object also contains an "other_headers" field:

{
 "signatures": [
  {
   "keyid": "aad6ec15d80aca160e1a0e7041fc235573127eb3",
   "other_headers": "04000108001d162104aad6ec15d80aca160e1a0e7041fc235573127eb30502615bdc17",
   "signature": "5be45bc0d63301d487d3eac7605d0440aba0017fcaef987066d1451ed02e696b53ffa6404bb97698becc69ec4cbe2efc58e4148151aa48c2d0d6b3f5544672cf4d596c4ac0bdff6350e4a8c4a034fae3286e123e3e934c2c14bace75126d8b9e2fea08055a21674875dac6284e1c8eba142cab8f0b14b2547f50135745dc5b051b2b7a59c1e5578b68917ca2d3c061235c0a3c93c649df4140e0acda9b779b2fefd0e767e376afb67c7bde86fd904e75d99efef664ed1561471645b3bb642adf1cf2707819e9246cb71410445e00cc130d237e8904710266411d3bc166ffb3c90407f54d52cc3f5e514bfc8823053a681136a230983c3c2bf38371b456cb1d1eae322119e3a697e0cfed27504eb55bcf8bae8cb33c88ef155477aa97b523834a8efe1efc0fb12de6a32b84c4eb4230682c5554d34e6fd54de6928c675a98df633a1ed17abb77958e2da0b5230c57f7e4c817b42aa86d26a6583fdbb2c96e0d6c46e7bafe02b1d600a146476b7434bd511741414e40309d6d99715f75b1644bb04470311520b76114a4cf75abceaf805f3b2d8ce2b0cca56dbba231116b83b0aabf920b3602e06c334de13ceccb8b5ef9c0f3fd1b85a2c07126d513d9c45ccc54d5036d4a31a4f4299f8081811e83fe6d50aeb85c09959e43f46ce8d9053da9dba330f57faacbadeaa5bc1533fc03bcd8d3f1b24f66ad403691d6118a43df7a74"
  }

Verification fails if this field is removed, so it is clearly important/required in some scenarios, but the spec does not provide any details on this. It also doesn't indicate that a signature object can have other arbitrary/opaque fields.

It would be helpful if the spec could be updated/fixed to properly explain this.

@adityasaky
Copy link
Member

While this is indeed undocumented, I wonder how we should tackle this now that ITE-5 has been accepted. @SantiagoTorres is it time to move the signature spec definition here out, or at least mark as legacy? We should probably take up GPG-specific fields in the signature in the DSSE issue tracker...

@nmiyake
Copy link
Contributor Author

nmiyake commented Oct 6, 2021

@adityasaky thanks for the information on ITE-5. Given that it has been accepted, are there any specific goals/timelines for implementation (or even for updating the actual language of the spec)? I'm looking to set up in-toto in an ecosystem that uses multiple different languages, and the simplifications that would come from using the DSSE format do sound appealing so this is of interest to me.

@adityasaky
Copy link
Member

ecosystem that uses multiple different languages

Will you be using existing in-toto implementations, or do you plan to write your own?

@adityasaky
Copy link
Member

adityasaky commented Oct 6, 2021

So, there are DSSE implementations / developments in a few different places.

There's a library in Go maintained by the in-toto/TUF teams: https://github.com/secure-systems-lab/go-securesystemslib. The plan is for in-toto-golang to use this, and I just opened a ticket to track this: in-toto/in-toto-golang#148.

in-toto-java is getting a significant overhaul, and this again includes DSSE support: in-toto/in-toto-java#24.

For in-toto-python (reference implementation which has hit v1.0), the idea is for DSSE support to live in the current crypto provider, and there's an issue here: secure-systems-lab/securesystemslib#370. There's a Python implementation in the DSSE specification itself (https://github.com/secure-systems-lab/dsse/tree/master/implementation) that securesystemslib can borrow from. After this support is present, we can likely begin by adding it as an option in the Python implementation. The switch to DSSE by default in the Python implementation in particular would necessitate bumping the major version as it follows SemVer. The timeline for this will be controlled via the in-toto roadmap.

@adityasaky
Copy link
Member

I have left out the rust implementation for now, but we haven't had a chance to approach it there yet. I can open a ticket to track it there as well...

@nmiyake
Copy link
Contributor Author

nmiyake commented Oct 6, 2021

Thanks, that extra context is extremely helpful!

I'd like to leverage the existing in-toto implementations as much as possible, but am also making local modifications/writing my own as needed. I may be able to help with contributing to the existing implementations if things line up properly -- will engage in the appropriate issues.

Is there a timeline/plan for updating the spec to incorporate ITE-5? From a practical perspective I think there are a few interactions there -- for example, in DSSE it's made clear that the key ID is optional, but not sure if the in-toto spec side will be opinionated on this.

@nmiyake
Copy link
Contributor Author

nmiyake commented Oct 7, 2021

Noting #56 as well.

Both of these basically have to do with trying to determine the bounds between what is formally described in the spec versus the behavior in the reference implementation (and is relevant in us trying to determine what to support as we build out our deployment/implementation).

@adityasaky
Copy link
Member

With #69, we're removing the signature wrapper altogether from the spec. That said, I'm still for adding some information about other_headers to https://github.com/in-toto/ITE/blob/master/ITE/5/README.adoc#appendix where the legacy signature wrapper is now described.

@adityasaky
Copy link
Member

I'm going to close this issue now in favour of opening a similar issue against ITE-5. The signature wrapper isn't in this spec anymore. Thanks, @nmiyake!

@adityasaky
Copy link
Member

in-toto/ITE#42

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

No branches or pull requests

2 participants