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

added ite6 processor with unit tests #39

Merged
merged 4 commits into from Sep 1, 2022
Merged

Conversation

pxp928
Copy link
Collaborator

@pxp928 pxp928 commented Aug 26, 2022

Signed-off-by: pxp928 parth.psu@gmail.com

Added ITE6 processor that will run and determine if the predicate type is SLSA and unpack the predicate. This will need to be replaced with runtime checks of the predicate such that we dont have to keep updating GAUC as new predicate becomes available.

Fixes: #36

case string(slsaPredicateType):
doc = &processor.Document{
Blob: predicatePayload,
Type: processor.DocumentSLSA,
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm, i think this is a point of discussion, is DocumentSLSA the entire intoto JSON or the contents of the predicate? This does make sense, but also SLSA provenance definition is based on the entire document as well (https://slsa.dev/provenance/v0.2).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right. Should it be the whole provenance or just the predicate was the question.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I have some thoughts around this. Hard to type out. In the graph it might make sense because we establish the relationship through edges anyway.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also FWIW, folks have recognized that the SLSA stuff is a bit backwards. We should be separating provenance from the in-toto wrapper, otherwise we end up with situations where we make it difficult to have composite types.

Copy link
Contributor

Choose a reason for hiding this comment

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

i think the tricky part here though is that some of the information is missing as as a standalone predicate payload - i.e. the subject will not be available. Which makes me want to tend towards having SLSA be the entire ITE6, for ease of implementation initially.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Isn't that why the doc tree was created. This would be similar to the DSSE processor where we just store the payload. Losing the signature information.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the main motivation was more for trust validation to simplify the processor.

This makes the ingestor less modular since a SLSA document ingestor now needs to somehow feed information from the parent node - which may or may not exist. Then some kind of map would need to be passed down but figuring out if a field was from the parent or previous ancestors would need to be considered as well.

I guess the question would be if a document (minus trust validation) should be self contained. Like SBOMs describe the subject within itself through an identifier. Slsa should also duplicate that information within its predicate payload?

pkg/handler/processor/ite6/ite6_test.go Outdated Show resolved Hide resolved
pkg/handler/processor/ite6/ite6_test.go Outdated Show resolved Hide resolved
pkg/handler/processor/ite6/ite6_test.go Outdated Show resolved Hide resolved
@lumjjb
Copy link
Contributor

lumjjb commented Aug 27, 2022 via email

@mlieberman85
Copy link
Collaborator

Yeah. Might be worthwhile to get @SantiagoTorres thoughts on it?

When unpacking documents we're always going to lose something.

  1. Unpack envelope -> lose signature info
  2. Unpack ite6 -> lose subject

There are a few different approaches here though.

  1. Don't unpack, just transform the original doc directly
  2. Unpack but encode it into the DocTree somehow?
  3. Some hybrid of above?

I think I still prefer 2, because of the proposed changes to sigstore with bundles that would allow multiple things.

@lumjjb
Copy link
Contributor

lumjjb commented Aug 30, 2022

Documenting what was discussed during our conversation:

We agreed that in the long term, the ingestor would need to have a way to communicate information up/down the tree in order to make edges and annotations between the elements of each node in the document tree.

However, in the meantime, it would be simpler to assume that the information is all encapsulated within the same document. Thus, for now, we will have

  • DocumentITE6SLSA
  • DocumentITE6Unknown

and ingestor plugins for each that will be handled independently.

@lumjjb lumjjb mentioned this pull request Aug 30, 2022
@pxp928 pxp928 force-pushed the ite6_processor branch 2 times, most recently from 6cde0cd to feeb4d8 Compare August 30, 2022 19:48
Signed-off-by: pxp928 <parth.psu@gmail.com>
Signed-off-by: pxp928 <parth.psu@gmail.com>
Signed-off-by: pxp928 <parth.psu@gmail.com>
@pxp928 pxp928 force-pushed the ite6_processor branch 2 times, most recently from 02807f1 to e09d777 Compare August 30, 2022 19:52
@pxp928
Copy link
Collaborator Author

pxp928 commented Aug 30, 2022

Updated based on the conversation. #53 to update for the future implementation.


// Unpack takes in the document and tries to unpack the provenance.
// if the predicate is of SLSA type the predicate is stored in the blob
func (e *ITE6Processor) Unpack(i *processor.Document) ([]*processor.Document, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this just returns nil, nil

Copy link
Contributor

Choose a reason for hiding this comment

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

unpack of len 0 output means that it is a leaf node. The guesser would be able to determine if its SLSA or ITE6 so this should not be the case where a processor needs to relabel a document.

Copy link
Collaborator Author

@pxp928 pxp928 Aug 31, 2022

Choose a reason for hiding this comment

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

The guesser would be able to determine if its SLSA or ITE6 so this should not be the case where a processor needs to relabel a document.

In that case is the ITE6 Processor needed at all? Seems like its not doing anything that the guesser isnt already doing.

Signed-off-by: pxp928 <parth.psu@gmail.com>
Copy link
Contributor

@lumjjb lumjjb left a comment

Choose a reason for hiding this comment

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

lgtm

@lumjjb lumjjb merged commit 4fbd5f9 into guacsec:main Sep 1, 2022
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

Successfully merging this pull request may close these issues.

task: [processor] create ITE6 Processor
3 participants