Skip to content

Commit

Permalink
fix: Appends annotations returned by plugin to signature manifest's a…
Browse files Browse the repository at this point in the history
…nnotations

Signed-off-by: Pritesh Bandi <[email protected]>
  • Loading branch information
Pritesh Bandi committed Feb 1, 2023
1 parent 6c88d3d commit 62eec92
Show file tree
Hide file tree
Showing 6 changed files with 93 additions and 83 deletions.
17 changes: 10 additions & 7 deletions notation.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ type SignOptions struct {
type Signer interface {
// Sign signs the artifact described by its descriptor,
// and returns the signature and SignerInfo.
Sign(ctx context.Context, desc ocispec.Descriptor, opts SignOptions) ([]byte, *signature.SignerInfo, error)
Sign(ctx context.Context, desc ocispec.Descriptor, opts SignOptions) ([]byte, map[string]string, *signature.SignerInfo, error)
}

// Sign signs the artifact in the remote registry and push the signature to the
Expand Down Expand Up @@ -85,12 +85,12 @@ func Sign(ctx context.Context, signer Signer, repo registry.Repository, opts Sig
logger.Infof("Resolved artifact tag `%s` to digest `%s` before signing", ref.Reference, targetDesc.Digest.String())
}

sig, signerInfo, err := signer.Sign(ctx, targetDesc, opts)
sig, ants, signerInfo, err := signer.Sign(ctx, targetDesc, opts)
if err != nil {
return ocispec.Descriptor{}, err
}
logger.Debug("Generating annotation")
annotations, err := generateAnnotations(signerInfo)
annotations, err := generateAnnotations(signerInfo, ants)
if err != nil {
return ocispec.Descriptor{}, err
}
Expand Down Expand Up @@ -310,7 +310,7 @@ func Verify(ctx context.Context, verifier Verifier, repo registry.Repository, re
return artifactDescriptor, verificationOutcomes, nil
}

func generateAnnotations(signerInfo *signature.SignerInfo) (map[string]string, error) {
func generateAnnotations(signerInfo *signature.SignerInfo, annotations map[string]string) (map[string]string, error) {
var thumbprints []string
for _, cert := range signerInfo.CertificateChain {
checkSum := sha256.Sum256(cert.Raw)
Expand All @@ -321,7 +321,10 @@ func generateAnnotations(signerInfo *signature.SignerInfo) (map[string]string, e
return nil, err
}

return map[string]string{
annotationX509ChainThumbprint: string(val),
}, nil
if annotations == nil {
annotations = make(map[string]string)
}

annotations[annotationX509ChainThumbprint] = string(val)
return annotations, nil
}
20 changes: 17 additions & 3 deletions notation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,18 @@ func TestSignSuccess(t *testing.T) {
}
}

func TestSignWithAnnotationsSuccess(t *testing.T) {
repo := mock.NewRepository()

opts := SignOptions{
ArtifactReference: mock.SampleArtifactUri,
}
_, err := Sign(context.Background(), &dummySigner{annotations: map[string]string{"foo": "bar"}}, repo, opts)
if err != nil {
t.Fatalf("Sign failed with error: %v", err)
}
}

func TestSignWithInvalidExpiry(t *testing.T) {
repo := mock.NewRepository()
testCases := []struct {
Expand Down Expand Up @@ -217,10 +229,12 @@ func dummyPolicyStatement() (policyStatement trustpolicy.TrustPolicy) {
return
}

type dummySigner struct{}
type dummySigner struct{
annotations map[string]string
}

func (s *dummySigner) Sign(ctx context.Context, desc ocispec.Descriptor, opts SignOptions) ([]byte, *signature.SignerInfo, error) {
return []byte("ABC"), &signature.SignerInfo{}, nil
func (s *dummySigner) Sign(ctx context.Context, desc ocispec.Descriptor, opts SignOptions) ([]byte, map[string]string, *signature.SignerInfo, error) {
return []byte("ABC"), s.annotations, &signature.SignerInfo{}, nil
}

type dummyVerifier struct {
Expand Down
40 changes: 21 additions & 19 deletions signer/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import (
"oras.land/oras-go/v2/content"
)

const annotationNotaryPrefix = "io.cncf.notary"

// pluginSigner signs artifacts and generates signatures.
// It implements notation.Signer
type pluginSigner struct {
Expand All @@ -26,7 +28,7 @@ type pluginSigner struct {
pluginConfig map[string]string
}

// NewSignerPlugin creates a notation.Signer that signs artifacts and generates
// NewFromPlugin creates a notation.Signer that signs artifacts and generates
// signatures by delegating the one or more operations to the named plugin,
// as defined in https://github.com/notaryproject/notaryproject/blob/main/specs/plugin-extensibility.md#signing-interfaces.
func NewFromPlugin(plugin plugin.Plugin, keyID string, pluginConfig map[string]string) (notation.Signer, error) {
Expand All @@ -46,15 +48,15 @@ func NewFromPlugin(plugin plugin.Plugin, keyID string, pluginConfig map[string]s

// Sign signs the artifact described by its descriptor and returns the
// marshalled envelope.
func (s *pluginSigner) Sign(ctx context.Context, desc ocispec.Descriptor, opts notation.SignOptions) ([]byte, *signature.SignerInfo, error) {
func (s *pluginSigner) Sign(ctx context.Context, desc ocispec.Descriptor, opts notation.SignOptions) ([]byte, map[string]string, *signature.SignerInfo, error) {
logger := log.GetLogger(ctx)
logger.Debug("Invoking plugin's get-plugin-metadata command")
req := &proto.GetMetadataRequest{
PluginConfig: s.mergeConfig(opts.PluginConfig),
}
metadata, err := s.plugin.GetMetadata(ctx, req)
if err != nil {
return nil, nil, err
return nil, nil, nil, err
}

logger.Debugf("Using plugin %v with capabilities %v to sign artifact %v in signature media type %v", metadata.Name, metadata.Capabilities, desc.Digest, opts.SignatureMediaType)
Expand All @@ -63,26 +65,26 @@ func (s *pluginSigner) Sign(ctx context.Context, desc ocispec.Descriptor, opts n
} else if metadata.HasCapability(proto.CapabilityEnvelopeGenerator) {
return s.generateSignatureEnvelope(ctx, desc, opts)
}
return nil, nil, fmt.Errorf("plugin does not have signing capabilities")
return nil, nil, nil, fmt.Errorf("plugin does not have signing capabilities")
}

func (s *pluginSigner) generateSignature(ctx context.Context, desc ocispec.Descriptor, opts notation.SignOptions, metadata *proto.GetMetadataResponse) ([]byte, *signature.SignerInfo, error) {
func (s *pluginSigner) generateSignature(ctx context.Context, desc ocispec.Descriptor, opts notation.SignOptions, metadata *proto.GetMetadataResponse) ([]byte, map[string]string, *signature.SignerInfo, error) {
logger := log.GetLogger(ctx)
logger.Debug("Generating signature by plugin")
config := s.mergeConfig(opts.PluginConfig)
// Get key info.
key, err := s.describeKey(ctx, config)
if err != nil {
return nil, nil, err
return nil, nil, nil,err
}

// Check keyID is honored.
if s.keyID != key.KeyID {
return nil, nil, fmt.Errorf("keyID in describeKey response %q does not match request %q", key.KeyID, s.keyID)
return nil, nil, nil, fmt.Errorf("keyID in describeKey response %q does not match request %q", key.KeyID, s.keyID)
}
ks, err := proto.DecodeKeySpec(key.KeySpec)
if err != nil {
return nil, nil, err
return nil, nil, nil, err
}

genericSigner := genericSigner{
Expand All @@ -99,13 +101,13 @@ func (s *pluginSigner) generateSignature(ctx context.Context, desc ocispec.Descr
return genericSigner.Sign(ctx, desc, opts)
}

func (s *pluginSigner) generateSignatureEnvelope(ctx context.Context, desc ocispec.Descriptor, opts notation.SignOptions) ([]byte, *signature.SignerInfo, error) {
func (s *pluginSigner) generateSignatureEnvelope(ctx context.Context, desc ocispec.Descriptor, opts notation.SignOptions) ([]byte, map[string]string, *signature.SignerInfo, error) {
logger := log.GetLogger(ctx)
logger.Debug("Generating signature envelope by plugin")
payload := envelope.Payload{TargetArtifact: envelope.SanitizeTargetArtifact(desc)}
payloadBytes, err := json.Marshal(payload)
if err != nil {
return nil, nil, fmt.Errorf("envelope payload can't be marshalled: %w", err)
return nil, nil, nil, fmt.Errorf("envelope payload can't be marshalled: %w", err)
}
// Execute plugin sign command.
req := &proto.GenerateEnvelopeRequest{
Expand All @@ -118,12 +120,12 @@ func (s *pluginSigner) generateSignatureEnvelope(ctx context.Context, desc ocisp
}
resp, err := s.plugin.GenerateEnvelope(ctx, req)
if err != nil {
return nil, nil, fmt.Errorf("plugin failed to sign with following error: %w", err)
return nil, nil, nil, fmt.Errorf("plugin failed to sign with following error: %w", err)
}

// Check signatureEnvelopeType is honored.
if resp.SignatureEnvelopeType != req.SignatureEnvelopeType {
return nil, nil, fmt.Errorf(
return nil, nil, nil, fmt.Errorf(
"signatureEnvelopeType in generateEnvelope response %q does not match request %q",
resp.SignatureEnvelopeType, req.SignatureEnvelopeType,
)
Expand All @@ -132,32 +134,32 @@ func (s *pluginSigner) generateSignatureEnvelope(ctx context.Context, desc ocisp
logger.Debug("Verifying signature envelope generated by the plugin")
sigEnv, err := signature.ParseEnvelope(opts.SignatureMediaType, resp.SignatureEnvelope)
if err != nil {
return nil, nil, err
return nil, nil, nil, err
}

envContent, err := sigEnv.Verify()
if err != nil {
return nil, nil, fmt.Errorf("generated signature failed verification: %w", err)
return nil, nil, nil, fmt.Errorf("generated signature failed verification: %w", err)
}
if err := envelope.ValidatePayloadContentType(&envContent.Payload); err != nil {
return nil, nil, err
return nil, nil, nil, err
}

content := envContent.Payload.Content
var signedPayload envelope.Payload
if err = json.Unmarshal(content, &signedPayload); err != nil {
return nil, nil, fmt.Errorf("signed envelope payload can't be unmarshalled: %w", err)
return nil, nil, nil, fmt.Errorf("signed envelope payload can't be unmarshalled: %w", err)
}

if !isPayloadDescriptorValid(desc, signedPayload.TargetArtifact) {
return nil, nil, fmt.Errorf("during signing descriptor subject has changed from %+v to %+v", desc, signedPayload.TargetArtifact)
return nil, nil, nil, fmt.Errorf("during signing descriptor subject has changed from %+v to %+v", desc, signedPayload.TargetArtifact)
}

if unknownAttributes := areUnknownAttributesAdded(content); len(unknownAttributes) != 0 {
return nil, nil, fmt.Errorf("during signing, following unknown attributes were added to subject descriptor: %+q", unknownAttributes)
return nil, nil, nil, fmt.Errorf("during signing, following unknown attributes were added to subject descriptor: %+q", unknownAttributes)
}

return resp.SignatureEnvelope, &envContent.SignerInfo, nil
return resp.SignatureEnvelope, resp.Annotations, &envContent.SignerInfo, nil
}

func (s *pluginSigner) mergeConfig(config map[string]string) map[string]string {
Expand Down
33 changes: 27 additions & 6 deletions signer/plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ type mockPlugin struct {
invalidSig bool
invalidCertChain bool
invalidDescriptor bool
annotations map[string]string
key crypto.PrivateKey
certs []*x509.Certificate
keySpec signature.KeySpec
Expand All @@ -64,7 +65,7 @@ func newMockPlugin(key crypto.PrivateKey, certs []*x509.Certificate, keySpec sig
}
}

func (p *mockPlugin) GetMetadata(ctx context.Context, req *proto.GetMetadataRequest) (*proto.GetMetadataResponse, error) {
func (p *mockPlugin) GetMetadata(context.Context, *proto.GetMetadataRequest) (*proto.GetMetadataResponse, error) {
if p.wantEnvelope {
return &proto.GetMetadataResponse{
Name: "testPlugin",
Expand Down Expand Up @@ -178,13 +179,14 @@ func (p *mockPlugin) GenerateEnvelope(ctx context.Context, req *proto.GenerateEn
return nil, err
}
validSignOpts.SignatureMediaType = req.SignatureEnvelopeType
data, _, err := internalPluginSigner.Sign(ctx, payload.TargetArtifact, validSignOpts)
data, _, _, err := internalPluginSigner.Sign(ctx, payload.TargetArtifact, validSignOpts)
if err != nil {
return nil, err
}
return &proto.GenerateEnvelopeResponse{
SignatureEnvelope: data,
SignatureEnvelopeType: req.SignatureEnvelopeType,
Annotations: p.annotations,
}, nil
}
return &proto.GenerateEnvelopeResponse{}, nil
Expand Down Expand Up @@ -226,7 +228,7 @@ func TestSigner_Sign_ExpiryInValid(t *testing.T) {
signer := pluginSigner{
plugin: newMockPlugin(keyCertPairCollections[0].key, keyCertPairCollections[0].certs, ks),
}
_, _, err := signer.Sign(context.Background(), ocispec.Descriptor{}, notation.SignOptions{ExpiryDuration: -24 * time.Hour, SignatureMediaType: envelopeType})
_, _, _, err := signer.Sign(context.Background(), ocispec.Descriptor{}, notation.SignOptions{ExpiryDuration: -24 * time.Hour, SignatureMediaType: envelopeType})
wantEr := "expiry cannot be equal or before the signing time"
if err == nil || !strings.Contains(err.Error(), wantEr) {
t.Errorf("Signer.Sign() error = %v, wantErr %v", err, wantEr)
Expand Down Expand Up @@ -289,6 +291,25 @@ func TestPluginSigner_Sign_Valid(t *testing.T) {
}
}

func TestPluginSigner_SignWithAnnotations_Valid(t *testing.T) {
for _, envelopeType := range signature.RegisteredEnvelopeTypes() {
for _, keyCert := range keyCertPairCollections {
t.Run(fmt.Sprintf("external plugin,envelopeType=%v_keySpec=%v", envelopeType, keyCert.keySpecName), func(t *testing.T) {
keySpec, _ := proto.DecodeKeySpec(proto.KeySpec(keyCert.keySpecName))
pluginSigner := pluginSigner{
plugin: &mockPlugin{
key: keyCert.key,
certs: keyCert.certs,
keySpec: keySpec,
annotations: map[string]string{"key": "value"},
},
}
basicSignTest(t, &pluginSigner, envelopeType, &validMetadata)
})
}
}
}

func TestPluginSigner_SignEnvelope_RunFailed(t *testing.T) {
for _, envelopeType := range signature.RegisteredEnvelopeTypes() {
t.Run(fmt.Sprintf("envelopeType=%v", envelopeType), func(t *testing.T) {
Expand Down Expand Up @@ -322,15 +343,15 @@ func TestPluginSigner_SignEnvelope_Valid(t *testing.T) {

func testSignerError(t *testing.T, signer pluginSigner, wantEr string, opts notation.SignOptions) {
t.Helper()
_, _, err := signer.Sign(context.Background(), ocispec.Descriptor{}, opts)
_, _, _, err := signer.Sign(context.Background(), ocispec.Descriptor{}, opts)
if err == nil || !strings.Contains(err.Error(), wantEr) {
t.Errorf("Signer.Sign() error = %v, wantErr %v", err, wantEr)
}
}

func basicSignTest(t *testing.T, pluginSigner *pluginSigner, envelopeType string, metadata *proto.GetMetadataResponse) {
validSignOpts.SignatureMediaType = envelopeType
data, signerInfo, err := pluginSigner.Sign(context.Background(), validSignDescriptor, validSignOpts)
data, ants, signerInfo, err := pluginSigner.Sign(context.Background(), validSignDescriptor, validSignOpts)
if err != nil {
t.Fatalf("Signer.Sign() error = %v, wantErr nil", err)
}
Expand Down Expand Up @@ -374,5 +395,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, ants, envelopeType, mockPlugin.certs[len(mockPlugin.certs)-1], metadata)
}
14 changes: 7 additions & 7 deletions signer/signer.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,14 @@ func NewFromFiles(keyPath, certChainPath string) (notation.Signer, error) {

// Sign signs the artifact described by its descriptor and returns the
// marshalled envelope.
func (s *genericSigner) Sign(ctx context.Context, desc ocispec.Descriptor, opts notation.SignOptions) ([]byte, *signature.SignerInfo, error) {
func (s *genericSigner) Sign(ctx context.Context, desc ocispec.Descriptor, opts notation.SignOptions) ([]byte, map[string]string ,*signature.SignerInfo, error) {
logger := log.GetLogger(ctx)
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)}
payloadBytes, err := json.Marshal(payload)
if err != nil {
return nil, nil, fmt.Errorf("envelope payload can't be marshalled: %w", err)
return nil, nil, nil, fmt.Errorf("envelope payload can't be marshalled: %w", err)
}

var signingAgentId string
Expand Down Expand Up @@ -114,22 +114,22 @@ func (s *genericSigner) Sign(ctx context.Context, desc ocispec.Descriptor, opts
// perform signing
sigEnv, err := signature.NewEnvelope(opts.SignatureMediaType)
if err != nil {
return nil, nil, err
return nil, nil, nil, err
}

sig, err := sigEnv.Sign(signReq)
if err != nil {
return nil, nil, err
return nil, nil, nil, err
}

envContent, err := sigEnv.Verify()
if err != nil {
return nil, nil, fmt.Errorf("generated signature failed verification: %v", err)
return nil, nil, nil, fmt.Errorf("generated signature failed verification: %v", err)
}
if err := envelope.ValidatePayloadContentType(&envContent.Payload); err != nil {
return nil, nil, err
return nil, nil, nil, err
}

// TODO: re-enable timestamping https://github.com/notaryproject/notation-go/issues/78
return sig, &envContent.SignerInfo, nil
return sig, nil, &envContent.SignerInfo, nil
}
Loading

0 comments on commit 62eec92

Please sign in to comment.