Add more validations to trust-policy parsing

go.mod

@@ -6,3 +6,8 @@ require (
 	github.com/golang-jwt/jwt/v4 v4.4.1
 	github.com/opencontainers/go-digest v1.0.0
 )
+
+require (
+	github.com/go-ldap/ldap v3.0.3+incompatible // indirect
+	gopkg.in/asn1-ber.v1 v1.0.0-20181015200546-f715ec2f112d // indirect
+)

go.sum

@@ -1,4 +1,8 @@
+github.com/go-ldap/ldap v3.0.3+incompatible h1:HTeSZO8hWMS1Rgb2Ziku6b8a7qRIZZMHjsvuZyatzwk=
+github.com/go-ldap/ldap v3.0.3+incompatible/go.mod h1:qfd9rJvER9Q0/D/Sqn1DfHRoBp40uXYvFoEVrNEPqRc=
 github.com/golang-jwt/jwt/v4 v4.4.1 h1:pC5DB52sCeK48Wlb9oPcdhnjkz1TKt1D/P7WKJ0kUcQ=
 github.com/golang-jwt/jwt/v4 v4.4.1/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0=
 github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8Oi/yOhh5U=
 github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM=
+gopkg.in/asn1-ber.v1 v1.0.0-20181015200546-f715ec2f112d h1:TxyelI5cVkbREznMhfzycHdkp5cLA7DpE+GKjSslYhM=
+gopkg.in/asn1-ber.v1 v1.0.0-20181015200546-f715ec2f112d/go.mod h1:cuepJuh7vyXfUyUwEgHQXw849cJrilpS5NeIjOWESAw=

verification/helpers.go

@@ -0,0 +1,74 @@
+package verification
+
+import (
+	"fmt"
+	"regexp"
+	"strings"
+
+	"github.com/go-ldap/ldap"
+)
+
+// isPresent is a utility function to check if a string exists in an array
+func isPresent(val string, values []string) bool {
+	for _, v := range values {
+		if v == val {
+			return true
+		}
+	}
+	return false
+}
+
+// validateRegistryScopeFormat validates if a scope is following the format defined in distribution spec
+func validateRegistryScopeFormat(scope string) error {
+	// Domain and Repository regexes are adapted from distribution implementation
+	// https://github.com/distribution/distribution/blob/main/reference/regexp.go#L31
+	domainRegexp := regexp.MustCompile(`^(?:[a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9])(?:(?:\.(?:[a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9]))+)?(?::[0-9]+)?$`)
+	repositoryRegexp := regexp.MustCompile(`^[a-z0-9]+(?:(?:(?:[._]|__|[-]*)[a-z0-9]+)+)?(?:(?:/[a-z0-9]+(?:(?:(?:[._]|__|[-]*)[a-z0-9]+)+)?)+)?$`)
+	errorMessage := "registry scope %q is not valid, make sure it is the fully qualified registry URL without the scheme/protocol. e.g domain.com/my/repository"
+	firstSlash := strings.Index(scope, "/")
+	if firstSlash < 0 {
+		return fmt.Errorf(errorMessage, scope)
+	}
+	domain := scope[:firstSlash]
+	repository := scope[firstSlash+1:]
+
+	if domain == "" || repository == "" || !domainRegexp.MatchString(domain) || !repositoryRegexp.MatchString(repository) {
+		return fmt.Errorf(errorMessage, scope)
+	}
+
+	// No errors
+	return nil
+}
+
+// validateDistinguishedName validates if a DN name parsable and follows Notary V2 rules
+func validateDistinguishedName(name string) error {
+	mandatoryFields := []string{"C", "ST", "O"}
+	rDnCount := make(map[string]int)
+	dn, err := ldap.ParseDN(name)
+
+	if err != nil {
+		return fmt.Errorf("distinguished name (DN) %q is not valid, make sure it is following rfc4514 standard", name)
+	}
+
+	for _, rdn := range dn.RDNs {
+		for _, attribute := range rdn.Attributes {
+			rDnCount[attribute.Type]++
+		}
+	}
+
+	// Verify there are no duplicate RDNs (multi-valdued RDNs are not supported)
+	for key := range rDnCount {
+		if rDnCount[key] > 1 {
+			return fmt.Errorf("distinguished name (DN) %q has duplicate RDNs, DN can only have unique RDNs", name)
+		}
+	}
+
+	// Verify mandatory fields are present
+	for _, field := range mandatoryFields {
+		if rDnCount[field] != 1 {
+			return fmt.Errorf("distinguished name (DN) %q has no mandatory RDN for %q", name, field)
+		}
+	}
+	// No errors
+	return nil
+}

