ITE7 Prototype Implementation #119
Conversation
|
We're still working on splitting up our fork into separate pull requests. We're thinking two more will be opened: one for the CLI and another for the SPIFFE/SPIRE plugin. We created end-to-end testing for all of this functionality but it relies upon the CLI pull request. In the meantime I'll continue to work and add unit tests and refining the documentation. |
This is a prototype implementation of ITE7 as drafted and proposed at: in-toto/ITE#21 * Adds the concept of certificate constraints. This allows layout creators to define the shape of a X509 certificate that is allowed to act as a functionary for each step of a layout. * Adds roots and intermediate CAs to the layout. This allows layout creators to define the chain of trust that a X509 certificate must fall within to act as a functionary. * Adds the leaf certificate to signatures. This will need to be updated to be compatible with DSSE if ITE5 is accepted into the specficiation.
mikhailswift
force-pushed
the
ite7/cert-constraints
branch
from
3ca6751 to
31b889b
Compare
|
@mikhailswift Thanks for the PR. I have added a few questions. These are not meant as request for changes, they are just open questions on my side :) |
|
@shibumi I added some tests to cover the functionality added and some documentation. I'll continue to go through and add tests and documentation as I can find areas that need them. I left the commits separate for ease of review but as we get closer to this being in a merge-able state I can squash them down to a singular commit if desired. |
|
@mikhailswift I have restarted the CI pipeline.. let's see how this looks like. We don't need a perfect code coverage result, a small decrease is fine for us. Squashing the commits to one feature commit for ITE7 is desirable. |
|
I just remembered one block of important code I left untested, currently working through writing a test for that |
|
@shibumi Okay, that should cover all the happy path test cases. Additional tests can be added to test the errors and improve coverage. |
|
@adityasaky can you have a look? |
|
@adityasaky if you are fine with this PR I think we can merge it. It slightly decreases our code coverage, but this is fine. We are still over 91% (we should maybe change the threshold for future PRs.. I think the current threshold is a little bit too conservative for agile development). |
|
Hi @mikhailswift, thank you for your work on this, this looks great! I was wondering if we could make the new fields optional so that the implementation continues to support non ITE-7 metadata. I'm wary of breaking, say, verification workflows where the metadata is generated using the python implementation but the verification uses the Go implementation, or even where folks have older metadata generated using the Go implementation. |
|
Absolutely, 100% in favor of that and will focus on getting that done ASAP :) |
|
Thank you, I appreciate it! |
This commit will make the new fields added by ITE7 optional. This allows the golang version to load and verify layouts and links created by versions of in-toto that adhere to the 0.9 specification.
|
Sorry for the delay in this, the past week has been chaotic. My latest commit makes the new fields optional. Coincidentally this also fixes the KeyID problem discussed before. |
|
looks good for me! :) |
| if _, ok := obj[field]; !ok { | ||
| return fmt.Errorf("required field %s missing", field) | ||
| if _, ok := obj[field.name]; !ok && !field.omitempty { | ||
| return fmt.Errorf("required field %s missing", field.name) |
There was a problem hiding this comment.
probably not super important, but this for loop and the one above could be combined so we only have to do one pass over all the fields.
|
Thanks @mikhailswift! Looking forward to the other two PRs :) |
This is a prototype implementation of ITE7 as drafted and proposed at:
in-toto/ITE#21
creators to define the shape of a X509 certificate that is allowed to
act as a functionary for each step of a layout.
creators to define the chain of trust that a X509 certificate must
fall within to act as a functionary.
to be compatible with DSSE if ITE5 is accepted into the specficiation.
Please fill in the fields below to submit a pull request. The more
information that is provided, the better.
Fixes issue #:
Description of pull request:
Please verify and check that the pull request fulfills the following
requirements: