some more

Signed-off-by: Pritesh Bandi <priteshbandi@gmail.com>
notaryproject · Apr 2, 2024 · 270f3d3 · 270f3d3
1 parent 56b9fbd
commit 270f3d3
Show file tree

Hide file tree

Showing 5 changed files with 58 additions and 81 deletions.
diff --git a/dir/path.go b/dir/path.go
@@ -11,7 +11,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// package dir implements Notation directory structure.
+// Package dir implements Notation directory structure.
 // [directory spec]: https://github.com/notaryproject/notation/blob/main/specs/directory.md
 //
 // Example:
@@ -58,7 +58,8 @@ const (
 	// PathSigningKeys is the signingkeys file relative path.
 	PathSigningKeys = "signingkeys.json"
 	// PathTrustPolicy is the OCI trust policy file relative path.
-	// Deprecated
+	// Deprecated: PathTrustPolicy exists for historical compatibility and should not be used.
+	// To get OCI trust policy path, use PathOCITrustPolicy.
 	PathTrustPolicy = PathOCITrustPolicy
 	// PathOCITrustPolicy is the OCI trust policy file relative path.
 	PathOCITrustPolicy = "trustpolicy.json"

diff --git a/verifier/trustpolicy/blob.go b/verifier/trustpolicy/blob.go
@@ -38,10 +38,10 @@ type BlobTrustPolicy struct {
 	SignatureVerification SignatureVerification `json:"signatureVerification"`
 
 	// TrustStores this policy statement uses
-	TrustStores []string `json:"trustStores,omitempty"`
+	TrustStores []string `json:"trustStores"`
 
 	// TrustedIdentities this policy statement pins
-	TrustedIdentities []string `json:"trustedIdentities,omitempty"`
+	TrustedIdentities []string `json:"trustedIdentities"`
 
 	// GlobalPolicy defines if policy statement is global or not
 	GlobalPolicy bool `json:"globalPolicy,omitempty"`
@@ -64,7 +64,7 @@ func (policyDoc *BlobDocument) Validate() error {
 
 	// Validate Version
 	if policyDoc.Version == "" {
-		return errors.New("trust policy document is missing or has empty version, it must be specified")
+		return errors.New("trust policy has empty version, version must be specified")
 	}
 	if !slices.Contains(supportedPolicyVersions, policyDoc.Version) {
 		return fmt.Errorf("trust policy document uses unsupported version %q", policyDoc.Version)
@@ -79,7 +79,7 @@ func (policyDoc *BlobDocument) Validate() error {
 	foundGlobalPolicy := false
 	for _, statement := range policyDoc.BlobTrustPolicies {
 		policyStatementNameCount[statement.Name]++
-		if err := validateCore(statement.Name, statement.SignatureVerification, statement.TrustStores, statement.TrustedIdentities); err != nil {
+		if err := validatePolicyCore(statement.Name, statement.SignatureVerification, statement.TrustStores, statement.TrustedIdentities); err != nil {
 			return err
 		}
 
@@ -98,41 +98,29 @@ func (policyDoc *BlobDocument) Validate() error {
 		}
 	}
 
-	// No errors
 	return nil
 }
 
 // GetApplicableTrustPolicy returns a pointer to the deep copied TrustPolicy
 // statement that applies to the given registry scope. If no applicable trust
 // policy is found, returns an error
 // see https://github.com/notaryproject/notaryproject/blob/v1.0.0-rc.2/specs/trust-store-trust-policy.md#selecting-a-trust-policy-based-on-artifact-uri
-// TODO optimize
 func (policyDoc *BlobDocument) GetApplicableTrustPolicy(policyName string) (*BlobTrustPolicy, error) {
-	exactPolicyMatch := true
-	if policyName == "" {
-		exactPolicyMatch = false
-	}
-
-	var applicablePolicy *BlobTrustPolicy
 	for _, policyStatement := range policyDoc.BlobTrustPolicies {
-		if exactPolicyMatch {
-			if policyStatement.Name == policyName {
-				applicablePolicy = (&policyStatement).clone()
-				break
+		if policyName == "" {
+			// global policy
+			if policyStatement.GlobalPolicy {
+				return (&policyStatement).clone(), nil
 			}
 		} else {
-			if policyStatement.GlobalPolicy {
-				applicablePolicy = (&policyStatement).clone()
-				break
+			// exact match
+			if policyStatement.Name == policyName {
+				return (&policyStatement).clone(), nil
 			}
 		}
 	}
 
-	if applicablePolicy != nil {
-		return applicablePolicy, nil
-	}
-
-	return nil, fmt.Errorf("no applicable blob trust policy. Applicability for a given blob artifact is determined by policy name")
+	return nil, fmt.Errorf("no applicable blob trust policy. Applicability for a given blob is determined by policy name")
 }
 
 // clone returns a pointer to the deeply copied TrustPolicy

diff --git a/verifier/trustpolicy/oci.go b/verifier/trustpolicy/oci.go
@@ -42,25 +42,28 @@ type OCITrustPolicy struct {
 	SignatureVerification SignatureVerification `json:"signatureVerification"`
 
 	// TrustStores this policy statement uses
-	TrustStores []string `json:"trustStores,omitempty"`
+	TrustStores []string `json:"trustStores"`
 
 	// TrustedIdentities this policy statement pins
-	TrustedIdentities []string `json:"trustedIdentities,omitempty"`
+	TrustedIdentities []string `json:"trustedIdentities"`
 
 	// RegistryScopes that this policy statement affects
 	RegistryScopes []string `json:"registryScopes"`
 }
 
 // Document represents a trustPolicy.json document
-// Deprecated
+// Deprecated: Document exists for historical compatibility and should not be used.
+// To create OCI Document, use OCIDocument.
 type Document = OCIDocument
 
 // TrustPolicy represents a policy statement in the policy document
-// Deprecated
+// Deprecated: TrustPolicy exists for historical compatibility and should not be used.
+// To create OCI TrustPolicy, use OCITrustPolicy.
 type TrustPolicy = OCITrustPolicy
 
 // LoadDocument loads a trust policy document from a local file system
-// Deprecated
+// Deprecated: LoadDocument function exists for historical compatibility and should not be used.
+// To load OCI Document, use LoadOCIDocument function.
 var LoadDocument = LoadOCIDocument
 
 // LoadOCIDocument loads a trust policy document from a local file system
@@ -80,7 +83,7 @@ func (policyDoc *OCIDocument) Validate() error {
 
 	// Validate Version
 	if policyDoc.Version == "" {
-		return errors.New("trust policy document is missing or has empty version, it must be specified")
+		return errors.New("trust policy document has empty version, version must be specified")
 	}
 	if !slices.Contains(supportedPolicyVersions, policyDoc.Version) {
 		return fmt.Errorf("trust policy document uses unsupported version %q", policyDoc.Version)
@@ -93,7 +96,7 @@ func (policyDoc *OCIDocument) Validate() error {
 
 	policyStatementNameCount := make(map[string]int)
 	for _, statement := range policyDoc.TrustPolicies {
-		if err := validateCore(statement.Name, statement.SignatureVerification, statement.TrustStores, statement.TrustedIdentities); err != nil {
+		if err := validatePolicyCore(statement.Name, statement.SignatureVerification, statement.TrustStores, statement.TrustedIdentities); err != nil {
 			return err
 		}
 		policyStatementNameCount[statement.Name]++
@@ -111,7 +114,6 @@ func (policyDoc *OCIDocument) Validate() error {
 		}
 	}
 
-	// No errors
 	return nil
 }
 

diff --git a/verifier/trustpolicy/trustpolicy.go b/verifier/trustpolicy/trustpolicy.go
@@ -141,23 +141,6 @@ type SignatureVerification struct {
 	Override          map[ValidationType]ValidationAction `json:"override,omitempty"`
 }
 
-type Type int
-
-const (
-	TypeBlob Type = iota
-	TypeOCI
-)
-
-func Get(p Type) (interface{}, error) {
-	switch p {
-	case TypeBlob:
-		return LoadBlobDocument()
-	case TypeOCI:
-		return LoadBlobDocument()
-	}
-	return 0, fmt.Errorf("invalid policy type:L %v", p)
-}
-
 func getDocument(path string, v any) error {
 	path, err := dir.ConfigFS().SysPath(path)
 	if err != nil {
@@ -192,7 +175,6 @@ func getDocument(path string, v any) error {
 		return fmt.Errorf("malformed trust policy. To create a trust policy, see: %s", trustPolicyLink)
 	}
 	return nil
-
 }
 
 // GetVerificationLevel returns VerificationLevel struct for the given
@@ -267,7 +249,7 @@ func (signatureVerification *SignatureVerification) GetVerificationLevel() (*Ver
 	return customVerificationLevel, nil
 }
 
-func validateCore(name string, signatureVerification SignatureVerification, trustStores, trustedIdentities []string) error {
+func validatePolicyCore(name string, signatureVerification SignatureVerification, trustStores, trustedIdentities []string) error {
 	// Verify statement name is valid
 	if name == "" {
 		return errors.New("a trust policy statement is missing a name, every statement requires a name")
@@ -391,7 +373,7 @@ func isValidTrustStoreType(s string) bool {
 	return false
 }
 
-// Internal type to hold raw and parsed Distinguished Names
+// parsedDN holds raw and parsed Distinguished Names
 type parsedDN struct {
 	RawString string
 	ParsedMap map[string]string

diff --git a/verifier/verifier.go b/verifier/verifier.go
@@ -11,7 +11,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// Package Verifier provides an implementation of notation.Verifier interface
+// Package verifier provides an implementation of notation.Verifier interface
 package verifier
 
 import (
@@ -63,14 +63,8 @@ type VerifierOptions struct {
 	RevocationClient revocation.Revocation
 }
 
-// NewFromConfig returns a Verifier based on local file system
-// Deprecated
-func NewFromConfig() (notation.Verifier, error) {
-	return NewOCIFromConfig()
-}
-
-// NewOCIFromConfig returns an OCI Verifier based on local file system
-func NewOCIFromConfig() (*Verifier, error) {
+// NewOCIVerifierFromConfig returns a OCI Verifier based on local file system
+func NewOCIVerifierFromConfig() (*Verifier, error) {
 	// load trust policy
 	policyDocument, err := trustpolicy.LoadOCIDocument()
 	if err != nil {
@@ -82,8 +76,8 @@ func NewOCIFromConfig() (*Verifier, error) {
 	return NewVerifier(policyDocument, nil, x509TrustStore, plugin.NewCLIManager(dir.PluginFS()))
 }
 
-// NewBlobIFromConfig returns an Blob Verifier based on local file system
-func NewBlobIFromConfig() (*Verifier, error) {
+// NewBlobVerifierFromConfig returns a Blob Verifier based on local file system
+func NewBlobVerifierFromConfig() (*Verifier, error) {
 	// load trust policy
 	policyDocument, err := trustpolicy.LoadBlobDocument()
 	if err != nil {
@@ -95,24 +89,19 @@ func NewBlobIFromConfig() (*Verifier, error) {
 	return NewVerifier(nil, policyDocument, x509TrustStore, plugin.NewCLIManager(dir.PluginFS()))
 }
 
-// New creates a new Verifier given trustPolicy, trustStore and pluginManager
-// Deprecated
-func New(trustPolicy *trustpolicy.OCIDocument, trustStore truststore.X509TrustStore, pluginManager plugin.Manager) (notation.Verifier, error) {
-	return NewVerifierWithOptions(trustPolicy, nil, trustStore, pluginManager, VerifierOptions{})
+// NewWithOptions creates a new Verifier given trustPolicy, trustStore,
+// pluginManager, and VerifierOptions
+// Deprecated: NewWithOptions function exists for historical compatibility and should not be used.
+// To create Verifier, use NewVerifierWithOptions function.
+func NewWithOptions(trustPolicy *trustpolicy.OCIDocument, trustStore truststore.X509TrustStore, pluginManager plugin.Manager, opts VerifierOptions) (notation.Verifier, error) {
+	return NewVerifierWithOptions(trustPolicy, nil, trustStore, pluginManager, opts)
 }
 
 // NewVerifier creates a new Verifier given trustPolicies, trustStore and pluginManager
 func NewVerifier(ociTrustPolicy *trustpolicy.OCIDocument, blobTrustPolicy *trustpolicy.BlobDocument, trustStore truststore.X509TrustStore, pluginManager plugin.Manager) (*Verifier, error) {
 	return NewVerifierWithOptions(ociTrustPolicy, blobTrustPolicy, trustStore, pluginManager, VerifierOptions{})
 }
 
-// NewWithOptions creates a new Verifier given trustPolicy, trustStore,
-// pluginManager, and VerifierOptions
-// Deprecated
-func NewWithOptions(trustPolicy *trustpolicy.OCIDocument, trustStore truststore.X509TrustStore, pluginManager plugin.Manager, opts VerifierOptions) (notation.Verifier, error) {
-	return NewVerifierWithOptions(trustPolicy, nil, trustStore, pluginManager, opts)
-}
-
 func NewVerifierWithOptions(ociTrustPolicy *trustpolicy.OCIDocument, blobTrustPolicy *trustpolicy.BlobDocument, trustStore truststore.X509TrustStore, pluginManager plugin.Manager, opts VerifierOptions) (*Verifier, error) {
 	revocationClient := opts.RevocationClient
 	if revocationClient == nil {
@@ -153,6 +142,20 @@ func NewVerifierWithOptions(ociTrustPolicy *trustpolicy.OCIDocument, blobTrustPo
 
 }
 
+// NewFromConfig returns a Verifier based on local file system
+// Deprecated: NewFromConfig function exists for historical compatibility and should not be used.
+// To create an OCI Verifier, use NewOCIVerifierFromConfig function.
+func NewFromConfig() (notation.Verifier, error) {
+	return NewOCIVerifierFromConfig()
+}
+
+// New creates a new Verifier given trustPolicy, trustStore and pluginManager
+// Deprecated: New function exists for historical compatibility and should not be used.
+// To create Verifier, use NewVerifier function.
+func New(trustPolicy *trustpolicy.OCIDocument, trustStore truststore.X509TrustStore, pluginManager plugin.Manager) (notation.Verifier, error) {
+	return NewVerifier(trustPolicy, nil, trustStore, pluginManager)
+}
+
 // SkipVerify validates whether the verification level is skip.
 func (v *Verifier) SkipVerify(ctx context.Context, opts notation.VerifierVerifyOptions) (bool, *trustpolicy.VerificationLevel, error) {
 	logger := log.GetLogger(ctx)
@@ -163,20 +166,20 @@ func (v *Verifier) SkipVerify(ctx context.Context, opts notation.VerifierVerifyO
 		return false, nil, notation.ErrorNoApplicableTrustPolicy{Msg: err.Error()}
 	}
 	logger.Infof("Trust policy configuration: %+v", trustPolicy)
+
 	// ignore the error since we already validated the policy document
 	verificationLevel, _ := trustPolicy.SignatureVerification.GetVerificationLevel()
-
 	// verificationLevel is skip
 	if reflect.DeepEqual(verificationLevel, trustpolicy.LevelSkip) {
 		logger.Debug("Skipping signature verification")
 		return true, trustpolicy.LevelSkip, nil
 	}
+
 	return false, verificationLevel, nil
 }
 
 func (v *Verifier) VerifyBlob(ctx context.Context, descGenFunc notation.BlobDescriptorGenerator, signature []byte, opts notation.BlobVerifierVerifyOptions) (*notation.VerificationOutcome, error) {
 	envelopeMediaType := opts.SignatureMediaType
-	pluginConfig := opts.PluginConfig
 	logger := log.GetLogger(ctx)
 	logger.Debugf("Verify blob signature %v of media type %v", envelopeMediaType)
 
@@ -185,6 +188,7 @@ func (v *Verifier) VerifyBlob(ctx context.Context, descGenFunc notation.BlobDesc
 		return nil, notation.ErrorNoApplicableTrustPolicy{Msg: err.Error()}
 	}
 	logger.Infof("Trust policy configuration: %+v", trustPolicy)
+
 	// ignore the error since we already validated the policy document
 	verificationLevel, _ := trustPolicy.SignatureVerification.GetVerificationLevel()
 	outcome := &notation.VerificationOutcome{
@@ -196,7 +200,7 @@ func (v *Verifier) VerifyBlob(ctx context.Context, descGenFunc notation.BlobDesc
 		logger.Debug("Skipping signature verification")
 		return outcome, nil
 	}
-	err = v.processSignature(ctx, signature, envelopeMediaType, trustPolicy.Name, trustPolicy.TrustedIdentities, trustPolicy.TrustStores, pluginConfig, outcome)
+	err = v.processSignature(ctx, signature, envelopeMediaType, trustPolicy.Name, trustPolicy.TrustedIdentities, trustPolicy.TrustStores, opts.PluginConfig, outcome)
 	if err != nil {
 		outcome.Error = err
 		return outcome, err
@@ -220,7 +224,7 @@ func (v *Verifier) VerifyBlob(ctx context.Context, descGenFunc notation.BlobDesc
 	if desc.Digest != payload.TargetArtifact.Digest || desc.Size != payload.TargetArtifact.Size ||
 		(desc.MediaType != "" && desc.MediaType != payload.TargetArtifact.MediaType) {
 		logger.Infof("payload.TargetArtifact in signature: %+v", payload.TargetArtifact)
-		logger.Infof("Target blob that want to be verified: %+v", desc)
+		logger.Infof("Target blob that want to be verify: %+v", desc)
 		outcome.Error = errors.New("content descriptor mismatch")
 	}
 
@@ -791,7 +795,7 @@ func executePlugin(ctx context.Context, installedPlugin pluginframework.VerifyPl
 
 	req := &pluginframework.VerifySignatureRequest{
 		ContractVersion: pluginframework.ContractVersion,
-		Signature:       signature,
+		Signature:       sig,
 		TrustPolicy:     policy,
 		PluginConfig:    pluginConfig,
 	}