verification/policy.go

@@ -11,6 +11,11 @@ import (
 	"strings"
 )
 
+const (
+	wildcard    = "*"
+	x509Subject = "x509.subject"
+)
+
 // PolicyDocument represents a trustPolicy.json document
 type PolicyDocument struct {
 	// Version of the policy document
@@ -33,23 +38,93 @@ type TrustPolicy struct {
 	TrustedIdentities []string `json:"trustedIdentities,omitempty"`
 }
 
-func isPresent(val string, values []string) bool {
-	for _, v := range values {
-		if v == val {
-			return true
+// validateRegistryScopes validates if the policy document is following the Notary V2 spec rules for registry scopes
+func validateRegistryScopes(policyDoc *PolicyDocument) error {
+	registryScopeCount := make(map[string]int)
+
+	for _, statement := range policyDoc.TrustPolicies {
+		// Verify registry scopes are valid
+		if len(statement.RegistryScopes) == 0 {
+			return fmt.Errorf("trust policy statement %q has zero registry scopes, it must specify registry scopes with at least one value", statement.Name)
+		}
+		if len(statement.RegistryScopes) > 1 && isPresent(wildcard, statement.RegistryScopes) {
+			return fmt.Errorf("trust policy statement %q uses wildcard registry scope '*', a wildcard scope cannot be used in conjunction with other scope values", statement.Name)
+		}
+		for _, scope := range statement.RegistryScopes {
+			if scope != wildcard {
+				if err := validateRegistryScopeFormat(scope); err != nil {
+					return err
+				}
+			}
+			registryScopeCount[scope]++
+		}
+	}
+
+	// Verify one policy statement per registry scope
+	for key := range registryScopeCount {
+		if registryScopeCount[key] > 1 {
+			return fmt.Errorf("registry scope %q is present in multiple trust policy statements, one registry scope value can only be associated with one statement", key)
+		}
+	}
+
+	// No error
+	return nil
+}
+
+// validateRegistryScopes validates if the policy statement is following the Notary V2 spec rules for trusted identities
+func validateTrustedIdentities(statement TrustPolicy) error {
+
+	// If there is a wildcard in trusted identies, there shouldn't be any other identities
+	if len(statement.TrustedIdentities) > 1 && isPresent(wildcard, statement.TrustedIdentities) {
+		return fmt.Errorf("trust policy statement %q uses a wildcard trusted identity '*', a wildcard identity cannot be used in conjunction with other values", statement.Name)
+	}
+
+	// If there are trusted identities, verify they are valid
+	for _, identity := range statement.TrustedIdentities {
+		if identity == "" {
+			return fmt.Errorf("trust policy statement %q has an empty trusted identity", statement.Name)
 		}
+
+		if identity != wildcard {
+			i := strings.Index(identity, ":")
+			if i < 0 {
+				return fmt.Errorf("trust policy statement %q has trusted identity %q without an identity prefix", statement.Name, identity)
+			}
+
+			identityPrefix := identity[:i]
+			identityValue := identity[i+1:]
+
+			// notation natively supports x509.subject identities only
+			if identityPrefix == x509Subject {
+				if err := validateDistinguishedName(identityValue); err != nil {
+					return err
+				}
+			}
+		}
+	}
+
+	// No error
+	return nil
+}
+
+// validateTrustStore validates if the policy statement is following the Notary V2 spec rules for truststores
+func validateTrustStore(statement TrustPolicy) error {
+	supportedTrustStorePrefixes := []string{"ca"}
+
+	i := strings.Index(statement.TrustStore, ":")
+	if i < 0 || !isPresent(statement.TrustStore[:i], supportedTrustStorePrefixes) {
+		return fmt.Errorf("trust policy statement %q uses an unsupported trust store type %q in trust store value %q", statement.Name, statement.TrustStore[:i], statement.TrustStore)
 	}
-	return false
+
+	return nil
 }
 
 // ValidatePolicyDocument validates a policy document according to it's version's rule set.
 // if any rule is violated, returns an error
 func ValidatePolicyDocument(policyDoc *PolicyDocument) error {
 	// Constants
-	wildcard := "*"
 	supportedPolicyVersions := []string{"1.0"}
 	supportedVerificationPresets := []string{"strict", "permissive", "audit", "skip"}
-	supportedTrustStorePrefixes := []string{"ca"}
 
 	// Validate Version
 	if !isPresent(policyDoc.Version, supportedPolicyVersions) {
@@ -60,8 +135,9 @@ func ValidatePolicyDocument(policyDoc *PolicyDocument) error {
 	if len(policyDoc.TrustPolicies) == 0 {
 		return errors.New("trust policy document can not have zero trust policy statements")
 	}
+
 	policyStatementNameCount := make(map[string]int)
-	registryScopeCount := make(map[string]int)
+
 	for _, statement := range policyDoc.TrustPolicies {
 
 		// Verify statement name is valid
@@ -70,45 +146,37 @@ func ValidatePolicyDocument(policyDoc *PolicyDocument) error {
 		}
 		policyStatementNameCount[statement.Name]++
 
-		// Verify registry scopes are valid
-		if len(statement.RegistryScopes) == 0 {
-			return fmt.Errorf("trust policy statement %q has zero registry scopes, it must specify registry scopes with at least one value", statement.Name)
-		}
-		if len(statement.RegistryScopes) > 1 && isPresent(wildcard, statement.RegistryScopes) {
-			return fmt.Errorf("trust policy statement %q uses wildcard registry scope '*', a wildcard scope cannot be used in conjunction with other scope values", statement.Name)
-		}
-		for _, scope := range statement.RegistryScopes {
-			registryScopeCount[scope]++
-		}
-
 		// Verify signature verification preset is valid
 		if !isPresent(statement.SignatureVerification, supportedVerificationPresets) {
 			return fmt.Errorf("trust policy statement %q uses unsupported signatureVerification value %q", statement.Name, statement.SignatureVerification)
 		}
 
-		// Any signature verification other than "skip" needs a trust store
-		if statement.SignatureVerification != "skip" && (statement.TrustStore == "" || len(statement.TrustedIdentities) == 0) {
-			return fmt.Errorf("trust policy statement %q is either missing a trust store or trusted identities, both must be specified", statement.Name)
-		}
+		// Any signature verification other than "skip" needs a trust store and trusted identities
+		if statement.SignatureVerification == "skip" {
+			if statement.TrustStore != "" || len(statement.TrustedIdentities) > 0 {
+				return fmt.Errorf("trust policy statement %q is set to skip signature verification but configured with a trust store or trusted identities, remove them if signature verification needs to be skipped", statement.Name)
+			}
+		} else {
+			if statement.TrustStore == "" || len(statement.TrustedIdentities) == 0 {
+				return fmt.Errorf("trust policy statement %q is either missing a trust store or trusted identities, both must be specified", statement.Name)
+			}
 
-		// Verify trust store type is valid if it is present (trust store is optional for "skip" signature verification)
-		if statement.TrustStore != "" {
-			i := strings.Index(statement.TrustStore, ":")
-			if i < 0 || !isPresent(statement.TrustStore[:i], supportedTrustStorePrefixes) {
-				return fmt.Errorf("trust policy statement %q uses an unsupported trust store type %q in trust store value %q", statement.Name, statement.TrustStore[:i], statement.TrustStore)
+			// Verify Trust Store is valid
+			if err := validateTrustStore(statement); err != nil {
+				return err
 			}
-		}
 
-		// If there are trusted identities, verify they are not empty
-		for _, identity := range statement.TrustedIdentities {
-			if identity == "" {
-				return fmt.Errorf("trust policy statement %q has an empty trusted identity", statement.Name)
+			// Verify Trusted Identities are valid
+			if err := validateTrustedIdentities(statement); err != nil {
+				return err
 			}
 		}
-		// If there is a wildcard in trusted identies, there shouldn't be any other identities
-		if len(statement.TrustedIdentities) > 1 && isPresent(wildcard, statement.TrustedIdentities) {
-			return fmt.Errorf("trust policy statement %q uses a wildcard trusted identity '*', a wildcard identity cannot be used in conjunction with other values", statement.Name)
-		}
+
+	}
+
+	// Verify registry scopes are valid
+	if err := validateRegistryScopes(policyDoc); err != nil {
+		return err
 	}
 
 	// Verify unique policy statement names across the policy document
@@ -118,13 +186,6 @@ func ValidatePolicyDocument(policyDoc *PolicyDocument) error {
 		}
 	}
 
-	// Verify one policy statement per registry scope
-	for key := range registryScopeCount {
-		if registryScopeCount[key] > 1 {
-			return fmt.Errorf("registry scope %q is present in multiple trust policy statements, one registry scope value can only be associated with one statement", key)
-		}
-	}
-
 	// No errors
 	return nil
 }