[feature] Add more malicious SLSA subject layout tests to generate-attestations

[asraa]

Is your feature request related to a problem? Please describe. Add strict validation to the SLSA subject layout for generate-attestations.

Validate that malformed subject layouts cause errors.

Describe the solution you'd like A clear and concise description of what you want to happen.

Describe alternatives you've considered A clear and concise description of any alternative solutions or features you've considered.

Additional context Add any other context or screenshots about the feature request here.

[laurentsimon]

@asraa can you elaborate on "strict validation"?

[asraa]

Yes! I'm fairly certain it doesn't even have to be a valid JSON of the type we have - i don't think javascript enforced that. Meaning, we could produce some arbitrary JSON and have that incorporated as the subjects part which would then make it not a valid intoto statement

[laurentsimon]

so iiuc, this line https://github.com/slsa-framework/slsa-github-generator/blob/main/.github/actions/generate-attestations/src/attestation.ts#L70-L72 is what needs further validation, correct?

[laurentsimon]

Given that subject is defined in https://github.com/slsa-framework/slsa-github-generator/blob/main/.github/actions/generate-attestations/src/intoto.ts#L17-L20, only the defined fields should be present (I'll see if that's true when I write the PR).

So I think validation would essentially be (this would also take care of the case when the layout file is not json or not properly formatted):

• verify the hash algorithm
• verify the length of its value
• verify the name of the subject is not empty

The name is arbitrary.

Is the above correct?

[asraa]

I think it is this entire piece https://github.com/slsa-framework/slsa-github-generator/blob/7f9306a166326f33f0aeb3c1a217cedf3fb981c5/.github/actions/generate-attestations/src/attestation.ts#L42

I'm not sure how the type casting to types.Layout works with errors, but basically the idea is that if I add in test cases with completely bogus JSON, will it fail gracefully?

[laurentsimon]

I don't' know :) If suppose a non-JSON string would fail; but a JSON string with more fields than we expect may not. Will need to test.

In terms of priority, is this something we need to tackle for a pre-release (say, next month), or can wait for GA, say, a month later? It seems that the only attack / problem is that the TRW would break its own provenance, so not a security problem.

Wdut?

[asraa]

Yes - it was more for robustness for the TRW author to see if they are getting an error.