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.

Sign up for GitHub

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

Jump to bottom

feat: verify claims in provenance match the certificate #572

Merged
merged 31 commits into from
May 9, 2023
Merged

feat: verify claims in provenance match the certificate #572

merged 31 commits into from
May 9, 2023

Conversation

laurentsimon
Copy link
Contributor

@laurentsimon laurentsimon commented Apr 24, 2023 •
edited
Loading

I made a change to cache the TUF root during the execution. This should help with regression tests (see function TrustedRootSingleton()).
Additional e2e tests need to be created #571

closes #580
closes #570

@laurentsimon laurentsimon mentioned this pull request Apr 27, 2023
Closed
ianlewis
ianlewis reviewed May 2, 2023
View reviewed changes
errors/errors.go Outdated Show resolved Hide resolved
verifiers/internal/gha/builder.go Outdated Show resolved Hide resolved
verifiers/internal/gha/builder.go Outdated Show resolved Hide resolved
verifiers/internal/gha/builder.go Outdated Show resolved Hide resolved
verifiers/internal/gha/builder.go Outdated Show resolved Hide resolved
verifiers/internal/gha/builder.go Outdated Show resolved Hide resolved
verifiers/internal/gha/builder.go Show resolved Hide resolved
verifiers/internal/gha/builder.go Outdated Show resolved Hide resolved
@ianlewis
Copy link
Member

ianlewis commented May 2, 2023

I didn't take a really close look at the tests but they looked mostly ok to me.

@laurentsimon laurentsimon enabled auto-merge (squash) May 2, 2023 19:56
@laurentsimon
Copy link
Contributor Author

All comments addressed. PTAL

@laurentsimon
Copy link
Contributor Author

@asraa can you take a look?

ianlewis
ianlewis approved these changes May 8, 2023
View reviewed changes
Copy link
Member

@ianlewis ianlewis left a comment

Choose a reason for hiding this comment

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

Sorry forgot to approve.

@laurentsimon
Copy link
Contributor Author

I completely messed up the unit tests. Will take a look later

@ianlewis ianlewis mentioned this pull request May 8, 2023
Closed
@laurentsimon laurentsimon mentioned this pull request May 9, 2023
Merged
@laurentsimon
update
7241f81 
Signed-off-by: laurentsimon <laurentsimon@google.com>
@laurentsimon
update
18541c7 
Signed-off-by: laurentsimon <laurentsimon@google.com>
@laurentsimon
update
e2e1b79 
Signed-off-by: laurentsimon <laurentsimon@google.com>
@laurentsimon
update
ff8fdda 
Signed-off-by: laurentsimon <laurentsimon@google.com>
@laurentsimon
update
6dd5255 
Signed-off-by: laurentsimon <laurentsimon@google.com>
@laurentsimon
update
d84147c 
Signed-off-by: laurentsimon <laurentsimon@google.com>
@laurentsimon
update
ecb4f9b 
Signed-off-by: laurentsimon <laurentsimon@google.com>
@laurentsimon
update
73b2678 
Signed-off-by: laurentsimon <laurentsimon@google.com>
@laurentsimon
update
850cd1b 
Signed-off-by: laurentsimon <laurentsimon@google.com>
@laurentsimon
update
768fd0d 
Signed-off-by: laurentsimon <laurentsimon@google.com>
@laurentsimon
update
bfe5913 
Signed-off-by: laurentsimon <laurentsimon@google.com>
@laurentsimon
update
cc5b79f 
Signed-off-by: laurentsimon <laurentsimon@google.com>
@laurentsimon
update
2639884 
Signed-off-by: laurentsimon <laurentsimon@google.com>
@laurentsimon
update
7606efd 
Signed-off-by: laurentsimon <laurentsimon@google.com>
@laurentsimon
update
03bd38b 
Signed-off-by: laurentsimon <laurentsimon@google.com>
@laurentsimon
update
f8d35f0 
Signed-off-by: laurentsimon <laurentsimon@google.com>
@laurentsimon
update
784fec4 
Signed-off-by: laurentsimon <laurentsimon@google.com>
@laurentsimon
update
cbafe52 
Signed-off-by: laurentsimon <laurentsimon@google.com>
@laurentsimon
update
38d631b 
Signed-off-by: laurentsimon <laurentsimon@google.com>
@laurentsimon
update
2e8be27 
Signed-off-by: laurentsimon <laurentsimon@google.com>
@laurentsimon
update
6b0106a 
Signed-off-by: laurentsimon <laurentsimon@google.com>
@laurentsimon
update
693486a 
Signed-off-by: laurentsimon <laurentsimon@google.com>
@laurentsimon
update
4b7f159 
Signed-off-by: laurentsimon <laurentsimon@google.com>
@laurentsimon
update
3a769a9 
Signed-off-by: laurentsimon <laurentsimon@google.com>
@laurentsimon
update
172cd95 
Signed-off-by: laurentsimon <laurentsimon@google.com>
@laurentsimon
update
a36c23e 
Signed-off-by: laurentsimon <laurentsimon@google.com>
@laurentsimon
update
b57e722 
Signed-off-by: laurentsimon <laurentsimon@google.com>
@laurentsimon
update
b179065 
Signed-off-by: laurentsimon <laurentsimon@google.com>
@laurentsimon
update
b305527 
Signed-off-by: laurentsimon <laurentsimon@google.com>
@laurentsimon
update
89ea911 
Signed-off-by: laurentsimon <laurentsimon@google.com>
@laurentsimon
Copy link
Contributor Author

