From f46c01009242b47a039be2322a6ad8b193d4c09d Mon Sep 17 00:00:00 2001 From: Travis Cosgrave Date: Thu, 30 Nov 2017 14:54:47 -0500 Subject: [PATCH 1/6] Use Custom Cert Extensions as Constraint This change allows the use of custom certificate extensions as a configurable constraint for certificate authentication. Opening the ability to use CAs like Puppet or Vault it self to inject more detailed information into the certificate as a means of more narrowly defining access to specific profiles. --- builtin/credential/cert/backend_test.go | 94 +++++++++++++++---- builtin/credential/cert/path_certs.go | 31 +++--- builtin/credential/cert/path_login.go | 43 ++++++++- .../cert/test-fixtures/root/rootcacert.srl | 1 + .../cert/test-fixtures/root/rootcawext.cnf | 21 +++++ .../cert/test-fixtures/root/rootcawext.csr | 19 ++++ .../test-fixtures/root/rootcawextcert.pem | 20 ++++ .../cert/test-fixtures/root/rootcawextkey.pem | 28 ++++++ website/source/api/auth/cert/index.html.md | 5 +- 9 files changed, 227 insertions(+), 35 deletions(-) create mode 100644 builtin/credential/cert/test-fixtures/root/rootcacert.srl create mode 100644 builtin/credential/cert/test-fixtures/root/rootcawext.cnf create mode 100644 builtin/credential/cert/test-fixtures/root/rootcawext.csr create mode 100644 builtin/credential/cert/test-fixtures/root/rootcawextcert.pem create mode 100644 builtin/credential/cert/test-fixtures/root/rootcawextkey.pem diff --git a/builtin/credential/cert/backend_test.go b/builtin/credential/cert/backend_test.go index 4680d6154323..13e61a326334 100644 --- a/builtin/credential/cert/backend_test.go +++ b/builtin/credential/cert/backend_test.go @@ -619,9 +619,9 @@ func TestBackend_CertWrites(t *testing.T) { tc := logicaltest.TestCase{ Backend: testFactory(t), Steps: []logicaltest.TestStep{ - testAccStepCert(t, "aaa", ca1, "foo", "", false), - testAccStepCert(t, "bbb", ca2, "foo", "", false), - testAccStepCert(t, "ccc", ca3, "foo", "", true), + testAccStepCert(t, "aaa", ca1, "foo", "", "", false), + testAccStepCert(t, "bbb", ca2, "foo", "", "", false), + testAccStepCert(t, "ccc", ca3, "foo", "", "", true), }, } tc.Steps = append(tc.Steps, testAccStepListCerts(t, []string{"aaa", "bbb"})...) @@ -642,16 +642,16 @@ func TestBackend_basic_CA(t *testing.T) { logicaltest.Test(t, logicaltest.TestCase{ Backend: testFactory(t), Steps: []logicaltest.TestStep{ - testAccStepCert(t, "web", ca, "foo", "", false), + testAccStepCert(t, "web", ca, "foo", "", "", false), testAccStepLogin(t, connState), testAccStepCertLease(t, "web", ca, "foo"), testAccStepCertTTL(t, "web", ca, "foo"), testAccStepLogin(t, connState), testAccStepCertNoLease(t, "web", ca, "foo"), testAccStepLoginDefaultLease(t, connState), - testAccStepCert(t, "web", ca, "foo", "*.example.com", false), + testAccStepCert(t, "web", ca, "foo", "*.example.com", "", false), testAccStepLogin(t, connState), - testAccStepCert(t, "web", ca, "foo", "*.invalid.com", false), + testAccStepCert(t, "web", ca, "foo", "*.invalid.com", "", false), testAccStepLoginInvalid(t, connState), }, }) @@ -700,11 +700,68 @@ func TestBackend_basic_singleCert(t *testing.T) { logicaltest.Test(t, logicaltest.TestCase{ Backend: testFactory(t), Steps: []logicaltest.TestStep{ - testAccStepCert(t, "web", ca, "foo", "", false), + testAccStepCert(t, "web", ca, "foo", "", "", false), testAccStepLogin(t, connState), - testAccStepCert(t, "web", ca, "foo", "example.com", false), + testAccStepCert(t, "web", ca, "foo", "example.com", "", false), testAccStepLogin(t, connState), - testAccStepCert(t, "web", ca, "foo", "invalid", false), + testAccStepCert(t, "web", ca, "foo", "invalid", "", false), + testAccStepLoginInvalid(t, connState), + testAccStepCert(t, "web", ca, "foo", "", "1.2.3.4:invalid", false), + testAccStepLoginInvalid(t, connState), + }, + }) +} + +// Test a self-signed client with custom extensions (root CA) that is trusted +func TestBackend_extensions_signleCert(t *testing.T) { + connState, err := testConnState( + "test-fixtures/root/rootcawextcert.pem", + "test-fixtures/root/rootcawextkey.pem", + "test-fixtures/root/rootcacert.pem", + ) + if err != nil { + t.Fatalf("error testing connection state: %v", err) + } + ca, err := ioutil.ReadFile("test-fixtures/root/rootcacert.pem") + if err != nil { + t.Fatalf("err: %v", err) + } + logicaltest.Test(t, logicaltest.TestCase{ + Backend: testFactory(t), + Steps: []logicaltest.TestStep{ + testAccStepCert(t, "web", ca, "foo", "", "2.1.1.1:A UTF8String Extension", false), + testAccStepLogin(t, connState), + testAccStepCert(t, "web", ca, "foo", "", "2.1.1.1:*,2.1.1.2:A UTF8*", false), + testAccStepLogin(t, connState), + testAccStepCert(t, "web", ca, "foo", "", "1.2.3.45:*", false), + testAccStepLoginInvalid(t, connState), + testAccStepCert(t, "web", ca, "foo", "", "2.1.1.1:The Wrong Value", false), + testAccStepLoginInvalid(t, connState), + testAccStepCert(t, "web", ca, "foo", "", "2.1.1.1:*,2.1.1.2:The Wrong Value", false), + testAccStepLoginInvalid(t, connState), + testAccStepCert(t, "web", ca, "foo", "", "2.1.1.1:", false), + testAccStepLoginInvalid(t, connState), + testAccStepCert(t, "web", ca, "foo", "", "2.1.1.1:,2.1.1.2:*", false), + testAccStepLoginInvalid(t, connState), + testAccStepCert(t, "web", ca, "foo", "example.com", "2.1.1.1:A UTF8String Extension", false), + testAccStepLogin(t, connState), + testAccStepCert(t, "web", ca, "foo", "example.com", "2.1.1.1:*,2.1.1.2:A UTF8*", false), + testAccStepLogin(t, connState), + testAccStepCert(t, "web", ca, "foo", "example.com", "1.2.3.45:*", false), + testAccStepLoginInvalid(t, connState), + testAccStepCert(t, "web", ca, "foo", "example.com", "2.1.1.1:The Wrong Value", false), + testAccStepLoginInvalid(t, connState), + testAccStepCert(t, "web", ca, "foo", "example.com", "2.1.1.1:*,2.1.1.2:The Wrong Value", false), + testAccStepLoginInvalid(t, connState), + testAccStepCert(t, "web", ca, "foo", "invalid", "2.1.1.1:A UTF8String Extension", false), + testAccStepLoginInvalid(t, connState), + testAccStepCert(t, "web", ca, "foo", "invalid", "2.1.1.1:*,2.1.1.2:A UTF8*", false), + testAccStepLoginInvalid(t, connState), + testAccStepCert(t, "web", ca, "foo", "invalid", "1.2.3.45:*", false), + testAccStepLoginInvalid(t, connState), + testAccStepCert(t, "web", ca, "foo", "invalid", "2.1.1.1:The Wrong Value", false), + testAccStepLoginInvalid(t, connState), + testAccStepCert(t, "web", ca, "foo", "invalid", "2.1.1.1:*,2.1.1.2:The Wrong Value", false), testAccStepLoginInvalid(t, connState), }, }) @@ -724,9 +781,9 @@ func TestBackend_mixed_constraints(t *testing.T) { logicaltest.Test(t, logicaltest.TestCase{ Backend: testFactory(t), Steps: []logicaltest.TestStep{ - testAccStepCert(t, "1unconstrained", ca, "foo", "", false), - testAccStepCert(t, "2matching", ca, "foo", "*.example.com,whatever", false), - testAccStepCert(t, "3invalid", ca, "foo", "invalid", false), + testAccStepCert(t, "1unconstrained", ca, "foo", "", "", false), + testAccStepCert(t, "2matching", ca, "foo", "*.example.com,whatever", "", false), + testAccStepCert(t, "3invalid", ca, "foo", "invalid", "", false), testAccStepLogin(t, connState), // Assumes CertEntries are processed in alphabetical order (due to store.List), so we only match 2matching if 1unconstrained doesn't match testAccStepLoginWithName(t, connState, "2matching"), @@ -906,17 +963,18 @@ func testAccStepListCerts( } func testAccStepCert( - t *testing.T, name string, cert []byte, policies string, allowedNames string, expectError bool) logicaltest.TestStep { + t *testing.T, name string, cert []byte, policies string, allowedNames string, requiredExtensions string, expectError bool) logicaltest.TestStep { return logicaltest.TestStep{ Operation: logical.UpdateOperation, Path: "certs/" + name, ErrorOk: expectError, Data: map[string]interface{}{ - "certificate": string(cert), - "policies": policies, - "display_name": name, - "allowed_names": allowedNames, - "lease": 1000, + "certificate": string(cert), + "policies": policies, + "display_name": name, + "allowed_names": allowedNames, + "required_extensions": requiredExtensions, + "lease": 1000, }, Check: func(resp *logical.Response) error { if resp == nil && expectError { diff --git a/builtin/credential/cert/path_certs.go b/builtin/credential/cert/path_certs.go index fc5254f2891e..23467a0bc376 100644 --- a/builtin/credential/cert/path_certs.go +++ b/builtin/credential/cert/path_certs.go @@ -45,6 +45,12 @@ Must be x509 PEM encoded.`, At least one must exist in either the Common Name or SANs. Supports globbing.`, }, + "required_extensions": &framework.FieldSchema{ + Type: framework.TypeCommaStringSlice, + Description: `A comma-separated list of extensions +formatted as "$oid:value". All values much match. Supports globbing on $value.`, + }, + "display_name": &framework.FieldSchema{ Type: framework.TypeString, Description: `The display name to use for clients using this @@ -146,6 +152,7 @@ func (b *backend) pathCertWrite( displayName := d.Get("display_name").(string) policies := policyutil.ParsePolicies(d.Get("policies")) allowedNames := d.Get("allowed_names").([]string) + requiredExtensions := d.Get("required_extensions").([]string) // Default the display name to the certificate name if not given if displayName == "" { @@ -172,11 +179,12 @@ func (b *backend) pathCertWrite( } certEntry := &CertEntry{ - Name: name, - Certificate: certificate, - DisplayName: displayName, - Policies: policies, - AllowedNames: allowedNames, + Name: name, + Certificate: certificate, + DisplayName: displayName, + Policies: policies, + AllowedNames: allowedNames, + RequiredExtensions: requiredExtensions, } // Parse the lease duration or default to backend/system default @@ -204,12 +212,13 @@ func (b *backend) pathCertWrite( } type CertEntry struct { - Name string - Certificate string - DisplayName string - Policies []string - TTL time.Duration - AllowedNames []string + Name string + Certificate string + DisplayName string + Policies []string + TTL time.Duration + AllowedNames []string + RequiredExtensions []string } const pathCertHelpSyn = ` diff --git a/builtin/credential/cert/path_login.go b/builtin/credential/cert/path_login.go index 85246c411f98..823f3e79a0e2 100644 --- a/builtin/credential/cert/path_login.go +++ b/builtin/credential/cert/path_login.go @@ -237,30 +237,63 @@ func (b *backend) verifyCredentials(req *logical.Request, d *framework.FieldData } func (b *backend) matchesConstraints(clientCert *x509.Certificate, trustedChain []*x509.Certificate, config *ParsedCert) bool { + return !b.checkForChainInCRLs(trustedChain) && + b.matchesNames(clientCert, config) && + b.matchesCertificateExtenions(clientCert, config) +} + +// matchesNames verifies that the certificate matches at least one configured +// allowed name +func (b *backend) matchesNames(clientCert *x509.Certificate, config *ParsedCert) bool { // Default behavior (no names) is to allow all names - nameMatched := len(config.Entry.AllowedNames) == 0 + if len(config.Entry.AllowedNames) == 0 { + return true + } // At least one pattern must match at least one name if any patterns are specified for _, allowedName := range config.Entry.AllowedNames { if glob.Glob(allowedName, clientCert.Subject.CommonName) { - nameMatched = true + return true } for _, name := range clientCert.DNSNames { if glob.Glob(allowedName, name) { - nameMatched = true + return true } } for _, name := range clientCert.EmailAddresses { if glob.Glob(allowedName, name) { - nameMatched = true + return true } } } + return false +} - return !b.checkForChainInCRLs(trustedChain) && nameMatched +// matchesCertificateExtenions verifies that the certificate matches configured +// required extensions +func (b *backend) matchesCertificateExtenions(clientCert *x509.Certificate, config *ParsedCert) bool { + // Build Client Extensions Map + clientExtMap := map[string]string{} + for _, ext := range clientCert.Extensions { + clientExtMap[ext.Id.String()] = strings.TrimLeftFunc(string(ext.Value[:]), b.isControlRune) + } + + // If we match all of the expected extensions, the requirement is satisfied + matchedExts := 0 + for _, requiredExt := range config.Entry.RequiredExtensions { + reqExt := strings.Split(requiredExt, ":") + clientExtValue, clientExtValueExists := clientExtMap[reqExt[0]] + if glob.Glob(reqExt[1], clientExtValue) && clientExtValueExists { + matchedExts++ + } + } + return matchedExts == len(config.Entry.RequiredExtensions) } +// isControlRune returns true for control charaters for trimming extension values +func (b *backend) isControlRune(r rune) bool { return r <= 32 || r == 127 } + // loadTrustedCerts is used to load all the trusted certificates from the backend func (b *backend) loadTrustedCerts(store logical.Storage, certName string) (pool *x509.CertPool, trusted []*ParsedCert, trustedNonCAs []*ParsedCert) { pool = x509.NewCertPool() diff --git a/builtin/credential/cert/test-fixtures/root/rootcacert.srl b/builtin/credential/cert/test-fixtures/root/rootcacert.srl new file mode 100644 index 000000000000..219a6be4b13b --- /dev/null +++ b/builtin/credential/cert/test-fixtures/root/rootcacert.srl @@ -0,0 +1 @@ +92223EAFBBEE17A3 diff --git a/builtin/credential/cert/test-fixtures/root/rootcawext.cnf b/builtin/credential/cert/test-fixtures/root/rootcawext.cnf new file mode 100644 index 000000000000..524efd2e4013 --- /dev/null +++ b/builtin/credential/cert/test-fixtures/root/rootcawext.cnf @@ -0,0 +1,21 @@ +[ req ] +default_bits = 2048 +encrypt_key = no +prompt = no +default_md = sha256 +req_extensions = req_v3 +distinguished_name = dn + +[ dn ] +CN = example.com + +[ req_v3 ] +subjectAltName = @alt_names +2.1.1.1=ASN1:UTF8String:A UTF8String Extension +2.1.1.2=ASN1:UTF8:A UTF8 Extension +2.1.1.3=ASN1:IA5:An IA5 Extension +2.1.1.4=ASN1:VISIBLE:A Visible Extension + +[ alt_names ] +DNS.1 = example.com +IP.1 = 127.0.0.1 diff --git a/builtin/credential/cert/test-fixtures/root/rootcawext.csr b/builtin/credential/cert/test-fixtures/root/rootcawext.csr new file mode 100644 index 000000000000..55e22eedeb0a --- /dev/null +++ b/builtin/credential/cert/test-fixtures/root/rootcawext.csr @@ -0,0 +1,19 @@ +-----BEGIN CERTIFICATE REQUEST----- +MIIDAzCCAesCAQAwFjEUMBIGA1UEAwwLZXhhbXBsZS5jb20wggEiMA0GCSqGSIb3 +DQEBAQUAA4IBDwAwggEKAoIBAQDM2PrLyK/wVQIcnK362ZylDrIVMjFQzps/0AxM +ke+8MNPMArBlSAhnZus6qb0nN0nJrDLkHQgYqnSvK9N7VUv/xFblEcOLBlciLhyN +Wkm92+q/M/xOvUVmnYkN3XgTI5QNxF7ZWDFHmwCNV27RraQZou0hG7yvyoILLMQE +3MnMCNM1nZ9JIuBMcRsZLGqQ1XNaQljboRVIUjimzkcfYyTruhLosTIbwForp78J +MzHHqVjtLJXPqUnRMS7KhGMj1f2mIswQzCv6F2PWEzNBbP4Gb67znKikKDs0RgyL +RyfizFNFJSC58XntK8jwHK1D8W3UepFf4K8xNFnhPoKWtWfJAgMBAAGggacwgaQG +CSqGSIb3DQEJDjGBljCBkzAcBgNVHREEFTATggtleGFtcGxlLmNvbYcEfwAAATAf +BgNRAQEEGAwWQSBVVEY4U3RyaW5nIEV4dGVuc2lvbjAZBgNRAQIEEgwQQSBVVEY4 +IEV4dGVuc2lvbjAZBgNRAQMEEhYQQW4gSUE1IEV4dGVuc2lvbjAcBgNRAQQEFRoT +QSBWaXNpYmxlIEV4dGVuc2lvbjANBgkqhkiG9w0BAQsFAAOCAQEAtYjewBcqAXxk +tDY0lpZid6ZvfngdDlDZX0vrs3zNppKNe5Sl+jsoDOexqTA7HQA/y1ru117sAEeB +yiqMeZ7oPk8b3w+BZUpab7p2qPMhZypKl93y/jGXGscc3jRbUBnym9S91PSq6wUd +f2aigSqFc9+ywFVdx5PnnZUfcrUQ2a+AweYEkGOzXX2Ga+Ige8grDMCzRgCoP5cW +kM5ghwZp5wYIBGrKBU9iDcBlmnNhYaGWf+dD00JtVDPNn2bJnCsJHIO0nklZgnrS +fli8VQ1nYPkONdkiRYLt6//6at1iNDoDgsVCChtlVkLpxFIKcDFUHlffZsc1kMFI +HTX579k8hA== +-----END CERTIFICATE REQUEST----- diff --git a/builtin/credential/cert/test-fixtures/root/rootcawextcert.pem b/builtin/credential/cert/test-fixtures/root/rootcawextcert.pem new file mode 100644 index 000000000000..2c8591735f5a --- /dev/null +++ b/builtin/credential/cert/test-fixtures/root/rootcawextcert.pem @@ -0,0 +1,20 @@ +-----BEGIN CERTIFICATE----- +MIIDRjCCAi6gAwIBAgIJAJIiPq+77hejMA0GCSqGSIb3DQEBCwUAMBYxFDASBgNV +BAMTC2V4YW1wbGUuY29tMB4XDTE3MTEyOTE5MTgwM1oXDTI3MTEyNzE5MTgwM1ow +FjEUMBIGA1UEAwwLZXhhbXBsZS5jb20wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAw +ggEKAoIBAQDM2PrLyK/wVQIcnK362ZylDrIVMjFQzps/0AxMke+8MNPMArBlSAhn +Zus6qb0nN0nJrDLkHQgYqnSvK9N7VUv/xFblEcOLBlciLhyNWkm92+q/M/xOvUVm +nYkN3XgTI5QNxF7ZWDFHmwCNV27RraQZou0hG7yvyoILLMQE3MnMCNM1nZ9JIuBM +cRsZLGqQ1XNaQljboRVIUjimzkcfYyTruhLosTIbwForp78JMzHHqVjtLJXPqUnR +MS7KhGMj1f2mIswQzCv6F2PWEzNBbP4Gb67znKikKDs0RgyLRyfizFNFJSC58Xnt +K8jwHK1D8W3UepFf4K8xNFnhPoKWtWfJAgMBAAGjgZYwgZMwHAYDVR0RBBUwE4IL +ZXhhbXBsZS5jb22HBH8AAAEwHwYDUQEBBBgMFkEgVVRGOFN0cmluZyBFeHRlbnNp +b24wGQYDUQECBBIMEEEgVVRGOCBFeHRlbnNpb24wGQYDUQEDBBIWEEFuIElBNSBF +eHRlbnNpb24wHAYDUQEEBBUaE0EgVmlzaWJsZSBFeHRlbnNpb24wDQYJKoZIhvcN +AQELBQADggEBAGU/iA6saupEaGn/veVNCknFGDL7pst5D6eX/y9atXlBOdJe7ZJJ +XQRkeHJldA0khVpzH7Ryfi+/25WDuNz+XTZqmb4ppeV8g9amtqBwxziQ9UUwYrza +eDBqdXBaYp/iHUEHoceX4F44xuo80BIqwF0lD9TFNUFoILnF26ajhKX0xkGaiKTH +6SbjBfHoQVMzOHokVRWregmgNycV+MAI9Ne9XkIZvdOYeNlcS9drZeJI3szkiaxB +WWaWaAr5UU2Z0yUCZnAIDMRcIiUbSEjIDz504sSuCzTctMOxWZu0r/0UrXRzwZZi +HAaKm3MUmBh733ChP4rTB58nr5DEr5rJ9P8= +-----END CERTIFICATE----- diff --git a/builtin/credential/cert/test-fixtures/root/rootcawextkey.pem b/builtin/credential/cert/test-fixtures/root/rootcawextkey.pem new file mode 100644 index 000000000000..3f8d8ebed9c0 --- /dev/null +++ b/builtin/credential/cert/test-fixtures/root/rootcawextkey.pem @@ -0,0 +1,28 @@ +-----BEGIN PRIVATE KEY----- +MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQDM2PrLyK/wVQIc +nK362ZylDrIVMjFQzps/0AxMke+8MNPMArBlSAhnZus6qb0nN0nJrDLkHQgYqnSv +K9N7VUv/xFblEcOLBlciLhyNWkm92+q/M/xOvUVmnYkN3XgTI5QNxF7ZWDFHmwCN +V27RraQZou0hG7yvyoILLMQE3MnMCNM1nZ9JIuBMcRsZLGqQ1XNaQljboRVIUjim +zkcfYyTruhLosTIbwForp78JMzHHqVjtLJXPqUnRMS7KhGMj1f2mIswQzCv6F2PW +EzNBbP4Gb67znKikKDs0RgyLRyfizFNFJSC58XntK8jwHK1D8W3UepFf4K8xNFnh +PoKWtWfJAgMBAAECggEAW7hLkzMok9N8PpNo0wjcuor58cOnkSbxHIFrAF3XmcvD +CXWqxa6bFLFgYcPejdCTmVkg8EKPfXvVAxn8dxyaCss+nRJ3G6ibGxLKdgAXRItT +cIk2T4svp+KhmzOur+MeR4vFbEuwxP8CIEclt3yoHVJ2Gnzw30UtNRO2MPcq48/C +ZODGeBqUif1EGjDAvlqu5kl/pcDBJ3ctIZdVUMYYW4R9JtzKsmwhX7CRCBm8k5hG +2uzn8AKwpuVtfWcnX59UUmHGJ8mjETuNLARRAwWBWhl8f7wckmi+PKERJGEM2QE5 +/Voy0p22zmQ3waS8LgiI7YHCAEFqjVWNziVGdR36gQKBgQDxkpfkEsfa5PieIaaF +iQOO0rrjEJ9MBOQqmTDeclmDPNkM9qvCF/dqpJfOtliYFxd7JJ3OR2wKrBb5vGHt +qIB51Rnm9aDTM4OUEhnhvbPlERD0W+yWYXWRvqyHz0GYwEFGQ83h95GC/qfTosqy +LEzYLDafiPeNP+DG/HYRljAxUwKBgQDZFOWHEcZkSFPLNZiksHqs90OR2zIFxZcx +SrbkjqXjRjehWEAwgpvQ/quSBxrE2E8xXgVm90G1JpWzxjUfKKQRM6solQeEpnwY +kCy2Ozij/TtbLNRlU65UQ+nMto8KTSIyJbxxdOZxYdtJAJQp1FJO1a1WC11z4+zh +lnLV1O5S8wKBgQCDf/QU4DBQtNGtas315Oa96XJ4RkUgoYz+r1NN09tsOERC7UgE +KP2y3JQSn2pMqE1M6FrKvlBO4uzC10xLja0aJOmrssvwDBu1D8FtA9IYgJjFHAEG +v1i7lJrgdu7TUtx1flVli1l3gF4lM3m5UaonBrJZV7rB9iLKzwUKf8IOJwKBgFt/ +QktPA6brEV56Za8sr1hOFA3bLNdf9B0Tl8j4ExWbWAFKeCu6MUDCxsAS/IZxgdeW +AILovqpC7CBM78EFWTni5EaDohqYLYAQ7LeWeIYuSyFf4Nogjj74LQha/iliX4Jx +g17y3dp2W34Gn2yOEG8oAxpcSfR54jMnPZnBWP5fAoGBAMNAd3oa/xq9A5v719ik +naD7PdrjBdhnPk4egzMDv54y6pCFlvFbEiBduBWTmiVa7dSzhYtmEbri2WrgARlu +vkfTnVH9E8Hnm4HTbNn+ebxrofq1AOAvdApSoslsOP1NT9J6zB89RzChJyzjbIQR +Gevrutb4uO9qpB1jDVoMmGde +-----END PRIVATE KEY----- diff --git a/website/source/api/auth/cert/index.html.md b/website/source/api/auth/cert/index.html.md index 0de838e6f540..7e1994bc8aff 100644 --- a/website/source/api/auth/cert/index.html.md +++ b/website/source/api/auth/cert/index.html.md @@ -33,6 +33,8 @@ Sets a CA cert and associated parameters in a role name. the client certificate with a [globbed pattern] (https://github.com/ryanuber/go-glob/blob/master/README.md#example). Value is a comma-separated list of patterns. Authentication requires at least one Name matching at least one pattern. If not set, defaults to allowing all names. +- `required_extensions` `(string: "")` - Require specific Custom Extension OIDs to exist and match the pattern. + Value is a comma separated list of `oid:glob,oid:glob`. All conditions _must_ be met. - `policies` `(string: "")` - A comma-separated list of policies to set on tokens issued when authenticating against this CA certificate. - `display_name` `(string: "")` - The `display_name` to set on tokens issued @@ -93,6 +95,7 @@ $ curl \ "display_name": "test", "policies": "", "allowed_names": "", + "required_extensions": "", "ttl": 2764800 }, "warnings": null, @@ -327,4 +330,4 @@ $ curl \ "renewable": true, } } -``` \ No newline at end of file +``` From 47e9816ae02a04017b6d953c7e1144ccbd4e99f7 Mon Sep 17 00:00:00 2001 From: Travis Cosgrave Date: Mon, 4 Dec 2017 15:39:34 -0500 Subject: [PATCH 2/6] PR Comment Changes Use SplitN over Split for custom extension configuration to avoid missing values containing :. Simply return false on the first required custom extension faliure. Format oneline function correctly. --- builtin/credential/cert/path_login.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/builtin/credential/cert/path_login.go b/builtin/credential/cert/path_login.go index 823f3e79a0e2..d63dff522f79 100644 --- a/builtin/credential/cert/path_login.go +++ b/builtin/credential/cert/path_login.go @@ -276,23 +276,25 @@ func (b *backend) matchesCertificateExtenions(clientCert *x509.Certificate, conf // Build Client Extensions Map clientExtMap := map[string]string{} for _, ext := range clientCert.Extensions { + // Trim prefix control characters from the Custom Extension Value for human readable configs clientExtMap[ext.Id.String()] = strings.TrimLeftFunc(string(ext.Value[:]), b.isControlRune) } - // If we match all of the expected extensions, the requirement is satisfied - matchedExts := 0 + // If any of the required extensions don't match the constraint fails for _, requiredExt := range config.Entry.RequiredExtensions { - reqExt := strings.Split(requiredExt, ":") + reqExt := strings.SplitN(requiredExt, ":", 2) clientExtValue, clientExtValueExists := clientExtMap[reqExt[0]] - if glob.Glob(reqExt[1], clientExtValue) && clientExtValueExists { - matchedExts++ + if !clientExtValueExists || !glob.Glob(reqExt[1], clientExtValue) { + return false } } - return matchedExts == len(config.Entry.RequiredExtensions) + return true } // isControlRune returns true for control charaters for trimming extension values -func (b *backend) isControlRune(r rune) bool { return r <= 32 || r == 127 } +func (b *backend) isControlRune(r rune) bool { + return r <= 32 || r == 127 +} // loadTrustedCerts is used to load all the trusted certificates from the backend func (b *backend) loadTrustedCerts(store logical.Storage, certName string) (pool *x509.CertPool, trusted []*ParsedCert, trustedNonCAs []*ParsedCert) { From 459860e1980a8c2e191d887eaef2c977d482b8f5 Mon Sep 17 00:00:00 2001 From: Travis Cosgrave Date: Fri, 8 Dec 2017 22:14:31 -0500 Subject: [PATCH 3/6] Trim ASN.1 Tag from Custom Extensions value bytes by dropping the first two bytes and assume the value is a string. --- builtin/credential/cert/path_login.go | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/builtin/credential/cert/path_login.go b/builtin/credential/cert/path_login.go index d63dff522f79..730dcaf4efc1 100644 --- a/builtin/credential/cert/path_login.go +++ b/builtin/credential/cert/path_login.go @@ -273,29 +273,25 @@ func (b *backend) matchesNames(clientCert *x509.Certificate, config *ParsedCert) // matchesCertificateExtenions verifies that the certificate matches configured // required extensions func (b *backend) matchesCertificateExtenions(clientCert *x509.Certificate, config *ParsedCert) bool { - // Build Client Extensions Map - clientExtMap := map[string]string{} + // Build Client Extensions Map for Constraint Matching + // x509 Writes Extensions in ASN1 with a bitstring tag, which results in the field + // including its ASN.1 type tag bytes. For the sake of simplicity, assume string type + // and drop the tag bytes. And get the number of bytes from the tag. + clientExtMap := make(map[string]string, len(clientCert.Extensions)) for _, ext := range clientCert.Extensions { - // Trim prefix control characters from the Custom Extension Value for human readable configs - clientExtMap[ext.Id.String()] = strings.TrimLeftFunc(string(ext.Value[:]), b.isControlRune) + clientExtMap[ext.Id.String()] = string(ext.Value[2 : 2+ext.Value[1]]) } - // If any of the required extensions don't match the constraint fails for _, requiredExt := range config.Entry.RequiredExtensions { reqExt := strings.SplitN(requiredExt, ":", 2) - clientExtValue, clientExtValueExists := clientExtMap[reqExt[0]] - if !clientExtValueExists || !glob.Glob(reqExt[1], clientExtValue) { + clientExtValue, clientExtValueOk := clientExtMap[reqExt[0]] + if !clientExtValueOk || !glob.Glob(reqExt[1], clientExtValue) { return false } } return true } -// isControlRune returns true for control charaters for trimming extension values -func (b *backend) isControlRune(r rune) bool { - return r <= 32 || r == 127 -} - // loadTrustedCerts is used to load all the trusted certificates from the backend func (b *backend) loadTrustedCerts(store logical.Storage, certName string) (pool *x509.CertPool, trusted []*ParsedCert, trustedNonCAs []*ParsedCert) { pool = x509.NewCertPool() From b97f47865fc4a103e6e1ab6fedaabe9451998d5e Mon Sep 17 00:00:00 2001 From: Travis Cosgrave Date: Sun, 10 Dec 2017 15:39:03 +0100 Subject: [PATCH 4/6] Fixes typo in test name --- builtin/credential/cert/backend_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/credential/cert/backend_test.go b/builtin/credential/cert/backend_test.go index 13e61a326334..c50127b170ce 100644 --- a/builtin/credential/cert/backend_test.go +++ b/builtin/credential/cert/backend_test.go @@ -713,7 +713,7 @@ func TestBackend_basic_singleCert(t *testing.T) { } // Test a self-signed client with custom extensions (root CA) that is trusted -func TestBackend_extensions_signleCert(t *testing.T) { +func TestBackend_extensions_singleCert(t *testing.T) { connState, err := testConnState( "test-fixtures/root/rootcawextcert.pem", "test-fixtures/root/rootcawextkey.pem", From ffc84871a8349d3754adf6eacbba2d6e3ba8644b Mon Sep 17 00:00:00 2001 From: Travis Cosgrave Date: Fri, 15 Dec 2017 00:17:49 +0100 Subject: [PATCH 5/6] Use ASN1 to Unmarshal extensions into string values --- builtin/credential/cert/path_login.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/builtin/credential/cert/path_login.go b/builtin/credential/cert/path_login.go index 730dcaf4efc1..0461f2a91ee7 100644 --- a/builtin/credential/cert/path_login.go +++ b/builtin/credential/cert/path_login.go @@ -4,6 +4,7 @@ import ( "bytes" "crypto/tls" "crypto/x509" + "encoding/asn1" "encoding/base64" "encoding/pem" "errors" @@ -279,7 +280,9 @@ func (b *backend) matchesCertificateExtenions(clientCert *x509.Certificate, conf // and drop the tag bytes. And get the number of bytes from the tag. clientExtMap := make(map[string]string, len(clientCert.Extensions)) for _, ext := range clientCert.Extensions { - clientExtMap[ext.Id.String()] = string(ext.Value[2 : 2+ext.Value[1]]) + var parsedValue string + asn1.Unmarshal(ext.Value, &parsedValue) + clientExtMap[ext.Id.String()] = parsedValue } // If any of the required extensions don't match the constraint fails for _, requiredExt := range config.Entry.RequiredExtensions { From f5e1929b77918853c18df900c63ef486d0c9c1a5 Mon Sep 17 00:00:00 2001 From: Travis Cosgrave Date: Fri, 15 Dec 2017 23:05:23 +0100 Subject: [PATCH 6/6] updating docs based on review feedback --- builtin/credential/cert/path_certs.go | 5 +++-- website/source/api/auth/cert/index.html.md | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/builtin/credential/cert/path_certs.go b/builtin/credential/cert/path_certs.go index 23467a0bc376..63cbec4b183b 100644 --- a/builtin/credential/cert/path_certs.go +++ b/builtin/credential/cert/path_certs.go @@ -47,8 +47,9 @@ At least one must exist in either the Common Name or SANs. Supports globbing.`, "required_extensions": &framework.FieldSchema{ Type: framework.TypeCommaStringSlice, - Description: `A comma-separated list of extensions -formatted as "$oid:value". All values much match. Supports globbing on $value.`, + Description: `A comma-separated string or array of extensions +formatted as "oid:value". Expects the extension value to be some type of ASN1 encoded string. +All values much match. Supports globbing on "value".`, }, "display_name": &framework.FieldSchema{ diff --git a/website/source/api/auth/cert/index.html.md b/website/source/api/auth/cert/index.html.md index 7e1994bc8aff..f94f5f659ab5 100644 --- a/website/source/api/auth/cert/index.html.md +++ b/website/source/api/auth/cert/index.html.md @@ -33,8 +33,9 @@ Sets a CA cert and associated parameters in a role name. the client certificate with a [globbed pattern] (https://github.com/ryanuber/go-glob/blob/master/README.md#example). Value is a comma-separated list of patterns. Authentication requires at least one Name matching at least one pattern. If not set, defaults to allowing all names. -- `required_extensions` `(string: "")` - Require specific Custom Extension OIDs to exist and match the pattern. - Value is a comma separated list of `oid:glob,oid:glob`. All conditions _must_ be met. +- `required_extensions` `(string: "" or array:[])` - Require specific Custom Extension OIDs to exist and match the pattern. + Value is a comma separated string or array of `oid:value`. Expects the extension value to be some type of ASN1 encoded string. + All conditions _must_ be met. Supports globbing on `value`. - `policies` `(string: "")` - A comma-separated list of policies to set on tokens issued when authenticating against this CA certificate. - `display_name` `(string: "")` - The `display_name` to set on tokens issued