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

Offline verification: verify content checksum #317

Closed
wlynch opened this issue May 22, 2023 · 1 comment · Fixed by #319
Closed

Offline verification: verify content checksum #317

wlynch opened this issue May 22, 2023 · 1 comment · Fixed by #319
Assignees
Labels
bug Something isn't working

Comments

@wlynch
Copy link
Member

wlynch commented May 22, 2023

Description

For offline verification, we are taking the checksum from the signature - we should really be re-computing the checksum ourselves and verifying it matches the signature checksum.

Cut a v0.7.0 release a little too quickly here, so going to retract it.

Version

v0.7.0

@wlynch wlynch added the bug Something isn't working label May 22, 2023
@wlynch wlynch self-assigned this May 22, 2023
@wlynch wlynch mentioned this issue May 22, 2023
@wlynch
Copy link
Member Author

wlynch commented May 22, 2023

Looking at this more closely - we are doing the right thing, but it's very easy for a consumer of the API to do the wrong thing without realizing.

In typical gitsign usage we're verifying the cert, then using VerifyOffline really as the Rekor inclusion check:

func Verify(ctx context.Context, git Verifier, rekor rekor.Verifier, data, sig []byte, detached bool) (*VerificationSummary, error) {
claims := []Claim{}
cert, err := git.Verify(ctx, data, sig, detached)
if err != nil {
return nil, err
}
claims = append(claims, NewClaim(ClaimValidatedSignature, true))
if tlog, err := rekor.VerifyOffline(ctx, sig); err == nil {
return &VerificationSummary{
Cert: cert,
LogEntry: tlog,
Claims: claims,
}, nil
}

But if you look at just rekor.OfflineVerify, it's not obvious that it's only doing the inclusion check, rather than the complete verification.

I'm going to refactor things a bit to make it harder to hold wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant