Skip to content

Commit

Permalink
chore: clean up (#502)
Browse files Browse the repository at this point in the history 
Signed-off-by: Patrick Zheng <[email protected]>
  • Loading branch information
@Two-Hearts
Two-Hearts authored Jan 13, 2025
1 parent 26ce089 commit b40566d
Show file tree
Hide file tree
Showing 8 changed files with 23 additions and 84 deletions.
34 changes: 8 additions & 26 deletions notation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,13 +159,15 @@ func Sign(ctx context.Context, signer Signer, repo registry.Repository, signOpts
if err != nil {
return ocispec.Descriptor{}, fmt.Errorf("failed to resolve reference: %w", err)
}

// artifactRef is a tag or a digest, if it's a digest it has to match
// the resolved digest
if artifactRef != targetDesc.Digest.String() {
if _, err := digest.Parse(artifactRef); err == nil {
// artifactRef is a digest, but does not match the resolved digest
return ocispec.Descriptor{}, fmt.Errorf("user input digest %s does not match the resolved digest %s", artifactRef, targetDesc.Digest.String())
}

// artifactRef is a tag
logger.Warnf("Always sign the artifact using digest(`@sha256:...`) rather than a tag(`:%s`) because tags are mutable and a tag reference can point to a different artifact than the one signed", artifactRef)
logger.Infof("Resolved artifact tag `%s` to digest `%v` before signing", artifactRef, targetDesc.Digest)
Expand All @@ -178,11 +180,11 @@ func Sign(ctx context.Context, signer Signer, repo registry.Repository, signOpts
if err != nil {
return ocispec.Descriptor{}, err
}

var pluginAnnotations map[string]string
if signerAnts, ok := signer.(signerAnnotation); ok {
pluginAnnotations = signerAnts.PluginAnnotations()
}

logger.Debug("Generating annotation")
annotations, err := generateAnnotations(signerInfo, pluginAnnotations)
if err != nil {
Expand All @@ -193,13 +195,13 @@ func Sign(ctx context.Context, signer Signer, repo registry.Repository, signOpts
_, _, err = repo.PushSignature(ctx, signOpts.SignatureMediaType, sig, targetDesc, annotations)
if err != nil {
var referrerError *remote.ReferrersError

// do not log an error for failing to delete referral index
if !errors.As(err, &referrerError) || !referrerError.IsReferrersIndexDelete() {
logger.Error("Failed to push the signature")
}
return ocispec.Descriptor{}, ErrorPushSignatureFailed{Msg: err.Error()}
}

return targetDesc, nil
}

Expand All @@ -210,15 +212,12 @@ func SignBlob(ctx context.Context, signer BlobSigner, blobReader io.Reader, sign
if err := validateSignArguments(signer, signBlobOpts.SignerSignOptions); err != nil {
return nil, nil, err
}

if blobReader == nil {
return nil, nil, errors.New("blobReader cannot be nil")
}

if signBlobOpts.ContentMediaType == "" {
return nil, nil, errors.New("content media-type cannot be empty")
}

if err := validateContentMediaType(signBlobOpts.ContentMediaType); err != nil {
return nil, nil, err
}
Expand All @@ -243,33 +242,26 @@ func validateSignArguments(signer any, signOpts SignerSignOptions) error {
if err := validateSigMediaType(signOpts.SignatureMediaType); err != nil {
return err
}

return nil
}

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

if desc.Annotations == nil && len(userMetadata) > 0 {
desc.Annotations = map[string]string{}
}

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 desc, fmt.Errorf("error adding user metadata: metadata key %v has reserved prefix %v", k, reservedPrefix)
}
}

if _, ok := desc.Annotations[k]; ok {
return desc, fmt.Errorf("error adding user metadata: metadata key %v is already present in the target artifact", k)
}

desc.Annotations[k] = v
}

return desc, nil
}

Expand Down Expand Up @@ -311,6 +303,7 @@ type VerificationOutcome struct {
Error error
}

// UserMetadata returns the user metadata from the signature envelope.
func (outcome *VerificationOutcome) UserMetadata() (map[string]string, error) {
if outcome.EnvelopeContent == nil {
return nil, errors.New("unable to find envelope content for verification outcome")
Expand All @@ -321,11 +314,9 @@ func (outcome *VerificationOutcome) UserMetadata() (map[string]string, error) {
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
}

Expand Down Expand Up @@ -382,7 +373,7 @@ type BlobVerifierVerifyOptions struct {
type BlobVerifier interface {
// VerifyBlob verifies the `signature` against the target blob using the
// descriptor returned by descGenFunc parameter and
// returns the outcome upon successful verification.
// returns the outcome upon successful verification.
VerifyBlob(ctx context.Context, descGenFunc BlobDescriptorGenerator, signature []byte, opts BlobVerifierVerifyOptions) (*VerificationOutcome, error)
}

Expand Down Expand Up @@ -428,23 +419,18 @@ func VerifyBlob(ctx context.Context, blobVerifier BlobVerifier, blobReader io.Re
if blobVerifier == nil {
return ocispec.Descriptor{}, nil, errors.New("blobVerifier cannot be nil")
}

if blobReader == nil {
return ocispec.Descriptor{}, nil, errors.New("blobReader cannot be nil")
}

if len(signature) == 0 {
return ocispec.Descriptor{}, nil, errors.New("signature cannot be nil or empty")
}

if err := validateContentMediaType(verifyBlobOpts.ContentMediaType); err != nil {
return ocispec.Descriptor{}, nil, err
}

if err := validateSigMediaType(verifyBlobOpts.SignatureMediaType); err != nil {
return ocispec.Descriptor{}, nil, err
}

getDescFunc := getDescriptorFunc(ctx, blobReader, verifyBlobOpts.ContentMediaType, verifyBlobOpts.UserMetadata)
vo, err := blobVerifier.VerifyBlob(ctx, getDescFunc, signature, verifyBlobOpts.BlobVerifierVerifyOptions)
if err != nil {
Expand All @@ -455,12 +441,11 @@ func VerifyBlob(ctx context.Context, blobVerifier BlobVerifier, blobReader io.Re
if err = json.Unmarshal(vo.EnvelopeContent.Payload.Content, &desc); err != nil {
return ocispec.Descriptor{}, nil, err
}

return desc, vo, nil
}

// Verify performs signature verification on each of the notation supported
// verification types (like integrity, authenticity, etc.) and return the
// verification types (like integrity, authenticity, etc.) and returns the
// successful signature verification outcome.
// For more details on signature verification, see
// https://github.com/notaryproject/notaryproject/blob/main/specs/trust-store-trust-policy.md#signature-verification
Expand All @@ -484,7 +469,6 @@ func Verify(ctx context.Context, verifier Verifier, repo registry.Repository, ve
PluginConfig: verifyOpts.PluginConfig,
UserMetadata: verifyOpts.UserMetadata,
}

if skipChecker, ok := verifier.(verifySkipper); ok {
logger.Info("Checking whether signature verification should be skipped or not")
skip, verificationLevel, err := skipChecker.SkipVerify(ctx, opts)
Expand Down Expand Up @@ -558,6 +542,7 @@ func Verify(ctx context.Context, verifier Verifier, repo registry.Repository, ve
}
// at this point, the signature is verified successfully
verificationSucceeded = true

// on success, verificationOutcomes only contains the
// succeeded outcome
verificationOutcomes = []*VerificationOutcome{outcome}
Expand All @@ -566,14 +551,11 @@ func Verify(ctx context.Context, verifier Verifier, repo registry.Repository, ve
// early break on success
return errDoneVerification
}

if numOfSignatureProcessed >= verifyOpts.MaxSignatureAttempts {
return errExceededMaxVerificationLimit
}

return nil
})

if err != nil && !errors.Is(err, errDoneVerification) {
if errors.Is(err, errExceededMaxVerificationLimit) {
return ocispec.Descriptor{}, verificationOutcomes, err
Expand Down
26 changes: 5 additions & 21 deletions signer/plugin.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ func NewPluginSigner(plugin plugin.SignPlugin, keyID string, pluginConfig map[st
if keyID == "" {
return nil, errors.New("keyID not specified")
}

return &PluginSigner{
plugin: plugin,
keyID: keyID,
Expand All @@ -88,20 +87,17 @@ func (s *PluginSigner) PluginAnnotations() map[string]string {
func (s *PluginSigner) Sign(ctx context.Context, desc ocispec.Descriptor, opts notation.SignerSignOptions) ([]byte, *signature.SignerInfo, error) {
logger := log.GetLogger(ctx)
mergedConfig := s.mergeConfig(opts.PluginConfig)

logger.Debug("Invoking plugin's get-plugin-metadata command")
metadata, err := s.plugin.GetMetadata(ctx, &plugin.GetMetadataRequest{PluginConfig: mergedConfig})
if err != nil {
return nil, nil, err
}

logger.Debugf("Using plugin %v with capabilities %v to sign oci artifact %v in signature media type %v", metadata.Name, metadata.Capabilities, desc.Digest, opts.SignatureMediaType)
if metadata.HasCapability(plugin.CapabilitySignatureGenerator) {
ks, err := s.getKeySpec(ctx, mergedConfig)
if err != nil {
return nil, nil, fmt.Errorf("failed to sign with the plugin %s: %w", metadata.Name, err)
}

sig, signerInfo, err := s.generateSignature(ctx, desc, opts, ks, metadata, mergedConfig)
if err != nil {
return nil, nil, fmt.Errorf("failed to sign with the plugin %s: %w", metadata.Name, err)
Expand All @@ -114,7 +110,6 @@ func (s *PluginSigner) Sign(ctx context.Context, desc ocispec.Descriptor, opts n
}
return sig, signerInfo, nil
}

return nil, nil, fmt.Errorf("plugin does not have signing capabilities")
}

Expand All @@ -123,13 +118,11 @@ func (s *PluginSigner) Sign(ctx context.Context, desc ocispec.Descriptor, opts n
func (s *PluginSigner) SignBlob(ctx context.Context, descGenFunc notation.BlobDescriptorGenerator, opts notation.SignerSignOptions) ([]byte, *signature.SignerInfo, error) {
logger := log.GetLogger(ctx)
mergedConfig := s.mergeConfig(opts.PluginConfig)

logger.Debug("Invoking plugin's get-plugin-metadata command")
metadata, err := s.plugin.GetMetadata(ctx, &plugin.GetMetadataRequest{PluginConfig: mergedConfig})
if err != nil {
return nil, nil, err
}

logger.Debug("Invoking plugin's describe-key command")
ks, err := s.getKeySpec(ctx, mergedConfig)
if err != nil {
Expand All @@ -141,7 +134,6 @@ func (s *PluginSigner) SignBlob(ctx context.Context, descGenFunc notation.BlobDe
if err != nil {
return nil, nil, err
}

logger.Debugf("Using plugin %v with capabilities %v to sign blob using descriptor %+v", metadata.Name, metadata.Capabilities, desc)
if metadata.HasCapability(plugin.CapabilitySignatureGenerator) {
return s.generateSignature(ctx, desc, opts, ks, metadata, mergedConfig)
Expand All @@ -158,11 +150,9 @@ func (s *PluginSigner) getKeySpec(ctx context.Context, config map[string]string)
if err != nil {
return signature.KeySpec{}, err
}

if s.keyID != descKeyResp.KeyID {
return signature.KeySpec{}, fmt.Errorf("keyID in describeKey response %q does not match request %q", descKeyResp.KeyID, s.keyID)
}

return proto.DecodeKeySpec(descKeyResp.KeySpec)
}

Expand All @@ -178,7 +168,6 @@ func (s *PluginSigner) generateSignature(ctx context.Context, desc ocispec.Descr
keySpec: ks,
},
}

opts.SigningAgent = fmt.Sprintf("%s %s/%s", signingAgent, metadata.Name, metadata.Version)
return genericSigner.Sign(ctx, desc, opts)
}
Expand All @@ -191,6 +180,7 @@ func (s *PluginSigner) generateSignatureEnvelope(ctx context.Context, desc ocisp
if err != nil {
return nil, nil, fmt.Errorf("envelope payload can't be marshalled: %w", err)
}

// Execute plugin sign command.
req := &plugin.GenerateEnvelopeRequest{
ContractVersion: plugin.ContractVersion,
Expand All @@ -213,45 +203,41 @@ func (s *PluginSigner) generateSignatureEnvelope(ctx context.Context, desc ocisp
resp.SignatureEnvelopeType, req.SignatureEnvelopeType,
)
}

logger.Debug("Verifying signature envelope generated by the plugin")
sigEnv, err := signature.ParseEnvelope(opts.SignatureMediaType, resp.SignatureEnvelope)
if err != nil {
return nil, nil, err
}

envContent, err := sigEnv.Verify()
if err != nil {
return nil, nil, fmt.Errorf("generated signature failed verification: %w", err)
}
if err := envelope.ValidatePayloadContentType(&envContent.Payload); err != nil {
return 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)
}

if !isPayloadDescriptorValid(desc, signedPayload.TargetArtifact) {
return 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)
}

s.manifestAnnotations = resp.Annotations
return resp.SignatureEnvelope, &envContent.SignerInfo, nil
}

func (s *PluginSigner) mergeConfig(config map[string]string) map[string]string {
c := make(map[string]string, len(s.pluginConfig)+len(config))

// First clone s.PluginConfig.
for k, v := range s.pluginConfig {
c[k] = v
}

// Then set or override entries from config.
for k, v := range config {
c[k] = v
Expand All @@ -269,7 +255,6 @@ func (s *PluginSigner) describeKey(ctx context.Context, config map[string]string
if err != nil {
return nil, err
}

return resp, nil
}

Expand All @@ -279,6 +264,7 @@ func isDescriptorSubset(original, newDesc ocispec.Descriptor) bool {
if !content.Equal(original, newDesc) {
return false
}

// Plugins may append additional annotations but not replace/override
// existing.
for k, v := range original.Annotations {
Expand All @@ -296,6 +282,7 @@ func isPayloadDescriptorValid(originalDesc, newDesc ocispec.Descriptor) bool {

func areUnknownAttributesAdded(content []byte) []string {
var targetArtifactMap map[string]interface{}

// Ignoring error because we already successfully unmarshalled before this
// point
_ = json.Unmarshal(content, &targetArtifactMap)
Expand Down Expand Up @@ -352,12 +339,10 @@ func (s *pluginPrimitiveSigner) Sign(payload []byte) ([]byte, []*x509.Certificat
if err != nil {
return nil, nil, err
}

keySpecHash, err := proto.HashAlgorithmFromKeySpec(s.keySpec)
if err != nil {
return nil, nil, err
}

req := &plugin.GenerateSignatureRequest{
ContractVersion: plugin.ContractVersion,
KeyID: s.keyID,
Expand All @@ -366,7 +351,6 @@ func (s *pluginPrimitiveSigner) Sign(payload []byte) ([]byte, []*x509.Certificat
Payload: payload,
PluginConfig: s.pluginConfig,
}

resp, err := s.plugin.GenerateSignature(s.ctx, req)
if err != nil {
return nil, nil, err
Expand Down
Loading

0 comments on commit b40566d

Please sign in to comment.