@ianlewis not sure what's wrong with the linter. I've tried to disable the deprecated method check but it's still failing:

Error: directive `//nolint:staticcheck we want to disable SA1019 only to use deprecated methods but there is a bug in golangci-lint.` should match `//nolint:staticcheck we want to disable SA1019 only to use deprecated methods but there is a bug in golangci-lint.[:<comma-separated-linters>] [// <explanation>]` (nolintlint)

Context: golangci/golangci-lint#741 (comment)
I tried disabling the linter in builder.go and builder_test.go.

@ianlewis
Fix linter errors
f35f0a1 
Signed-off-by: Ian Lewis <ianlewis@google.com>
@ianlewis
Copy link
Member

ianlewis commented May 9, 2023

@ianlewis not sure what's wrong with the linter. I've tried to disable the deprecated method check but it's still failing:

Error: directive `//nolint:staticcheck we want to disable SA1019 only to use deprecated methods but there is a bug in golangci-lint.` should match `//nolint:staticcheck we want to disable SA1019 only to use deprecated methods but there is a bug in golangci-lint.[:<comma-separated-linters>] [// <explanation>]` (nolintlint)

Context: golangci/golangci-lint#741 (comment) I tried disabling the linter in builder.go and builder_test.go.

The //nolint declarations are fairly particular and can't have a leading space and need to have a // delimiter before the comment message. I updated the comments.

@laurentsimon laurentsimon merged commit 3a4e992 into slsa-framework:main May 9, 2023
@laurentsimon
Copy link
Contributor Author

The //nolint declarations are fairly particular and can't have a leading space and need to have a // delimiter before the comment message. I updated the comments.

Thanks!

@laurentsimon laurentsimon mentioned this pull request May 18, 2023
Open
ramonpetgrave64 pushed a commit to ramonpetgrave64/slsa-verifier that referenced this pull request Apr 18, 2024
@laurentsimon
doc: Updates to Go builder README (slsa-framework#572)
f6b684f 
* update

* update

* update

* update

* update
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ianlewis ianlewis ianlewis approved these changes

@asraa asraa Awaiting requested review from asraa

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

feat: use OID.Equal in certificate handling code remove certificate claims after fulcio update
2 participants
@laurentsimon @ianlewis