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: add support for signed user metadata #242

Merged
merged 12 commits into from Feb 8, 2023
11 changes: 11 additions & 0 deletions errors.go
Expand Up @@ -47,3 +47,14 @@ func (e ErrorVerificationFailed) Error() string {
}
return "signature verification failed"
}

type ErrorUserMetadataVerificationFailed struct {
byronchien marked this conversation as resolved.
Show resolved Hide resolved
Msg string
}

func (e ErrorUserMetadataVerificationFailed) Error() string {
if e.Msg != "" {
return e.Msg
}
return "unable to find specified metadata in the signature"
}
9 changes: 9 additions & 0 deletions internal/mock/mocks.go
Expand Up @@ -35,6 +35,9 @@ var MockSaExpiredSigEnv []byte
//go:embed testdata/sa_plugin_sig_env.json
var MockSaPluginSigEnv []byte // extended attributes are "SomeKey":"SomeValue", "io.cncf.notary.verificationPlugin":"plugin-name"

//go:embed testdata/sig_env_with_metadata.json
var MockSigEnvWithMetadata []byte

var (
SampleArtifactUri = "registry.acme-rockets.io/software/net-monitor@sha256:60043cf45eaebc4c0867fea485a039b598f52fd09fd5b07b0b2d2f88fad9d74e"
SampleDigest = digest.Digest("sha256:60043cf45eaebc4c0867fea485a039b598f52fd09fd5b07b0b2d2f88fad9d74e")
Expand Down Expand Up @@ -62,6 +65,12 @@ var (
Critical: true,
Value: "SomeValue",
}
MetadataSigEnvDescriptor = ocispec.Descriptor{
MediaType: "application/vnd.docker.distribution.manifest.v2+json",
Digest: digest.Digest("sha256:5a07385af4e6b6af81b0ebfd435aedccdfa3507f0609c658209e1aba57159b2b"),
Size: 942,
Annotations: map[string]string{"io.wabbit-networks.buildId": "123", "io.wabbit-networks.buildTime": "1672944615"},
}
)

