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

Allow configurable algorithms in verification #76

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

tetsuo-cpp
Copy link

Summary

Support configurable signing algorithms in sigstore-go's verification flow. We already have the signing key/certificate so we can use the artifact digest algorithm information to figure out what hash function is being used.

Release Note

Documentation

Signed-off-by: Alex Cameron <asc@tetsuo.sh>
@tetsuo-cpp tetsuo-cpp changed the title Allow configurable algorthms in verification Allow configurable algorithms in verification Jan 16, 2024
@tetsuo-cpp
Copy link
Author

CC: @ret2libc

@tetsuo-cpp
Copy link
Author

I think it's best to get this in first and follow up with the change to restrict algorithms via a --allowed-signing-algorithms flag since it's not straightforward with the current CLI flags library.

@tetsuo-cpp tetsuo-cpp marked this pull request as draft January 16, 2024 09:09
if envelope := sigContent.EnvelopeContent(); envelope != nil {
verifier, err = getSignatureVerifier(verificationContent, trustedMaterial)
Copy link
Author

Choose a reason for hiding this comment

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

As discussed, the envelope flow just uses the defaults that we had before.

cmd/sigstore-go/main.go Outdated Show resolved Hide resolved
Signed-off-by: Alex Cameron <asc@tetsuo.sh>
Signed-off-by: Alex Cameron <asc@tetsuo.sh>
Copy link
Author

@tetsuo-cpp tetsuo-cpp left a comment

Choose a reason for hiding this comment

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

Unfortunately, unit testing this was a bit of a pain. I've left a few comments about things we probably need to discuss.

github.com/google/certificate-transparency-go v1.1.7
github.com/in-toto/in-toto-golang v0.9.0
github.com/secure-systems-lab/go-securesystemslib v0.8.0
github.com/sigstore/protobuf-specs v0.2.1
github.com/sigstore/rekor v1.3.4
github.com/sigstore/sigstore v1.8.0
github.com/sigstore/rekor v1.3.5-0.20240129120212-63aa08fd13eb
Copy link
Author

Choose a reason for hiding this comment

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

  • Pin to a version once a new Rekor release is cut.

@@ -28,11 +28,11 @@ type CertificateChain struct {
}

type PublicKey struct {
hint string
HintString string
Copy link
Author

Choose a reason for hiding this comment

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

I had to make this public so I could create one in the test module.

@@ -113,3 +120,50 @@ func TestSignatureVerifierMessageSignature(t *testing.T) {
assert.Error(t, err)
assert.Nil(t, result)
}

type nonExpiringVerifier struct {
Copy link
Author

Choose a reason for hiding this comment

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

This struct and the trustedPublicKeyMaterial helper is just copied from sigstore-go/main.go which isn't great. Not 100% sure what to do here because we need this to test the relevant functionality, however it probably doesn't belong in the public API.

return hashFunc, nil
}

func getHashForHashAlgorithmString(hashAlgorithm string) (crypto.Hash, error) {
Copy link
Author

Choose a reason for hiding this comment

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

This is the string representation of the HashAlgorithm enum which gets used in the case where we're providing a trusted root (rather than public key material). Currently, different hash algorithms only work in the case where we provide trusted public key material since this enum only has a SHA2_256 value. But if we add more values to this enum, we should be able to "unlock" different hash algorithms.

@@ -234,3 +242,29 @@ func verifyMessageSignatureWithArtifactDigest(verifier signature.Verifier, msg M

return nil
}

func GetHashForDigestAlgorithm(digestAlgorithm string) (crypto.Hash, error) {
Copy link
Author

Choose a reason for hiding this comment

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

  • Figure out what this value should look like for Ed25519.

Signed-off-by: Alex Cameron <asc@tetsuo.sh>
result, err = verifier.Verify(entity, verify.NewPolicy(verify.WithArtifact(bytes.NewBufferString(artifact)), verify.WithoutIdentitiesUnsafe()))
assert.NoError(t, err)

assert.Equal(t, result.VerifiedTimestamps[0].Type, "Tlog")
Copy link
Author

Choose a reason for hiding this comment

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

We should add a test for Ed25519 here. We'll need sigstore/rekor#1945 for that.

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.

None yet

2 participants