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

feat: Enable granular regression tests #709

Open
laurentsimon opened this issue Oct 2, 2023 · 5 comments
Open

feat: Enable granular regression tests #709

laurentsimon opened this issue Oct 2, 2023 · 5 comments

Comments

@laurentsimon
Copy link
Contributor

Per discussion in #707, we'd like to be able to verify certain things end-to-end and need a way to ignore signature verification.

@trishankatdatadog @ianlewis

@trishankatdatadog
Copy link
Member

need a way to ignore signature verification.

Or at least use test keys :)

@trishankatdatadog
Copy link
Member

The more I think about it, I'm increasingly of the opinion that we should never forgo signature verification, even while testing. Better to err on the side of caution and make sure we are always verifying signatures correctly.

@laurentsimon
Copy link
Contributor Author

Indeed, that's one reason I have not built this in the first place. Some things we do is set special env variables and check the identity of the platform it runs on to turn on things like this. Like "CI": "true" and "repository"="slsa-framework/slsa-verifier" and "SLSA_VERIFIER_TEST"="1". Eg see.

if (id.SourceRepository == trustedBuilderRepository ||
id.SourceRepository == e2eTestRepository) &&
options.TestingEnabled() {
// Allow verification on the main branch to support e2e tests.
if ref == "refs/heads/main" {
return nil
}
for our end-to-end tests

@ianlewis
Copy link
Member

ianlewis commented Oct 2, 2023

Maybe I'm misunderstanding but for regression tests we just need to make sure the thing that used to fail doesn't fail anymore and run the test in an ongoing way to make sure we don't regress. So it should just be a matter of modifying the digest in the publish attestation JSON and check to make sure the signature validation fails no?

For the regression test the actual digest check shouldn't really come into play (as it's hidden behind the signature check that will fail), and we shouldn't necessarily need to touch the signature I don't think. We can capture tests for behavior here in unit-tests of more granular functions directly with mocks etc.

@trishankatdatadog
Copy link
Member

trishankatdatadog commented Oct 3, 2023

So it should just be a matter of modifying the digest in the publish attestation JSON and check to make sure the signature validation fails no?

Sorry, maybe I didn't sufficiently clarify what I meant: suppose a publish attestation was correctly signed, but for some reason (e.g., a bug or, worse, a key compromise) the subject digest in it did not match the provenance attestation. We should test whether we can catch this, no?

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

3 participants