type Repository struct {
Expand Down
11 changes: 11 additions & 0 deletions internal/mock/testdata/sig_env_with_metadata.json
@@ -0,0 +1,11 @@
{
"payload":"eyJ0YXJnZXRBcnRpZmFjdCI6eyJhbm5vdGF0aW9ucyI6eyJpby53YWJiaXQtbmV0d29ya3MuYnVpbGRJZCI6IjEyMyIsImlvLndhYmJpdC1uZXR3b3Jrcy5idWlsZFRpbWUiOiIxNjcyOTQ0NjE1In0sImRpZ2VzdCI6InNoYTI1Njo1YTA3Mzg1YWY0ZTZiNmFmODFiMGViZmQ0MzVhZWRjY2RmYTM1MDdmMDYwOWM2NTgyMDllMWFiYTU3MTU5YjJiIiwibWVkaWFUeXBlIjoiYXBwbGljYXRpb24vdm5kLmRvY2tlci5kaXN0cmlidXRpb24ubWFuaWZlc3QudjIranNvbiIsInNpemUiOjk0Mn19",
"protected":"eyJhbGciOiJQUzI1NiIsImNyaXQiOlsiaW8uY25jZi5ub3Rhcnkuc2lnbmluZ1NjaGVtZSJdLCJjdHkiOiJhcHBsaWNhdGlvbi92bmQuY25jZi5ub3RhcnkucGF5bG9hZC52MStqc29uIiwiaW8uY25jZi5ub3Rhcnkuc2lnbmluZ1NjaGVtZSI6Im5vdGFyeS54NTA5IiwiaW8uY25jZi5ub3Rhcnkuc2lnbmluZ1RpbWUiOiIyMDIzLTAxLTExVDEwOjAyOjU0LTA4OjAwIn0",
"header": {
"x5c": [
"MIIDVjCCAj6gAwIBAgIBUTANBgkqhkiG9w0BAQsFADBaMQswCQYDVQQGEwJVUzELMAkGA1UECBMCV0ExEDAOBgNVBAcTB1NlYXR0bGUxDzANBgNVBAoTBk5vdGFyeTEbMBkGA1UEAxMSd2FiYml0LW5ldHdvcmtzLmlvMB4XDTIzMDExMTAwNTIxMloXDTIzMDExMjAwNTIxMlowWjELMAkGA1UEBhMCVVMxCzAJBgNVBAgTAldBMRAwDgYDVQQHEwdTZWF0dGxlMQ8wDQYDVQQKEwZOb3RhcnkxGzAZBgNVBAMTEndhYmJpdC1uZXR3b3Jrcy5pbzCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBANH4GCn0bO8LurJvnDh9F6E5iU8MydVw5bypnPlRpP3Mt9AmdWgBYhTegHT9DecA7smkLP3FAZG33Z9c1oxeZaeMnkWmiPGtuGQtXRHoj3+ioe4zH8LKYtCDW2uNs0xaDI1CldDXf4xZGa1mYqXVT1SeYXLwHf2dAL9q6FY98lYLax139PIwJwgEiod1hyIJyQZ2Zf9+IHe+v+Aja0wNLp/w4tO9Q5FR6VNhtmeGL/zPLD8chcj4iBzArsPyos2jBDUwogsEPTYoa6Rtn6IrUyrg4aJ8S3W0qGX7qGPeSY3wbsI63Q7XYQkRrD+cb1yvwt1+YhqN8nnvM/ujVtT+JfsCAwEAAaMnMCUwDgYDVR0PAQH/BAQDAgeAMBMGA1UdJQQMMAoGCCsGAQUFBwMDMA0GCSqGSIb3DQEBCwUAA4IBAQBs475D3dkDhjTksg+ff0zhu2MaO0UR0kVuW+7tLFkgGptfos7Z6WN4xsjpMOL44xYx3DIKHkPybTFFEr75TGsfXUFRjYRoXCYW6L72p53kzR27Im14xiELGQoIw9n0/7ajIh1j4qKg+jP7dNSGg5234QllmZZMiRWl1/X2UlE1TEgJP26vuLKsw0bPsmRPaxoKcAAQxSWuOG5gdpZVw2p08rEwsaleK2Hbh7rIQwyL7JOGrUMYyEXuF/gE72Az4NYBVlLYPE5up/Cuq4bhjpRZ4qmVTQfiDoyhn5gSCJh+1wVewbqS/DECRpKETHTCYtrfrnxsROOkB8jtaSp7vTLk"
],
"io.cncf.notary.signingAgent":"Notation/1.0.0"
},
"signature":"Fqe_cSgUlbYXKYz5K-O_iZobcmwUdQVaT_mPsI-fnp2ibsFbWOfokYS-DJboJJJEJyzDH41WWAi9Xxr_yieub3Eq9vD4TIz5iVm7oJxI-x92mqe3MhgeybIQDyivtChmb2ufwmr1bFCtj4girLeYc_kUVj_BZDIUYo8rlx8nyr6ucFsxK-YyNYez9ySeInWCGz-Lce4ySuXCopgiGB-lVAeDzpxBwQHVYacKfvhvoXJgmsw372dBYUVVOHbfK5PX04r2ArpysNpvlPT7iY3t6pUVsRniDNFQ1nh2t7ZttuG9qQMTrpeegAIVDJ4i-PZnLS_8LQmF07Z6rpU8e1E6_Q"
}
41 changes: 40 additions & 1 deletion notation.go
Expand Up @@ -12,6 +12,7 @@ import (
"time"

"github.com/notaryproject/notation-core-go/signature"
"github.com/notaryproject/notation-go/internal/envelope"
"github.com/notaryproject/notation-go/log"
"github.com/notaryproject/notation-go/registry"
"github.com/notaryproject/notation-go/verifier/trustpolicy"
Expand Down Expand Up @@ -40,6 +41,9 @@ type SignOptions struct {
// PluginConfig sets or overrides the plugin configuration.
PluginConfig map[string]string

// UserMetadata contains key-value pairs that are added to the signature payload
UserMetadata map[string]string
byronchien marked this conversation as resolved.
Show resolved Hide resolved

// SigningAgent sets the signing agent name
SigningAgent string
}
Expand Down Expand Up @@ -156,6 +160,9 @@ type VerifyOptions struct {

// PluginConfig is a map of plugin configs.
PluginConfig map[string]string

// UserMetadata contains key-value pairs that must be present in the signature
UserMetadata map[string]string
byronchien marked this conversation as resolved.
Show resolved Hide resolved
}

// Verifier is a generic interface for verifying an artifact.
Expand All @@ -181,6 +188,9 @@ type RemoteVerifyOptions struct {
// will be processed for verification. If set to less than or equals
// to zero, an error will be returned.
MaxSignatureAttempts int

// UserMetadata contains key-value pairs that must be present in the signature
UserMetadata map[string]string
byronchien marked this conversation as resolved.
Show resolved Hide resolved
}

type skipVerifier interface {
Expand All @@ -200,6 +210,7 @@ func Verify(ctx context.Context, verifier Verifier, repo registry.Repository, re
opts := VerifyOptions{
ArtifactReference: remoteOpts.ArtifactReference,
PluginConfig: remoteOpts.PluginConfig,
UserMetadata: remoteOpts.UserMetadata,
}

if skipChecker, ok := verifier.(skipVerifier); ok {
Expand Down Expand Up @@ -240,6 +251,7 @@ func Verify(ctx context.Context, verifier Verifier, repo registry.Repository, re
}

var verificationOutcomes []*VerificationOutcome
var failedOutcomes []*VerificationOutcome
byronchien marked this conversation as resolved.
Show resolved Hide resolved
errExceededMaxVerificationLimit := ErrorVerificationFailed{Msg: fmt.Sprintf("total number of signatures associated with an artifact should be less than: %d", remoteOpts.MaxSignatureAttempts)}
numOfSignatureProcessed := 0

Expand Down Expand Up @@ -270,6 +282,7 @@ func Verify(ctx context.Context, verifier Verifier, repo registry.Repository, re
logger.Error("Got nil outcome. Expecting non-nil outcome on verification failure")
return err
}
failedOutcomes = append(failedOutcomes, outcome)
continue
}
// at this point, the signature is verified successfully. Add
Expand Down Expand Up @@ -303,7 +316,7 @@ func Verify(ctx context.Context, verifier Verifier, repo registry.Repository, re
// Verification Failed
if len(verificationOutcomes) == 0 {
logger.Debugf("Signature verification failed for all the signatures associated with artifact %v", artifactDescriptor.Digest)
return ocispec.Descriptor{}, verificationOutcomes, ErrorVerificationFailed{}
return ocispec.Descriptor{}, verificationOutcomes, getFinalVerificationError(failedOutcomes)
}

// Verification Succeeded
Expand All @@ -325,3 +338,29 @@ func generateAnnotations(signerInfo *signature.SignerInfo) (map[string]string, e
annotationX509ChainThumbprint: string(val),
}, nil
}

func (outcome *VerificationOutcome) GetUserMetadata() (map[string]string, error) {
byronchien marked this conversation as resolved.
Show resolved Hide resolved
var payload envelope.Payload
err := json.Unmarshal(outcome.EnvelopeContent.Payload.Content, &payload)
byronchien marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, errors.New("Failed to unmarshal the payload content in the signature blob to envelope.Payload")
}

if payload.TargetArtifact.Annotations == nil {
return map[string]string{}, nil
}

return payload.TargetArtifact.Annotations, nil
byronchien marked this conversation as resolved.
Show resolved Hide resolved
}
byronchien marked this conversation as resolved.
Show resolved Hide resolved

func getFinalVerificationError(outcomes []*VerificationOutcome) error {
err := ErrorVerificationFailed{}

for _, outcome := range outcomes {
if _, ok := outcome.Error.(ErrorUserMetadataVerificationFailed); ok {
return outcome.Error
}
}

return err
}
6 changes: 6 additions & 0 deletions signer/plugin.go
Expand Up @@ -103,6 +103,12 @@ func (s *pluginSigner) generateSignatureEnvelope(ctx context.Context, desc ocisp
logger := log.GetLogger(ctx)
logger.Debug("Generating signature envelope by plugin")
payload := envelope.Payload{TargetArtifact: envelope.SanitizeTargetArtifact(desc)}

err := addUserMetadataToDescriptor(ctx, &payload.TargetArtifact, opts.UserMetadata)
if err != nil {
return nil, nil, fmt.Errorf("error adding user metadata: %w", err)
}

payloadBytes, err := json.Marshal(payload)
if err != nil {
return nil, nil, fmt.Errorf("envelope payload can't be marshalled: %w", err)
Expand Down
14 changes: 13 additions & 1 deletion signer/plugin_test.go
Expand Up @@ -206,6 +206,18 @@ func TestSigner_Sign_EnvelopeNotSupported(t *testing.T) {
testSignerError(t, signer, fmt.Sprintf("signature envelope format with media type %q is not supported", opts.SignatureMediaType), opts)
}

func TestSigner_Sign_InvalidUserMetadata(t *testing.T) {
signer := pluginSigner{
plugin: newMockPlugin(nil, nil, signature.KeySpec{Type: signature.KeyTypeRSA, Size: 2048}),
}

for _, reservedPrefix := range reservedAnnotationPrefixes {
invalidKey := reservedPrefix + ".invalid"
opts := notation.SignOptions{UserMetadata: map[string]string{invalidKey: "value"}}
testSignerError(t, signer, fmt.Sprintf("metadata key %v has reserved prefix %v", invalidKey, reservedPrefix), opts)
}
}

func TestSigner_Sign_DescribeKeyIDMismatch(t *testing.T) {
respKeyId := ""
for _, envelopeType := range signature.RegisteredEnvelopeTypes() {
Expand Down Expand Up @@ -374,5 +386,5 @@ func basicSignTest(t *testing.T, pluginSigner *pluginSigner, envelopeType string
if !reflect.DeepEqual(mockPlugin.certs, signerInfo.CertificateChain) {
t.Fatalf(" Signer.Sign() cert chain changed")
}
basicVerification(t, data, envelopeType, mockPlugin.certs[len(mockPlugin.certs)-1], metadata)
basicVerification(t, data, envelopeType, mockPlugin.certs[len(mockPlugin.certs)-1], metadata, validSignOpts.UserMetadata)
}
36 changes: 36 additions & 0 deletions signer/signer.go
Expand Up @@ -11,6 +11,7 @@ import (
"encoding/json"
"errors"
"fmt"
"strings"
"time"

"github.com/notaryproject/notation-core-go/signature"
Expand All @@ -23,6 +24,13 @@ import (
// signingAgent is the unprotected header field used by signature.
const signingAgent = "Notation/1.0.0"

var (
reservedAnnotationPrefixes = []string{
"org.opencontainers",
byronchien marked this conversation as resolved.
Show resolved Hide resolved
"io.cncf.notary",
}
)

// genericSigner implements notation.Signer and embeds signature.Signer
type genericSigner struct {
signature.Signer
Expand Down Expand Up @@ -77,6 +85,12 @@ func (s *genericSigner) Sign(ctx context.Context, desc ocispec.Descriptor, opts
logger.Debugf("Generic signing for %v in signature media type %v", desc.Digest, opts.SignatureMediaType)
// Generate payload to be signed.
payload := envelope.Payload{TargetArtifact: envelope.SanitizeTargetArtifact(desc)}

err := addUserMetadataToDescriptor(ctx, &payload.TargetArtifact, opts.UserMetadata)
if err != nil {
return nil, nil, fmt.Errorf("error adding user metadata: %w", err)
}

payloadBytes, err := json.Marshal(payload)
if err != nil {
return nil, nil, fmt.Errorf("envelope payload can't be marshalled: %w", err)
Expand Down Expand Up @@ -133,3 +147,25 @@ func (s *genericSigner) Sign(ctx context.Context, desc ocispec.Descriptor, opts
// TODO: re-enable timestamping https://github.com/notaryproject/notation-go/issues/78
return sig, &envContent.SignerInfo, nil
}

func addUserMetadataToDescriptor(ctx context.Context, desc *ocispec.Descriptor, userMetadata map[string]string) error {
logger := log.GetLogger(ctx)

if desc.Annotations == nil && len(userMetadata) > 0 {
desc.Annotations = map[string]string{}
}
priteshbandi marked this conversation as resolved.
Show resolved Hide resolved

for k, v := range userMetadata {
logger.Debugf("Adding metadata %v=%v to annotations", k, v)

for _, reservedPrefix := range reservedAnnotationPrefixes {
if strings.HasPrefix(k, reservedPrefix) {
return fmt.Errorf("metadata key %v has reserved prefix %v", k, reservedPrefix)
}
}

desc.Annotations[k] = v
}

return nil
}
59 changes: 53 additions & 6 deletions signer/signer_test.go
Expand Up @@ -8,12 +8,14 @@ import (
"crypto/rand"
"crypto/rsa"
"crypto/x509"
"encoding/json"
"encoding/pem"
"errors"
"fmt"
"os"
"path/filepath"
"regexp"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -141,7 +143,7 @@ func testSignerFromFile(t *testing.T, keyCert *keyCertPair, envelopeType, dir st
t.Fatalf("Sign() failed: %v", err)
}
// basic verification
basicVerification(t, sig, envelopeType, keyCert.certs[len(keyCert.certs)-1], nil)
basicVerification(t, sig, envelopeType, keyCert.certs[len(keyCert.certs)-1], nil, opts.UserMetadata)
}

func TestNewFromFiles(t *testing.T) {
Expand Down Expand Up @@ -195,7 +197,7 @@ func TestSignWithTimestamp(t *testing.T) {
}

// basic verification
basicVerification(t, sig, envelopeType, keyCert.certs[len(keyCert.certs)-1], &validMetadata)
basicVerification(t, sig, envelopeType, keyCert.certs[len(keyCert.certs)-1], &validMetadata, sOpts.UserMetadata)
})
}
}
Expand All @@ -221,7 +223,34 @@ func TestSignWithoutExpiry(t *testing.T) {
}

// basic verification
basicVerification(t, sig, envelopeType, keyCert.certs[len(keyCert.certs)-1], nil)
basicVerification(t, sig, envelopeType, keyCert.certs[len(keyCert.certs)-1], nil, sOpts.UserMetadata)
})
}
}
}

func TestSignWithInvalidUserMetadata(t *testing.T) {
for _, envelopeType := range signature.RegisteredEnvelopeTypes() {
for _, keyCert := range keyCertPairCollections {
t.Run(fmt.Sprintf("envelopeType=%v_keySpec=%v", envelopeType, keyCert.keySpecName), func(t *testing.T) {
s, err := New(keyCert.key, keyCert.certs)
if err != nil {
t.Fatalf("NewSigner() error = %v", err)
}

ctx := context.Background()

for _, reservedPrefix := range reservedAnnotationPrefixes {
desc, sOpts := generateSigningContent(nil)
invalidKey := reservedPrefix + ".invalid"
sOpts.UserMetadata[invalidKey] = "value"
expectedError := fmt.Sprintf("metadata key %v has reserved prefix %v", invalidKey, reservedPrefix)

_, _, err := s.Sign(ctx, desc, sOpts)
if err == nil || !strings.Contains(err.Error(), expectedError) {
t.Fatalf("expected error %+v but was %+v instead", expectedError, err)
}
}
})
}
}
Expand Down Expand Up @@ -269,15 +298,19 @@ func generateSigningContent(tsa *timestamptest.TSA) (ocispec.Descriptor, notatio
"foo": "bar",
},
}
sOpts := notation.SignOptions{ExpiryDuration: 24 * time.Hour}
sOpts := notation.SignOptions{
ExpiryDuration: 24 * time.Hour,
UserMetadata: map[string]string{"example": "metadata"},
}
if tsa != nil {
tsaRoots := x509.NewCertPool()
tsaRoots.AddCert(tsa.Certificate())
}

return desc, sOpts
}

func basicVerification(t *testing.T, sig []byte, envelopeType string, trust *x509.Certificate, metadata *proto.GetMetadataResponse) {
func basicVerification(t *testing.T, sig []byte, envelopeType string, trust *x509.Certificate, metadata *proto.GetMetadataResponse, userMetadata map[string]string) {
// basic verification
sigEnv, err := signature.ParseEnvelope(envelopeType, sig)
if err != nil {
Expand All @@ -299,6 +332,7 @@ func basicVerification(t *testing.T, sig []byte, envelopeType string, trust *x50
}

verifySigningAgent(t, envContent.SignerInfo.UnsignedAttributes.SigningAgent, metadata)
verifyUserMetadataInPayload(t, envContent.Payload.Content, userMetadata)
}

func verifySigningAgent(t *testing.T, signingAgentId string, metadata *proto.GetMetadataResponse) {
Expand Down Expand Up @@ -334,5 +368,18 @@ func validateSignWithCerts(t *testing.T, envelopeType string, key crypto.Private
}

// basic verification
basicVerification(t, sig, envelopeType, certs[len(certs)-1], nil)
basicVerification(t, sig, envelopeType, certs[len(certs)-1], nil, sOpts.UserMetadata)
}

func verifyUserMetadataInPayload(t *testing.T, payloadBytes []byte, userMetadata map[string]string) {
var payload envelope.Payload
if err := json.Unmarshal(payloadBytes, &payload); err != nil {
t.Fatalf("unable to unmarshal payload")
}

for k, v := range userMetadata {
if payload.TargetArtifact.Annotations[k] != v {
t.Fatalf("unable to find expected user metadata %v=%v in payload", k, v)
}
}
}