From 0825b5df446bfefc3f8ed37013648dcb708a13dd Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Tue, 8 Nov 2022 14:33:50 +0800 Subject: [PATCH 01/19] tp/ts refactoring Signed-off-by: Patrick Zheng --- internal/common/common.go | 97 ++++ .../ca/trust-store-with-invalid-certs/invalid | 1 + .../RootAndLeafCerts.crt | 43 ++ .../GlobalSignRootCA.crt | 22 + .../ca/trust-store-with-leaf-certs/non-ca.crt | 21 + .../valid-trust-store_SYMLINK | 1 + .../valid-trust-store-2/GlobalSign.der | Bin 0 -> 867 bytes .../valid-trust-store-2/GlobalSignRootCA.crt | 21 + verification/trustpolicy/trustpolicy.go | 378 +++++++++++++++ verification/trustpolicy/trustpolicy_test.go | 446 ++++++++++++++++++ verification/truststore/truststore.go | 126 +++++ verification/truststore/truststore_test.go | 58 +++ 12 files changed, 1214 insertions(+) create mode 100644 internal/common/common.go create mode 100644 verification/testdata/truststore/x509/ca/trust-store-with-invalid-certs/invalid create mode 100644 verification/testdata/truststore/x509/ca/trust-store-with-leaf-certs-in-single-file/RootAndLeafCerts.crt create mode 100644 verification/testdata/truststore/x509/ca/trust-store-with-leaf-certs/GlobalSignRootCA.crt create mode 100644 verification/testdata/truststore/x509/ca/trust-store-with-leaf-certs/non-ca.crt create mode 100644 verification/testdata/truststore/x509/ca/valid-trust-store_SYMLINK/valid-trust-store_SYMLINK create mode 100644 verification/testdata/truststore/x509/signingAuthority/valid-trust-store-2/GlobalSign.der create mode 100644 verification/testdata/truststore/x509/signingAuthority/valid-trust-store-2/GlobalSignRootCA.crt create mode 100644 verification/trustpolicy/trustpolicy.go create mode 100644 verification/trustpolicy/trustpolicy_test.go create mode 100644 verification/truststore/truststore.go create mode 100644 verification/truststore/truststore_test.go diff --git a/internal/common/common.go b/internal/common/common.go new file mode 100644 index 00000000..92a09f04 --- /dev/null +++ b/internal/common/common.go @@ -0,0 +1,97 @@ +package common + +import ( + "fmt" + "regexp" + "strings" + + ldapv3 "github.com/go-ldap/ldap/v3" +) + +const ( + Wildcard = "*" + X509Subject = "x509.subject" +) + +// 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 +} + +// Internal type to hold raw and parsed Distinguished Names +type ParsedDN struct { + RawString string + ParsedMap map[string]string +} + +// ParseDistinguishedName parses a DN name and validates Notary V2 rules +func ParseDistinguishedName(name string) (map[string]string, error) { + mandatoryFields := []string{"C", "ST", "O"} + attrKeyValue := make(map[string]string) + dn, err := ldapv3.ParseDN(name) + + if err != nil { + return nil, fmt.Errorf("distinguished name (DN) %q is not valid, it must contain 'C', 'ST', and 'O' RDN attributes at a minimum, and follow RFC 4514 standard", name) + } + + for _, rdn := range dn.RDNs { + + // multi-valued RDNs are not supported (TODO: add spec reference here) + if len(rdn.Attributes) > 1 { + return nil, fmt.Errorf("distinguished name (DN) %q has multi-valued RDN attributes, remove multi-valued RDN attributes as they are not supported", name) + } + for _, attribute := range rdn.Attributes { + if attrKeyValue[attribute.Type] == "" { + attrKeyValue[attribute.Type] = attribute.Value + } else { + return nil, fmt.Errorf("distinguished name (DN) %q has duplicate RDN attribute for %q, DN can only have unique RDN attributes", name, attribute.Type) + } + } + } + + // Verify mandatory fields are present + for _, field := range mandatoryFields { + if attrKeyValue[field] == "" { + return nil, fmt.Errorf("distinguished name (DN) %q has no mandatory RDN attribute for %q, it must contain 'C', 'ST', and 'O' RDN attributes at a minimum", name, field) + } + } + // No errors + return attrKeyValue, nil +} + +// IsSubsetDN returns true if dn1 is a subset of dn2 i.e. every key/value pair of dn1 has a matching key/value pair in dn2, otherwise returns false +func IsSubsetDN(dn1 map[string]string, dn2 map[string]string) bool { + for key := range dn1 { + if dn1[key] != dn2[key] { + return false + } + } + return true +} + +// 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 +} diff --git a/verification/testdata/truststore/x509/ca/trust-store-with-invalid-certs/invalid b/verification/testdata/truststore/x509/ca/trust-store-with-invalid-certs/invalid new file mode 100644 index 00000000..9977a283 --- /dev/null +++ b/verification/testdata/truststore/x509/ca/trust-store-with-invalid-certs/invalid @@ -0,0 +1 @@ +invalid diff --git a/verification/testdata/truststore/x509/ca/trust-store-with-leaf-certs-in-single-file/RootAndLeafCerts.crt b/verification/testdata/truststore/x509/ca/trust-store-with-leaf-certs-in-single-file/RootAndLeafCerts.crt new file mode 100644 index 00000000..e6c38cfb --- /dev/null +++ b/verification/testdata/truststore/x509/ca/trust-store-with-leaf-certs-in-single-file/RootAndLeafCerts.crt @@ -0,0 +1,43 @@ +-----BEGIN CERTIFICATE----- +MIIDejCCAmKgAwIBAgIBAjANBgkqhkiG9w0BAQsFADBdMQswCQYDVQQGEwJVUzEL +MAkGA1UECBMCV0ExEDAOBgNVBAcTB1NlYXR0bGUxDzANBgNVBAoTBk5vdGFyeTEe +MBwGA1UEAxMVd2FiYml0LW5ldHdvcmtzLmlvIENBMB4XDTIyMDkyMDA2MzExM1oX +DTIyMDkyMTA2MzExM1owWjELMAkGA1UEBhMCVVMxCzAJBgNVBAgTAldBMRAwDgYD +VQQHEwdTZWF0dGxlMQ8wDQYDVQQKEwZOb3RhcnkxGzAZBgNVBAMTEndhYmJpdC1u +ZXR3b3Jrcy5pbzCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBALiZp5O+ +6YtaNO5GbWaZUxvJPXktJ7k7LBX5G/Kn6eh9JkJln1agqbax9MRDB/5YCdQBKMBq +NE2wYIwmCs7ArFU5DxvRhoBnCGLjcsIZ9pfaZ6lBppEvxMmUAYDmgjze0J13PwRp +WAZMfBlisZnJAWokgE5sWtggUXURyFk67H0R+4sWlm8SSZOiJCA/e0bYPCHTfFA/ +2zg6koNRSwvI6zvftGnnJ9ny0BTuGOjZ6lDfIX5awFrgRdO8wmwejo4oJ45tUotF +/Rt/yHkmjdGhONbJjcMLf9AIyVwMHg6t6mj2SYbHqzIyTcpjk90HgeiU5eS5JMqj +Jkug5U9XrGGCqIcCAwEAAaNIMEYwDgYDVR0PAQH/BAQDAgeAMBMGA1UdJQQMMAoG +CCsGAQUFBwMDMB8GA1UdIwQYMBaAFLAy4Il5S9zOd/AMWF8hATmldAjYMA0GCSqG +SIb3DQEBCwUAA4IBAQBLYBnSuMNCzzLmeqH/wBr6kKUtF10AN9VF8/3iZW8iCj4B +Bx7VDq7iZR/G9UTLsWdZqkkxnOGu4QffBHz2Lc1v9D923EEPDAP5mJYvUchvdXYT +lmyQr9QEjRC6IFhlBB27Bi207QJ8UxYgmbseQ3FQFE16Usdmlg9iWDn5tx/DZn9/ +yUd81yKKYp2uLx0x2sQDJh61QSZB6jtzjN7w4Xax2NViabLaH7raMrDbIqigkXJh +iXG9fWx1Ax7S3dJVIglbZGPgYDW14Ass40gs8vcOBg8CwszrKiEuwp20d12Ky87/ +0pLsOWJmcNyXbd3gztX01N1frSEbvTBJNI9E/jmI +-----END CERTIFICATE----- +-----BEGIN CERTIFICATE----- +MIIDjzCCAnegAwIBAgIBATANBgkqhkiG9w0BAQsFADBdMQswCQYDVQQGEwJVUzEL +MAkGA1UECBMCV0ExEDAOBgNVBAcTB1NlYXR0bGUxDzANBgNVBAoTBk5vdGFyeTEe +MBwGA1UEAxMVd2FiYml0LW5ldHdvcmtzLmlvIENBMB4XDTIyMDkyMDA2MzExM1oX +DTIyMTAyMDA2MzExM1owXTELMAkGA1UEBhMCVVMxCzAJBgNVBAgTAldBMRAwDgYD +VQQHEwdTZWF0dGxlMQ8wDQYDVQQKEwZOb3RhcnkxHjAcBgNVBAMTFXdhYmJpdC1u +ZXR3b3Jrcy5pbyBDQTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAKNM +3dUToC4TyegGMw47ax9aZt13pQgTeV7xZbVsOmZiv/8gZ9tEZWgQbvBJrWUH8y4o +eQLCVQOTESNP2TSyTqizNtG1ex6YfSpWKSqUkfGX2II9xCX8hNXZqTphAjrGGf2Z +EOLRIIkbhjkuiAR+7q4TF/KJhdfYD1HQBJ2PF92egV5JEZTrxIjVIi+WK19VKSwx +m7oFiijve4VPaQYQnWgj0dk+Tn9cMB/OMX6cszoJbn98ogQIvWaY3dd1qba4uGJ9 +vmkNKDJcUd1PbkaVlikXC4UM+PxXy7/ZvSihOXurAPIChS6JgWC8Ru2vxm9SC+BN +5J/hr92W2TdsrvLkrc8CAwEAAaNaMFgwDgYDVR0PAQH/BAQDAgIEMBMGA1UdJQQM +MAoGCCsGAQUFBwMDMBIGA1UdEwEB/wQIMAYBAf8CAQEwHQYDVR0OBBYEFLAy4Il5 +S9zOd/AMWF8hATmldAjYMA0GCSqGSIb3DQEBCwUAA4IBAQCTf6GbT5Z0x5ciNr9i +8i+QsIAg7ZHzv5RLLJuocGcKwbdi+btU6BPl/X4U5ZB6OArv4oiyPSbECoxkgGRq +cj+mfzXdm/3jEyRskHDfoxcJFYmcBsEykS7DoLYEy5HxgKSaGOLl4dMWbbj/E8mR +e9XC5ruvPNZX52pQMqSqUUTYlbR4YQojsp7ShcLLD/Iea90wXk44+wHAKNFpwkN1 +h5JMlYm+jKkol6u/Nmd3vNqhzrL91ZLPVtSWpfsBxh7l4BsDns2uPl+/fgCav9MJ +jUkWJbEaDPY5bSbHDhCbxMO37VbvkkFUvz7lfKAkXj6DnkPzMj3++KTFNdw3fJ4+ +WzLe +-----END CERTIFICATE----- diff --git a/verification/testdata/truststore/x509/ca/trust-store-with-leaf-certs/GlobalSignRootCA.crt b/verification/testdata/truststore/x509/ca/trust-store-with-leaf-certs/GlobalSignRootCA.crt new file mode 100644 index 00000000..c26fd442 --- /dev/null +++ b/verification/testdata/truststore/x509/ca/trust-store-with-leaf-certs/GlobalSignRootCA.crt @@ -0,0 +1,22 @@ +-----BEGIN CERTIFICATE----- +MIIDjzCCAnegAwIBAgIBATANBgkqhkiG9w0BAQsFADBdMQswCQYDVQQGEwJVUzEL +MAkGA1UECBMCV0ExEDAOBgNVBAcTB1NlYXR0bGUxDzANBgNVBAoTBk5vdGFyeTEe +MBwGA1UEAxMVd2FiYml0LW5ldHdvcmtzLmlvIENBMB4XDTIyMDkyMDA2MzExM1oX +DTIyMTAyMDA2MzExM1owXTELMAkGA1UEBhMCVVMxCzAJBgNVBAgTAldBMRAwDgYD +VQQHEwdTZWF0dGxlMQ8wDQYDVQQKEwZOb3RhcnkxHjAcBgNVBAMTFXdhYmJpdC1u +ZXR3b3Jrcy5pbyBDQTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAKNM +3dUToC4TyegGMw47ax9aZt13pQgTeV7xZbVsOmZiv/8gZ9tEZWgQbvBJrWUH8y4o +eQLCVQOTESNP2TSyTqizNtG1ex6YfSpWKSqUkfGX2II9xCX8hNXZqTphAjrGGf2Z +EOLRIIkbhjkuiAR+7q4TF/KJhdfYD1HQBJ2PF92egV5JEZTrxIjVIi+WK19VKSwx +m7oFiijve4VPaQYQnWgj0dk+Tn9cMB/OMX6cszoJbn98ogQIvWaY3dd1qba4uGJ9 +vmkNKDJcUd1PbkaVlikXC4UM+PxXy7/ZvSihOXurAPIChS6JgWC8Ru2vxm9SC+BN +5J/hr92W2TdsrvLkrc8CAwEAAaNaMFgwDgYDVR0PAQH/BAQDAgIEMBMGA1UdJQQM +MAoGCCsGAQUFBwMDMBIGA1UdEwEB/wQIMAYBAf8CAQEwHQYDVR0OBBYEFLAy4Il5 +S9zOd/AMWF8hATmldAjYMA0GCSqGSIb3DQEBCwUAA4IBAQCTf6GbT5Z0x5ciNr9i +8i+QsIAg7ZHzv5RLLJuocGcKwbdi+btU6BPl/X4U5ZB6OArv4oiyPSbECoxkgGRq +cj+mfzXdm/3jEyRskHDfoxcJFYmcBsEykS7DoLYEy5HxgKSaGOLl4dMWbbj/E8mR +e9XC5ruvPNZX52pQMqSqUUTYlbR4YQojsp7ShcLLD/Iea90wXk44+wHAKNFpwkN1 +h5JMlYm+jKkol6u/Nmd3vNqhzrL91ZLPVtSWpfsBxh7l4BsDns2uPl+/fgCav9MJ +jUkWJbEaDPY5bSbHDhCbxMO37VbvkkFUvz7lfKAkXj6DnkPzMj3++KTFNdw3fJ4+ +WzLe +-----END CERTIFICATE----- diff --git a/verification/testdata/truststore/x509/ca/trust-store-with-leaf-certs/non-ca.crt b/verification/testdata/truststore/x509/ca/trust-store-with-leaf-certs/non-ca.crt new file mode 100644 index 00000000..74bc7b00 --- /dev/null +++ b/verification/testdata/truststore/x509/ca/trust-store-with-leaf-certs/non-ca.crt @@ -0,0 +1,21 @@ +-----BEGIN CERTIFICATE----- +MIIDejCCAmKgAwIBAgIBAjANBgkqhkiG9w0BAQsFADBdMQswCQYDVQQGEwJVUzEL +MAkGA1UECBMCV0ExEDAOBgNVBAcTB1NlYXR0bGUxDzANBgNVBAoTBk5vdGFyeTEe +MBwGA1UEAxMVd2FiYml0LW5ldHdvcmtzLmlvIENBMB4XDTIyMDkyMDA2MzExM1oX +DTIyMDkyMTA2MzExM1owWjELMAkGA1UEBhMCVVMxCzAJBgNVBAgTAldBMRAwDgYD +VQQHEwdTZWF0dGxlMQ8wDQYDVQQKEwZOb3RhcnkxGzAZBgNVBAMTEndhYmJpdC1u +ZXR3b3Jrcy5pbzCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBALiZp5O+ +6YtaNO5GbWaZUxvJPXktJ7k7LBX5G/Kn6eh9JkJln1agqbax9MRDB/5YCdQBKMBq +NE2wYIwmCs7ArFU5DxvRhoBnCGLjcsIZ9pfaZ6lBppEvxMmUAYDmgjze0J13PwRp +WAZMfBlisZnJAWokgE5sWtggUXURyFk67H0R+4sWlm8SSZOiJCA/e0bYPCHTfFA/ +2zg6koNRSwvI6zvftGnnJ9ny0BTuGOjZ6lDfIX5awFrgRdO8wmwejo4oJ45tUotF +/Rt/yHkmjdGhONbJjcMLf9AIyVwMHg6t6mj2SYbHqzIyTcpjk90HgeiU5eS5JMqj +Jkug5U9XrGGCqIcCAwEAAaNIMEYwDgYDVR0PAQH/BAQDAgeAMBMGA1UdJQQMMAoG +CCsGAQUFBwMDMB8GA1UdIwQYMBaAFLAy4Il5S9zOd/AMWF8hATmldAjYMA0GCSqG +SIb3DQEBCwUAA4IBAQBLYBnSuMNCzzLmeqH/wBr6kKUtF10AN9VF8/3iZW8iCj4B +Bx7VDq7iZR/G9UTLsWdZqkkxnOGu4QffBHz2Lc1v9D923EEPDAP5mJYvUchvdXYT +lmyQr9QEjRC6IFhlBB27Bi207QJ8UxYgmbseQ3FQFE16Usdmlg9iWDn5tx/DZn9/ +yUd81yKKYp2uLx0x2sQDJh61QSZB6jtzjN7w4Xax2NViabLaH7raMrDbIqigkXJh +iXG9fWx1Ax7S3dJVIglbZGPgYDW14Ass40gs8vcOBg8CwszrKiEuwp20d12Ky87/ +0pLsOWJmcNyXbd3gztX01N1frSEbvTBJNI9E/jmI +-----END CERTIFICATE----- diff --git a/verification/testdata/truststore/x509/ca/valid-trust-store_SYMLINK/valid-trust-store_SYMLINK b/verification/testdata/truststore/x509/ca/valid-trust-store_SYMLINK/valid-trust-store_SYMLINK new file mode 100644 index 00000000..ca626091 --- /dev/null +++ b/verification/testdata/truststore/x509/ca/valid-trust-store_SYMLINK/valid-trust-store_SYMLINK @@ -0,0 +1 @@ +ca/valid-trust-store \ No newline at end of file diff --git a/verification/testdata/truststore/x509/signingAuthority/valid-trust-store-2/GlobalSign.der b/verification/testdata/truststore/x509/signingAuthority/valid-trust-store-2/GlobalSign.der new file mode 100644 index 0000000000000000000000000000000000000000..232c4b6121f7236eb429572c591127e8aa60166f GIT binary patch literal 867 zcmXqLVvaXxVsc-=%*4pV#LdD01dNIi!5oVWc-c6$+C196^D;7WvoaX?7%CXZu`!3T za0`pO=j10P<^*S^=P3l`=a(orJ1XcZ1Q{C&8wi3_a0zoERKNt8kp&Ip#CZ)Y4U7#f z3=KdaN}SgSnM-Bcni!Rky~D`Lz}&>h&tTBR$i>ve$jER;wQNEqTZNL?*8|PlT25)q z`^#D;cyw(?(H}P^=i{5Y=CZ`AoYwPxn9$_*FlaSTqkBQl-IR;3zv?XJZ?fglUN`;v zHjy@g%H7t&4dp!?4?QnsCF#q@{hF3>zf*mx#eBBwb|+7(Me-Kk+i>Eg8eg;MvG>v4 zmsk=`c`noVmTCR%^a+iLPv>?ehMTV`5xG?_`;jk@3_*EunPN{PO=3Mc%Jd@cgsZLDb_S zlVPG{+>Yt**OqTjnN_tv{-E&t*-5{7a~_0bimhb6mG`oFa$(uA%+@AxCT2zk#>Gws z4hDR{_><*lWc<&<0!$HX2K*qtFo@4;zzn1eWI+OaEMhDo{U5yRSLnH_tn&@{l{~Ba z-lMzHdyqpJm}r2Z%*f#FaQi^Os(&YV-hZDK;_A738Uv@}n$5y(Z5r&xr?Q`w?A*nm zyKV{B<*y$<@^|eoPWNg)?owUxV0~UrKC@<@v8C(bz9&1wb5{Kkn)W96nC=smoSjpW zf8PntNDs4X-f`Yk@$kuvf9Gx;3SM<)Lf6X=v2Hz6?^Z=gm=bbRTvM6jq*l?z>U;6vh=ZpX(UQTUo1KYI8Vet 0 || len(statement.TrustedIdentities) > 0 { + return fmt.Errorf("trust policy statement %q is set to skip signature verification but configured with trust stores and/or trusted identities, remove them if signature verification needs to be skipped", statement.Name) + } + } else { + if len(statement.TrustStores) == 0 || len(statement.TrustedIdentities) == 0 { + return fmt.Errorf("trust policy statement %q is either missing trust stores or trusted identities, both must be specified", statement.Name) + } + + // Verify Trust Store is valid + if err := validateTrustStore(statement); err != nil { + return err + } + + // Verify Trusted Identities are valid + if err := validateTrustedIdentities(statement); err != nil { + return err + } + } + + } + + // Verify registry scopes are valid + if err := validateRegistryScopes(policyDoc); err != nil { + return err + } + + // Verify unique policy statement names across the policy document + for key := range policyStatementNameCount { + if policyStatementNameCount[key] > 1 { + return fmt.Errorf("multiple trust policy statements use the same name %q, statement names must be unique", key) + } + } + + // No errors + return nil +} + +// GetVerificationLevel returns VerificationLevel struct for the given SignatureVerification struct +// throws error if SignatureVerification is invalid +func GetVerificationLevel(signatureVerification SignatureVerification) (*VerificationLevel, error) { + var baseLevel *VerificationLevel + for _, l := range VerificationLevels { + if l.Name == signatureVerification.VerificationLevel { + baseLevel = l + } + } + if baseLevel == nil { + return nil, fmt.Errorf("invalid signature verification %q", signatureVerification.VerificationLevel) + } + + if len(signatureVerification.Override) == 0 { + // nothing to override, return the base verification level + return baseLevel, nil + } + + if baseLevel == LevelSkip { + return nil, fmt.Errorf("signature verification %q can't be used to customize signature verification", baseLevel.Name) + } + + customVerificationLevel := &VerificationLevel{ + Name: "custom", + Enforcement: make(map[ValidationType]ValidationAction), + } + + // populate the custom verification level with the base verification settings + for k, v := range baseLevel.Enforcement { + customVerificationLevel.Enforcement[k] = v + } + + // override the verification actions with the user configured settings + for key, value := range signatureVerification.Override { + var validationType ValidationType + for _, t := range ValidationTypes { + if t == key { + validationType = t + break + } + } + if validationType == "" { + return nil, fmt.Errorf("verification type %q in custom signature verification is not supported, supported values are %q", key, ValidationTypes) + } + + var validationAction ValidationAction + for _, action := range ValidationActions { + if action == value { + validationAction = action + break + } + } + if validationAction == "" { + return nil, fmt.Errorf("verification action %q in custom signature verification is not supported, supported values are %q", value, ValidationActions) + } + + if validationType == TypeIntegrity { + return nil, fmt.Errorf("%q verification can not be overridden in custom signature verification", key) + } else if validationType != TypeRevocation && validationAction == ActionSkip { + return nil, fmt.Errorf("%q verification can not be skipped in custom signature verification", key) + } + + customVerificationLevel.Enforcement[validationType] = validationAction + } + return customVerificationLevel, nil +} + +// validateTrustStore validates if the policy statement is following the Notary V2 spec rules for truststores +func validateTrustStore(statement TrustPolicy) error { + for _, trustStore := range statement.TrustStores { + i := strings.Index(trustStore, ":") + if i < 0 || !isValidTrustStoreType(trustStore[:i]) { + return fmt.Errorf("trust policy statement %q uses an unsupported trust store type %q in trust store value %q", statement.Name, trustStore[:i], trustStore) + } + } + + return nil +} + +// validateTrustedIdentities 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 && common.IsPresent(common.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) + } + + var parsedDNs []common.ParsedDN + // 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 != common.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 == common.X509Subject { + dn, err := common.ParseDistinguishedName(identityValue) + if err != nil { + return err + } + parsedDNs = append(parsedDNs, common.ParsedDN{RawString: identity, ParsedMap: dn}) + } + } + } + + // Verify there are no overlapping DNs + if err := validateOverlappingDNs(statement.Name, parsedDNs); err != nil { + return err + } + + // No error + return nil +} + +// validateRegistryScopes validates if the policy document is following the Notary V2 spec rules for registry scopes +func validateRegistryScopes(policyDoc *Document) 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 && common.IsPresent(common.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 != common.Wildcard { + if err := common.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 +} + +func validateOverlappingDNs(policyName string, parsedDNs []common.ParsedDN) error { + for i, dn1 := range parsedDNs { + for j, dn2 := range parsedDNs { + if i != j && common.IsSubsetDN(dn1.ParsedMap, dn2.ParsedMap) { + return fmt.Errorf("trust policy statement %q has overlapping x509 trustedIdentities, %q overlaps with %q", policyName, dn1.RawString, dn2.RawString) + } + } + } + + return nil +} + +// isValidTrustStoreType returns true if the given string is a valid truststore.Type, otherwise false. +func isValidTrustStoreType(s string) bool { + for _, p := range truststore.Types { + if s == string(p) { + return true + } + } + return false +} diff --git a/verification/trustpolicy/trustpolicy_test.go b/verification/trustpolicy/trustpolicy_test.go new file mode 100644 index 00000000..b8d4ee54 --- /dev/null +++ b/verification/trustpolicy/trustpolicy_test.go @@ -0,0 +1,446 @@ +package trustpolicy + +import ( + "strconv" + "testing" +) + +func dummyPolicyStatement() (policyStatement TrustPolicy) { + policyStatement = TrustPolicy{ + Name: "test-statement-name", + RegistryScopes: []string{"registry.acme-rockets.io/software/net-monitor"}, + SignatureVerification: SignatureVerification{VerificationLevel: "strict"}, + TrustStores: []string{"ca:valid-trust-store", "signingAuthority:valid-trust-store"}, + TrustedIdentities: []string{"x509.subject:CN=Notation Test Root,O=Notary,L=Seattle,ST=WA,C=US"}, + } + return +} + +func dummyPolicyDocument() (policyDoc Document) { + policyDoc = Document{ + Version: "1.0", + TrustPolicies: []TrustPolicy{dummyPolicyStatement()}, + } + return +} + +// TestValidateValidPolicyDocument tests a happy policy document +func TestValidateValidPolicyDocument(t *testing.T) { + policyDoc := dummyPolicyDocument() + + policyStatement1 := dummyPolicyStatement() + + policyStatement2 := dummyPolicyStatement() + policyStatement2.Name = "test-statement-name-2" + policyStatement2.RegistryScopes = []string{"registry.wabbit-networks.io/software/unsigned/net-utils"} + policyStatement2.SignatureVerification = SignatureVerification{VerificationLevel: "permissive"} + + policyStatement3 := dummyPolicyStatement() + policyStatement3.Name = "test-statement-name-3" + policyStatement3.RegistryScopes = []string{"registry.acme-rockets.io/software/legacy/metrics"} + policyStatement3.TrustStores = []string{} + policyStatement3.TrustedIdentities = []string{} + policyStatement3.SignatureVerification = SignatureVerification{VerificationLevel: "skip"} + + policyStatement4 := dummyPolicyStatement() + policyStatement4.Name = "test-statement-name-4" + policyStatement4.TrustStores = []string{"ca:valid-trust-store", "signingAuthority:valid-trust-store-2"} + policyStatement4.RegistryScopes = []string{"*"} + policyStatement4.SignatureVerification = SignatureVerification{VerificationLevel: "audit"} + + policyStatement5 := dummyPolicyStatement() + policyStatement5.Name = "test-statement-name-5" + policyStatement5.RegistryScopes = []string{"registry.acme-rockets2.io/software"} + policyStatement5.TrustedIdentities = []string{"*"} + policyStatement5.SignatureVerification = SignatureVerification{VerificationLevel: "strict"} + + policyDoc.TrustPolicies = []TrustPolicy{ + policyStatement1, + policyStatement2, + policyStatement3, + policyStatement4, + policyStatement5, + } + err := policyDoc.Validate() + if err != nil { + t.Fatalf("validation failed on a good policy document. Error : %q", err) + } +} + +// TestValidateTrustedIdentities tests only valid x509.subjects are accepted +func TestValidateTrustedIdentities(t *testing.T) { + + // No trusted identity prefix throws error + policyDoc := dummyPolicyDocument() + policyStatement := dummyPolicyStatement() + policyStatement.TrustedIdentities = []string{"C=US, ST=WA, O=wabbit-network.io, OU=org1"} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err := policyDoc.Validate() + if err == nil || err.Error() != "trust policy statement \"test-statement-name\" has trusted identity \"C=US, ST=WA, O=wabbit-network.io, OU=org1\" without an identity prefix" { + t.Fatalf("trusted identity without a prefix should return error") + } + + // Accept unknown identity prefixes + policyDoc = dummyPolicyDocument() + policyStatement = dummyPolicyStatement() + policyStatement.TrustedIdentities = []string{"unknown:my-trusted-idenity"} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err = policyDoc.Validate() + if err != nil { + t.Fatalf("unknown identity prefix should not return an error. Error: %q", err) + } + + // Validate x509.subject identities + policyDoc = dummyPolicyDocument() + policyStatement = dummyPolicyStatement() + invalidDN := "x509.subject:,,," + policyStatement.TrustedIdentities = []string{invalidDN} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err = policyDoc.Validate() + if err == nil || err.Error() != "distinguished name (DN) \",,,\" is not valid, it must contain 'C', 'ST', and 'O' RDN attributes at a minimum, and follow RFC 4514 standard" { + t.Fatalf("invalid x509.subject identity should return error. Error : %q", err) + } + + // Validate duplicate RDNs + policyDoc = dummyPolicyDocument() + policyStatement = dummyPolicyStatement() + invalidDN = "x509.subject:C=US,C=IN" + policyStatement.TrustedIdentities = []string{invalidDN} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err = policyDoc.Validate() + if err == nil || err.Error() != "distinguished name (DN) \"C=US,C=IN\" has duplicate RDN attribute for \"C\", DN can only have unique RDN attributes" { + t.Fatalf("invalid x509.subject identity should return error. Error : %q", err) + } + + // Validate mandatory RDNs + policyDoc = dummyPolicyDocument() + policyStatement = dummyPolicyStatement() + invalidDN = "x509.subject:C=US,ST=WA" + policyStatement.TrustedIdentities = []string{invalidDN} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err = policyDoc.Validate() + if err == nil || err.Error() != "distinguished name (DN) \"C=US,ST=WA\" has no mandatory RDN attribute for \"O\", it must contain 'C', 'ST', and 'O' RDN attributes at a minimum" { + t.Fatalf("invalid x509.subject identity should return error. Error : %q", err) + } + + // DN may have optional RDNs + policyDoc = dummyPolicyDocument() + policyStatement = dummyPolicyStatement() + validDN := "x509.subject:C=US,ST=WA,O=MyOrg,CustomRDN=CustomValue" + policyStatement.TrustedIdentities = []string{validDN} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err = policyDoc.Validate() + if err != nil { + t.Fatalf("valid x509.subject identity should not return error. Error : %q", err) + } + + // Validate rfc4514 DNs + policyDoc = dummyPolicyDocument() + policyStatement = dummyPolicyStatement() + validDN1 := "x509.subject:C=US,ST=WA,O=MyOrg" + validDN2 := "x509.subject:C=US,ST=WA,O= My. Org" + validDN3 := "x509.subject:C=US,ST=WA,O=My \"special\" Org \\, \\; \\\\ others" + validDN4 := "x509.subject:C=US,ST=WA,O=My Org,1.3.6.1.4.1.1466.0=#04024869" + policyStatement.TrustedIdentities = []string{validDN1, validDN2, validDN3, validDN4} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err = policyDoc.Validate() + if err != nil { + t.Fatalf("valid x509.subject identity should not return error. Error : %q", err) + } + + // Validate overlapping DNs + policyDoc = dummyPolicyDocument() + policyStatement = dummyPolicyStatement() + validDN1 = "x509.subject:C=US,ST=WA,O=MyOrg" + validDN2 = "x509.subject:C=US,ST=WA,O=MyOrg,X=Y" + policyStatement.TrustedIdentities = []string{validDN1, validDN2} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err = policyDoc.Validate() + if err == nil || err.Error() != "trust policy statement \"test-statement-name\" has overlapping x509 trustedIdentities, \"x509.subject:C=US,ST=WA,O=MyOrg\" overlaps with \"x509.subject:C=US,ST=WA,O=MyOrg,X=Y\"" { + t.Fatalf("overlapping DNs should return error") + } + + // Validate multi-valued RDNs + policyDoc = dummyPolicyDocument() + policyStatement = dummyPolicyStatement() + multiValduedRDN := "x509.subject:C=US+ST=WA,O=MyOrg" + policyStatement.TrustedIdentities = []string{multiValduedRDN} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err = policyDoc.Validate() + if err == nil || err.Error() != "distinguished name (DN) \"C=US+ST=WA,O=MyOrg\" has multi-valued RDN attributes, remove multi-valued RDN attributes as they are not supported" { + t.Fatalf("multi-valued RDN should return error. Error : %q", err) + } +} + +// TestInvalidRegistryScopes tests invalid scopes are rejected +func TestInvalidRegistryScopes(t *testing.T) { + invalidScopes := []string{ + "", "1:1", "a,b", "abcd", "1111", "1,2", "example.com/rep:tag", + "example.com/rep/subrep/sub:latest", "example.com", "rep/rep2:latest", + "repository", "10.10.10.10", "10.10.10.10:8080/rep/rep2:latest", + } + + for _, scope := range invalidScopes { + policyDoc := dummyPolicyDocument() + policyStatement := dummyPolicyStatement() + policyStatement.RegistryScopes = []string{scope} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err := policyDoc.Validate() + if err == nil || err.Error() != "registry scope \""+scope+"\" is not valid, make sure it is the fully qualified registry URL without the scheme/protocol. e.g domain.com/my/repository" { + t.Fatalf("invalid registry scope should return error. Error : %q", err) + } + } +} + +// TestValidRegistryScopes tests valid scopes are accepted +func TestValidRegistryScopes(t *testing.T) { + validScopes := []string{ + "example.com/rep", "example.com:8080/rep/rep2", "example.com/rep/subrep/subsub", + "10.10.10.10:8080/rep/rep2", "domain/rep", "domain:1234/rep", + } + + for _, scope := range validScopes { + policyDoc := dummyPolicyDocument() + policyStatement := dummyPolicyStatement() + policyStatement.RegistryScopes = []string{scope} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err := policyDoc.Validate() + if err != nil { + t.Fatalf("valid registry scope should not return error. Error : %q", err) + } + } +} + +// TestValidatePolicyDocument calls policyDoc.Validate() +// and tests various validations on policy eliments +func TestValidateInvalidPolicyDocument(t *testing.T) { + + // Invalid Version + policyDoc := dummyPolicyDocument() + policyDoc.Version = "invalid" + err := policyDoc.Validate() + if err == nil || err.Error() != "trust policy document uses unsupported version \"invalid\"" { + t.Fatalf("invalid version should return error") + } + + // No Policy Satements + policyDoc = dummyPolicyDocument() + policyDoc.TrustPolicies = nil + err = policyDoc.Validate() + if err == nil || err.Error() != "trust policy document can not have zero trust policy statements" { + t.Fatalf("zero policy statements should return error") + } + + // No Policy Satement Name + policyDoc = dummyPolicyDocument() + policyStatement := dummyPolicyStatement() + policyStatement.Name = "" + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err = policyDoc.Validate() + if err == nil || err.Error() != "a trust policy statement is missing a name, every statement requires a name" { + t.Fatalf("policy statement with no name should return an error") + } + + // No Registry Scopes + policyDoc = dummyPolicyDocument() + policyStatement = dummyPolicyStatement() + policyStatement.RegistryScopes = nil + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err = policyDoc.Validate() + if err == nil || err.Error() != "trust policy statement \"test-statement-name\" has zero registry scopes, it must specify registry scopes with at least one value" { + t.Fatalf("policy statement with registry scopes should return error") + } + + // Multiple policy statements with same registry scope + policyDoc = dummyPolicyDocument() + policyStatement1 := dummyPolicyStatement() + policyStatement2 := dummyPolicyStatement() + policyStatement2.Name = "test-statement-name-2" + policyDoc.TrustPolicies = []TrustPolicy{policyStatement1, policyStatement2} + err = policyDoc.Validate() + if err == nil || err.Error() != "registry scope \"registry.acme-rockets.io/software/net-monitor\" is present in multiple trust policy statements, one registry scope value can only be associated with one statement" { + t.Fatalf("Policy statements with same registry scope should return error %q", err) + } + + // Registry scopes with a wildcard + policyDoc = dummyPolicyDocument() + policyStatement = dummyPolicyStatement() + policyStatement.RegistryScopes = []string{"*", "registry.acme-rockets.io/software/net-monitor"} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err = policyDoc.Validate() + if err == nil || err.Error() != "trust policy statement \"test-statement-name\" uses wildcard registry scope '*', a wildcard scope cannot be used in conjunction with other scope values" { + t.Fatalf("policy statement with more than a wildcard registry scope should return error") + } + + // Invlaid SignatureVerification + policyDoc = dummyPolicyDocument() + policyStatement = dummyPolicyStatement() + policyStatement.SignatureVerification = SignatureVerification{VerificationLevel: "invalid"} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err = policyDoc.Validate() + if err == nil || err.Error() != "trust policy statement \"test-statement-name\" uses invalid signatureVerification value \"invalid\"" { + t.Fatalf("policy statement with invalid SignatureVerification should return error") + } + + // strict SignatureVerification should have a trust store + policyDoc = dummyPolicyDocument() + policyStatement = dummyPolicyStatement() + policyStatement.TrustStores = []string{} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err = policyDoc.Validate() + if err == nil || err.Error() != "trust policy statement \"test-statement-name\" is either missing trust stores or trusted identities, both must be specified" { + t.Fatalf("strict SignatureVerification should have a trust store") + } + + // strict SignatureVerification should have trusted identities + policyDoc = dummyPolicyDocument() + policyStatement = dummyPolicyStatement() + policyStatement.TrustedIdentities = []string{} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err = policyDoc.Validate() + if err == nil || err.Error() != "trust policy statement \"test-statement-name\" is either missing trust stores or trusted identities, both must be specified" { + t.Fatalf("strict SignatureVerification should have trusted identities") + } + + // skip SignatureVerification should not have trust store or trusted identities + policyDoc = dummyPolicyDocument() + policyStatement = dummyPolicyStatement() + policyStatement.SignatureVerification = SignatureVerification{VerificationLevel: "skip"} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err = policyDoc.Validate() + if err == nil || err.Error() != "trust policy statement \"test-statement-name\" is set to skip signature verification but configured with trust stores and/or trusted identities, remove them if signature verification needs to be skipped" { + t.Fatalf("strict SignatureVerification should have trusted identities") + } + + // Empty Trusted Identity should throw error + policyDoc = dummyPolicyDocument() + policyStatement = dummyPolicyStatement() + policyStatement.TrustedIdentities = []string{""} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err = policyDoc.Validate() + if err == nil || err.Error() != "trust policy statement \"test-statement-name\" has an empty trusted identity" { + t.Fatalf("policy statement with empty trusted identity should return error") + } + + // trust store/trusted identites are optional for skip SignatureVerification + policyDoc = dummyPolicyDocument() + policyStatement = dummyPolicyStatement() + policyStatement.SignatureVerification = SignatureVerification{VerificationLevel: "skip"} + policyStatement.TrustStores = []string{} + policyStatement.TrustedIdentities = []string{} + err = policyDoc.Validate() + if err != nil { + t.Fatalf("skip SignatureVerification should not require a trust store or trusted identities") + } + + // Invalid Trust Store type + policyDoc = dummyPolicyDocument() + policyStatement = dummyPolicyStatement() + policyStatement.TrustStores = []string{"invalid:test-trust-store"} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err = policyDoc.Validate() + if err == nil || err.Error() != "trust policy statement \"test-statement-name\" uses an unsupported trust store type \"invalid\" in trust store value \"invalid:test-trust-store\"" { + t.Fatalf("policy statement with invalid trust store type should return error") + } + + // trusted identities with a wildcard + policyDoc = dummyPolicyDocument() + policyStatement = dummyPolicyStatement() + policyStatement.TrustedIdentities = []string{"*", "test-identity"} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err = policyDoc.Validate() + if err == nil || err.Error() != "trust policy statement \"test-statement-name\" uses a wildcard trusted identity '*', a wildcard identity cannot be used in conjunction with other values" { + t.Fatalf("policy statement with more than a wildcard trusted identity should return error") + } + + // Policy Document with duplicate policy statement names + policyDoc = dummyPolicyDocument() + policyStatement1 = dummyPolicyStatement() + policyStatement2 = dummyPolicyStatement() + policyStatement2.RegistryScopes = []string{"registry.acme-rockets.io/software/legacy/metrics"} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement1, policyStatement2} + err = policyDoc.Validate() + if err == nil || err.Error() != "multiple trust policy statements use the same name \"test-statement-name\", statement names must be unique" { + t.Fatalf("policy statements with same name should return error") + } +} + +func TestGetVerificationLevel(t *testing.T) { + tests := []struct { + verificationLevel SignatureVerification + wantErr bool + verificationActions []ValidationAction + }{ + {SignatureVerification{VerificationLevel: "strict"}, false, []ValidationAction{ActionEnforce, ActionEnforce, ActionEnforce, ActionEnforce, ActionEnforce}}, + {SignatureVerification{VerificationLevel: "permissive"}, false, []ValidationAction{ActionEnforce, ActionEnforce, ActionLog, ActionLog, ActionLog}}, + {SignatureVerification{VerificationLevel: "audit"}, false, []ValidationAction{ActionEnforce, ActionLog, ActionLog, ActionLog, ActionLog}}, + {SignatureVerification{VerificationLevel: "skip"}, false, []ValidationAction{ActionSkip, ActionSkip, ActionSkip, ActionSkip, ActionSkip}}, + {SignatureVerification{VerificationLevel: "invalid"}, true, []ValidationAction{}}, + } + for i, tt := range tests { + t.Run(strconv.Itoa(i), func(t *testing.T) { + + level, err := GetVerificationLevel(tt.verificationLevel) + + if tt.wantErr != (err != nil) { + t.Fatalf("TestFindVerificationLevel Error: %q WantErr: %v", err, tt.wantErr) + } else { + for index, action := range tt.verificationActions { + if action != level.Enforcement[ValidationTypes[index]] { + t.Errorf("%q verification action should be %q for Verification Level %q", ValidationTypes[index], action, tt.verificationLevel) + } + } + } + }) + } +} + +func TestCustomVerificationLevel(t *testing.T) { + tests := []struct { + customVerification SignatureVerification + wantErr bool + verificationActions []ValidationAction + }{ + {SignatureVerification{VerificationLevel: "strict", Override: map[ValidationType]ValidationAction{"integrity": "log"}}, true, []ValidationAction{}}, + {SignatureVerification{VerificationLevel: "strict", Override: map[ValidationType]ValidationAction{"authenticity": "skip"}}, true, []ValidationAction{}}, + {SignatureVerification{VerificationLevel: "strict", Override: map[ValidationType]ValidationAction{"authenticTimestamp": "skip"}}, true, []ValidationAction{}}, + {SignatureVerification{VerificationLevel: "strict", Override: map[ValidationType]ValidationAction{"expiry": "skip"}}, true, []ValidationAction{}}, + {SignatureVerification{VerificationLevel: "skip", Override: map[ValidationType]ValidationAction{"authenticity": "log"}}, true, []ValidationAction{}}, + {SignatureVerification{VerificationLevel: "invalid", Override: map[ValidationType]ValidationAction{"authenticity": "log"}}, true, []ValidationAction{}}, + {SignatureVerification{VerificationLevel: "strict", Override: map[ValidationType]ValidationAction{"invalid": "log"}}, true, []ValidationAction{}}, + {SignatureVerification{VerificationLevel: "strict", Override: map[ValidationType]ValidationAction{"authenticity": "invalid"}}, true, []ValidationAction{}}, + {SignatureVerification{VerificationLevel: "strict", Override: map[ValidationType]ValidationAction{"authenticity": "log"}}, false, []ValidationAction{ActionEnforce, ActionLog, ActionEnforce, ActionEnforce, ActionEnforce}}, + {SignatureVerification{VerificationLevel: "permissive", Override: map[ValidationType]ValidationAction{"authenticity": "log"}}, false, []ValidationAction{ActionEnforce, ActionLog, ActionLog, ActionLog, ActionLog}}, + {SignatureVerification{VerificationLevel: "audit", Override: map[ValidationType]ValidationAction{"authenticity": "log"}}, false, []ValidationAction{ActionEnforce, ActionLog, ActionLog, ActionLog, ActionLog}}, + {SignatureVerification{VerificationLevel: "strict", Override: map[ValidationType]ValidationAction{"expiry": "log"}}, false, []ValidationAction{ActionEnforce, ActionEnforce, ActionEnforce, ActionLog, ActionEnforce}}, + {SignatureVerification{VerificationLevel: "permissive", Override: map[ValidationType]ValidationAction{"expiry": "log"}}, false, []ValidationAction{ActionEnforce, ActionEnforce, ActionLog, ActionLog, ActionLog}}, + {SignatureVerification{VerificationLevel: "audit", Override: map[ValidationType]ValidationAction{"expiry": "log"}}, false, []ValidationAction{ActionEnforce, ActionLog, ActionLog, ActionLog, ActionLog}}, + {SignatureVerification{VerificationLevel: "strict", Override: map[ValidationType]ValidationAction{"revocation": "log"}}, false, []ValidationAction{ActionEnforce, ActionEnforce, ActionEnforce, ActionEnforce, ActionLog}}, + {SignatureVerification{VerificationLevel: "permissive", Override: map[ValidationType]ValidationAction{"revocation": "log"}}, false, []ValidationAction{ActionEnforce, ActionEnforce, ActionLog, ActionLog, ActionLog}}, + {SignatureVerification{VerificationLevel: "audit", Override: map[ValidationType]ValidationAction{"revocation": "log"}}, false, []ValidationAction{ActionEnforce, ActionLog, ActionLog, ActionLog, ActionLog}}, + {SignatureVerification{VerificationLevel: "strict", Override: map[ValidationType]ValidationAction{"revocation": "skip"}}, false, []ValidationAction{ActionEnforce, ActionEnforce, ActionEnforce, ActionEnforce, ActionSkip}}, + {SignatureVerification{VerificationLevel: "permissive", Override: map[ValidationType]ValidationAction{"revocation": "skip"}}, false, []ValidationAction{ActionEnforce, ActionEnforce, ActionLog, ActionLog, ActionSkip}}, + {SignatureVerification{VerificationLevel: "audit", Override: map[ValidationType]ValidationAction{"revocation": "skip"}}, false, []ValidationAction{ActionEnforce, ActionLog, ActionLog, ActionLog, ActionSkip}}, + {SignatureVerification{VerificationLevel: "permissive", Override: map[ValidationType]ValidationAction{"authenticTimestamp": "log"}}, false, []ValidationAction{ActionEnforce, ActionEnforce, ActionLog, ActionLog, ActionLog}}, + {SignatureVerification{VerificationLevel: "audit", Override: map[ValidationType]ValidationAction{"authenticTimestamp": "log"}}, false, []ValidationAction{ActionEnforce, ActionLog, ActionLog, ActionLog, ActionLog}}, + {SignatureVerification{VerificationLevel: "strict", Override: map[ValidationType]ValidationAction{"authenticTimestamp": "log"}}, false, []ValidationAction{ActionEnforce, ActionEnforce, ActionLog, ActionEnforce, ActionEnforce}}, + } + for i, tt := range tests { + t.Run(strconv.Itoa(i), func(t *testing.T) { + level, err := GetVerificationLevel(tt.customVerification) + + if tt.wantErr != (err != nil) { + t.Fatalf("TestCustomVerificationLevel Error: %q WantErr: %v", err, tt.wantErr) + } else { + if !tt.wantErr && len(tt.verificationActions) == 0 { + t.Errorf("test case isn't configured with VerificationActions") + } + for index, action := range tt.verificationActions { + if action != level.Enforcement[ValidationTypes[index]] { + t.Errorf("%q verification action should be %q for custom verification %q", ValidationTypes[index], action, tt.customVerification) + } + } + } + }) + } +} diff --git a/verification/truststore/truststore.go b/verification/truststore/truststore.go new file mode 100644 index 00000000..10cef927 --- /dev/null +++ b/verification/truststore/truststore.go @@ -0,0 +1,126 @@ +// Package truststore reads certificates in a trust store +package truststore + +import ( + "context" + "crypto/x509" + "errors" + "fmt" + "io/fs" + "os" + "path/filepath" + + corex509 "github.com/notaryproject/notation-core-go/x509" + "github.com/notaryproject/notation-go/dir" +) + +// Type is an enum for trust store types supported such as +// "ca" and "signingAuthority" +type Type string + +const ( + TypeCA Type = "ca" + TypeSigningAuthority Type = "signingAuthority" +) + +var ( + Types = []Type{ + TypeCA, + TypeSigningAuthority, + } +) + +// X509TrustStore provide list and get behaviors for the trust store +type X509TrustStore interface { + // List named stores under storeType + // List(ctx context.Context, storeType Type) ([]string, error) + + // GetCertificates returns certificates under storeType/namedStore + GetCertificates(ctx context.Context, storeType Type, namedStore string) ([]*x509.Certificate, error) +} + +// NewX509TrustStore generates a new X509TrustStore +func NewX509TrustStore(trustStorefs dir.SysFS) X509TrustStore { + return x509TrustStore{trustStorefs} +} + +// x509TrustStore implements X509TrustStore +type x509TrustStore struct { + trustStorefs dir.SysFS +} + +// GetCertificates returns certificates under storeType/namedStore +func (trustStore x509TrustStore) GetCertificates(ctx context.Context, storeType Type, namedStore string) ([]*x509.Certificate, error) { + if storeType == "" || namedStore == "" { + return nil, errors.New("storeType and namedStore cannot be empty") + } + path, err := trustStore.trustStorefs.SysPath(dir.X509TrustStoreDir(string(storeType), namedStore)) + if err != nil { + return nil, err + } + // check path is valid + if _, err := os.Stat(path); os.IsNotExist(err) { + return nil, fmt.Errorf("%q does not exist", path) + } + + // throw error if path is not a directory or is a symlink + fileInfo, err := os.Lstat(path) + if err != nil { + return nil, err + } + mode := fileInfo.Mode() + if !mode.IsDir() || mode&fs.ModeSymlink != 0 { + return nil, fmt.Errorf("%q is not a regular directory (symlinks are not supported)", path) + } + + files, err := os.ReadDir(path) + if err != nil { + return nil, err + } + + var certificates []*x509.Certificate + for _, file := range files { + joinedPath := filepath.Join(path, file.Name()) + if file.IsDir() || file.Type()&fs.ModeSymlink != 0 { + return nil, fmt.Errorf("%q is not a regular file (directories or symlinks are not supported)", joinedPath) + } + certs, err := corex509.ReadCertificateFile(joinedPath) + if err != nil { + return nil, fmt.Errorf("error while reading certificates from %q: %w", joinedPath, err) + } + + if err := validateCerts(certs, joinedPath); err != nil { + return nil, err + } + + certificates = append(certificates, certs...) + } + + if len(certificates) < 1 { + return nil, fmt.Errorf("trust store %q has no x509 certificates", path) + } + + return certificates, nil +} + +func validateCerts(certs []*x509.Certificate, path string) error { + // to prevent any trust store misconfigurations, ensure there is at least + // one certificate from each file. + if len(certs) < 1 { + return fmt.Errorf("could not parse a certificate from %q, every file in a trust store must have a PEM or DER certificate in it", path) + } + + for _, cert := range certs { + if !cert.IsCA { + if err := cert.CheckSignature(cert.SignatureAlgorithm, cert.RawTBSCertificate, cert.Signature); err != nil { + return fmt.Errorf( + "certificate with subject %q from file %q is not a CA certificate or self-signed signing certificate", + cert.Subject, + path, + ) + } + } + } + + return nil +} diff --git a/verification/truststore/truststore_test.go b/verification/truststore/truststore_test.go new file mode 100644 index 00000000..c4f2bb95 --- /dev/null +++ b/verification/truststore/truststore_test.go @@ -0,0 +1,58 @@ +package truststore + +import ( + "context" + "fmt" + "path/filepath" + "testing" + + "github.com/notaryproject/notation-go/dir" +) + +var trustStore = NewX509TrustStore(dir.NewSysFS(filepath.FromSlash("../testdata/"))) + +// TestLoadTrustStore tests a valid trust store +func TestLoadValidTrustStore(t *testing.T) { + certs, err := trustStore.GetCertificates(context.Background(), "ca", "valid-trust-store") + if err != nil { + t.Fatalf("could not get certificates from trust store. %q", err) + } + if len(certs) != 3 { + t.Fatalf("unexpected number of certificates in the trust store, expected: %d, got: %d", 3, len(certs)) + } +} + +// TestLoadValidTrustStoreWithSelfSignedSigningCertificate tests a valid trust store with self-signed signing certificate +func TestLoadValidTrustStoreWithSelfSignedSigningCertificate(t *testing.T) { + certs, err := trustStore.GetCertificates(context.Background(), "ca", "valid-trust-store-self-signed") + if err != nil { + t.Fatalf("could not get certificates from trust store. %q", err) + } + if len(certs) != 1 { + t.Fatalf("unexpected number of certificates in the trust store, expected: %d, got: %d", 1, len(certs)) + } +} + +func TestLoadTrustStoreWithInvalidCerts(t *testing.T) { + failurePath := filepath.FromSlash("../testdata/truststore/x509/ca/trust-store-with-invalid-certs/invalid") + _, err := trustStore.GetCertificates(context.Background(), "ca", "trust-store-with-invalid-certs") + if err == nil || err.Error() != fmt.Sprintf("error while reading certificates from %q: x509: malformed certificate", failurePath) { + t.Fatalf("invalid certs should return error : %q", err) + } +} + +func TestLoadTrustStoreWithLeafCerts(t *testing.T) { + failurePath := filepath.FromSlash("../testdata/truststore/x509/ca/trust-store-with-leaf-certs/non-ca.crt") + _, err := trustStore.GetCertificates(context.Background(), "ca", "trust-store-with-leaf-certs") + if err == nil || err.Error() != fmt.Sprintf("certificate with subject \"CN=wabbit-networks.io,O=Notary,L=Seattle,ST=WA,C=US\" from file %q is not a CA certificate or self-signed signing certificate", failurePath) { + t.Fatalf("leaf cert in a trust store should return error : %q", err) + } +} + +func TestLoadTrustStoreWithLeafCertsInSingleFile(t *testing.T) { + failurePath := filepath.FromSlash("../testdata/truststore/x509/ca/trust-store-with-leaf-certs-in-single-file/RootAndLeafCerts.crt") + _, err := trustStore.GetCertificates(context.Background(), "ca", "trust-store-with-leaf-certs-in-single-file") + if err == nil || err.Error() != fmt.Sprintf("certificate with subject \"CN=wabbit-networks.io,O=Notary,L=Seattle,ST=WA,C=US\" from file %q is not a CA certificate or self-signed signing certificate", failurePath) { + t.Fatalf("leaf cert in a trust store should return error : %q", err) + } +} From fe548493896780da0690433d8588c771d2d8eeb4 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Tue, 8 Nov 2022 15:06:14 +0800 Subject: [PATCH 02/19] refactored tp/ts Signed-off-by: Patrick Zheng --- verification/trustpolicy/trustpolicy.go | 94 +++++++++++++++++--- verification/trustpolicy/trustpolicy_test.go | 45 ++++++++++ 2 files changed, 127 insertions(+), 12 deletions(-) diff --git a/verification/trustpolicy/trustpolicy.go b/verification/trustpolicy/trustpolicy.go index 245e6c98..dbba67f7 100644 --- a/verification/trustpolicy/trustpolicy.go +++ b/verification/trustpolicy/trustpolicy.go @@ -9,13 +9,16 @@ import ( "github.com/notaryproject/notation-go/verification/truststore" ) -// ValidationType is an enum for signature verification types such as Integrity, Authenticity, etc. +// ValidationType is an enum for signature verification types such as Integrity, +// Authenticity, etc. type ValidationType string -// ValidationAction is an enum for signature verification actions such as Enforced, Logged, Skipped. +// ValidationAction is an enum for signature verification actions such as +// Enforced, Logged, Skipped. type ValidationAction string -// VerificationLevel encapsulates the signature verification preset and it's actions for each verification type +// VerificationLevel encapsulates the signature verification preset and its +// actions for each verification type type VerificationLevel struct { Name string Enforcement map[ValidationType]ValidationAction @@ -160,7 +163,8 @@ func (policyDoc *Document) Validate() error { return fmt.Errorf("trust policy statement %q uses invalid signatureVerification value %q", statement.Name, statement.SignatureVerification.VerificationLevel) } - // Any signature verification other than "skip" needs a trust store and trusted identities + // Any signature verification other than "skip" needs a trust store and + // trusted identities if verificationLevel.Name == "skip" { if len(statement.TrustStores) > 0 || len(statement.TrustedIdentities) > 0 { return fmt.Errorf("trust policy statement %q is set to skip signature verification but configured with trust stores and/or trusted identities, remove them if signature verification needs to be skipped", statement.Name) @@ -199,8 +203,8 @@ func (policyDoc *Document) Validate() error { return nil } -// GetVerificationLevel returns VerificationLevel struct for the given SignatureVerification struct -// throws error if SignatureVerification is invalid +// GetVerificationLevel returns VerificationLevel struct for the given +// SignatureVerification struct throws error if SignatureVerification is invalid func GetVerificationLevel(signatureVerification SignatureVerification) (*VerificationLevel, error) { var baseLevel *VerificationLevel for _, l := range VerificationLevels { @@ -226,7 +230,8 @@ func GetVerificationLevel(signatureVerification SignatureVerification) (*Verific Enforcement: make(map[ValidationType]ValidationAction), } - // populate the custom verification level with the base verification settings + // populate the custom verification level with the base verification + // settings for k, v := range baseLevel.Enforcement { customVerificationLevel.Enforcement[k] = v } @@ -266,7 +271,8 @@ func GetVerificationLevel(signatureVerification SignatureVerification) (*Verific return customVerificationLevel, nil } -// validateTrustStore validates if the policy statement is following the Notary V2 spec rules for truststores +// validateTrustStore validates if the policy statement is following the +// Notary V2 spec rules for truststores func validateTrustStore(statement TrustPolicy) error { for _, trustStore := range statement.TrustStores { i := strings.Index(trustStore, ":") @@ -278,10 +284,12 @@ func validateTrustStore(statement TrustPolicy) error { return nil } -// validateTrustedIdentities validates if the policy statement is following the Notary V2 spec rules for trusted identities +// validateTrustedIdentities 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 there is a wildcard in trusted identies, there shouldn't be any other + //identities if len(statement.TrustedIdentities) > 1 && common.IsPresent(common.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) } @@ -322,7 +330,8 @@ func validateTrustedIdentities(statement TrustPolicy) error { return nil } -// validateRegistryScopes validates if the policy document is following the Notary V2 spec rules for registry scopes +// validateRegistryScopes validates if the policy document is following the +// Notary V2 spec rules for registry scopes func validateRegistryScopes(policyDoc *Document) error { registryScopeCount := make(map[string]int) @@ -367,7 +376,8 @@ func validateOverlappingDNs(policyName string, parsedDNs []common.ParsedDN) erro return nil } -// isValidTrustStoreType returns true if the given string is a valid truststore.Type, otherwise false. +// isValidTrustStoreType returns true if the given string is a valid +// truststore.Type, otherwise false. func isValidTrustStoreType(s string) bool { for _, p := range truststore.Types { if s == string(p) { @@ -376,3 +386,63 @@ func isValidTrustStoreType(s string) bool { } return false } + +// GetApplicableTrustPolicy returns a pointer to the deep copied TrustPolicy +// statement that applies to the given registry URI. If no applicable trust +// policy is found, returns an error +// see https://github.com/notaryproject/notaryproject/blob/main/trust-store-trust-policy-specification.md#selecting-a-trust-policy-based-on-artifact-uri +func GetApplicableTrustPolicy(trustPolicyDoc *Document, artifactReference string) (*TrustPolicy, error) { + + artifactPath, err := getArtifactPathFromReference(artifactReference) + if err != nil { + return nil, err + } + + var wildcardPolicy *TrustPolicy + var applicablePolicy *TrustPolicy + for _, policyStatement := range trustPolicyDoc.TrustPolicies { + if common.IsPresent(common.Wildcard, policyStatement.RegistryScopes) { + // we need to deep copy because we can't use the loop variable + // address. see https://stackoverflow.com/a/45967429 + wildcardPolicy = deepCopy(&policyStatement) + } else if common.IsPresent(artifactPath, policyStatement.RegistryScopes) { + applicablePolicy = deepCopy(&policyStatement) + } + } + + if applicablePolicy != nil { + // a policy with exact match for registry URI takes precedence over + // a wildcard (*) policy. + return applicablePolicy, nil + } else if wildcardPolicy != nil { + return wildcardPolicy, nil + } else { + return nil, fmt.Errorf("artifact %q has no applicable trust policy", artifactReference) + } +} + +// deepCopy returns a pointer to the deeply copied TrustPolicy +func deepCopy(t *TrustPolicy) *TrustPolicy { + return &TrustPolicy{ + Name: t.Name, + SignatureVerification: t.SignatureVerification, + RegistryScopes: append([]string(nil), t.RegistryScopes...), + TrustedIdentities: append([]string(nil), t.TrustedIdentities...), + TrustStores: append([]string(nil), t.TrustStores...), + } +} + +func getArtifactPathFromReference(artifactReference string) (string, error) { + // TODO support more types of URI like "domain.com/repository", + // "domain.com/repository:tag" + i := strings.LastIndex(artifactReference, "@") + if i < 0 { + return "", fmt.Errorf("artifact URI %q could not be parsed, make sure it is the fully qualified OCI artifact URI without the scheme/protocol. e.g domain.com:80/my/repository@sha256:digest", artifactReference) + } + + artifactPath := artifactReference[:i] + if err := common.ValidateRegistryScopeFormat(artifactPath); err != nil { + return "", err + } + return artifactPath, nil +} diff --git a/verification/trustpolicy/trustpolicy_test.go b/verification/trustpolicy/trustpolicy_test.go index b8d4ee54..b08b106c 100644 --- a/verification/trustpolicy/trustpolicy_test.go +++ b/verification/trustpolicy/trustpolicy_test.go @@ -1,6 +1,7 @@ package trustpolicy import ( + "fmt" "strconv" "testing" ) @@ -444,3 +445,47 @@ func TestCustomVerificationLevel(t *testing.T) { }) } } + +// TestApplicableTrustPolicy tests filtering policies against registry scopes +func TestApplicableTrustPolicy(t *testing.T) { + policyDoc := dummyPolicyDocument() + + policyStatement := dummyPolicyStatement() + policyStatement.Name = "test-statement-name-1" + registryScope := "registry.wabbit-networks.io/software/unsigned/net-utils" + registryUri := fmt.Sprintf("%s@sha256:hash", registryScope) + policyStatement.RegistryScopes = []string{registryScope} + policyStatement.SignatureVerification = SignatureVerification{VerificationLevel: "strict"} + + policyDoc.TrustPolicies = []TrustPolicy{ + policyStatement, + } + // existing Registry Scope + policy, err := GetApplicableTrustPolicy(&policyDoc, registryUri) + if policy.Name != policyStatement.Name || err != nil { + t.Fatalf("getApplicableTrustPolicy should return %q for registry scope %q", policyStatement.Name, registryScope) + } + + // non-existing Registry Scope + policy, err = GetApplicableTrustPolicy(&policyDoc, "non.existing.scope/repo@sha256:hash") + if policy != nil || err == nil || err.Error() != "artifact \"non.existing.scope/repo@sha256:hash\" has no applicable trust policy" { + t.Fatalf("getApplicableTrustPolicy should return nil for non existing registry scope") + } + + // wildcard registry scope + wildcardStatement := dummyPolicyStatement() + wildcardStatement.Name = "test-statement-name-2" + wildcardStatement.RegistryScopes = []string{"*"} + wildcardStatement.TrustStores = []string{} + wildcardStatement.TrustedIdentities = []string{} + wildcardStatement.SignatureVerification = SignatureVerification{VerificationLevel: "skip"} + + policyDoc.TrustPolicies = []TrustPolicy{ + policyStatement, + wildcardStatement, + } + policy, err = GetApplicableTrustPolicy(&policyDoc, "some.registry.that/has.no.policy@sha256:hash") + if policy.Name != wildcardStatement.Name || err != nil { + t.Fatalf("getApplicableTrustPolicy should return wildcard policy for registry scope \"some.registry.that/has.no.policy\"") + } +} From 43483d35cb67783a17713773a9b696ab01658bf4 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Wed, 9 Nov 2022 11:14:39 +0800 Subject: [PATCH 03/19] update Signed-off-by: Patrick Zheng --- internal/common/common.go | 10 ---------- verification/trustpolicy/trustpolicy.go | 20 +++++++++++++++----- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/internal/common/common.go b/internal/common/common.go index 92a09f04..91506b2f 100644 --- a/internal/common/common.go +++ b/internal/common/common.go @@ -13,16 +13,6 @@ const ( X509Subject = "x509.subject" ) -// 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 -} - // Internal type to hold raw and parsed Distinguished Names type ParsedDN struct { RawString string diff --git a/verification/trustpolicy/trustpolicy.go b/verification/trustpolicy/trustpolicy.go index dbba67f7..924efc7f 100644 --- a/verification/trustpolicy/trustpolicy.go +++ b/verification/trustpolicy/trustpolicy.go @@ -138,7 +138,7 @@ func (policyDoc *Document) Validate() error { supportedPolicyVersions := []string{"1.0"} // Validate Version - if !common.IsPresent(policyDoc.Version, supportedPolicyVersions) { + if !contains(policyDoc.Version, supportedPolicyVersions) { return fmt.Errorf("trust policy document uses unsupported version %q", policyDoc.Version) } @@ -290,7 +290,7 @@ 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 && common.IsPresent(common.Wildcard, statement.TrustedIdentities) { + if len(statement.TrustedIdentities) > 1 && contains(common.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) } @@ -340,7 +340,7 @@ func validateRegistryScopes(policyDoc *Document) error { 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 && common.IsPresent(common.Wildcard, statement.RegistryScopes) { + if len(statement.RegistryScopes) > 1 && contains(common.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 { @@ -401,11 +401,11 @@ func GetApplicableTrustPolicy(trustPolicyDoc *Document, artifactReference string var wildcardPolicy *TrustPolicy var applicablePolicy *TrustPolicy for _, policyStatement := range trustPolicyDoc.TrustPolicies { - if common.IsPresent(common.Wildcard, policyStatement.RegistryScopes) { + if contains(common.Wildcard, policyStatement.RegistryScopes) { // we need to deep copy because we can't use the loop variable // address. see https://stackoverflow.com/a/45967429 wildcardPolicy = deepCopy(&policyStatement) - } else if common.IsPresent(artifactPath, policyStatement.RegistryScopes) { + } else if contains(artifactPath, policyStatement.RegistryScopes) { applicablePolicy = deepCopy(&policyStatement) } } @@ -446,3 +446,13 @@ func getArtifactPathFromReference(artifactReference string) (string, error) { } return artifactPath, nil } + +// contains is a utility function to check if a string exists in an array +func contains(val string, values []string) bool { + for _, v := range values { + if v == val { + return true + } + } + return false +} From dfdf99b45e459f9b78759f023ce9e78da6965fcf Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Wed, 9 Nov 2022 11:21:06 +0800 Subject: [PATCH 04/19] update Signed-off-by: Patrick Zheng --- internal/common/common.go | 30 -------------- internal/slice/slice.go | 11 +++++ verification/trustpolicy/trustpolicy.go | 54 +++++++++++++++++-------- 3 files changed, 48 insertions(+), 47 deletions(-) create mode 100644 internal/slice/slice.go diff --git a/internal/common/common.go b/internal/common/common.go index 91506b2f..ccf3ae7e 100644 --- a/internal/common/common.go +++ b/internal/common/common.go @@ -2,8 +2,6 @@ package common import ( "fmt" - "regexp" - "strings" ldapv3 "github.com/go-ldap/ldap/v3" ) @@ -13,12 +11,6 @@ const ( X509Subject = "x509.subject" ) -// Internal type to hold raw and parsed Distinguished Names -type ParsedDN struct { - RawString string - ParsedMap map[string]string -} - // ParseDistinguishedName parses a DN name and validates Notary V2 rules func ParseDistinguishedName(name string) (map[string]string, error) { mandatoryFields := []string{"C", "ST", "O"} @@ -63,25 +55,3 @@ func IsSubsetDN(dn1 map[string]string, dn2 map[string]string) bool { } return true } - -// 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 -} diff --git a/internal/slice/slice.go b/internal/slice/slice.go new file mode 100644 index 00000000..f95e0527 --- /dev/null +++ b/internal/slice/slice.go @@ -0,0 +1,11 @@ +package slice + +// Contains is a utility function to check if a string exists in an array +func Contains(val string, values []string) bool { + for _, v := range values { + if v == val { + return true + } + } + return false +} diff --git a/verification/trustpolicy/trustpolicy.go b/verification/trustpolicy/trustpolicy.go index 924efc7f..fb8d4348 100644 --- a/verification/trustpolicy/trustpolicy.go +++ b/verification/trustpolicy/trustpolicy.go @@ -3,9 +3,11 @@ package trustpolicy import ( "errors" "fmt" + "regexp" "strings" "github.com/notaryproject/notation-go/internal/common" + "github.com/notaryproject/notation-go/internal/slice" "github.com/notaryproject/notation-go/verification/truststore" ) @@ -138,7 +140,7 @@ func (policyDoc *Document) Validate() error { supportedPolicyVersions := []string{"1.0"} // Validate Version - if !contains(policyDoc.Version, supportedPolicyVersions) { + if !slice.Contains(policyDoc.Version, supportedPolicyVersions) { return fmt.Errorf("trust policy document uses unsupported version %q", policyDoc.Version) } @@ -290,11 +292,11 @@ 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 && contains(common.Wildcard, statement.TrustedIdentities) { + if len(statement.TrustedIdentities) > 1 && slice.Contains(common.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) } - var parsedDNs []common.ParsedDN + var parsedDNs []parsedDN // If there are trusted identities, verify they are valid for _, identity := range statement.TrustedIdentities { if identity == "" { @@ -316,7 +318,7 @@ func validateTrustedIdentities(statement TrustPolicy) error { if err != nil { return err } - parsedDNs = append(parsedDNs, common.ParsedDN{RawString: identity, ParsedMap: dn}) + parsedDNs = append(parsedDNs, parsedDN{RawString: identity, ParsedMap: dn}) } } } @@ -340,12 +342,12 @@ func validateRegistryScopes(policyDoc *Document) error { 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 && contains(common.Wildcard, statement.RegistryScopes) { + if len(statement.RegistryScopes) > 1 && slice.Contains(common.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 != common.Wildcard { - if err := common.ValidateRegistryScopeFormat(scope); err != nil { + if err := validateRegistryScopeFormat(scope); err != nil { return err } } @@ -364,7 +366,7 @@ func validateRegistryScopes(policyDoc *Document) error { return nil } -func validateOverlappingDNs(policyName string, parsedDNs []common.ParsedDN) error { +func validateOverlappingDNs(policyName string, parsedDNs []parsedDN) error { for i, dn1 := range parsedDNs { for j, dn2 := range parsedDNs { if i != j && common.IsSubsetDN(dn1.ParsedMap, dn2.ParsedMap) { @@ -401,11 +403,11 @@ func GetApplicableTrustPolicy(trustPolicyDoc *Document, artifactReference string var wildcardPolicy *TrustPolicy var applicablePolicy *TrustPolicy for _, policyStatement := range trustPolicyDoc.TrustPolicies { - if contains(common.Wildcard, policyStatement.RegistryScopes) { + if slice.Contains(common.Wildcard, policyStatement.RegistryScopes) { // we need to deep copy because we can't use the loop variable // address. see https://stackoverflow.com/a/45967429 wildcardPolicy = deepCopy(&policyStatement) - } else if contains(artifactPath, policyStatement.RegistryScopes) { + } else if slice.Contains(artifactPath, policyStatement.RegistryScopes) { applicablePolicy = deepCopy(&policyStatement) } } @@ -441,18 +443,36 @@ func getArtifactPathFromReference(artifactReference string) (string, error) { } artifactPath := artifactReference[:i] - if err := common.ValidateRegistryScopeFormat(artifactPath); err != nil { + if err := validateRegistryScopeFormat(artifactPath); err != nil { return "", err } return artifactPath, nil } -// contains is a utility function to check if a string exists in an array -func contains(val string, values []string) bool { - for _, v := range values { - if v == val { - return true - } +// Internal type to hold raw and parsed Distinguished Names +type parsedDN struct { + RawString string + ParsedMap map[string]string +} + +// 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) } - return false + 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 } From 90e1c688a850109b54c6008c3785eb26cd481ca9 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Wed, 9 Nov 2022 12:27:25 +0800 Subject: [PATCH 05/19] updated per code review Signed-off-by: Patrick Zheng --- internal/{common/common.go => pkix/pkix.go} | 7 +--- internal/slice/slice.go | 5 +++ verification/trustpolicy/trustpolicy.go | 40 ++++++++++++--------- verification/truststore/truststore.go | 9 +++-- 4 files changed, 34 insertions(+), 27 deletions(-) rename internal/{common/common.go => pkix/pkix.go} (95%) diff --git a/internal/common/common.go b/internal/pkix/pkix.go similarity index 95% rename from internal/common/common.go rename to internal/pkix/pkix.go index ccf3ae7e..c453b8d5 100644 --- a/internal/common/common.go +++ b/internal/pkix/pkix.go @@ -1,4 +1,4 @@ -package common +package pkix import ( "fmt" @@ -6,11 +6,6 @@ import ( ldapv3 "github.com/go-ldap/ldap/v3" ) -const ( - Wildcard = "*" - X509Subject = "x509.subject" -) - // ParseDistinguishedName parses a DN name and validates Notary V2 rules func ParseDistinguishedName(name string) (map[string]string, error) { mandatoryFields := []string{"C", "ST", "O"} diff --git a/internal/slice/slice.go b/internal/slice/slice.go index f95e0527..99491ffa 100644 --- a/internal/slice/slice.go +++ b/internal/slice/slice.go @@ -1,5 +1,10 @@ package slice +const ( + Wildcard = "*" + X509Subject = "x509.subject" +) + // Contains is a utility function to check if a string exists in an array func Contains(val string, values []string) bool { for _, v := range values { diff --git a/verification/trustpolicy/trustpolicy.go b/verification/trustpolicy/trustpolicy.go index fb8d4348..0cad0156 100644 --- a/verification/trustpolicy/trustpolicy.go +++ b/verification/trustpolicy/trustpolicy.go @@ -6,7 +6,7 @@ import ( "regexp" "strings" - "github.com/notaryproject/notation-go/internal/common" + "github.com/notaryproject/notation-go/internal/pkix" "github.com/notaryproject/notation-go/internal/slice" "github.com/notaryproject/notation-go/verification/truststore" ) @@ -32,7 +32,9 @@ const ( TypeAuthenticTimestamp ValidationType = "authenticTimestamp" TypeExpiry ValidationType = "expiry" TypeRevocation ValidationType = "revocation" +) +const ( ActionEnforce ValidationAction = "enforce" ActionLog ValidationAction = "log" ActionSkip ValidationAction = "skip" @@ -82,7 +84,9 @@ var ( TypeRevocation: ActionSkip, }, } +) +var ( ValidationTypes = []ValidationType{ TypeIntegrity, TypeAuthenticity, @@ -105,10 +109,13 @@ var ( } ) +var supportedPolicyVersions = []string{"1.0"} + // Document represents a trustPolicy.json document type Document struct { // Version of the policy document Version string `json:"version"` + // TrustPolicies include each policy statement TrustPolicies []TrustPolicy `json:"trustPolicies"` } @@ -117,12 +124,16 @@ type Document struct { type TrustPolicy struct { // Name of the policy statement Name string `json:"name"` + // RegistryScopes that this policy statement affects RegistryScopes []string `json:"registryScopes"` + // SignatureVerification setting for this policy statement SignatureVerification SignatureVerification `json:"signatureVerification"` + // TrustStores this policy statement uses TrustStores []string `json:"trustStores,omitempty"` + // TrustedIdentities this policy statement pins TrustedIdentities []string `json:"trustedIdentities,omitempty"` } @@ -136,9 +147,6 @@ type SignatureVerification struct { // Validate validates a policy document according to it's version's rule set. // if any rule is violated, returns an error func (policyDoc *Document) Validate() error { - // Constants - supportedPolicyVersions := []string{"1.0"} - // Validate Version if !slice.Contains(policyDoc.Version, supportedPolicyVersions) { return fmt.Errorf("trust policy document uses unsupported version %q", policyDoc.Version) @@ -292,7 +300,7 @@ 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 && slice.Contains(common.Wildcard, statement.TrustedIdentities) { + if len(statement.TrustedIdentities) > 1 && slice.Contains(slice.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) } @@ -303,7 +311,7 @@ func validateTrustedIdentities(statement TrustPolicy) error { return fmt.Errorf("trust policy statement %q has an empty trusted identity", statement.Name) } - if identity != common.Wildcard { + if identity != slice.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) @@ -313,8 +321,8 @@ func validateTrustedIdentities(statement TrustPolicy) error { identityValue := identity[i+1:] // notation natively supports x509.subject identities only - if identityPrefix == common.X509Subject { - dn, err := common.ParseDistinguishedName(identityValue) + if identityPrefix == slice.X509Subject { + dn, err := pkix.ParseDistinguishedName(identityValue) if err != nil { return err } @@ -342,11 +350,11 @@ func validateRegistryScopes(policyDoc *Document) error { 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 && slice.Contains(common.Wildcard, statement.RegistryScopes) { + if len(statement.RegistryScopes) > 1 && slice.Contains(slice.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 != common.Wildcard { + if scope != slice.Wildcard { if err := validateRegistryScopeFormat(scope); err != nil { return err } @@ -369,7 +377,7 @@ func validateRegistryScopes(policyDoc *Document) error { func validateOverlappingDNs(policyName string, parsedDNs []parsedDN) error { for i, dn1 := range parsedDNs { for j, dn2 := range parsedDNs { - if i != j && common.IsSubsetDN(dn1.ParsedMap, dn2.ParsedMap) { + if i != j && pkix.IsSubsetDN(dn1.ParsedMap, dn2.ParsedMap) { return fmt.Errorf("trust policy statement %q has overlapping x509 trustedIdentities, %q overlaps with %q", policyName, dn1.RawString, dn2.RawString) } } @@ -403,12 +411,12 @@ func GetApplicableTrustPolicy(trustPolicyDoc *Document, artifactReference string var wildcardPolicy *TrustPolicy var applicablePolicy *TrustPolicy for _, policyStatement := range trustPolicyDoc.TrustPolicies { - if slice.Contains(common.Wildcard, policyStatement.RegistryScopes) { + if slice.Contains(slice.Wildcard, policyStatement.RegistryScopes) { // we need to deep copy because we can't use the loop variable // address. see https://stackoverflow.com/a/45967429 - wildcardPolicy = deepCopy(&policyStatement) + wildcardPolicy = (&policyStatement).clone() } else if slice.Contains(artifactPath, policyStatement.RegistryScopes) { - applicablePolicy = deepCopy(&policyStatement) + applicablePolicy = (&policyStatement).clone() } } @@ -423,8 +431,8 @@ func GetApplicableTrustPolicy(trustPolicyDoc *Document, artifactReference string } } -// deepCopy returns a pointer to the deeply copied TrustPolicy -func deepCopy(t *TrustPolicy) *TrustPolicy { +// clone returns a pointer to the deeply copied TrustPolicy +func (t *TrustPolicy) clone() *TrustPolicy { return &TrustPolicy{ Name: t.Name, SignatureVerification: t.SignatureVerification, diff --git a/verification/truststore/truststore.go b/verification/truststore/truststore.go index 10cef927..5c8e6260 100644 --- a/verification/truststore/truststore.go +++ b/verification/truststore/truststore.go @@ -32,9 +32,6 @@ var ( // X509TrustStore provide list and get behaviors for the trust store type X509TrustStore interface { - // List named stores under storeType - // List(ctx context.Context, storeType Type) ([]string, error) - // GetCertificates returns certificates under storeType/namedStore GetCertificates(ctx context.Context, storeType Type, namedStore string) ([]*x509.Certificate, error) } @@ -59,8 +56,10 @@ func (trustStore x509TrustStore) GetCertificates(ctx context.Context, storeType return nil, err } // check path is valid - if _, err := os.Stat(path); os.IsNotExist(err) { - return nil, fmt.Errorf("%q does not exist", path) + if _, err := os.Stat(path); err != nil { + if os.IsNotExist(err) { + return nil, fmt.Errorf("%q does not exist", path) + } } // throw error if path is not a directory or is a symlink From a42ae05d8106d296e8a18d6511a11662370b73be Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Wed, 9 Nov 2022 15:41:01 +0800 Subject: [PATCH 06/19] update Signed-off-by: Patrick Zheng --- verification/truststore/truststore.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/verification/truststore/truststore.go b/verification/truststore/truststore.go index 5c8e6260..b6227bf2 100644 --- a/verification/truststore/truststore.go +++ b/verification/truststore/truststore.go @@ -36,16 +36,16 @@ type X509TrustStore interface { GetCertificates(ctx context.Context, storeType Type, namedStore string) ([]*x509.Certificate, error) } -// NewX509TrustStore generates a new X509TrustStore -func NewX509TrustStore(trustStorefs dir.SysFS) X509TrustStore { - return x509TrustStore{trustStorefs} -} - // x509TrustStore implements X509TrustStore type x509TrustStore struct { trustStorefs dir.SysFS } +// NewX509TrustStore generates a new X509TrustStore +func NewX509TrustStore(trustStorefs dir.SysFS) X509TrustStore { + return x509TrustStore{trustStorefs} +} + // GetCertificates returns certificates under storeType/namedStore func (trustStore x509TrustStore) GetCertificates(ctx context.Context, storeType Type, namedStore string) ([]*x509.Certificate, error) { if storeType == "" || namedStore == "" { From fdd182e17cf4507d97138e476eef927c51a6a289 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Wed, 9 Nov 2022 15:51:47 +0800 Subject: [PATCH 07/19] updated per code review Signed-off-by: Patrick Zheng --- internal/slice/slice.go | 4 ++-- verification/trustpolicy/trustpolicy.go | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/internal/slice/slice.go b/internal/slice/slice.go index 99491ffa..d7cb4959 100644 --- a/internal/slice/slice.go +++ b/internal/slice/slice.go @@ -5,8 +5,8 @@ const ( X509Subject = "x509.subject" ) -// Contains is a utility function to check if a string exists in an array -func Contains(val string, values []string) bool { +// ContainsString is a utility function to check if a string exists in an array +func ContainsString(val string, values []string) bool { for _, v := range values { if v == val { return true diff --git a/verification/trustpolicy/trustpolicy.go b/verification/trustpolicy/trustpolicy.go index 0cad0156..4fd4020b 100644 --- a/verification/trustpolicy/trustpolicy.go +++ b/verification/trustpolicy/trustpolicy.go @@ -148,7 +148,7 @@ type SignatureVerification struct { // if any rule is violated, returns an error func (policyDoc *Document) Validate() error { // Validate Version - if !slice.Contains(policyDoc.Version, supportedPolicyVersions) { + if !slice.ContainsString(policyDoc.Version, supportedPolicyVersions) { return fmt.Errorf("trust policy document uses unsupported version %q", policyDoc.Version) } @@ -300,7 +300,7 @@ 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 && slice.Contains(slice.Wildcard, statement.TrustedIdentities) { + if len(statement.TrustedIdentities) > 1 && slice.ContainsString(slice.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) } @@ -332,7 +332,7 @@ func validateTrustedIdentities(statement TrustPolicy) error { } // Verify there are no overlapping DNs - if err := validateOverlappingDNs(statement.Name, parsedDNs); err != nil { + if err := hasOverlappingDNs(statement.Name, parsedDNs); err != nil { return err } @@ -350,7 +350,7 @@ func validateRegistryScopes(policyDoc *Document) error { 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 && slice.Contains(slice.Wildcard, statement.RegistryScopes) { + if len(statement.RegistryScopes) > 1 && slice.ContainsString(slice.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 { @@ -374,7 +374,7 @@ func validateRegistryScopes(policyDoc *Document) error { return nil } -func validateOverlappingDNs(policyName string, parsedDNs []parsedDN) error { +func hasOverlappingDNs(policyName string, parsedDNs []parsedDN) error { for i, dn1 := range parsedDNs { for j, dn2 := range parsedDNs { if i != j && pkix.IsSubsetDN(dn1.ParsedMap, dn2.ParsedMap) { @@ -411,11 +411,11 @@ func GetApplicableTrustPolicy(trustPolicyDoc *Document, artifactReference string var wildcardPolicy *TrustPolicy var applicablePolicy *TrustPolicy for _, policyStatement := range trustPolicyDoc.TrustPolicies { - if slice.Contains(slice.Wildcard, policyStatement.RegistryScopes) { + if slice.ContainsString(slice.Wildcard, policyStatement.RegistryScopes) { // we need to deep copy because we can't use the loop variable // address. see https://stackoverflow.com/a/45967429 wildcardPolicy = (&policyStatement).clone() - } else if slice.Contains(artifactPath, policyStatement.RegistryScopes) { + } else if slice.ContainsString(artifactPath, policyStatement.RegistryScopes) { applicablePolicy = (&policyStatement).clone() } } From f51b71f78cd37cabbe74a55f40e78b29900b0911 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Wed, 9 Nov 2022 16:34:16 +0800 Subject: [PATCH 08/19] updated per code review Signed-off-by: Patrick Zheng --- internal/slice/slice.go | 5 - internal/trustpolicy/trustpolicy.go | 6 + verification/trustpolicy/trustpolicy.go | 109 ++++++++++--------- verification/trustpolicy/trustpolicy_test.go | 10 +- verification/truststore/truststore.go | 35 ++++-- 5 files changed, 93 insertions(+), 72 deletions(-) create mode 100644 internal/trustpolicy/trustpolicy.go diff --git a/internal/slice/slice.go b/internal/slice/slice.go index d7cb4959..b73c8794 100644 --- a/internal/slice/slice.go +++ b/internal/slice/slice.go @@ -1,10 +1,5 @@ package slice -const ( - Wildcard = "*" - X509Subject = "x509.subject" -) - // ContainsString is a utility function to check if a string exists in an array func ContainsString(val string, values []string) bool { for _, v := range values { diff --git a/internal/trustpolicy/trustpolicy.go b/internal/trustpolicy/trustpolicy.go new file mode 100644 index 00000000..82745017 --- /dev/null +++ b/internal/trustpolicy/trustpolicy.go @@ -0,0 +1,6 @@ +package trustpolicy + +const ( + Wildcard = "*" + X509Subject = "x509.subject" +) diff --git a/verification/trustpolicy/trustpolicy.go b/verification/trustpolicy/trustpolicy.go index 4fd4020b..c3f6e956 100644 --- a/verification/trustpolicy/trustpolicy.go +++ b/verification/trustpolicy/trustpolicy.go @@ -8,6 +8,7 @@ import ( "github.com/notaryproject/notation-go/internal/pkix" "github.com/notaryproject/notation-go/internal/slice" + "github.com/notaryproject/notation-go/internal/trustpolicy" "github.com/notaryproject/notation-go/verification/truststore" ) @@ -168,7 +169,7 @@ func (policyDoc *Document) Validate() error { policyStatementNameCount[statement.Name]++ // Verify signature verification level is valid - verificationLevel, err := GetVerificationLevel(statement.SignatureVerification) + verificationLevel, err := statement.SignatureVerification.GetVerificationLevel() if err != nil { return fmt.Errorf("trust policy statement %q uses invalid signatureVerification value %q", statement.Name, statement.SignatureVerification.VerificationLevel) } @@ -213,9 +214,43 @@ func (policyDoc *Document) Validate() error { return nil } +// GetApplicableTrustPolicy returns a pointer to the deep copied TrustPolicy +// statement that applies to the given registry URI. If no applicable trust +// policy is found, returns an error +// see https://github.com/notaryproject/notaryproject/blob/main/trust-store-trust-policy-specification.md#selecting-a-trust-policy-based-on-artifact-uri +func (trustPolicyDoc *Document) GetApplicableTrustPolicy(artifactReference string) (*TrustPolicy, error) { + + artifactPath, err := getArtifactPathFromReference(artifactReference) + if err != nil { + return nil, err + } + + var wildcardPolicy *TrustPolicy + var applicablePolicy *TrustPolicy + for _, policyStatement := range trustPolicyDoc.TrustPolicies { + if slice.ContainsString(trustpolicy.Wildcard, policyStatement.RegistryScopes) { + // we need to deep copy because we can't use the loop variable + // address. see https://stackoverflow.com/a/45967429 + wildcardPolicy = (&policyStatement).clone() + } else if slice.ContainsString(artifactPath, policyStatement.RegistryScopes) { + applicablePolicy = (&policyStatement).clone() + } + } + + if applicablePolicy != nil { + // a policy with exact match for registry URI takes precedence over + // a wildcard (*) policy. + return applicablePolicy, nil + } else if wildcardPolicy != nil { + return wildcardPolicy, nil + } else { + return nil, fmt.Errorf("artifact %q has no applicable trust policy", artifactReference) + } +} + // GetVerificationLevel returns VerificationLevel struct for the given // SignatureVerification struct throws error if SignatureVerification is invalid -func GetVerificationLevel(signatureVerification SignatureVerification) (*VerificationLevel, error) { +func (signatureVerification *SignatureVerification) GetVerificationLevel() (*VerificationLevel, error) { var baseLevel *VerificationLevel for _, l := range VerificationLevels { if l.Name == signatureVerification.VerificationLevel { @@ -281,6 +316,17 @@ func GetVerificationLevel(signatureVerification SignatureVerification) (*Verific return customVerificationLevel, nil } +// clone returns a pointer to the deeply copied TrustPolicy +func (t *TrustPolicy) clone() *TrustPolicy { + return &TrustPolicy{ + Name: t.Name, + SignatureVerification: t.SignatureVerification, + RegistryScopes: append([]string(nil), t.RegistryScopes...), + TrustedIdentities: append([]string(nil), t.TrustedIdentities...), + TrustStores: append([]string(nil), t.TrustStores...), + } +} + // validateTrustStore validates if the policy statement is following the // Notary V2 spec rules for truststores func validateTrustStore(statement TrustPolicy) error { @@ -300,7 +346,7 @@ 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 && slice.ContainsString(slice.Wildcard, statement.TrustedIdentities) { + if len(statement.TrustedIdentities) > 1 && slice.ContainsString(trustpolicy.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) } @@ -311,7 +357,7 @@ func validateTrustedIdentities(statement TrustPolicy) error { return fmt.Errorf("trust policy statement %q has an empty trusted identity", statement.Name) } - if identity != slice.Wildcard { + if identity != trustpolicy.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) @@ -321,7 +367,7 @@ func validateTrustedIdentities(statement TrustPolicy) error { identityValue := identity[i+1:] // notation natively supports x509.subject identities only - if identityPrefix == slice.X509Subject { + if identityPrefix == trustpolicy.X509Subject { dn, err := pkix.ParseDistinguishedName(identityValue) if err != nil { return err @@ -332,7 +378,7 @@ func validateTrustedIdentities(statement TrustPolicy) error { } // Verify there are no overlapping DNs - if err := hasOverlappingDNs(statement.Name, parsedDNs); err != nil { + if err := validateOverlappingDNs(statement.Name, parsedDNs); err != nil { return err } @@ -350,11 +396,11 @@ func validateRegistryScopes(policyDoc *Document) error { 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 && slice.ContainsString(slice.Wildcard, statement.RegistryScopes) { + if len(statement.RegistryScopes) > 1 && slice.ContainsString(trustpolicy.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 != slice.Wildcard { + if scope != trustpolicy.Wildcard { if err := validateRegistryScopeFormat(scope); err != nil { return err } @@ -374,7 +420,7 @@ func validateRegistryScopes(policyDoc *Document) error { return nil } -func hasOverlappingDNs(policyName string, parsedDNs []parsedDN) error { +func validateOverlappingDNs(policyName string, parsedDNs []parsedDN) error { for i, dn1 := range parsedDNs { for j, dn2 := range parsedDNs { if i != j && pkix.IsSubsetDN(dn1.ParsedMap, dn2.ParsedMap) { @@ -397,51 +443,6 @@ func isValidTrustStoreType(s string) bool { return false } -// GetApplicableTrustPolicy returns a pointer to the deep copied TrustPolicy -// statement that applies to the given registry URI. If no applicable trust -// policy is found, returns an error -// see https://github.com/notaryproject/notaryproject/blob/main/trust-store-trust-policy-specification.md#selecting-a-trust-policy-based-on-artifact-uri -func GetApplicableTrustPolicy(trustPolicyDoc *Document, artifactReference string) (*TrustPolicy, error) { - - artifactPath, err := getArtifactPathFromReference(artifactReference) - if err != nil { - return nil, err - } - - var wildcardPolicy *TrustPolicy - var applicablePolicy *TrustPolicy - for _, policyStatement := range trustPolicyDoc.TrustPolicies { - if slice.ContainsString(slice.Wildcard, policyStatement.RegistryScopes) { - // we need to deep copy because we can't use the loop variable - // address. see https://stackoverflow.com/a/45967429 - wildcardPolicy = (&policyStatement).clone() - } else if slice.ContainsString(artifactPath, policyStatement.RegistryScopes) { - applicablePolicy = (&policyStatement).clone() - } - } - - if applicablePolicy != nil { - // a policy with exact match for registry URI takes precedence over - // a wildcard (*) policy. - return applicablePolicy, nil - } else if wildcardPolicy != nil { - return wildcardPolicy, nil - } else { - return nil, fmt.Errorf("artifact %q has no applicable trust policy", artifactReference) - } -} - -// clone returns a pointer to the deeply copied TrustPolicy -func (t *TrustPolicy) clone() *TrustPolicy { - return &TrustPolicy{ - Name: t.Name, - SignatureVerification: t.SignatureVerification, - RegistryScopes: append([]string(nil), t.RegistryScopes...), - TrustedIdentities: append([]string(nil), t.TrustedIdentities...), - TrustStores: append([]string(nil), t.TrustStores...), - } -} - func getArtifactPathFromReference(artifactReference string) (string, error) { // TODO support more types of URI like "domain.com/repository", // "domain.com/repository:tag" diff --git a/verification/trustpolicy/trustpolicy_test.go b/verification/trustpolicy/trustpolicy_test.go index b08b106c..00ac83c5 100644 --- a/verification/trustpolicy/trustpolicy_test.go +++ b/verification/trustpolicy/trustpolicy_test.go @@ -381,7 +381,7 @@ func TestGetVerificationLevel(t *testing.T) { for i, tt := range tests { t.Run(strconv.Itoa(i), func(t *testing.T) { - level, err := GetVerificationLevel(tt.verificationLevel) + level, err := tt.verificationLevel.GetVerificationLevel() if tt.wantErr != (err != nil) { t.Fatalf("TestFindVerificationLevel Error: %q WantErr: %v", err, tt.wantErr) @@ -428,7 +428,7 @@ func TestCustomVerificationLevel(t *testing.T) { } for i, tt := range tests { t.Run(strconv.Itoa(i), func(t *testing.T) { - level, err := GetVerificationLevel(tt.customVerification) + level, err := tt.customVerification.GetVerificationLevel() if tt.wantErr != (err != nil) { t.Fatalf("TestCustomVerificationLevel Error: %q WantErr: %v", err, tt.wantErr) @@ -461,13 +461,13 @@ func TestApplicableTrustPolicy(t *testing.T) { policyStatement, } // existing Registry Scope - policy, err := GetApplicableTrustPolicy(&policyDoc, registryUri) + policy, err := (&policyDoc).GetApplicableTrustPolicy(registryUri) if policy.Name != policyStatement.Name || err != nil { t.Fatalf("getApplicableTrustPolicy should return %q for registry scope %q", policyStatement.Name, registryScope) } // non-existing Registry Scope - policy, err = GetApplicableTrustPolicy(&policyDoc, "non.existing.scope/repo@sha256:hash") + policy, err = (&policyDoc).GetApplicableTrustPolicy("non.existing.scope/repo@sha256:hash") if policy != nil || err == nil || err.Error() != "artifact \"non.existing.scope/repo@sha256:hash\" has no applicable trust policy" { t.Fatalf("getApplicableTrustPolicy should return nil for non existing registry scope") } @@ -484,7 +484,7 @@ func TestApplicableTrustPolicy(t *testing.T) { policyStatement, wildcardStatement, } - policy, err = GetApplicableTrustPolicy(&policyDoc, "some.registry.that/has.no.policy@sha256:hash") + policy, err = (&policyDoc).GetApplicableTrustPolicy("some.registry.that/has.no.policy@sha256:hash") if policy.Name != wildcardStatement.Name || err != nil { t.Fatalf("getApplicableTrustPolicy should return wildcard policy for registry scope \"some.registry.that/has.no.policy\"") } diff --git a/verification/truststore/truststore.go b/verification/truststore/truststore.go index b6227bf2..0d736722 100644 --- a/verification/truststore/truststore.go +++ b/verification/truststore/truststore.go @@ -9,6 +9,7 @@ import ( "io/fs" "os" "path/filepath" + "regexp" corex509 "github.com/notaryproject/notation-core-go/x509" "github.com/notaryproject/notation-go/dir" @@ -36,20 +37,23 @@ type X509TrustStore interface { GetCertificates(ctx context.Context, storeType Type, namedStore string) ([]*x509.Certificate, error) } +// NewX509TrustStore generates a new X509TrustStore +func NewX509TrustStore(trustStorefs dir.SysFS) X509TrustStore { + return &x509TrustStore{trustStorefs} +} + // x509TrustStore implements X509TrustStore type x509TrustStore struct { trustStorefs dir.SysFS } -// NewX509TrustStore generates a new X509TrustStore -func NewX509TrustStore(trustStorefs dir.SysFS) X509TrustStore { - return x509TrustStore{trustStorefs} -} - // GetCertificates returns certificates under storeType/namedStore -func (trustStore x509TrustStore) GetCertificates(ctx context.Context, storeType Type, namedStore string) ([]*x509.Certificate, error) { - if storeType == "" || namedStore == "" { - return nil, errors.New("storeType and namedStore cannot be empty") +func (trustStore *x509TrustStore) GetCertificates(ctx context.Context, storeType Type, namedStore string) ([]*x509.Certificate, error) { + if !isValidStoreType(storeType) { + return nil, fmt.Errorf("unsupported store type: %s", storeType) + } + if !isValidNamedStore(namedStore) { + return nil, errors.New("named store name needs to follow [a-zA-Z0-9_.-]+ format") } path, err := trustStore.trustStorefs.SysPath(dir.X509TrustStoreDir(string(storeType), namedStore)) if err != nil { @@ -123,3 +127,18 @@ func validateCerts(certs []*x509.Certificate, path string) error { return nil } + +// isValidStoreType checks if storeType is supported +func isValidStoreType(storeType Type) bool { + for _, t := range Types { + if storeType == t { + return true + } + } + return false +} + +// isValidFileName checks if a file name is cross-platform compatible +func isValidNamedStore(namedStore string) bool { + return regexp.MustCompile(`^[a-zA-Z0-9_.-]+$`).MatchString(namedStore) +} From 89740d73b6fafbe9ac13ddc2bee738d0523b3827 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Tue, 8 Nov 2022 14:33:50 +0800 Subject: [PATCH 09/19] tp/ts refactoring Signed-off-by: Patrick Zheng --- internal/common/common.go | 97 ++++ .../ca/trust-store-with-invalid-certs/invalid | 1 + .../RootAndLeafCerts.crt | 43 ++ .../GlobalSignRootCA.crt | 22 + .../ca/trust-store-with-leaf-certs/non-ca.crt | 21 + .../valid-trust-store_SYMLINK | 1 + .../valid-trust-store-2/GlobalSign.der | Bin 0 -> 867 bytes .../valid-trust-store-2/GlobalSignRootCA.crt | 21 + verification/trustpolicy/trustpolicy.go | 378 +++++++++++++++ verification/trustpolicy/trustpolicy_test.go | 446 ++++++++++++++++++ verification/truststore/truststore.go | 126 +++++ verification/truststore/truststore_test.go | 58 +++ 12 files changed, 1214 insertions(+) create mode 100644 internal/common/common.go create mode 100644 verification/testdata/truststore/x509/ca/trust-store-with-invalid-certs/invalid create mode 100644 verification/testdata/truststore/x509/ca/trust-store-with-leaf-certs-in-single-file/RootAndLeafCerts.crt create mode 100644 verification/testdata/truststore/x509/ca/trust-store-with-leaf-certs/GlobalSignRootCA.crt create mode 100644 verification/testdata/truststore/x509/ca/trust-store-with-leaf-certs/non-ca.crt create mode 100644 verification/testdata/truststore/x509/ca/valid-trust-store_SYMLINK/valid-trust-store_SYMLINK create mode 100644 verification/testdata/truststore/x509/signingAuthority/valid-trust-store-2/GlobalSign.der create mode 100644 verification/testdata/truststore/x509/signingAuthority/valid-trust-store-2/GlobalSignRootCA.crt create mode 100644 verification/trustpolicy/trustpolicy.go create mode 100644 verification/trustpolicy/trustpolicy_test.go create mode 100644 verification/truststore/truststore.go create mode 100644 verification/truststore/truststore_test.go diff --git a/internal/common/common.go b/internal/common/common.go new file mode 100644 index 00000000..92a09f04 --- /dev/null +++ b/internal/common/common.go @@ -0,0 +1,97 @@ +package common + +import ( + "fmt" + "regexp" + "strings" + + ldapv3 "github.com/go-ldap/ldap/v3" +) + +const ( + Wildcard = "*" + X509Subject = "x509.subject" +) + +// 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 +} + +// Internal type to hold raw and parsed Distinguished Names +type ParsedDN struct { + RawString string + ParsedMap map[string]string +} + +// ParseDistinguishedName parses a DN name and validates Notary V2 rules +func ParseDistinguishedName(name string) (map[string]string, error) { + mandatoryFields := []string{"C", "ST", "O"} + attrKeyValue := make(map[string]string) + dn, err := ldapv3.ParseDN(name) + + if err != nil { + return nil, fmt.Errorf("distinguished name (DN) %q is not valid, it must contain 'C', 'ST', and 'O' RDN attributes at a minimum, and follow RFC 4514 standard", name) + } + + for _, rdn := range dn.RDNs { + + // multi-valued RDNs are not supported (TODO: add spec reference here) + if len(rdn.Attributes) > 1 { + return nil, fmt.Errorf("distinguished name (DN) %q has multi-valued RDN attributes, remove multi-valued RDN attributes as they are not supported", name) + } + for _, attribute := range rdn.Attributes { + if attrKeyValue[attribute.Type] == "" { + attrKeyValue[attribute.Type] = attribute.Value + } else { + return nil, fmt.Errorf("distinguished name (DN) %q has duplicate RDN attribute for %q, DN can only have unique RDN attributes", name, attribute.Type) + } + } + } + + // Verify mandatory fields are present + for _, field := range mandatoryFields { + if attrKeyValue[field] == "" { + return nil, fmt.Errorf("distinguished name (DN) %q has no mandatory RDN attribute for %q, it must contain 'C', 'ST', and 'O' RDN attributes at a minimum", name, field) + } + } + // No errors + return attrKeyValue, nil +} + +// IsSubsetDN returns true if dn1 is a subset of dn2 i.e. every key/value pair of dn1 has a matching key/value pair in dn2, otherwise returns false +func IsSubsetDN(dn1 map[string]string, dn2 map[string]string) bool { + for key := range dn1 { + if dn1[key] != dn2[key] { + return false + } + } + return true +} + +// 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 +} diff --git a/verification/testdata/truststore/x509/ca/trust-store-with-invalid-certs/invalid b/verification/testdata/truststore/x509/ca/trust-store-with-invalid-certs/invalid new file mode 100644 index 00000000..9977a283 --- /dev/null +++ b/verification/testdata/truststore/x509/ca/trust-store-with-invalid-certs/invalid @@ -0,0 +1 @@ +invalid diff --git a/verification/testdata/truststore/x509/ca/trust-store-with-leaf-certs-in-single-file/RootAndLeafCerts.crt b/verification/testdata/truststore/x509/ca/trust-store-with-leaf-certs-in-single-file/RootAndLeafCerts.crt new file mode 100644 index 00000000..e6c38cfb --- /dev/null +++ b/verification/testdata/truststore/x509/ca/trust-store-with-leaf-certs-in-single-file/RootAndLeafCerts.crt @@ -0,0 +1,43 @@ +-----BEGIN CERTIFICATE----- +MIIDejCCAmKgAwIBAgIBAjANBgkqhkiG9w0BAQsFADBdMQswCQYDVQQGEwJVUzEL +MAkGA1UECBMCV0ExEDAOBgNVBAcTB1NlYXR0bGUxDzANBgNVBAoTBk5vdGFyeTEe +MBwGA1UEAxMVd2FiYml0LW5ldHdvcmtzLmlvIENBMB4XDTIyMDkyMDA2MzExM1oX +DTIyMDkyMTA2MzExM1owWjELMAkGA1UEBhMCVVMxCzAJBgNVBAgTAldBMRAwDgYD +VQQHEwdTZWF0dGxlMQ8wDQYDVQQKEwZOb3RhcnkxGzAZBgNVBAMTEndhYmJpdC1u +ZXR3b3Jrcy5pbzCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBALiZp5O+ +6YtaNO5GbWaZUxvJPXktJ7k7LBX5G/Kn6eh9JkJln1agqbax9MRDB/5YCdQBKMBq +NE2wYIwmCs7ArFU5DxvRhoBnCGLjcsIZ9pfaZ6lBppEvxMmUAYDmgjze0J13PwRp +WAZMfBlisZnJAWokgE5sWtggUXURyFk67H0R+4sWlm8SSZOiJCA/e0bYPCHTfFA/ +2zg6koNRSwvI6zvftGnnJ9ny0BTuGOjZ6lDfIX5awFrgRdO8wmwejo4oJ45tUotF +/Rt/yHkmjdGhONbJjcMLf9AIyVwMHg6t6mj2SYbHqzIyTcpjk90HgeiU5eS5JMqj +Jkug5U9XrGGCqIcCAwEAAaNIMEYwDgYDVR0PAQH/BAQDAgeAMBMGA1UdJQQMMAoG +CCsGAQUFBwMDMB8GA1UdIwQYMBaAFLAy4Il5S9zOd/AMWF8hATmldAjYMA0GCSqG +SIb3DQEBCwUAA4IBAQBLYBnSuMNCzzLmeqH/wBr6kKUtF10AN9VF8/3iZW8iCj4B +Bx7VDq7iZR/G9UTLsWdZqkkxnOGu4QffBHz2Lc1v9D923EEPDAP5mJYvUchvdXYT +lmyQr9QEjRC6IFhlBB27Bi207QJ8UxYgmbseQ3FQFE16Usdmlg9iWDn5tx/DZn9/ +yUd81yKKYp2uLx0x2sQDJh61QSZB6jtzjN7w4Xax2NViabLaH7raMrDbIqigkXJh +iXG9fWx1Ax7S3dJVIglbZGPgYDW14Ass40gs8vcOBg8CwszrKiEuwp20d12Ky87/ +0pLsOWJmcNyXbd3gztX01N1frSEbvTBJNI9E/jmI +-----END CERTIFICATE----- +-----BEGIN CERTIFICATE----- +MIIDjzCCAnegAwIBAgIBATANBgkqhkiG9w0BAQsFADBdMQswCQYDVQQGEwJVUzEL +MAkGA1UECBMCV0ExEDAOBgNVBAcTB1NlYXR0bGUxDzANBgNVBAoTBk5vdGFyeTEe +MBwGA1UEAxMVd2FiYml0LW5ldHdvcmtzLmlvIENBMB4XDTIyMDkyMDA2MzExM1oX +DTIyMTAyMDA2MzExM1owXTELMAkGA1UEBhMCVVMxCzAJBgNVBAgTAldBMRAwDgYD +VQQHEwdTZWF0dGxlMQ8wDQYDVQQKEwZOb3RhcnkxHjAcBgNVBAMTFXdhYmJpdC1u +ZXR3b3Jrcy5pbyBDQTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAKNM +3dUToC4TyegGMw47ax9aZt13pQgTeV7xZbVsOmZiv/8gZ9tEZWgQbvBJrWUH8y4o +eQLCVQOTESNP2TSyTqizNtG1ex6YfSpWKSqUkfGX2II9xCX8hNXZqTphAjrGGf2Z +EOLRIIkbhjkuiAR+7q4TF/KJhdfYD1HQBJ2PF92egV5JEZTrxIjVIi+WK19VKSwx +m7oFiijve4VPaQYQnWgj0dk+Tn9cMB/OMX6cszoJbn98ogQIvWaY3dd1qba4uGJ9 +vmkNKDJcUd1PbkaVlikXC4UM+PxXy7/ZvSihOXurAPIChS6JgWC8Ru2vxm9SC+BN +5J/hr92W2TdsrvLkrc8CAwEAAaNaMFgwDgYDVR0PAQH/BAQDAgIEMBMGA1UdJQQM +MAoGCCsGAQUFBwMDMBIGA1UdEwEB/wQIMAYBAf8CAQEwHQYDVR0OBBYEFLAy4Il5 +S9zOd/AMWF8hATmldAjYMA0GCSqGSIb3DQEBCwUAA4IBAQCTf6GbT5Z0x5ciNr9i +8i+QsIAg7ZHzv5RLLJuocGcKwbdi+btU6BPl/X4U5ZB6OArv4oiyPSbECoxkgGRq +cj+mfzXdm/3jEyRskHDfoxcJFYmcBsEykS7DoLYEy5HxgKSaGOLl4dMWbbj/E8mR +e9XC5ruvPNZX52pQMqSqUUTYlbR4YQojsp7ShcLLD/Iea90wXk44+wHAKNFpwkN1 +h5JMlYm+jKkol6u/Nmd3vNqhzrL91ZLPVtSWpfsBxh7l4BsDns2uPl+/fgCav9MJ +jUkWJbEaDPY5bSbHDhCbxMO37VbvkkFUvz7lfKAkXj6DnkPzMj3++KTFNdw3fJ4+ +WzLe +-----END CERTIFICATE----- diff --git a/verification/testdata/truststore/x509/ca/trust-store-with-leaf-certs/GlobalSignRootCA.crt b/verification/testdata/truststore/x509/ca/trust-store-with-leaf-certs/GlobalSignRootCA.crt new file mode 100644 index 00000000..c26fd442 --- /dev/null +++ b/verification/testdata/truststore/x509/ca/trust-store-with-leaf-certs/GlobalSignRootCA.crt @@ -0,0 +1,22 @@ +-----BEGIN CERTIFICATE----- +MIIDjzCCAnegAwIBAgIBATANBgkqhkiG9w0BAQsFADBdMQswCQYDVQQGEwJVUzEL +MAkGA1UECBMCV0ExEDAOBgNVBAcTB1NlYXR0bGUxDzANBgNVBAoTBk5vdGFyeTEe +MBwGA1UEAxMVd2FiYml0LW5ldHdvcmtzLmlvIENBMB4XDTIyMDkyMDA2MzExM1oX +DTIyMTAyMDA2MzExM1owXTELMAkGA1UEBhMCVVMxCzAJBgNVBAgTAldBMRAwDgYD +VQQHEwdTZWF0dGxlMQ8wDQYDVQQKEwZOb3RhcnkxHjAcBgNVBAMTFXdhYmJpdC1u +ZXR3b3Jrcy5pbyBDQTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAKNM +3dUToC4TyegGMw47ax9aZt13pQgTeV7xZbVsOmZiv/8gZ9tEZWgQbvBJrWUH8y4o +eQLCVQOTESNP2TSyTqizNtG1ex6YfSpWKSqUkfGX2II9xCX8hNXZqTphAjrGGf2Z +EOLRIIkbhjkuiAR+7q4TF/KJhdfYD1HQBJ2PF92egV5JEZTrxIjVIi+WK19VKSwx +m7oFiijve4VPaQYQnWgj0dk+Tn9cMB/OMX6cszoJbn98ogQIvWaY3dd1qba4uGJ9 +vmkNKDJcUd1PbkaVlikXC4UM+PxXy7/ZvSihOXurAPIChS6JgWC8Ru2vxm9SC+BN +5J/hr92W2TdsrvLkrc8CAwEAAaNaMFgwDgYDVR0PAQH/BAQDAgIEMBMGA1UdJQQM +MAoGCCsGAQUFBwMDMBIGA1UdEwEB/wQIMAYBAf8CAQEwHQYDVR0OBBYEFLAy4Il5 +S9zOd/AMWF8hATmldAjYMA0GCSqGSIb3DQEBCwUAA4IBAQCTf6GbT5Z0x5ciNr9i +8i+QsIAg7ZHzv5RLLJuocGcKwbdi+btU6BPl/X4U5ZB6OArv4oiyPSbECoxkgGRq +cj+mfzXdm/3jEyRskHDfoxcJFYmcBsEykS7DoLYEy5HxgKSaGOLl4dMWbbj/E8mR +e9XC5ruvPNZX52pQMqSqUUTYlbR4YQojsp7ShcLLD/Iea90wXk44+wHAKNFpwkN1 +h5JMlYm+jKkol6u/Nmd3vNqhzrL91ZLPVtSWpfsBxh7l4BsDns2uPl+/fgCav9MJ +jUkWJbEaDPY5bSbHDhCbxMO37VbvkkFUvz7lfKAkXj6DnkPzMj3++KTFNdw3fJ4+ +WzLe +-----END CERTIFICATE----- diff --git a/verification/testdata/truststore/x509/ca/trust-store-with-leaf-certs/non-ca.crt b/verification/testdata/truststore/x509/ca/trust-store-with-leaf-certs/non-ca.crt new file mode 100644 index 00000000..74bc7b00 --- /dev/null +++ b/verification/testdata/truststore/x509/ca/trust-store-with-leaf-certs/non-ca.crt @@ -0,0 +1,21 @@ +-----BEGIN CERTIFICATE----- +MIIDejCCAmKgAwIBAgIBAjANBgkqhkiG9w0BAQsFADBdMQswCQYDVQQGEwJVUzEL +MAkGA1UECBMCV0ExEDAOBgNVBAcTB1NlYXR0bGUxDzANBgNVBAoTBk5vdGFyeTEe +MBwGA1UEAxMVd2FiYml0LW5ldHdvcmtzLmlvIENBMB4XDTIyMDkyMDA2MzExM1oX +DTIyMDkyMTA2MzExM1owWjELMAkGA1UEBhMCVVMxCzAJBgNVBAgTAldBMRAwDgYD +VQQHEwdTZWF0dGxlMQ8wDQYDVQQKEwZOb3RhcnkxGzAZBgNVBAMTEndhYmJpdC1u +ZXR3b3Jrcy5pbzCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBALiZp5O+ +6YtaNO5GbWaZUxvJPXktJ7k7LBX5G/Kn6eh9JkJln1agqbax9MRDB/5YCdQBKMBq +NE2wYIwmCs7ArFU5DxvRhoBnCGLjcsIZ9pfaZ6lBppEvxMmUAYDmgjze0J13PwRp +WAZMfBlisZnJAWokgE5sWtggUXURyFk67H0R+4sWlm8SSZOiJCA/e0bYPCHTfFA/ +2zg6koNRSwvI6zvftGnnJ9ny0BTuGOjZ6lDfIX5awFrgRdO8wmwejo4oJ45tUotF +/Rt/yHkmjdGhONbJjcMLf9AIyVwMHg6t6mj2SYbHqzIyTcpjk90HgeiU5eS5JMqj +Jkug5U9XrGGCqIcCAwEAAaNIMEYwDgYDVR0PAQH/BAQDAgeAMBMGA1UdJQQMMAoG +CCsGAQUFBwMDMB8GA1UdIwQYMBaAFLAy4Il5S9zOd/AMWF8hATmldAjYMA0GCSqG +SIb3DQEBCwUAA4IBAQBLYBnSuMNCzzLmeqH/wBr6kKUtF10AN9VF8/3iZW8iCj4B +Bx7VDq7iZR/G9UTLsWdZqkkxnOGu4QffBHz2Lc1v9D923EEPDAP5mJYvUchvdXYT +lmyQr9QEjRC6IFhlBB27Bi207QJ8UxYgmbseQ3FQFE16Usdmlg9iWDn5tx/DZn9/ +yUd81yKKYp2uLx0x2sQDJh61QSZB6jtzjN7w4Xax2NViabLaH7raMrDbIqigkXJh +iXG9fWx1Ax7S3dJVIglbZGPgYDW14Ass40gs8vcOBg8CwszrKiEuwp20d12Ky87/ +0pLsOWJmcNyXbd3gztX01N1frSEbvTBJNI9E/jmI +-----END CERTIFICATE----- diff --git a/verification/testdata/truststore/x509/ca/valid-trust-store_SYMLINK/valid-trust-store_SYMLINK b/verification/testdata/truststore/x509/ca/valid-trust-store_SYMLINK/valid-trust-store_SYMLINK new file mode 100644 index 00000000..ca626091 --- /dev/null +++ b/verification/testdata/truststore/x509/ca/valid-trust-store_SYMLINK/valid-trust-store_SYMLINK @@ -0,0 +1 @@ +ca/valid-trust-store \ No newline at end of file diff --git a/verification/testdata/truststore/x509/signingAuthority/valid-trust-store-2/GlobalSign.der b/verification/testdata/truststore/x509/signingAuthority/valid-trust-store-2/GlobalSign.der new file mode 100644 index 0000000000000000000000000000000000000000..232c4b6121f7236eb429572c591127e8aa60166f GIT binary patch literal 867 zcmXqLVvaXxVsc-=%*4pV#LdD01dNIi!5oVWc-c6$+C196^D;7WvoaX?7%CXZu`!3T za0`pO=j10P<^*S^=P3l`=a(orJ1XcZ1Q{C&8wi3_a0zoERKNt8kp&Ip#CZ)Y4U7#f z3=KdaN}SgSnM-Bcni!Rky~D`Lz}&>h&tTBR$i>ve$jER;wQNEqTZNL?*8|PlT25)q z`^#D;cyw(?(H}P^=i{5Y=CZ`AoYwPxn9$_*FlaSTqkBQl-IR;3zv?XJZ?fglUN`;v zHjy@g%H7t&4dp!?4?QnsCF#q@{hF3>zf*mx#eBBwb|+7(Me-Kk+i>Eg8eg;MvG>v4 zmsk=`c`noVmTCR%^a+iLPv>?ehMTV`5xG?_`;jk@3_*EunPN{PO=3Mc%Jd@cgsZLDb_S zlVPG{+>Yt**OqTjnN_tv{-E&t*-5{7a~_0bimhb6mG`oFa$(uA%+@AxCT2zk#>Gws z4hDR{_><*lWc<&<0!$HX2K*qtFo@4;zzn1eWI+OaEMhDo{U5yRSLnH_tn&@{l{~Ba z-lMzHdyqpJm}r2Z%*f#FaQi^Os(&YV-hZDK;_A738Uv@}n$5y(Z5r&xr?Q`w?A*nm zyKV{B<*y$<@^|eoPWNg)?owUxV0~UrKC@<@v8C(bz9&1wb5{Kkn)W96nC=smoSjpW zf8PntNDs4X-f`Yk@$kuvf9Gx;3SM<)Lf6X=v2Hz6?^Z=gm=bbRTvM6jq*l?z>U;6vh=ZpX(UQTUo1KYI8Vet 0 || len(statement.TrustedIdentities) > 0 { + return fmt.Errorf("trust policy statement %q is set to skip signature verification but configured with trust stores and/or trusted identities, remove them if signature verification needs to be skipped", statement.Name) + } + } else { + if len(statement.TrustStores) == 0 || len(statement.TrustedIdentities) == 0 { + return fmt.Errorf("trust policy statement %q is either missing trust stores or trusted identities, both must be specified", statement.Name) + } + + // Verify Trust Store is valid + if err := validateTrustStore(statement); err != nil { + return err + } + + // Verify Trusted Identities are valid + if err := validateTrustedIdentities(statement); err != nil { + return err + } + } + + } + + // Verify registry scopes are valid + if err := validateRegistryScopes(policyDoc); err != nil { + return err + } + + // Verify unique policy statement names across the policy document + for key := range policyStatementNameCount { + if policyStatementNameCount[key] > 1 { + return fmt.Errorf("multiple trust policy statements use the same name %q, statement names must be unique", key) + } + } + + // No errors + return nil +} + +// GetVerificationLevel returns VerificationLevel struct for the given SignatureVerification struct +// throws error if SignatureVerification is invalid +func GetVerificationLevel(signatureVerification SignatureVerification) (*VerificationLevel, error) { + var baseLevel *VerificationLevel + for _, l := range VerificationLevels { + if l.Name == signatureVerification.VerificationLevel { + baseLevel = l + } + } + if baseLevel == nil { + return nil, fmt.Errorf("invalid signature verification %q", signatureVerification.VerificationLevel) + } + + if len(signatureVerification.Override) == 0 { + // nothing to override, return the base verification level + return baseLevel, nil + } + + if baseLevel == LevelSkip { + return nil, fmt.Errorf("signature verification %q can't be used to customize signature verification", baseLevel.Name) + } + + customVerificationLevel := &VerificationLevel{ + Name: "custom", + Enforcement: make(map[ValidationType]ValidationAction), + } + + // populate the custom verification level with the base verification settings + for k, v := range baseLevel.Enforcement { + customVerificationLevel.Enforcement[k] = v + } + + // override the verification actions with the user configured settings + for key, value := range signatureVerification.Override { + var validationType ValidationType + for _, t := range ValidationTypes { + if t == key { + validationType = t + break + } + } + if validationType == "" { + return nil, fmt.Errorf("verification type %q in custom signature verification is not supported, supported values are %q", key, ValidationTypes) + } + + var validationAction ValidationAction + for _, action := range ValidationActions { + if action == value { + validationAction = action + break + } + } + if validationAction == "" { + return nil, fmt.Errorf("verification action %q in custom signature verification is not supported, supported values are %q", value, ValidationActions) + } + + if validationType == TypeIntegrity { + return nil, fmt.Errorf("%q verification can not be overridden in custom signature verification", key) + } else if validationType != TypeRevocation && validationAction == ActionSkip { + return nil, fmt.Errorf("%q verification can not be skipped in custom signature verification", key) + } + + customVerificationLevel.Enforcement[validationType] = validationAction + } + return customVerificationLevel, nil +} + +// validateTrustStore validates if the policy statement is following the Notary V2 spec rules for truststores +func validateTrustStore(statement TrustPolicy) error { + for _, trustStore := range statement.TrustStores { + i := strings.Index(trustStore, ":") + if i < 0 || !isValidTrustStoreType(trustStore[:i]) { + return fmt.Errorf("trust policy statement %q uses an unsupported trust store type %q in trust store value %q", statement.Name, trustStore[:i], trustStore) + } + } + + return nil +} + +// validateTrustedIdentities 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 && common.IsPresent(common.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) + } + + var parsedDNs []common.ParsedDN + // 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 != common.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 == common.X509Subject { + dn, err := common.ParseDistinguishedName(identityValue) + if err != nil { + return err + } + parsedDNs = append(parsedDNs, common.ParsedDN{RawString: identity, ParsedMap: dn}) + } + } + } + + // Verify there are no overlapping DNs + if err := validateOverlappingDNs(statement.Name, parsedDNs); err != nil { + return err + } + + // No error + return nil +} + +// validateRegistryScopes validates if the policy document is following the Notary V2 spec rules for registry scopes +func validateRegistryScopes(policyDoc *Document) 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 && common.IsPresent(common.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 != common.Wildcard { + if err := common.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 +} + +func validateOverlappingDNs(policyName string, parsedDNs []common.ParsedDN) error { + for i, dn1 := range parsedDNs { + for j, dn2 := range parsedDNs { + if i != j && common.IsSubsetDN(dn1.ParsedMap, dn2.ParsedMap) { + return fmt.Errorf("trust policy statement %q has overlapping x509 trustedIdentities, %q overlaps with %q", policyName, dn1.RawString, dn2.RawString) + } + } + } + + return nil +} + +// isValidTrustStoreType returns true if the given string is a valid truststore.Type, otherwise false. +func isValidTrustStoreType(s string) bool { + for _, p := range truststore.Types { + if s == string(p) { + return true + } + } + return false +} diff --git a/verification/trustpolicy/trustpolicy_test.go b/verification/trustpolicy/trustpolicy_test.go new file mode 100644 index 00000000..b8d4ee54 --- /dev/null +++ b/verification/trustpolicy/trustpolicy_test.go @@ -0,0 +1,446 @@ +package trustpolicy + +import ( + "strconv" + "testing" +) + +func dummyPolicyStatement() (policyStatement TrustPolicy) { + policyStatement = TrustPolicy{ + Name: "test-statement-name", + RegistryScopes: []string{"registry.acme-rockets.io/software/net-monitor"}, + SignatureVerification: SignatureVerification{VerificationLevel: "strict"}, + TrustStores: []string{"ca:valid-trust-store", "signingAuthority:valid-trust-store"}, + TrustedIdentities: []string{"x509.subject:CN=Notation Test Root,O=Notary,L=Seattle,ST=WA,C=US"}, + } + return +} + +func dummyPolicyDocument() (policyDoc Document) { + policyDoc = Document{ + Version: "1.0", + TrustPolicies: []TrustPolicy{dummyPolicyStatement()}, + } + return +} + +// TestValidateValidPolicyDocument tests a happy policy document +func TestValidateValidPolicyDocument(t *testing.T) { + policyDoc := dummyPolicyDocument() + + policyStatement1 := dummyPolicyStatement() + + policyStatement2 := dummyPolicyStatement() + policyStatement2.Name = "test-statement-name-2" + policyStatement2.RegistryScopes = []string{"registry.wabbit-networks.io/software/unsigned/net-utils"} + policyStatement2.SignatureVerification = SignatureVerification{VerificationLevel: "permissive"} + + policyStatement3 := dummyPolicyStatement() + policyStatement3.Name = "test-statement-name-3" + policyStatement3.RegistryScopes = []string{"registry.acme-rockets.io/software/legacy/metrics"} + policyStatement3.TrustStores = []string{} + policyStatement3.TrustedIdentities = []string{} + policyStatement3.SignatureVerification = SignatureVerification{VerificationLevel: "skip"} + + policyStatement4 := dummyPolicyStatement() + policyStatement4.Name = "test-statement-name-4" + policyStatement4.TrustStores = []string{"ca:valid-trust-store", "signingAuthority:valid-trust-store-2"} + policyStatement4.RegistryScopes = []string{"*"} + policyStatement4.SignatureVerification = SignatureVerification{VerificationLevel: "audit"} + + policyStatement5 := dummyPolicyStatement() + policyStatement5.Name = "test-statement-name-5" + policyStatement5.RegistryScopes = []string{"registry.acme-rockets2.io/software"} + policyStatement5.TrustedIdentities = []string{"*"} + policyStatement5.SignatureVerification = SignatureVerification{VerificationLevel: "strict"} + + policyDoc.TrustPolicies = []TrustPolicy{ + policyStatement1, + policyStatement2, + policyStatement3, + policyStatement4, + policyStatement5, + } + err := policyDoc.Validate() + if err != nil { + t.Fatalf("validation failed on a good policy document. Error : %q", err) + } +} + +// TestValidateTrustedIdentities tests only valid x509.subjects are accepted +func TestValidateTrustedIdentities(t *testing.T) { + + // No trusted identity prefix throws error + policyDoc := dummyPolicyDocument() + policyStatement := dummyPolicyStatement() + policyStatement.TrustedIdentities = []string{"C=US, ST=WA, O=wabbit-network.io, OU=org1"} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err := policyDoc.Validate() + if err == nil || err.Error() != "trust policy statement \"test-statement-name\" has trusted identity \"C=US, ST=WA, O=wabbit-network.io, OU=org1\" without an identity prefix" { + t.Fatalf("trusted identity without a prefix should return error") + } + + // Accept unknown identity prefixes + policyDoc = dummyPolicyDocument() + policyStatement = dummyPolicyStatement() + policyStatement.TrustedIdentities = []string{"unknown:my-trusted-idenity"} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err = policyDoc.Validate() + if err != nil { + t.Fatalf("unknown identity prefix should not return an error. Error: %q", err) + } + + // Validate x509.subject identities + policyDoc = dummyPolicyDocument() + policyStatement = dummyPolicyStatement() + invalidDN := "x509.subject:,,," + policyStatement.TrustedIdentities = []string{invalidDN} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err = policyDoc.Validate() + if err == nil || err.Error() != "distinguished name (DN) \",,,\" is not valid, it must contain 'C', 'ST', and 'O' RDN attributes at a minimum, and follow RFC 4514 standard" { + t.Fatalf("invalid x509.subject identity should return error. Error : %q", err) + } + + // Validate duplicate RDNs + policyDoc = dummyPolicyDocument() + policyStatement = dummyPolicyStatement() + invalidDN = "x509.subject:C=US,C=IN" + policyStatement.TrustedIdentities = []string{invalidDN} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err = policyDoc.Validate() + if err == nil || err.Error() != "distinguished name (DN) \"C=US,C=IN\" has duplicate RDN attribute for \"C\", DN can only have unique RDN attributes" { + t.Fatalf("invalid x509.subject identity should return error. Error : %q", err) + } + + // Validate mandatory RDNs + policyDoc = dummyPolicyDocument() + policyStatement = dummyPolicyStatement() + invalidDN = "x509.subject:C=US,ST=WA" + policyStatement.TrustedIdentities = []string{invalidDN} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err = policyDoc.Validate() + if err == nil || err.Error() != "distinguished name (DN) \"C=US,ST=WA\" has no mandatory RDN attribute for \"O\", it must contain 'C', 'ST', and 'O' RDN attributes at a minimum" { + t.Fatalf("invalid x509.subject identity should return error. Error : %q", err) + } + + // DN may have optional RDNs + policyDoc = dummyPolicyDocument() + policyStatement = dummyPolicyStatement() + validDN := "x509.subject:C=US,ST=WA,O=MyOrg,CustomRDN=CustomValue" + policyStatement.TrustedIdentities = []string{validDN} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err = policyDoc.Validate() + if err != nil { + t.Fatalf("valid x509.subject identity should not return error. Error : %q", err) + } + + // Validate rfc4514 DNs + policyDoc = dummyPolicyDocument() + policyStatement = dummyPolicyStatement() + validDN1 := "x509.subject:C=US,ST=WA,O=MyOrg" + validDN2 := "x509.subject:C=US,ST=WA,O= My. Org" + validDN3 := "x509.subject:C=US,ST=WA,O=My \"special\" Org \\, \\; \\\\ others" + validDN4 := "x509.subject:C=US,ST=WA,O=My Org,1.3.6.1.4.1.1466.0=#04024869" + policyStatement.TrustedIdentities = []string{validDN1, validDN2, validDN3, validDN4} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err = policyDoc.Validate() + if err != nil { + t.Fatalf("valid x509.subject identity should not return error. Error : %q", err) + } + + // Validate overlapping DNs + policyDoc = dummyPolicyDocument() + policyStatement = dummyPolicyStatement() + validDN1 = "x509.subject:C=US,ST=WA,O=MyOrg" + validDN2 = "x509.subject:C=US,ST=WA,O=MyOrg,X=Y" + policyStatement.TrustedIdentities = []string{validDN1, validDN2} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err = policyDoc.Validate() + if err == nil || err.Error() != "trust policy statement \"test-statement-name\" has overlapping x509 trustedIdentities, \"x509.subject:C=US,ST=WA,O=MyOrg\" overlaps with \"x509.subject:C=US,ST=WA,O=MyOrg,X=Y\"" { + t.Fatalf("overlapping DNs should return error") + } + + // Validate multi-valued RDNs + policyDoc = dummyPolicyDocument() + policyStatement = dummyPolicyStatement() + multiValduedRDN := "x509.subject:C=US+ST=WA,O=MyOrg" + policyStatement.TrustedIdentities = []string{multiValduedRDN} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err = policyDoc.Validate() + if err == nil || err.Error() != "distinguished name (DN) \"C=US+ST=WA,O=MyOrg\" has multi-valued RDN attributes, remove multi-valued RDN attributes as they are not supported" { + t.Fatalf("multi-valued RDN should return error. Error : %q", err) + } +} + +// TestInvalidRegistryScopes tests invalid scopes are rejected +func TestInvalidRegistryScopes(t *testing.T) { + invalidScopes := []string{ + "", "1:1", "a,b", "abcd", "1111", "1,2", "example.com/rep:tag", + "example.com/rep/subrep/sub:latest", "example.com", "rep/rep2:latest", + "repository", "10.10.10.10", "10.10.10.10:8080/rep/rep2:latest", + } + + for _, scope := range invalidScopes { + policyDoc := dummyPolicyDocument() + policyStatement := dummyPolicyStatement() + policyStatement.RegistryScopes = []string{scope} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err := policyDoc.Validate() + if err == nil || err.Error() != "registry scope \""+scope+"\" is not valid, make sure it is the fully qualified registry URL without the scheme/protocol. e.g domain.com/my/repository" { + t.Fatalf("invalid registry scope should return error. Error : %q", err) + } + } +} + +// TestValidRegistryScopes tests valid scopes are accepted +func TestValidRegistryScopes(t *testing.T) { + validScopes := []string{ + "example.com/rep", "example.com:8080/rep/rep2", "example.com/rep/subrep/subsub", + "10.10.10.10:8080/rep/rep2", "domain/rep", "domain:1234/rep", + } + + for _, scope := range validScopes { + policyDoc := dummyPolicyDocument() + policyStatement := dummyPolicyStatement() + policyStatement.RegistryScopes = []string{scope} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err := policyDoc.Validate() + if err != nil { + t.Fatalf("valid registry scope should not return error. Error : %q", err) + } + } +} + +// TestValidatePolicyDocument calls policyDoc.Validate() +// and tests various validations on policy eliments +func TestValidateInvalidPolicyDocument(t *testing.T) { + + // Invalid Version + policyDoc := dummyPolicyDocument() + policyDoc.Version = "invalid" + err := policyDoc.Validate() + if err == nil || err.Error() != "trust policy document uses unsupported version \"invalid\"" { + t.Fatalf("invalid version should return error") + } + + // No Policy Satements + policyDoc = dummyPolicyDocument() + policyDoc.TrustPolicies = nil + err = policyDoc.Validate() + if err == nil || err.Error() != "trust policy document can not have zero trust policy statements" { + t.Fatalf("zero policy statements should return error") + } + + // No Policy Satement Name + policyDoc = dummyPolicyDocument() + policyStatement := dummyPolicyStatement() + policyStatement.Name = "" + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err = policyDoc.Validate() + if err == nil || err.Error() != "a trust policy statement is missing a name, every statement requires a name" { + t.Fatalf("policy statement with no name should return an error") + } + + // No Registry Scopes + policyDoc = dummyPolicyDocument() + policyStatement = dummyPolicyStatement() + policyStatement.RegistryScopes = nil + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err = policyDoc.Validate() + if err == nil || err.Error() != "trust policy statement \"test-statement-name\" has zero registry scopes, it must specify registry scopes with at least one value" { + t.Fatalf("policy statement with registry scopes should return error") + } + + // Multiple policy statements with same registry scope + policyDoc = dummyPolicyDocument() + policyStatement1 := dummyPolicyStatement() + policyStatement2 := dummyPolicyStatement() + policyStatement2.Name = "test-statement-name-2" + policyDoc.TrustPolicies = []TrustPolicy{policyStatement1, policyStatement2} + err = policyDoc.Validate() + if err == nil || err.Error() != "registry scope \"registry.acme-rockets.io/software/net-monitor\" is present in multiple trust policy statements, one registry scope value can only be associated with one statement" { + t.Fatalf("Policy statements with same registry scope should return error %q", err) + } + + // Registry scopes with a wildcard + policyDoc = dummyPolicyDocument() + policyStatement = dummyPolicyStatement() + policyStatement.RegistryScopes = []string{"*", "registry.acme-rockets.io/software/net-monitor"} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err = policyDoc.Validate() + if err == nil || err.Error() != "trust policy statement \"test-statement-name\" uses wildcard registry scope '*', a wildcard scope cannot be used in conjunction with other scope values" { + t.Fatalf("policy statement with more than a wildcard registry scope should return error") + } + + // Invlaid SignatureVerification + policyDoc = dummyPolicyDocument() + policyStatement = dummyPolicyStatement() + policyStatement.SignatureVerification = SignatureVerification{VerificationLevel: "invalid"} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err = policyDoc.Validate() + if err == nil || err.Error() != "trust policy statement \"test-statement-name\" uses invalid signatureVerification value \"invalid\"" { + t.Fatalf("policy statement with invalid SignatureVerification should return error") + } + + // strict SignatureVerification should have a trust store + policyDoc = dummyPolicyDocument() + policyStatement = dummyPolicyStatement() + policyStatement.TrustStores = []string{} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err = policyDoc.Validate() + if err == nil || err.Error() != "trust policy statement \"test-statement-name\" is either missing trust stores or trusted identities, both must be specified" { + t.Fatalf("strict SignatureVerification should have a trust store") + } + + // strict SignatureVerification should have trusted identities + policyDoc = dummyPolicyDocument() + policyStatement = dummyPolicyStatement() + policyStatement.TrustedIdentities = []string{} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err = policyDoc.Validate() + if err == nil || err.Error() != "trust policy statement \"test-statement-name\" is either missing trust stores or trusted identities, both must be specified" { + t.Fatalf("strict SignatureVerification should have trusted identities") + } + + // skip SignatureVerification should not have trust store or trusted identities + policyDoc = dummyPolicyDocument() + policyStatement = dummyPolicyStatement() + policyStatement.SignatureVerification = SignatureVerification{VerificationLevel: "skip"} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err = policyDoc.Validate() + if err == nil || err.Error() != "trust policy statement \"test-statement-name\" is set to skip signature verification but configured with trust stores and/or trusted identities, remove them if signature verification needs to be skipped" { + t.Fatalf("strict SignatureVerification should have trusted identities") + } + + // Empty Trusted Identity should throw error + policyDoc = dummyPolicyDocument() + policyStatement = dummyPolicyStatement() + policyStatement.TrustedIdentities = []string{""} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err = policyDoc.Validate() + if err == nil || err.Error() != "trust policy statement \"test-statement-name\" has an empty trusted identity" { + t.Fatalf("policy statement with empty trusted identity should return error") + } + + // trust store/trusted identites are optional for skip SignatureVerification + policyDoc = dummyPolicyDocument() + policyStatement = dummyPolicyStatement() + policyStatement.SignatureVerification = SignatureVerification{VerificationLevel: "skip"} + policyStatement.TrustStores = []string{} + policyStatement.TrustedIdentities = []string{} + err = policyDoc.Validate() + if err != nil { + t.Fatalf("skip SignatureVerification should not require a trust store or trusted identities") + } + + // Invalid Trust Store type + policyDoc = dummyPolicyDocument() + policyStatement = dummyPolicyStatement() + policyStatement.TrustStores = []string{"invalid:test-trust-store"} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err = policyDoc.Validate() + if err == nil || err.Error() != "trust policy statement \"test-statement-name\" uses an unsupported trust store type \"invalid\" in trust store value \"invalid:test-trust-store\"" { + t.Fatalf("policy statement with invalid trust store type should return error") + } + + // trusted identities with a wildcard + policyDoc = dummyPolicyDocument() + policyStatement = dummyPolicyStatement() + policyStatement.TrustedIdentities = []string{"*", "test-identity"} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement} + err = policyDoc.Validate() + if err == nil || err.Error() != "trust policy statement \"test-statement-name\" uses a wildcard trusted identity '*', a wildcard identity cannot be used in conjunction with other values" { + t.Fatalf("policy statement with more than a wildcard trusted identity should return error") + } + + // Policy Document with duplicate policy statement names + policyDoc = dummyPolicyDocument() + policyStatement1 = dummyPolicyStatement() + policyStatement2 = dummyPolicyStatement() + policyStatement2.RegistryScopes = []string{"registry.acme-rockets.io/software/legacy/metrics"} + policyDoc.TrustPolicies = []TrustPolicy{policyStatement1, policyStatement2} + err = policyDoc.Validate() + if err == nil || err.Error() != "multiple trust policy statements use the same name \"test-statement-name\", statement names must be unique" { + t.Fatalf("policy statements with same name should return error") + } +} + +func TestGetVerificationLevel(t *testing.T) { + tests := []struct { + verificationLevel SignatureVerification + wantErr bool + verificationActions []ValidationAction + }{ + {SignatureVerification{VerificationLevel: "strict"}, false, []ValidationAction{ActionEnforce, ActionEnforce, ActionEnforce, ActionEnforce, ActionEnforce}}, + {SignatureVerification{VerificationLevel: "permissive"}, false, []ValidationAction{ActionEnforce, ActionEnforce, ActionLog, ActionLog, ActionLog}}, + {SignatureVerification{VerificationLevel: "audit"}, false, []ValidationAction{ActionEnforce, ActionLog, ActionLog, ActionLog, ActionLog}}, + {SignatureVerification{VerificationLevel: "skip"}, false, []ValidationAction{ActionSkip, ActionSkip, ActionSkip, ActionSkip, ActionSkip}}, + {SignatureVerification{VerificationLevel: "invalid"}, true, []ValidationAction{}}, + } + for i, tt := range tests { + t.Run(strconv.Itoa(i), func(t *testing.T) { + + level, err := GetVerificationLevel(tt.verificationLevel) + + if tt.wantErr != (err != nil) { + t.Fatalf("TestFindVerificationLevel Error: %q WantErr: %v", err, tt.wantErr) + } else { + for index, action := range tt.verificationActions { + if action != level.Enforcement[ValidationTypes[index]] { + t.Errorf("%q verification action should be %q for Verification Level %q", ValidationTypes[index], action, tt.verificationLevel) + } + } + } + }) + } +} + +func TestCustomVerificationLevel(t *testing.T) { + tests := []struct { + customVerification SignatureVerification + wantErr bool + verificationActions []ValidationAction + }{ + {SignatureVerification{VerificationLevel: "strict", Override: map[ValidationType]ValidationAction{"integrity": "log"}}, true, []ValidationAction{}}, + {SignatureVerification{VerificationLevel: "strict", Override: map[ValidationType]ValidationAction{"authenticity": "skip"}}, true, []ValidationAction{}}, + {SignatureVerification{VerificationLevel: "strict", Override: map[ValidationType]ValidationAction{"authenticTimestamp": "skip"}}, true, []ValidationAction{}}, + {SignatureVerification{VerificationLevel: "strict", Override: map[ValidationType]ValidationAction{"expiry": "skip"}}, true, []ValidationAction{}}, + {SignatureVerification{VerificationLevel: "skip", Override: map[ValidationType]ValidationAction{"authenticity": "log"}}, true, []ValidationAction{}}, + {SignatureVerification{VerificationLevel: "invalid", Override: map[ValidationType]ValidationAction{"authenticity": "log"}}, true, []ValidationAction{}}, + {SignatureVerification{VerificationLevel: "strict", Override: map[ValidationType]ValidationAction{"invalid": "log"}}, true, []ValidationAction{}}, + {SignatureVerification{VerificationLevel: "strict", Override: map[ValidationType]ValidationAction{"authenticity": "invalid"}}, true, []ValidationAction{}}, + {SignatureVerification{VerificationLevel: "strict", Override: map[ValidationType]ValidationAction{"authenticity": "log"}}, false, []ValidationAction{ActionEnforce, ActionLog, ActionEnforce, ActionEnforce, ActionEnforce}}, + {SignatureVerification{VerificationLevel: "permissive", Override: map[ValidationType]ValidationAction{"authenticity": "log"}}, false, []ValidationAction{ActionEnforce, ActionLog, ActionLog, ActionLog, ActionLog}}, + {SignatureVerification{VerificationLevel: "audit", Override: map[ValidationType]ValidationAction{"authenticity": "log"}}, false, []ValidationAction{ActionEnforce, ActionLog, ActionLog, ActionLog, ActionLog}}, + {SignatureVerification{VerificationLevel: "strict", Override: map[ValidationType]ValidationAction{"expiry": "log"}}, false, []ValidationAction{ActionEnforce, ActionEnforce, ActionEnforce, ActionLog, ActionEnforce}}, + {SignatureVerification{VerificationLevel: "permissive", Override: map[ValidationType]ValidationAction{"expiry": "log"}}, false, []ValidationAction{ActionEnforce, ActionEnforce, ActionLog, ActionLog, ActionLog}}, + {SignatureVerification{VerificationLevel: "audit", Override: map[ValidationType]ValidationAction{"expiry": "log"}}, false, []ValidationAction{ActionEnforce, ActionLog, ActionLog, ActionLog, ActionLog}}, + {SignatureVerification{VerificationLevel: "strict", Override: map[ValidationType]ValidationAction{"revocation": "log"}}, false, []ValidationAction{ActionEnforce, ActionEnforce, ActionEnforce, ActionEnforce, ActionLog}}, + {SignatureVerification{VerificationLevel: "permissive", Override: map[ValidationType]ValidationAction{"revocation": "log"}}, false, []ValidationAction{ActionEnforce, ActionEnforce, ActionLog, ActionLog, ActionLog}}, + {SignatureVerification{VerificationLevel: "audit", Override: map[ValidationType]ValidationAction{"revocation": "log"}}, false, []ValidationAction{ActionEnforce, ActionLog, ActionLog, ActionLog, ActionLog}}, + {SignatureVerification{VerificationLevel: "strict", Override: map[ValidationType]ValidationAction{"revocation": "skip"}}, false, []ValidationAction{ActionEnforce, ActionEnforce, ActionEnforce, ActionEnforce, ActionSkip}}, + {SignatureVerification{VerificationLevel: "permissive", Override: map[ValidationType]ValidationAction{"revocation": "skip"}}, false, []ValidationAction{ActionEnforce, ActionEnforce, ActionLog, ActionLog, ActionSkip}}, + {SignatureVerification{VerificationLevel: "audit", Override: map[ValidationType]ValidationAction{"revocation": "skip"}}, false, []ValidationAction{ActionEnforce, ActionLog, ActionLog, ActionLog, ActionSkip}}, + {SignatureVerification{VerificationLevel: "permissive", Override: map[ValidationType]ValidationAction{"authenticTimestamp": "log"}}, false, []ValidationAction{ActionEnforce, ActionEnforce, ActionLog, ActionLog, ActionLog}}, + {SignatureVerification{VerificationLevel: "audit", Override: map[ValidationType]ValidationAction{"authenticTimestamp": "log"}}, false, []ValidationAction{ActionEnforce, ActionLog, ActionLog, ActionLog, ActionLog}}, + {SignatureVerification{VerificationLevel: "strict", Override: map[ValidationType]ValidationAction{"authenticTimestamp": "log"}}, false, []ValidationAction{ActionEnforce, ActionEnforce, ActionLog, ActionEnforce, ActionEnforce}}, + } + for i, tt := range tests { + t.Run(strconv.Itoa(i), func(t *testing.T) { + level, err := GetVerificationLevel(tt.customVerification) + + if tt.wantErr != (err != nil) { + t.Fatalf("TestCustomVerificationLevel Error: %q WantErr: %v", err, tt.wantErr) + } else { + if !tt.wantErr && len(tt.verificationActions) == 0 { + t.Errorf("test case isn't configured with VerificationActions") + } + for index, action := range tt.verificationActions { + if action != level.Enforcement[ValidationTypes[index]] { + t.Errorf("%q verification action should be %q for custom verification %q", ValidationTypes[index], action, tt.customVerification) + } + } + } + }) + } +} diff --git a/verification/truststore/truststore.go b/verification/truststore/truststore.go new file mode 100644 index 00000000..10cef927 --- /dev/null +++ b/verification/truststore/truststore.go @@ -0,0 +1,126 @@ +// Package truststore reads certificates in a trust store +package truststore + +import ( + "context" + "crypto/x509" + "errors" + "fmt" + "io/fs" + "os" + "path/filepath" + + corex509 "github.com/notaryproject/notation-core-go/x509" + "github.com/notaryproject/notation-go/dir" +) + +// Type is an enum for trust store types supported such as +// "ca" and "signingAuthority" +type Type string + +const ( + TypeCA Type = "ca" + TypeSigningAuthority Type = "signingAuthority" +) + +var ( + Types = []Type{ + TypeCA, + TypeSigningAuthority, + } +) + +// X509TrustStore provide list and get behaviors for the trust store +type X509TrustStore interface { + // List named stores under storeType + // List(ctx context.Context, storeType Type) ([]string, error) + + // GetCertificates returns certificates under storeType/namedStore + GetCertificates(ctx context.Context, storeType Type, namedStore string) ([]*x509.Certificate, error) +} + +// NewX509TrustStore generates a new X509TrustStore +func NewX509TrustStore(trustStorefs dir.SysFS) X509TrustStore { + return x509TrustStore{trustStorefs} +} + +// x509TrustStore implements X509TrustStore +type x509TrustStore struct { + trustStorefs dir.SysFS +} + +// GetCertificates returns certificates under storeType/namedStore +func (trustStore x509TrustStore) GetCertificates(ctx context.Context, storeType Type, namedStore string) ([]*x509.Certificate, error) { + if storeType == "" || namedStore == "" { + return nil, errors.New("storeType and namedStore cannot be empty") + } + path, err := trustStore.trustStorefs.SysPath(dir.X509TrustStoreDir(string(storeType), namedStore)) + if err != nil { + return nil, err + } + // check path is valid + if _, err := os.Stat(path); os.IsNotExist(err) { + return nil, fmt.Errorf("%q does not exist", path) + } + + // throw error if path is not a directory or is a symlink + fileInfo, err := os.Lstat(path) + if err != nil { + return nil, err + } + mode := fileInfo.Mode() + if !mode.IsDir() || mode&fs.ModeSymlink != 0 { + return nil, fmt.Errorf("%q is not a regular directory (symlinks are not supported)", path) + } + + files, err := os.ReadDir(path) + if err != nil { + return nil, err + } + + var certificates []*x509.Certificate + for _, file := range files { + joinedPath := filepath.Join(path, file.Name()) + if file.IsDir() || file.Type()&fs.ModeSymlink != 0 { + return nil, fmt.Errorf("%q is not a regular file (directories or symlinks are not supported)", joinedPath) + } + certs, err := corex509.ReadCertificateFile(joinedPath) + if err != nil { + return nil, fmt.Errorf("error while reading certificates from %q: %w", joinedPath, err) + } + + if err := validateCerts(certs, joinedPath); err != nil { + return nil, err + } + + certificates = append(certificates, certs...) + } + + if len(certificates) < 1 { + return nil, fmt.Errorf("trust store %q has no x509 certificates", path) + } + + return certificates, nil +} + +func validateCerts(certs []*x509.Certificate, path string) error { + // to prevent any trust store misconfigurations, ensure there is at least + // one certificate from each file. + if len(certs) < 1 { + return fmt.Errorf("could not parse a certificate from %q, every file in a trust store must have a PEM or DER certificate in it", path) + } + + for _, cert := range certs { + if !cert.IsCA { + if err := cert.CheckSignature(cert.SignatureAlgorithm, cert.RawTBSCertificate, cert.Signature); err != nil { + return fmt.Errorf( + "certificate with subject %q from file %q is not a CA certificate or self-signed signing certificate", + cert.Subject, + path, + ) + } + } + } + + return nil +} diff --git a/verification/truststore/truststore_test.go b/verification/truststore/truststore_test.go new file mode 100644 index 00000000..c4f2bb95 --- /dev/null +++ b/verification/truststore/truststore_test.go @@ -0,0 +1,58 @@ +package truststore + +import ( + "context" + "fmt" + "path/filepath" + "testing" + + "github.com/notaryproject/notation-go/dir" +) + +var trustStore = NewX509TrustStore(dir.NewSysFS(filepath.FromSlash("../testdata/"))) + +// TestLoadTrustStore tests a valid trust store +func TestLoadValidTrustStore(t *testing.T) { + certs, err := trustStore.GetCertificates(context.Background(), "ca", "valid-trust-store") + if err != nil { + t.Fatalf("could not get certificates from trust store. %q", err) + } + if len(certs) != 3 { + t.Fatalf("unexpected number of certificates in the trust store, expected: %d, got: %d", 3, len(certs)) + } +} + +// TestLoadValidTrustStoreWithSelfSignedSigningCertificate tests a valid trust store with self-signed signing certificate +func TestLoadValidTrustStoreWithSelfSignedSigningCertificate(t *testing.T) { + certs, err := trustStore.GetCertificates(context.Background(), "ca", "valid-trust-store-self-signed") + if err != nil { + t.Fatalf("could not get certificates from trust store. %q", err) + } + if len(certs) != 1 { + t.Fatalf("unexpected number of certificates in the trust store, expected: %d, got: %d", 1, len(certs)) + } +} + +func TestLoadTrustStoreWithInvalidCerts(t *testing.T) { + failurePath := filepath.FromSlash("../testdata/truststore/x509/ca/trust-store-with-invalid-certs/invalid") + _, err := trustStore.GetCertificates(context.Background(), "ca", "trust-store-with-invalid-certs") + if err == nil || err.Error() != fmt.Sprintf("error while reading certificates from %q: x509: malformed certificate", failurePath) { + t.Fatalf("invalid certs should return error : %q", err) + } +} + +func TestLoadTrustStoreWithLeafCerts(t *testing.T) { + failurePath := filepath.FromSlash("../testdata/truststore/x509/ca/trust-store-with-leaf-certs/non-ca.crt") + _, err := trustStore.GetCertificates(context.Background(), "ca", "trust-store-with-leaf-certs") + if err == nil || err.Error() != fmt.Sprintf("certificate with subject \"CN=wabbit-networks.io,O=Notary,L=Seattle,ST=WA,C=US\" from file %q is not a CA certificate or self-signed signing certificate", failurePath) { + t.Fatalf("leaf cert in a trust store should return error : %q", err) + } +} + +func TestLoadTrustStoreWithLeafCertsInSingleFile(t *testing.T) { + failurePath := filepath.FromSlash("../testdata/truststore/x509/ca/trust-store-with-leaf-certs-in-single-file/RootAndLeafCerts.crt") + _, err := trustStore.GetCertificates(context.Background(), "ca", "trust-store-with-leaf-certs-in-single-file") + if err == nil || err.Error() != fmt.Sprintf("certificate with subject \"CN=wabbit-networks.io,O=Notary,L=Seattle,ST=WA,C=US\" from file %q is not a CA certificate or self-signed signing certificate", failurePath) { + t.Fatalf("leaf cert in a trust store should return error : %q", err) + } +} From ed1a33da0a2f94ff766dbfbc34c17616377465ad Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Tue, 8 Nov 2022 15:06:14 +0800 Subject: [PATCH 10/19] refactored tp/ts Signed-off-by: Patrick Zheng --- verification/trustpolicy/trustpolicy.go | 94 +++++++++++++++++--- verification/trustpolicy/trustpolicy_test.go | 45 ++++++++++ 2 files changed, 127 insertions(+), 12 deletions(-) diff --git a/verification/trustpolicy/trustpolicy.go b/verification/trustpolicy/trustpolicy.go index 245e6c98..dbba67f7 100644 --- a/verification/trustpolicy/trustpolicy.go +++ b/verification/trustpolicy/trustpolicy.go @@ -9,13 +9,16 @@ import ( "github.com/notaryproject/notation-go/verification/truststore" ) -// ValidationType is an enum for signature verification types such as Integrity, Authenticity, etc. +// ValidationType is an enum for signature verification types such as Integrity, +// Authenticity, etc. type ValidationType string -// ValidationAction is an enum for signature verification actions such as Enforced, Logged, Skipped. +// ValidationAction is an enum for signature verification actions such as +// Enforced, Logged, Skipped. type ValidationAction string -// VerificationLevel encapsulates the signature verification preset and it's actions for each verification type +// VerificationLevel encapsulates the signature verification preset and its +// actions for each verification type type VerificationLevel struct { Name string Enforcement map[ValidationType]ValidationAction @@ -160,7 +163,8 @@ func (policyDoc *Document) Validate() error { return fmt.Errorf("trust policy statement %q uses invalid signatureVerification value %q", statement.Name, statement.SignatureVerification.VerificationLevel) } - // Any signature verification other than "skip" needs a trust store and trusted identities + // Any signature verification other than "skip" needs a trust store and + // trusted identities if verificationLevel.Name == "skip" { if len(statement.TrustStores) > 0 || len(statement.TrustedIdentities) > 0 { return fmt.Errorf("trust policy statement %q is set to skip signature verification but configured with trust stores and/or trusted identities, remove them if signature verification needs to be skipped", statement.Name) @@ -199,8 +203,8 @@ func (policyDoc *Document) Validate() error { return nil } -// GetVerificationLevel returns VerificationLevel struct for the given SignatureVerification struct -// throws error if SignatureVerification is invalid +// GetVerificationLevel returns VerificationLevel struct for the given +// SignatureVerification struct throws error if SignatureVerification is invalid func GetVerificationLevel(signatureVerification SignatureVerification) (*VerificationLevel, error) { var baseLevel *VerificationLevel for _, l := range VerificationLevels { @@ -226,7 +230,8 @@ func GetVerificationLevel(signatureVerification SignatureVerification) (*Verific Enforcement: make(map[ValidationType]ValidationAction), } - // populate the custom verification level with the base verification settings + // populate the custom verification level with the base verification + // settings for k, v := range baseLevel.Enforcement { customVerificationLevel.Enforcement[k] = v } @@ -266,7 +271,8 @@ func GetVerificationLevel(signatureVerification SignatureVerification) (*Verific return customVerificationLevel, nil } -// validateTrustStore validates if the policy statement is following the Notary V2 spec rules for truststores +// validateTrustStore validates if the policy statement is following the +// Notary V2 spec rules for truststores func validateTrustStore(statement TrustPolicy) error { for _, trustStore := range statement.TrustStores { i := strings.Index(trustStore, ":") @@ -278,10 +284,12 @@ func validateTrustStore(statement TrustPolicy) error { return nil } -// validateTrustedIdentities validates if the policy statement is following the Notary V2 spec rules for trusted identities +// validateTrustedIdentities 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 there is a wildcard in trusted identies, there shouldn't be any other + //identities if len(statement.TrustedIdentities) > 1 && common.IsPresent(common.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) } @@ -322,7 +330,8 @@ func validateTrustedIdentities(statement TrustPolicy) error { return nil } -// validateRegistryScopes validates if the policy document is following the Notary V2 spec rules for registry scopes +// validateRegistryScopes validates if the policy document is following the +// Notary V2 spec rules for registry scopes func validateRegistryScopes(policyDoc *Document) error { registryScopeCount := make(map[string]int) @@ -367,7 +376,8 @@ func validateOverlappingDNs(policyName string, parsedDNs []common.ParsedDN) erro return nil } -// isValidTrustStoreType returns true if the given string is a valid truststore.Type, otherwise false. +// isValidTrustStoreType returns true if the given string is a valid +// truststore.Type, otherwise false. func isValidTrustStoreType(s string) bool { for _, p := range truststore.Types { if s == string(p) { @@ -376,3 +386,63 @@ func isValidTrustStoreType(s string) bool { } return false } + +// GetApplicableTrustPolicy returns a pointer to the deep copied TrustPolicy +// statement that applies to the given registry URI. If no applicable trust +// policy is found, returns an error +// see https://github.com/notaryproject/notaryproject/blob/main/trust-store-trust-policy-specification.md#selecting-a-trust-policy-based-on-artifact-uri +func GetApplicableTrustPolicy(trustPolicyDoc *Document, artifactReference string) (*TrustPolicy, error) { + + artifactPath, err := getArtifactPathFromReference(artifactReference) + if err != nil { + return nil, err + } + + var wildcardPolicy *TrustPolicy + var applicablePolicy *TrustPolicy + for _, policyStatement := range trustPolicyDoc.TrustPolicies { + if common.IsPresent(common.Wildcard, policyStatement.RegistryScopes) { + // we need to deep copy because we can't use the loop variable + // address. see https://stackoverflow.com/a/45967429 + wildcardPolicy = deepCopy(&policyStatement) + } else if common.IsPresent(artifactPath, policyStatement.RegistryScopes) { + applicablePolicy = deepCopy(&policyStatement) + } + } + + if applicablePolicy != nil { + // a policy with exact match for registry URI takes precedence over + // a wildcard (*) policy. + return applicablePolicy, nil + } else if wildcardPolicy != nil { + return wildcardPolicy, nil + } else { + return nil, fmt.Errorf("artifact %q has no applicable trust policy", artifactReference) + } +} + +// deepCopy returns a pointer to the deeply copied TrustPolicy +func deepCopy(t *TrustPolicy) *TrustPolicy { + return &TrustPolicy{ + Name: t.Name, + SignatureVerification: t.SignatureVerification, + RegistryScopes: append([]string(nil), t.RegistryScopes...), + TrustedIdentities: append([]string(nil), t.TrustedIdentities...), + TrustStores: append([]string(nil), t.TrustStores...), + } +} + +func getArtifactPathFromReference(artifactReference string) (string, error) { + // TODO support more types of URI like "domain.com/repository", + // "domain.com/repository:tag" + i := strings.LastIndex(artifactReference, "@") + if i < 0 { + return "", fmt.Errorf("artifact URI %q could not be parsed, make sure it is the fully qualified OCI artifact URI without the scheme/protocol. e.g domain.com:80/my/repository@sha256:digest", artifactReference) + } + + artifactPath := artifactReference[:i] + if err := common.ValidateRegistryScopeFormat(artifactPath); err != nil { + return "", err + } + return artifactPath, nil +} diff --git a/verification/trustpolicy/trustpolicy_test.go b/verification/trustpolicy/trustpolicy_test.go index b8d4ee54..b08b106c 100644 --- a/verification/trustpolicy/trustpolicy_test.go +++ b/verification/trustpolicy/trustpolicy_test.go @@ -1,6 +1,7 @@ package trustpolicy import ( + "fmt" "strconv" "testing" ) @@ -444,3 +445,47 @@ func TestCustomVerificationLevel(t *testing.T) { }) } } + +// TestApplicableTrustPolicy tests filtering policies against registry scopes +func TestApplicableTrustPolicy(t *testing.T) { + policyDoc := dummyPolicyDocument() + + policyStatement := dummyPolicyStatement() + policyStatement.Name = "test-statement-name-1" + registryScope := "registry.wabbit-networks.io/software/unsigned/net-utils" + registryUri := fmt.Sprintf("%s@sha256:hash", registryScope) + policyStatement.RegistryScopes = []string{registryScope} + policyStatement.SignatureVerification = SignatureVerification{VerificationLevel: "strict"} + + policyDoc.TrustPolicies = []TrustPolicy{ + policyStatement, + } + // existing Registry Scope + policy, err := GetApplicableTrustPolicy(&policyDoc, registryUri) + if policy.Name != policyStatement.Name || err != nil { + t.Fatalf("getApplicableTrustPolicy should return %q for registry scope %q", policyStatement.Name, registryScope) + } + + // non-existing Registry Scope + policy, err = GetApplicableTrustPolicy(&policyDoc, "non.existing.scope/repo@sha256:hash") + if policy != nil || err == nil || err.Error() != "artifact \"non.existing.scope/repo@sha256:hash\" has no applicable trust policy" { + t.Fatalf("getApplicableTrustPolicy should return nil for non existing registry scope") + } + + // wildcard registry scope + wildcardStatement := dummyPolicyStatement() + wildcardStatement.Name = "test-statement-name-2" + wildcardStatement.RegistryScopes = []string{"*"} + wildcardStatement.TrustStores = []string{} + wildcardStatement.TrustedIdentities = []string{} + wildcardStatement.SignatureVerification = SignatureVerification{VerificationLevel: "skip"} + + policyDoc.TrustPolicies = []TrustPolicy{ + policyStatement, + wildcardStatement, + } + policy, err = GetApplicableTrustPolicy(&policyDoc, "some.registry.that/has.no.policy@sha256:hash") + if policy.Name != wildcardStatement.Name || err != nil { + t.Fatalf("getApplicableTrustPolicy should return wildcard policy for registry scope \"some.registry.that/has.no.policy\"") + } +} From 0a31c97e85163b66e1266e463a6ee0676befb284 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Wed, 9 Nov 2022 11:14:39 +0800 Subject: [PATCH 11/19] update Signed-off-by: Patrick Zheng --- internal/common/common.go | 10 ---------- verification/trustpolicy/trustpolicy.go | 20 +++++++++++++++----- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/internal/common/common.go b/internal/common/common.go index 92a09f04..91506b2f 100644 --- a/internal/common/common.go +++ b/internal/common/common.go @@ -13,16 +13,6 @@ const ( X509Subject = "x509.subject" ) -// 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 -} - // Internal type to hold raw and parsed Distinguished Names type ParsedDN struct { RawString string diff --git a/verification/trustpolicy/trustpolicy.go b/verification/trustpolicy/trustpolicy.go index dbba67f7..924efc7f 100644 --- a/verification/trustpolicy/trustpolicy.go +++ b/verification/trustpolicy/trustpolicy.go @@ -138,7 +138,7 @@ func (policyDoc *Document) Validate() error { supportedPolicyVersions := []string{"1.0"} // Validate Version - if !common.IsPresent(policyDoc.Version, supportedPolicyVersions) { + if !contains(policyDoc.Version, supportedPolicyVersions) { return fmt.Errorf("trust policy document uses unsupported version %q", policyDoc.Version) } @@ -290,7 +290,7 @@ 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 && common.IsPresent(common.Wildcard, statement.TrustedIdentities) { + if len(statement.TrustedIdentities) > 1 && contains(common.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) } @@ -340,7 +340,7 @@ func validateRegistryScopes(policyDoc *Document) error { 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 && common.IsPresent(common.Wildcard, statement.RegistryScopes) { + if len(statement.RegistryScopes) > 1 && contains(common.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 { @@ -401,11 +401,11 @@ func GetApplicableTrustPolicy(trustPolicyDoc *Document, artifactReference string var wildcardPolicy *TrustPolicy var applicablePolicy *TrustPolicy for _, policyStatement := range trustPolicyDoc.TrustPolicies { - if common.IsPresent(common.Wildcard, policyStatement.RegistryScopes) { + if contains(common.Wildcard, policyStatement.RegistryScopes) { // we need to deep copy because we can't use the loop variable // address. see https://stackoverflow.com/a/45967429 wildcardPolicy = deepCopy(&policyStatement) - } else if common.IsPresent(artifactPath, policyStatement.RegistryScopes) { + } else if contains(artifactPath, policyStatement.RegistryScopes) { applicablePolicy = deepCopy(&policyStatement) } } @@ -446,3 +446,13 @@ func getArtifactPathFromReference(artifactReference string) (string, error) { } return artifactPath, nil } + +// contains is a utility function to check if a string exists in an array +func contains(val string, values []string) bool { + for _, v := range values { + if v == val { + return true + } + } + return false +} From 25a49dac0fb337560e74ecff1858dfd46418bc24 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Wed, 9 Nov 2022 11:21:06 +0800 Subject: [PATCH 12/19] update Signed-off-by: Patrick Zheng --- internal/common/common.go | 30 -------------- internal/slice/slice.go | 11 +++++ verification/trustpolicy/trustpolicy.go | 54 +++++++++++++++++-------- 3 files changed, 48 insertions(+), 47 deletions(-) create mode 100644 internal/slice/slice.go diff --git a/internal/common/common.go b/internal/common/common.go index 91506b2f..ccf3ae7e 100644 --- a/internal/common/common.go +++ b/internal/common/common.go @@ -2,8 +2,6 @@ package common import ( "fmt" - "regexp" - "strings" ldapv3 "github.com/go-ldap/ldap/v3" ) @@ -13,12 +11,6 @@ const ( X509Subject = "x509.subject" ) -// Internal type to hold raw and parsed Distinguished Names -type ParsedDN struct { - RawString string - ParsedMap map[string]string -} - // ParseDistinguishedName parses a DN name and validates Notary V2 rules func ParseDistinguishedName(name string) (map[string]string, error) { mandatoryFields := []string{"C", "ST", "O"} @@ -63,25 +55,3 @@ func IsSubsetDN(dn1 map[string]string, dn2 map[string]string) bool { } return true } - -// 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 -} diff --git a/internal/slice/slice.go b/internal/slice/slice.go new file mode 100644 index 00000000..f95e0527 --- /dev/null +++ b/internal/slice/slice.go @@ -0,0 +1,11 @@ +package slice + +// Contains is a utility function to check if a string exists in an array +func Contains(val string, values []string) bool { + for _, v := range values { + if v == val { + return true + } + } + return false +} diff --git a/verification/trustpolicy/trustpolicy.go b/verification/trustpolicy/trustpolicy.go index 924efc7f..fb8d4348 100644 --- a/verification/trustpolicy/trustpolicy.go +++ b/verification/trustpolicy/trustpolicy.go @@ -3,9 +3,11 @@ package trustpolicy import ( "errors" "fmt" + "regexp" "strings" "github.com/notaryproject/notation-go/internal/common" + "github.com/notaryproject/notation-go/internal/slice" "github.com/notaryproject/notation-go/verification/truststore" ) @@ -138,7 +140,7 @@ func (policyDoc *Document) Validate() error { supportedPolicyVersions := []string{"1.0"} // Validate Version - if !contains(policyDoc.Version, supportedPolicyVersions) { + if !slice.Contains(policyDoc.Version, supportedPolicyVersions) { return fmt.Errorf("trust policy document uses unsupported version %q", policyDoc.Version) } @@ -290,11 +292,11 @@ 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 && contains(common.Wildcard, statement.TrustedIdentities) { + if len(statement.TrustedIdentities) > 1 && slice.Contains(common.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) } - var parsedDNs []common.ParsedDN + var parsedDNs []parsedDN // If there are trusted identities, verify they are valid for _, identity := range statement.TrustedIdentities { if identity == "" { @@ -316,7 +318,7 @@ func validateTrustedIdentities(statement TrustPolicy) error { if err != nil { return err } - parsedDNs = append(parsedDNs, common.ParsedDN{RawString: identity, ParsedMap: dn}) + parsedDNs = append(parsedDNs, parsedDN{RawString: identity, ParsedMap: dn}) } } } @@ -340,12 +342,12 @@ func validateRegistryScopes(policyDoc *Document) error { 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 && contains(common.Wildcard, statement.RegistryScopes) { + if len(statement.RegistryScopes) > 1 && slice.Contains(common.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 != common.Wildcard { - if err := common.ValidateRegistryScopeFormat(scope); err != nil { + if err := validateRegistryScopeFormat(scope); err != nil { return err } } @@ -364,7 +366,7 @@ func validateRegistryScopes(policyDoc *Document) error { return nil } -func validateOverlappingDNs(policyName string, parsedDNs []common.ParsedDN) error { +func validateOverlappingDNs(policyName string, parsedDNs []parsedDN) error { for i, dn1 := range parsedDNs { for j, dn2 := range parsedDNs { if i != j && common.IsSubsetDN(dn1.ParsedMap, dn2.ParsedMap) { @@ -401,11 +403,11 @@ func GetApplicableTrustPolicy(trustPolicyDoc *Document, artifactReference string var wildcardPolicy *TrustPolicy var applicablePolicy *TrustPolicy for _, policyStatement := range trustPolicyDoc.TrustPolicies { - if contains(common.Wildcard, policyStatement.RegistryScopes) { + if slice.Contains(common.Wildcard, policyStatement.RegistryScopes) { // we need to deep copy because we can't use the loop variable // address. see https://stackoverflow.com/a/45967429 wildcardPolicy = deepCopy(&policyStatement) - } else if contains(artifactPath, policyStatement.RegistryScopes) { + } else if slice.Contains(artifactPath, policyStatement.RegistryScopes) { applicablePolicy = deepCopy(&policyStatement) } } @@ -441,18 +443,36 @@ func getArtifactPathFromReference(artifactReference string) (string, error) { } artifactPath := artifactReference[:i] - if err := common.ValidateRegistryScopeFormat(artifactPath); err != nil { + if err := validateRegistryScopeFormat(artifactPath); err != nil { return "", err } return artifactPath, nil } -// contains is a utility function to check if a string exists in an array -func contains(val string, values []string) bool { - for _, v := range values { - if v == val { - return true - } +// Internal type to hold raw and parsed Distinguished Names +type parsedDN struct { + RawString string + ParsedMap map[string]string +} + +// 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) } - return false + 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 } From 001d3145bc2474f0c28b1f14bb551becb9a2e584 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Wed, 9 Nov 2022 12:27:25 +0800 Subject: [PATCH 13/19] updated per code review Signed-off-by: Patrick Zheng --- internal/{common/common.go => pkix/pkix.go} | 7 +--- internal/slice/slice.go | 5 +++ verification/trustpolicy/trustpolicy.go | 40 ++++++++++++--------- verification/truststore/truststore.go | 9 +++-- 4 files changed, 34 insertions(+), 27 deletions(-) rename internal/{common/common.go => pkix/pkix.go} (95%) diff --git a/internal/common/common.go b/internal/pkix/pkix.go similarity index 95% rename from internal/common/common.go rename to internal/pkix/pkix.go index ccf3ae7e..c453b8d5 100644 --- a/internal/common/common.go +++ b/internal/pkix/pkix.go @@ -1,4 +1,4 @@ -package common +package pkix import ( "fmt" @@ -6,11 +6,6 @@ import ( ldapv3 "github.com/go-ldap/ldap/v3" ) -const ( - Wildcard = "*" - X509Subject = "x509.subject" -) - // ParseDistinguishedName parses a DN name and validates Notary V2 rules func ParseDistinguishedName(name string) (map[string]string, error) { mandatoryFields := []string{"C", "ST", "O"} diff --git a/internal/slice/slice.go b/internal/slice/slice.go index f95e0527..99491ffa 100644 --- a/internal/slice/slice.go +++ b/internal/slice/slice.go @@ -1,5 +1,10 @@ package slice +const ( + Wildcard = "*" + X509Subject = "x509.subject" +) + // Contains is a utility function to check if a string exists in an array func Contains(val string, values []string) bool { for _, v := range values { diff --git a/verification/trustpolicy/trustpolicy.go b/verification/trustpolicy/trustpolicy.go index fb8d4348..0cad0156 100644 --- a/verification/trustpolicy/trustpolicy.go +++ b/verification/trustpolicy/trustpolicy.go @@ -6,7 +6,7 @@ import ( "regexp" "strings" - "github.com/notaryproject/notation-go/internal/common" + "github.com/notaryproject/notation-go/internal/pkix" "github.com/notaryproject/notation-go/internal/slice" "github.com/notaryproject/notation-go/verification/truststore" ) @@ -32,7 +32,9 @@ const ( TypeAuthenticTimestamp ValidationType = "authenticTimestamp" TypeExpiry ValidationType = "expiry" TypeRevocation ValidationType = "revocation" +) +const ( ActionEnforce ValidationAction = "enforce" ActionLog ValidationAction = "log" ActionSkip ValidationAction = "skip" @@ -82,7 +84,9 @@ var ( TypeRevocation: ActionSkip, }, } +) +var ( ValidationTypes = []ValidationType{ TypeIntegrity, TypeAuthenticity, @@ -105,10 +109,13 @@ var ( } ) +var supportedPolicyVersions = []string{"1.0"} + // Document represents a trustPolicy.json document type Document struct { // Version of the policy document Version string `json:"version"` + // TrustPolicies include each policy statement TrustPolicies []TrustPolicy `json:"trustPolicies"` } @@ -117,12 +124,16 @@ type Document struct { type TrustPolicy struct { // Name of the policy statement Name string `json:"name"` + // RegistryScopes that this policy statement affects RegistryScopes []string `json:"registryScopes"` + // SignatureVerification setting for this policy statement SignatureVerification SignatureVerification `json:"signatureVerification"` + // TrustStores this policy statement uses TrustStores []string `json:"trustStores,omitempty"` + // TrustedIdentities this policy statement pins TrustedIdentities []string `json:"trustedIdentities,omitempty"` } @@ -136,9 +147,6 @@ type SignatureVerification struct { // Validate validates a policy document according to it's version's rule set. // if any rule is violated, returns an error func (policyDoc *Document) Validate() error { - // Constants - supportedPolicyVersions := []string{"1.0"} - // Validate Version if !slice.Contains(policyDoc.Version, supportedPolicyVersions) { return fmt.Errorf("trust policy document uses unsupported version %q", policyDoc.Version) @@ -292,7 +300,7 @@ 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 && slice.Contains(common.Wildcard, statement.TrustedIdentities) { + if len(statement.TrustedIdentities) > 1 && slice.Contains(slice.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) } @@ -303,7 +311,7 @@ func validateTrustedIdentities(statement TrustPolicy) error { return fmt.Errorf("trust policy statement %q has an empty trusted identity", statement.Name) } - if identity != common.Wildcard { + if identity != slice.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) @@ -313,8 +321,8 @@ func validateTrustedIdentities(statement TrustPolicy) error { identityValue := identity[i+1:] // notation natively supports x509.subject identities only - if identityPrefix == common.X509Subject { - dn, err := common.ParseDistinguishedName(identityValue) + if identityPrefix == slice.X509Subject { + dn, err := pkix.ParseDistinguishedName(identityValue) if err != nil { return err } @@ -342,11 +350,11 @@ func validateRegistryScopes(policyDoc *Document) error { 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 && slice.Contains(common.Wildcard, statement.RegistryScopes) { + if len(statement.RegistryScopes) > 1 && slice.Contains(slice.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 != common.Wildcard { + if scope != slice.Wildcard { if err := validateRegistryScopeFormat(scope); err != nil { return err } @@ -369,7 +377,7 @@ func validateRegistryScopes(policyDoc *Document) error { func validateOverlappingDNs(policyName string, parsedDNs []parsedDN) error { for i, dn1 := range parsedDNs { for j, dn2 := range parsedDNs { - if i != j && common.IsSubsetDN(dn1.ParsedMap, dn2.ParsedMap) { + if i != j && pkix.IsSubsetDN(dn1.ParsedMap, dn2.ParsedMap) { return fmt.Errorf("trust policy statement %q has overlapping x509 trustedIdentities, %q overlaps with %q", policyName, dn1.RawString, dn2.RawString) } } @@ -403,12 +411,12 @@ func GetApplicableTrustPolicy(trustPolicyDoc *Document, artifactReference string var wildcardPolicy *TrustPolicy var applicablePolicy *TrustPolicy for _, policyStatement := range trustPolicyDoc.TrustPolicies { - if slice.Contains(common.Wildcard, policyStatement.RegistryScopes) { + if slice.Contains(slice.Wildcard, policyStatement.RegistryScopes) { // we need to deep copy because we can't use the loop variable // address. see https://stackoverflow.com/a/45967429 - wildcardPolicy = deepCopy(&policyStatement) + wildcardPolicy = (&policyStatement).clone() } else if slice.Contains(artifactPath, policyStatement.RegistryScopes) { - applicablePolicy = deepCopy(&policyStatement) + applicablePolicy = (&policyStatement).clone() } } @@ -423,8 +431,8 @@ func GetApplicableTrustPolicy(trustPolicyDoc *Document, artifactReference string } } -// deepCopy returns a pointer to the deeply copied TrustPolicy -func deepCopy(t *TrustPolicy) *TrustPolicy { +// clone returns a pointer to the deeply copied TrustPolicy +func (t *TrustPolicy) clone() *TrustPolicy { return &TrustPolicy{ Name: t.Name, SignatureVerification: t.SignatureVerification, diff --git a/verification/truststore/truststore.go b/verification/truststore/truststore.go index 10cef927..5c8e6260 100644 --- a/verification/truststore/truststore.go +++ b/verification/truststore/truststore.go @@ -32,9 +32,6 @@ var ( // X509TrustStore provide list and get behaviors for the trust store type X509TrustStore interface { - // List named stores under storeType - // List(ctx context.Context, storeType Type) ([]string, error) - // GetCertificates returns certificates under storeType/namedStore GetCertificates(ctx context.Context, storeType Type, namedStore string) ([]*x509.Certificate, error) } @@ -59,8 +56,10 @@ func (trustStore x509TrustStore) GetCertificates(ctx context.Context, storeType return nil, err } // check path is valid - if _, err := os.Stat(path); os.IsNotExist(err) { - return nil, fmt.Errorf("%q does not exist", path) + if _, err := os.Stat(path); err != nil { + if os.IsNotExist(err) { + return nil, fmt.Errorf("%q does not exist", path) + } } // throw error if path is not a directory or is a symlink From 3a6dc6210c15bcbb8d38a17c64932304b99fdb34 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Wed, 9 Nov 2022 15:41:01 +0800 Subject: [PATCH 14/19] update Signed-off-by: Patrick Zheng --- verification/truststore/truststore.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/verification/truststore/truststore.go b/verification/truststore/truststore.go index 5c8e6260..b6227bf2 100644 --- a/verification/truststore/truststore.go +++ b/verification/truststore/truststore.go @@ -36,16 +36,16 @@ type X509TrustStore interface { GetCertificates(ctx context.Context, storeType Type, namedStore string) ([]*x509.Certificate, error) } -// NewX509TrustStore generates a new X509TrustStore -func NewX509TrustStore(trustStorefs dir.SysFS) X509TrustStore { - return x509TrustStore{trustStorefs} -} - // x509TrustStore implements X509TrustStore type x509TrustStore struct { trustStorefs dir.SysFS } +// NewX509TrustStore generates a new X509TrustStore +func NewX509TrustStore(trustStorefs dir.SysFS) X509TrustStore { + return x509TrustStore{trustStorefs} +} + // GetCertificates returns certificates under storeType/namedStore func (trustStore x509TrustStore) GetCertificates(ctx context.Context, storeType Type, namedStore string) ([]*x509.Certificate, error) { if storeType == "" || namedStore == "" { From e23a91ccadf02c29de90d11afb8d819ad6f9e125 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Wed, 9 Nov 2022 15:51:47 +0800 Subject: [PATCH 15/19] updated per code review Signed-off-by: Patrick Zheng --- internal/slice/slice.go | 4 ++-- verification/trustpolicy/trustpolicy.go | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/internal/slice/slice.go b/internal/slice/slice.go index 99491ffa..d7cb4959 100644 --- a/internal/slice/slice.go +++ b/internal/slice/slice.go @@ -5,8 +5,8 @@ const ( X509Subject = "x509.subject" ) -// Contains is a utility function to check if a string exists in an array -func Contains(val string, values []string) bool { +// ContainsString is a utility function to check if a string exists in an array +func ContainsString(val string, values []string) bool { for _, v := range values { if v == val { return true diff --git a/verification/trustpolicy/trustpolicy.go b/verification/trustpolicy/trustpolicy.go index 0cad0156..4fd4020b 100644 --- a/verification/trustpolicy/trustpolicy.go +++ b/verification/trustpolicy/trustpolicy.go @@ -148,7 +148,7 @@ type SignatureVerification struct { // if any rule is violated, returns an error func (policyDoc *Document) Validate() error { // Validate Version - if !slice.Contains(policyDoc.Version, supportedPolicyVersions) { + if !slice.ContainsString(policyDoc.Version, supportedPolicyVersions) { return fmt.Errorf("trust policy document uses unsupported version %q", policyDoc.Version) } @@ -300,7 +300,7 @@ 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 && slice.Contains(slice.Wildcard, statement.TrustedIdentities) { + if len(statement.TrustedIdentities) > 1 && slice.ContainsString(slice.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) } @@ -332,7 +332,7 @@ func validateTrustedIdentities(statement TrustPolicy) error { } // Verify there are no overlapping DNs - if err := validateOverlappingDNs(statement.Name, parsedDNs); err != nil { + if err := hasOverlappingDNs(statement.Name, parsedDNs); err != nil { return err } @@ -350,7 +350,7 @@ func validateRegistryScopes(policyDoc *Document) error { 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 && slice.Contains(slice.Wildcard, statement.RegistryScopes) { + if len(statement.RegistryScopes) > 1 && slice.ContainsString(slice.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 { @@ -374,7 +374,7 @@ func validateRegistryScopes(policyDoc *Document) error { return nil } -func validateOverlappingDNs(policyName string, parsedDNs []parsedDN) error { +func hasOverlappingDNs(policyName string, parsedDNs []parsedDN) error { for i, dn1 := range parsedDNs { for j, dn2 := range parsedDNs { if i != j && pkix.IsSubsetDN(dn1.ParsedMap, dn2.ParsedMap) { @@ -411,11 +411,11 @@ func GetApplicableTrustPolicy(trustPolicyDoc *Document, artifactReference string var wildcardPolicy *TrustPolicy var applicablePolicy *TrustPolicy for _, policyStatement := range trustPolicyDoc.TrustPolicies { - if slice.Contains(slice.Wildcard, policyStatement.RegistryScopes) { + if slice.ContainsString(slice.Wildcard, policyStatement.RegistryScopes) { // we need to deep copy because we can't use the loop variable // address. see https://stackoverflow.com/a/45967429 wildcardPolicy = (&policyStatement).clone() - } else if slice.Contains(artifactPath, policyStatement.RegistryScopes) { + } else if slice.ContainsString(artifactPath, policyStatement.RegistryScopes) { applicablePolicy = (&policyStatement).clone() } } From dc17f6f564b1e015b0cb9bc5e884a5df59ce5794 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Wed, 9 Nov 2022 16:34:16 +0800 Subject: [PATCH 16/19] updated per code review Signed-off-by: Patrick Zheng --- internal/slice/slice.go | 5 - internal/trustpolicy/trustpolicy.go | 6 + verification/trustpolicy/trustpolicy.go | 109 ++++++++++--------- verification/trustpolicy/trustpolicy_test.go | 10 +- verification/truststore/truststore.go | 35 ++++-- 5 files changed, 93 insertions(+), 72 deletions(-) create mode 100644 internal/trustpolicy/trustpolicy.go diff --git a/internal/slice/slice.go b/internal/slice/slice.go index d7cb4959..b73c8794 100644 --- a/internal/slice/slice.go +++ b/internal/slice/slice.go @@ -1,10 +1,5 @@ package slice -const ( - Wildcard = "*" - X509Subject = "x509.subject" -) - // ContainsString is a utility function to check if a string exists in an array func ContainsString(val string, values []string) bool { for _, v := range values { diff --git a/internal/trustpolicy/trustpolicy.go b/internal/trustpolicy/trustpolicy.go new file mode 100644 index 00000000..82745017 --- /dev/null +++ b/internal/trustpolicy/trustpolicy.go @@ -0,0 +1,6 @@ +package trustpolicy + +const ( + Wildcard = "*" + X509Subject = "x509.subject" +) diff --git a/verification/trustpolicy/trustpolicy.go b/verification/trustpolicy/trustpolicy.go index 4fd4020b..c3f6e956 100644 --- a/verification/trustpolicy/trustpolicy.go +++ b/verification/trustpolicy/trustpolicy.go @@ -8,6 +8,7 @@ import ( "github.com/notaryproject/notation-go/internal/pkix" "github.com/notaryproject/notation-go/internal/slice" + "github.com/notaryproject/notation-go/internal/trustpolicy" "github.com/notaryproject/notation-go/verification/truststore" ) @@ -168,7 +169,7 @@ func (policyDoc *Document) Validate() error { policyStatementNameCount[statement.Name]++ // Verify signature verification level is valid - verificationLevel, err := GetVerificationLevel(statement.SignatureVerification) + verificationLevel, err := statement.SignatureVerification.GetVerificationLevel() if err != nil { return fmt.Errorf("trust policy statement %q uses invalid signatureVerification value %q", statement.Name, statement.SignatureVerification.VerificationLevel) } @@ -213,9 +214,43 @@ func (policyDoc *Document) Validate() error { return nil } +// GetApplicableTrustPolicy returns a pointer to the deep copied TrustPolicy +// statement that applies to the given registry URI. If no applicable trust +// policy is found, returns an error +// see https://github.com/notaryproject/notaryproject/blob/main/trust-store-trust-policy-specification.md#selecting-a-trust-policy-based-on-artifact-uri +func (trustPolicyDoc *Document) GetApplicableTrustPolicy(artifactReference string) (*TrustPolicy, error) { + + artifactPath, err := getArtifactPathFromReference(artifactReference) + if err != nil { + return nil, err + } + + var wildcardPolicy *TrustPolicy + var applicablePolicy *TrustPolicy + for _, policyStatement := range trustPolicyDoc.TrustPolicies { + if slice.ContainsString(trustpolicy.Wildcard, policyStatement.RegistryScopes) { + // we need to deep copy because we can't use the loop variable + // address. see https://stackoverflow.com/a/45967429 + wildcardPolicy = (&policyStatement).clone() + } else if slice.ContainsString(artifactPath, policyStatement.RegistryScopes) { + applicablePolicy = (&policyStatement).clone() + } + } + + if applicablePolicy != nil { + // a policy with exact match for registry URI takes precedence over + // a wildcard (*) policy. + return applicablePolicy, nil + } else if wildcardPolicy != nil { + return wildcardPolicy, nil + } else { + return nil, fmt.Errorf("artifact %q has no applicable trust policy", artifactReference) + } +} + // GetVerificationLevel returns VerificationLevel struct for the given // SignatureVerification struct throws error if SignatureVerification is invalid -func GetVerificationLevel(signatureVerification SignatureVerification) (*VerificationLevel, error) { +func (signatureVerification *SignatureVerification) GetVerificationLevel() (*VerificationLevel, error) { var baseLevel *VerificationLevel for _, l := range VerificationLevels { if l.Name == signatureVerification.VerificationLevel { @@ -281,6 +316,17 @@ func GetVerificationLevel(signatureVerification SignatureVerification) (*Verific return customVerificationLevel, nil } +// clone returns a pointer to the deeply copied TrustPolicy +func (t *TrustPolicy) clone() *TrustPolicy { + return &TrustPolicy{ + Name: t.Name, + SignatureVerification: t.SignatureVerification, + RegistryScopes: append([]string(nil), t.RegistryScopes...), + TrustedIdentities: append([]string(nil), t.TrustedIdentities...), + TrustStores: append([]string(nil), t.TrustStores...), + } +} + // validateTrustStore validates if the policy statement is following the // Notary V2 spec rules for truststores func validateTrustStore(statement TrustPolicy) error { @@ -300,7 +346,7 @@ 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 && slice.ContainsString(slice.Wildcard, statement.TrustedIdentities) { + if len(statement.TrustedIdentities) > 1 && slice.ContainsString(trustpolicy.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) } @@ -311,7 +357,7 @@ func validateTrustedIdentities(statement TrustPolicy) error { return fmt.Errorf("trust policy statement %q has an empty trusted identity", statement.Name) } - if identity != slice.Wildcard { + if identity != trustpolicy.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) @@ -321,7 +367,7 @@ func validateTrustedIdentities(statement TrustPolicy) error { identityValue := identity[i+1:] // notation natively supports x509.subject identities only - if identityPrefix == slice.X509Subject { + if identityPrefix == trustpolicy.X509Subject { dn, err := pkix.ParseDistinguishedName(identityValue) if err != nil { return err @@ -332,7 +378,7 @@ func validateTrustedIdentities(statement TrustPolicy) error { } // Verify there are no overlapping DNs - if err := hasOverlappingDNs(statement.Name, parsedDNs); err != nil { + if err := validateOverlappingDNs(statement.Name, parsedDNs); err != nil { return err } @@ -350,11 +396,11 @@ func validateRegistryScopes(policyDoc *Document) error { 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 && slice.ContainsString(slice.Wildcard, statement.RegistryScopes) { + if len(statement.RegistryScopes) > 1 && slice.ContainsString(trustpolicy.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 != slice.Wildcard { + if scope != trustpolicy.Wildcard { if err := validateRegistryScopeFormat(scope); err != nil { return err } @@ -374,7 +420,7 @@ func validateRegistryScopes(policyDoc *Document) error { return nil } -func hasOverlappingDNs(policyName string, parsedDNs []parsedDN) error { +func validateOverlappingDNs(policyName string, parsedDNs []parsedDN) error { for i, dn1 := range parsedDNs { for j, dn2 := range parsedDNs { if i != j && pkix.IsSubsetDN(dn1.ParsedMap, dn2.ParsedMap) { @@ -397,51 +443,6 @@ func isValidTrustStoreType(s string) bool { return false } -// GetApplicableTrustPolicy returns a pointer to the deep copied TrustPolicy -// statement that applies to the given registry URI. If no applicable trust -// policy is found, returns an error -// see https://github.com/notaryproject/notaryproject/blob/main/trust-store-trust-policy-specification.md#selecting-a-trust-policy-based-on-artifact-uri -func GetApplicableTrustPolicy(trustPolicyDoc *Document, artifactReference string) (*TrustPolicy, error) { - - artifactPath, err := getArtifactPathFromReference(artifactReference) - if err != nil { - return nil, err - } - - var wildcardPolicy *TrustPolicy - var applicablePolicy *TrustPolicy - for _, policyStatement := range trustPolicyDoc.TrustPolicies { - if slice.ContainsString(slice.Wildcard, policyStatement.RegistryScopes) { - // we need to deep copy because we can't use the loop variable - // address. see https://stackoverflow.com/a/45967429 - wildcardPolicy = (&policyStatement).clone() - } else if slice.ContainsString(artifactPath, policyStatement.RegistryScopes) { - applicablePolicy = (&policyStatement).clone() - } - } - - if applicablePolicy != nil { - // a policy with exact match for registry URI takes precedence over - // a wildcard (*) policy. - return applicablePolicy, nil - } else if wildcardPolicy != nil { - return wildcardPolicy, nil - } else { - return nil, fmt.Errorf("artifact %q has no applicable trust policy", artifactReference) - } -} - -// clone returns a pointer to the deeply copied TrustPolicy -func (t *TrustPolicy) clone() *TrustPolicy { - return &TrustPolicy{ - Name: t.Name, - SignatureVerification: t.SignatureVerification, - RegistryScopes: append([]string(nil), t.RegistryScopes...), - TrustedIdentities: append([]string(nil), t.TrustedIdentities...), - TrustStores: append([]string(nil), t.TrustStores...), - } -} - func getArtifactPathFromReference(artifactReference string) (string, error) { // TODO support more types of URI like "domain.com/repository", // "domain.com/repository:tag" diff --git a/verification/trustpolicy/trustpolicy_test.go b/verification/trustpolicy/trustpolicy_test.go index b08b106c..00ac83c5 100644 --- a/verification/trustpolicy/trustpolicy_test.go +++ b/verification/trustpolicy/trustpolicy_test.go @@ -381,7 +381,7 @@ func TestGetVerificationLevel(t *testing.T) { for i, tt := range tests { t.Run(strconv.Itoa(i), func(t *testing.T) { - level, err := GetVerificationLevel(tt.verificationLevel) + level, err := tt.verificationLevel.GetVerificationLevel() if tt.wantErr != (err != nil) { t.Fatalf("TestFindVerificationLevel Error: %q WantErr: %v", err, tt.wantErr) @@ -428,7 +428,7 @@ func TestCustomVerificationLevel(t *testing.T) { } for i, tt := range tests { t.Run(strconv.Itoa(i), func(t *testing.T) { - level, err := GetVerificationLevel(tt.customVerification) + level, err := tt.customVerification.GetVerificationLevel() if tt.wantErr != (err != nil) { t.Fatalf("TestCustomVerificationLevel Error: %q WantErr: %v", err, tt.wantErr) @@ -461,13 +461,13 @@ func TestApplicableTrustPolicy(t *testing.T) { policyStatement, } // existing Registry Scope - policy, err := GetApplicableTrustPolicy(&policyDoc, registryUri) + policy, err := (&policyDoc).GetApplicableTrustPolicy(registryUri) if policy.Name != policyStatement.Name || err != nil { t.Fatalf("getApplicableTrustPolicy should return %q for registry scope %q", policyStatement.Name, registryScope) } // non-existing Registry Scope - policy, err = GetApplicableTrustPolicy(&policyDoc, "non.existing.scope/repo@sha256:hash") + policy, err = (&policyDoc).GetApplicableTrustPolicy("non.existing.scope/repo@sha256:hash") if policy != nil || err == nil || err.Error() != "artifact \"non.existing.scope/repo@sha256:hash\" has no applicable trust policy" { t.Fatalf("getApplicableTrustPolicy should return nil for non existing registry scope") } @@ -484,7 +484,7 @@ func TestApplicableTrustPolicy(t *testing.T) { policyStatement, wildcardStatement, } - policy, err = GetApplicableTrustPolicy(&policyDoc, "some.registry.that/has.no.policy@sha256:hash") + policy, err = (&policyDoc).GetApplicableTrustPolicy("some.registry.that/has.no.policy@sha256:hash") if policy.Name != wildcardStatement.Name || err != nil { t.Fatalf("getApplicableTrustPolicy should return wildcard policy for registry scope \"some.registry.that/has.no.policy\"") } diff --git a/verification/truststore/truststore.go b/verification/truststore/truststore.go index b6227bf2..0d736722 100644 --- a/verification/truststore/truststore.go +++ b/verification/truststore/truststore.go @@ -9,6 +9,7 @@ import ( "io/fs" "os" "path/filepath" + "regexp" corex509 "github.com/notaryproject/notation-core-go/x509" "github.com/notaryproject/notation-go/dir" @@ -36,20 +37,23 @@ type X509TrustStore interface { GetCertificates(ctx context.Context, storeType Type, namedStore string) ([]*x509.Certificate, error) } +// NewX509TrustStore generates a new X509TrustStore +func NewX509TrustStore(trustStorefs dir.SysFS) X509TrustStore { + return &x509TrustStore{trustStorefs} +} + // x509TrustStore implements X509TrustStore type x509TrustStore struct { trustStorefs dir.SysFS } -// NewX509TrustStore generates a new X509TrustStore -func NewX509TrustStore(trustStorefs dir.SysFS) X509TrustStore { - return x509TrustStore{trustStorefs} -} - // GetCertificates returns certificates under storeType/namedStore -func (trustStore x509TrustStore) GetCertificates(ctx context.Context, storeType Type, namedStore string) ([]*x509.Certificate, error) { - if storeType == "" || namedStore == "" { - return nil, errors.New("storeType and namedStore cannot be empty") +func (trustStore *x509TrustStore) GetCertificates(ctx context.Context, storeType Type, namedStore string) ([]*x509.Certificate, error) { + if !isValidStoreType(storeType) { + return nil, fmt.Errorf("unsupported store type: %s", storeType) + } + if !isValidNamedStore(namedStore) { + return nil, errors.New("named store name needs to follow [a-zA-Z0-9_.-]+ format") } path, err := trustStore.trustStorefs.SysPath(dir.X509TrustStoreDir(string(storeType), namedStore)) if err != nil { @@ -123,3 +127,18 @@ func validateCerts(certs []*x509.Certificate, path string) error { return nil } + +// isValidStoreType checks if storeType is supported +func isValidStoreType(storeType Type) bool { + for _, t := range Types { + if storeType == t { + return true + } + } + return false +} + +// isValidFileName checks if a file name is cross-platform compatible +func isValidNamedStore(namedStore string) bool { + return regexp.MustCompile(`^[a-zA-Z0-9_.-]+$`).MatchString(namedStore) +} From 2c1f27d2908dba8f910851ce6ea231ea3b2d1b6a Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Wed, 9 Nov 2022 16:57:06 +0800 Subject: [PATCH 17/19] updated per code review Signed-off-by: Patrick Zheng --- internal/slice/slice.go | 8 ++++---- verification/trustpolicy/trustpolicy.go | 10 +++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/internal/slice/slice.go b/internal/slice/slice.go index b73c8794..a57453d1 100644 --- a/internal/slice/slice.go +++ b/internal/slice/slice.go @@ -1,9 +1,9 @@ package slice -// ContainsString is a utility function to check if a string exists in an array -func ContainsString(val string, values []string) bool { - for _, v := range values { - if v == val { +// Contains reports whether v is present in s. +func Contains[E comparable](s []E, v E) bool { + for _, vs := range s { + if v == vs { return true } } diff --git a/verification/trustpolicy/trustpolicy.go b/verification/trustpolicy/trustpolicy.go index c3f6e956..5daad9f3 100644 --- a/verification/trustpolicy/trustpolicy.go +++ b/verification/trustpolicy/trustpolicy.go @@ -149,7 +149,7 @@ type SignatureVerification struct { // if any rule is violated, returns an error func (policyDoc *Document) Validate() error { // Validate Version - if !slice.ContainsString(policyDoc.Version, supportedPolicyVersions) { + if !slice.Contains(supportedPolicyVersions, policyDoc.Version) { return fmt.Errorf("trust policy document uses unsupported version %q", policyDoc.Version) } @@ -228,11 +228,11 @@ func (trustPolicyDoc *Document) GetApplicableTrustPolicy(artifactReference strin var wildcardPolicy *TrustPolicy var applicablePolicy *TrustPolicy for _, policyStatement := range trustPolicyDoc.TrustPolicies { - if slice.ContainsString(trustpolicy.Wildcard, policyStatement.RegistryScopes) { + if slice.Contains(policyStatement.RegistryScopes, trustpolicy.Wildcard) { // we need to deep copy because we can't use the loop variable // address. see https://stackoverflow.com/a/45967429 wildcardPolicy = (&policyStatement).clone() - } else if slice.ContainsString(artifactPath, policyStatement.RegistryScopes) { + } else if slice.Contains(policyStatement.RegistryScopes, artifactPath) { applicablePolicy = (&policyStatement).clone() } } @@ -346,7 +346,7 @@ 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 && slice.ContainsString(trustpolicy.Wildcard, statement.TrustedIdentities) { + if len(statement.TrustedIdentities) > 1 && slice.Contains(statement.TrustedIdentities, trustpolicy.Wildcard) { 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) } @@ -396,7 +396,7 @@ func validateRegistryScopes(policyDoc *Document) error { 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 && slice.ContainsString(trustpolicy.Wildcard, statement.RegistryScopes) { + if len(statement.RegistryScopes) > 1 && slice.Contains(statement.RegistryScopes, trustpolicy.Wildcard) { 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 { From 7e79d104717f65f3ae80e18a3a20ba6680bc66df Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Wed, 9 Nov 2022 17:05:09 +0800 Subject: [PATCH 18/19] updated per code review Signed-off-by: Patrick Zheng --- verification/truststore/truststore.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/verification/truststore/truststore.go b/verification/truststore/truststore.go index 0d736722..8f9ad002 100644 --- a/verification/truststore/truststore.go +++ b/verification/truststore/truststore.go @@ -13,6 +13,7 @@ import ( corex509 "github.com/notaryproject/notation-core-go/x509" "github.com/notaryproject/notation-go/dir" + "github.com/notaryproject/notation-go/internal/slice" ) // Type is an enum for trust store types supported such as @@ -64,6 +65,7 @@ func (trustStore *x509TrustStore) GetCertificates(ctx context.Context, storeType if os.IsNotExist(err) { return nil, fmt.Errorf("%q does not exist", path) } + return nil, err } // throw error if path is not a directory or is a symlink @@ -130,12 +132,7 @@ func validateCerts(certs []*x509.Certificate, path string) error { // isValidStoreType checks if storeType is supported func isValidStoreType(storeType Type) bool { - for _, t := range Types { - if storeType == t { - return true - } - } - return false + return slice.Contains(Types, storeType) } // isValidFileName checks if a file name is cross-platform compatible From c566b00d35dada38be6d0a2ba0127290a2b431cb Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Wed, 9 Nov 2022 17:08:30 +0800 Subject: [PATCH 19/19] update Signed-off-by: Patrick Zheng --- verification/truststore/truststore.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/verification/truststore/truststore.go b/verification/truststore/truststore.go index 8f9ad002..dd8e0412 100644 --- a/verification/truststore/truststore.go +++ b/verification/truststore/truststore.go @@ -60,17 +60,13 @@ func (trustStore *x509TrustStore) GetCertificates(ctx context.Context, storeType if err != nil { return nil, err } - // check path is valid - if _, err := os.Stat(path); err != nil { - if os.IsNotExist(err) { - return nil, fmt.Errorf("%q does not exist", path) - } - return nil, err - } - // throw error if path is not a directory or is a symlink + // throw error if path is not a directory or is a symlink or does not exist. fileInfo, err := os.Lstat(path) if err != nil { + if os.IsNotExist(err) { + return nil, fmt.Errorf("%q does not exist", path) + } return nil, err } mode := fileInfo.Mode()