Skip to content

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

@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.

Signed-off-by: Alex Cameron <[email protected]>
Signed-off-by: Alex Cameron <[email protected]>
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/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.


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.

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.

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 <[email protected]>
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.

@haydentherapper
Copy link
Contributor

Now implemented.

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.

3 participants