From d639e40fa02ea4270e2a43773b10fb64c1406074 Mon Sep 17 00:00:00 2001 From: Michael Ansel Date: Sat, 15 Apr 2017 10:49:58 -0700 Subject: [PATCH 01/13] Refactor to consolidate constraints on the matching chain --- builtin/credential/cert/path_login.go | 73 +++++++++++++-------------- 1 file changed, 36 insertions(+), 37 deletions(-) diff --git a/builtin/credential/cert/path_login.go b/builtin/credential/cert/path_login.go index d9fcbe8fd5f1..e4c82eda6e42 100644 --- a/builtin/credential/cert/path_login.go +++ b/builtin/credential/cert/path_login.go @@ -153,13 +153,19 @@ func (b *backend) verifyCredentials(req *logical.Request) (*ParsedCert, *logical // If trustedNonCAs is not empty it means that client had registered a non-CA cert // with the backend. if len(trustedNonCAs) != 0 { - policy := b.matchNonCAPolicy(connState.PeerCertificates[0], trustedNonCAs) - if policy != nil && !b.checkForChainInCRLs(policy.Certificates) { - return policy, nil, nil + clientCert := connState.PeerCertificates[0] + for _, trustedNonCA := range trustedNonCAs { + tCert := trustedNonCA.Certificates[0] + // Check for client cert being explicitly listed in the config (and matching other constraints) + if tCert.SerialNumber.Cmp(clientCert.SerialNumber) == 0 && + bytes.Equal(tCert.AuthorityKeyId, clientCert.AuthorityKeyId) && + b.matchPolicy(connState.PeerCertificates[0], trustedNonCA.Certificates, trustedNonCA) { + return trustedNonCA, nil, nil + } } } - // Validate the connection state is trusted + // Get the list of full chains matching the connection trustedChains, err := validateConnState(roots, connState) if err != nil { return nil, nil, err @@ -170,45 +176,38 @@ func (b *backend) verifyCredentials(req *logical.Request) (*ParsedCert, *logical return nil, logical.ErrorResponse("invalid certificate or no client certificate supplied"), nil } - validChain := b.checkForValidChain(trustedChains) - if !validChain { - return nil, logical.ErrorResponse( - "no chain containing non-revoked certificates could be found for this login certificate", - ), nil + // Search for a ParsedCert that intersects with the validated chains and any additional constraints + matches := make([]*ParsedCert, 0) + for _, trust := range trusted { // For each ParsedCert in the config + for _, tCert := range trust.Certificates { // For each certificate in the entry + for _, chain := range trustedChains { // For each root chain that we matched + for _, cCert := range chain { // For each cert in the matched chain + if tCert.Equal(cCert) && // ParsedCert intersects with matched chain + b.matchPolicy(connState.PeerCertificates[0], chain, trust) { // validate client cert + matched chain against the config + // Add the match to the list + matches = append(matches, trust) + } + } + } + } } - // Match the trusted chain with the policy - return b.matchPolicy(trustedChains, trusted), nil, nil -} + // Fail on no matches + if len(matches) == 0 { + return nil, logical.ErrorResponse("no chain matching all constraints could be found for this login certificate"), nil + } -// matchNonCAPolicy is used to match the client cert with the registered non-CA -// policies to establish client identity. -func (b *backend) matchNonCAPolicy(clientCert *x509.Certificate, trustedNonCAs []*ParsedCert) *ParsedCert { - for _, trustedNonCA := range trustedNonCAs { - tCert := trustedNonCA.Certificates[0] - if tCert.SerialNumber.Cmp(clientCert.SerialNumber) == 0 && bytes.Equal(tCert.AuthorityKeyId, clientCert.AuthorityKeyId) { - return trustedNonCA - } + // Fail on non-determinism + if len(trustedChains) > 1 { + return nil, logical.ErrorResponse("login certificate matched multiple configured roles; unsure which to apply"), nil } - return nil + + // Return the sole matching entry + return matches[0], nil, nil } -// matchPolicy is used to match the associated policy with the certificate that -// was used to establish the client identity. -func (b *backend) matchPolicy(chains [][]*x509.Certificate, trusted []*ParsedCert) *ParsedCert { - // There is probably a better way to do this... - for _, chain := range chains { - for _, trust := range trusted { - for _, tCert := range trust.Certificates { - for _, cCert := range chain { - if tCert.Equal(cCert) { - return trust - } - } - } - } - } - return nil +func (b *backend) matchPolicy(clientCert *x509.Certificate, trustedChain []*x509.Certificate, config *ParsedCert) bool { + return !b.checkForChainInCRLs(trustedChain) } // loadTrustedCerts is used to load all the trusted certificates from the backend From 1310297405e20994a7abde193f9034fa2293b765 Mon Sep 17 00:00:00 2001 From: Michael Ansel Date: Sat, 15 Apr 2017 10:51:15 -0700 Subject: [PATCH 02/13] Add CN prefix/suffix constraint --- builtin/credential/cert/backend_test.go | 35 +++++++++++++++++++++++++ builtin/credential/cert/path_certs.go | 16 +++++++++++ builtin/credential/cert/path_login.go | 4 ++- 3 files changed, 54 insertions(+), 1 deletion(-) diff --git a/builtin/credential/cert/backend_test.go b/builtin/credential/cert/backend_test.go index 766c095f8d79..87d6c071f17e 100644 --- a/builtin/credential/cert/backend_test.go +++ b/builtin/credential/cert/backend_test.go @@ -242,6 +242,41 @@ func TestBackend_CRLs(t *testing.T) { t.Fatalf("err:%v resp:%#v", err, resp) } + // Add a failing constraint on the Common Name + certData["cn_prefix"] = "invalid" + certData["cn_suffix"] = "" + + // And update the config + resp, err = b.HandleRequest(certReq) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%v resp:%#v", err, resp) + } + + // Attempt login with the updated configuration + resp, err = b.HandleRequest(loginReq) + if err != nil { + t.Fatal(err) + } + if resp == nil || !resp.IsError() { + t.Fatalf("expected failure due to violating prefix/suffix constraints") + } + + // Add a passing constraint on the Common Name + certData["cn_prefix"] = "" + certData["cn_suffix"] = "myvault.com" + + // And update the config + resp, err = b.HandleRequest(certReq) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%v resp:%#v", err, resp) + } + + // Attempt login with the updated configuration + resp, err = b.HandleRequest(loginReq) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%v resp:%#v", err, resp) + } + // Register a CRL containing the issued client certificate used above. issuedCRL, err := ioutil.ReadFile(testIssuedCertCRL) if err != nil { diff --git a/builtin/credential/cert/path_certs.go b/builtin/credential/cert/path_certs.go index d842bb841de6..285c58cfaa0b 100644 --- a/builtin/credential/cert/path_certs.go +++ b/builtin/credential/cert/path_certs.go @@ -39,6 +39,16 @@ func pathCerts(b *backend) *framework.Path { Must be x509 PEM encoded.`, }, + "cn_prefix": &framework.FieldSchema{ + Type: framework.TypeString, + Description: `A required prefix for the Common Name in the client certificate.`, + }, + + "cn_suffix": &framework.FieldSchema{ + Type: framework.TypeString, + Description: `A required suffix for the Common Name in the client certificate.`, + }, + "display_name": &framework.FieldSchema{ Type: framework.TypeString, Description: `The display name to use for clients using this @@ -139,6 +149,8 @@ func (b *backend) pathCertWrite( certificate := d.Get("certificate").(string) displayName := d.Get("display_name").(string) policies := policyutil.ParsePolicies(d.Get("policies").(string)) + cnPrefix := d.Get("cn_prefix").(string) + cnSuffix := d.Get("cn_suffix").(string) // Default the display name to the certificate name if not given if displayName == "" { @@ -169,6 +181,8 @@ func (b *backend) pathCertWrite( Certificate: certificate, DisplayName: displayName, Policies: policies, + CNPrefix: cnPrefix, + CNSuffix: cnSuffix, } // Parse the lease duration or default to backend/system default @@ -201,6 +215,8 @@ type CertEntry struct { DisplayName string Policies []string TTL time.Duration + CNPrefix string + CNSuffix string } const pathCertHelpSyn = ` diff --git a/builtin/credential/cert/path_login.go b/builtin/credential/cert/path_login.go index e4c82eda6e42..66921e9c34ac 100644 --- a/builtin/credential/cert/path_login.go +++ b/builtin/credential/cert/path_login.go @@ -207,7 +207,9 @@ func (b *backend) verifyCredentials(req *logical.Request) (*ParsedCert, *logical } func (b *backend) matchPolicy(clientCert *x509.Certificate, trustedChain []*x509.Certificate, config *ParsedCert) bool { - return !b.checkForChainInCRLs(trustedChain) + return !b.checkForChainInCRLs(trustedChain) && + strings.HasPrefix(clientCert.Subject.CommonName, config.Entry.CNPrefix) && + strings.HasSuffix(clientCert.Subject.CommonName, config.Entry.CNSuffix) } // loadTrustedCerts is used to load all the trusted certificates from the backend From 0abb476d058a61524f0aca252118e0f6564e8717 Mon Sep 17 00:00:00 2001 From: Michael Ansel Date: Sat, 15 Apr 2017 15:51:34 -0700 Subject: [PATCH 03/13] Maintain backwards compatibility (pick a random cert if multiple match) --- builtin/credential/cert/path_login.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/builtin/credential/cert/path_login.go b/builtin/credential/cert/path_login.go index 66921e9c34ac..34cfe8e82be0 100644 --- a/builtin/credential/cert/path_login.go +++ b/builtin/credential/cert/path_login.go @@ -197,12 +197,7 @@ func (b *backend) verifyCredentials(req *logical.Request) (*ParsedCert, *logical return nil, logical.ErrorResponse("no chain matching all constraints could be found for this login certificate"), nil } - // Fail on non-determinism - if len(trustedChains) > 1 { - return nil, logical.ErrorResponse("login certificate matched multiple configured roles; unsure which to apply"), nil - } - - // Return the sole matching entry + // Return the first matching entry (for backwards compatibility, we continue to just pick one if multiple match) return matches[0], nil, nil } From 8322e8556c7d2f64e91145f5b35c110be88b4e1b Mon Sep 17 00:00:00 2001 From: Michael Ansel Date: Sat, 15 Apr 2017 15:49:41 -0700 Subject: [PATCH 04/13] Vendor go-glob --- vendor/github.com/ryanuber/go-glob/LICENSE | 21 ++++++++ vendor/github.com/ryanuber/go-glob/README.md | 29 ++++++++++ vendor/github.com/ryanuber/go-glob/glob.go | 56 ++++++++++++++++++++ vendor/vendor.json | 6 +++ 4 files changed, 112 insertions(+) create mode 100644 vendor/github.com/ryanuber/go-glob/LICENSE create mode 100644 vendor/github.com/ryanuber/go-glob/README.md create mode 100644 vendor/github.com/ryanuber/go-glob/glob.go diff --git a/vendor/github.com/ryanuber/go-glob/LICENSE b/vendor/github.com/ryanuber/go-glob/LICENSE new file mode 100644 index 000000000000..bdfbd9514976 --- /dev/null +++ b/vendor/github.com/ryanuber/go-glob/LICENSE @@ -0,0 +1,21 @@ +The MIT License (MIT) + +Copyright (c) 2014 Ryan Uber + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/vendor/github.com/ryanuber/go-glob/README.md b/vendor/github.com/ryanuber/go-glob/README.md new file mode 100644 index 000000000000..48f7fcb05a44 --- /dev/null +++ b/vendor/github.com/ryanuber/go-glob/README.md @@ -0,0 +1,29 @@ +# String globbing in golang [![Build Status](https://travis-ci.org/ryanuber/go-glob.svg)](https://travis-ci.org/ryanuber/go-glob) + +`go-glob` is a single-function library implementing basic string glob support. + +Globs are an extremely user-friendly way of supporting string matching without +requiring knowledge of regular expressions or Go's particular regex engine. Most +people understand that if you put a `*` character somewhere in a string, it is +treated as a wildcard. Surprisingly, this functionality isn't found in Go's +standard library, except for `path.Match`, which is intended to be used while +comparing paths (not arbitrary strings), and contains specialized logic for this +use case. A better solution might be a POSIX basic (non-ERE) regular expression +engine for Go, which doesn't exist currently. + +Example +======= + +``` +package main + +import "github.com/ryanuber/go-glob" + +func main() { + glob.Glob("*World!", "Hello, World!") // true + glob.Glob("Hello,*", "Hello, World!") // true + glob.Glob("*ello,*", "Hello, World!") // true + glob.Glob("World!", "Hello, World!") // false + glob.Glob("/home/*", "/home/ryanuber/.bashrc") // true +} +``` diff --git a/vendor/github.com/ryanuber/go-glob/glob.go b/vendor/github.com/ryanuber/go-glob/glob.go new file mode 100644 index 000000000000..e67db3be183f --- /dev/null +++ b/vendor/github.com/ryanuber/go-glob/glob.go @@ -0,0 +1,56 @@ +package glob + +import "strings" + +// The character which is treated like a glob +const GLOB = "*" + +// Glob will test a string pattern, potentially containing globs, against a +// subject string. The result is a simple true/false, determining whether or +// not the glob pattern matched the subject text. +func Glob(pattern, subj string) bool { + // Empty pattern can only match empty subject + if pattern == "" { + return subj == pattern + } + + // If the pattern _is_ a glob, it matches everything + if pattern == GLOB { + return true + } + + parts := strings.Split(pattern, GLOB) + + if len(parts) == 1 { + // No globs in pattern, so test for equality + return subj == pattern + } + + leadingGlob := strings.HasPrefix(pattern, GLOB) + trailingGlob := strings.HasSuffix(pattern, GLOB) + end := len(parts) - 1 + + // Go over the leading parts and ensure they match. + for i := 0; i < end; i++ { + idx := strings.Index(subj, parts[i]) + + switch i { + case 0: + // Check the first section. Requires special handling. + if !leadingGlob && idx != 0 { + return false + } + default: + // Check that the middle parts match. + if idx < 0 { + return false + } + } + + // Trim evaluated text from subj as we loop over the pattern. + subj = subj[idx+len(parts[i]):] + } + + // Reached the last section. Requires special handling. + return trailingGlob || strings.HasSuffix(subj, parts[end]) +} diff --git a/vendor/vendor.json b/vendor/vendor.json index b16d636b0de2..d58c83499a17 100644 --- a/vendor/vendor.json +++ b/vendor/vendor.json @@ -1110,6 +1110,12 @@ "revision": "ddeb643de91b4ee0d9d87172c931a4ea3d81d49a", "revisionTime": "2017-02-08T17:17:27Z" }, + { + "checksumSHA1": "6JP37UqrI0H80Gpk0Y2P+KXgn5M=", + "path": "github.com/ryanuber/go-glob", + "revision": "256dc444b735e061061cf46c809487313d5b0065", + "revisionTime": "2017-01-28T01:21:29Z" + }, { "checksumSHA1": "5SYLEhADhdBVZAGPVHWggQl7H8k=", "path": "github.com/samuel/go-zookeeper/zk", From 7ac7d97d23e126f4d9a93af1887cd03e09766aa2 Mon Sep 17 00:00:00 2001 From: Michael Ansel Date: Sat, 15 Apr 2017 16:00:35 -0700 Subject: [PATCH 05/13] Replace cn_prefix/suffix with required_name/globbing Move all the new tests to acceptance-capable tests instead of embedding in the CRL test --- builtin/credential/cert/backend_test.go | 89 ++++++++++--------------- builtin/credential/cert/path_certs.go | 36 ++++------ builtin/credential/cert/path_login.go | 31 +++++++-- 3 files changed, 74 insertions(+), 82 deletions(-) diff --git a/builtin/credential/cert/backend_test.go b/builtin/credential/cert/backend_test.go index 87d6c071f17e..27286340997f 100644 --- a/builtin/credential/cert/backend_test.go +++ b/builtin/credential/cert/backend_test.go @@ -242,41 +242,6 @@ func TestBackend_CRLs(t *testing.T) { t.Fatalf("err:%v resp:%#v", err, resp) } - // Add a failing constraint on the Common Name - certData["cn_prefix"] = "invalid" - certData["cn_suffix"] = "" - - // And update the config - resp, err = b.HandleRequest(certReq) - if err != nil || (resp != nil && resp.IsError()) { - t.Fatalf("err:%v resp:%#v", err, resp) - } - - // Attempt login with the updated configuration - resp, err = b.HandleRequest(loginReq) - if err != nil { - t.Fatal(err) - } - if resp == nil || !resp.IsError() { - t.Fatalf("expected failure due to violating prefix/suffix constraints") - } - - // Add a passing constraint on the Common Name - certData["cn_prefix"] = "" - certData["cn_suffix"] = "myvault.com" - - // And update the config - resp, err = b.HandleRequest(certReq) - if err != nil || (resp != nil && resp.IsError()) { - t.Fatalf("err:%v resp:%#v", err, resp) - } - - // Attempt login with the updated configuration - resp, err = b.HandleRequest(loginReq) - if err != nil || (resp != nil && resp.IsError()) { - t.Fatalf("err:%v resp:%#v", err, resp) - } - // Register a CRL containing the issued client certificate used above. issuedCRL, err := ioutil.ReadFile(testIssuedCertCRL) if err != nil { @@ -383,9 +348,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"})...) @@ -403,13 +368,17 @@ 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), + testAccStepLogin(t, connState), + testAccStepCert(t, "web", ca, "foo", "*.invalid.com", false), + testAccStepLoginInvalid(t, connState), }, }) } @@ -440,10 +409,10 @@ func TestBackend_Basic_CRLs(t *testing.T) { }) } -// Test a self-signed client that is trusted +// Test a self-signed client (root CA) that is trusted func TestBackend_basic_singleCert(t *testing.T) { - connState := testConnState(t, "test-fixtures/keys/cert.pem", - "test-fixtures/keys/key.pem", "test-fixtures/root/rootcacert.pem") + connState := testConnState(t, "test-fixtures/root/rootcacert.pem", + "test-fixtures/root/rootcakey.pem", "test-fixtures/root/rootcacert.pem") ca, err := ioutil.ReadFile("test-fixtures/root/rootcacert.pem") if err != nil { t.Fatalf("err: %v", err) @@ -451,13 +420,17 @@ 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), testAccStepLogin(t, connState), + testAccStepCert(t, "web", ca, "foo", "invalid", false), + testAccStepLoginInvalid(t, connState), }, }) } -// Test an untrusted self-signed client +// Test an untrusted client func TestBackend_untrusted(t *testing.T) { connState := testConnState(t, "test-fixtures/keys/cert.pem", "test-fixtures/keys/key.pem", "test-fixtures/root/rootcacert.pem") @@ -607,16 +580,17 @@ func testAccStepListCerts( } func testAccStepCert( - t *testing.T, name string, cert []byte, policies string, expectError bool) logicaltest.TestStep { + t *testing.T, name string, cert []byte, policies string, requiredName 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, - "lease": 1000, + "certificate": string(cert), + "policies": policies, + "display_name": name, + "required_name": requiredName, + "lease": 1000, }, Check: func(resp *logical.Response) error { if resp == nil && expectError { @@ -765,10 +739,17 @@ func Test_Renew(t *testing.T) { t.Fatal(err) } - resp, err = b.pathLogin(req, nil) + empty_login_fd := &framework.FieldData{ + Raw: map[string]interface{}{}, + Schema: pathLogin(b).Fields, + } + resp, err = b.pathLogin(req, empty_login_fd) if err != nil { t.Fatal(err) } + if resp.IsError() { + t.Fatalf("got error: %#v", *resp) + } req.Auth.InternalData = resp.Auth.InternalData req.Auth.Metadata = resp.Auth.Metadata req.Auth.LeaseOptions = resp.Auth.LeaseOptions @@ -776,7 +757,7 @@ func Test_Renew(t *testing.T) { req.Auth.IssueTime = time.Now() // Normal renewal - resp, err = b.pathLoginRenew(req, nil) + resp, err = b.pathLoginRenew(req, empty_login_fd) if err != nil { t.Fatal(err) } @@ -794,7 +775,7 @@ func Test_Renew(t *testing.T) { t.Fatal(err) } - resp, err = b.pathLoginRenew(req, nil) + resp, err = b.pathLoginRenew(req, empty_login_fd) if err == nil { t.Fatal("expected error") } @@ -806,7 +787,7 @@ func Test_Renew(t *testing.T) { t.Fatal(err) } - resp, err = b.pathLoginRenew(req, nil) + resp, err = b.pathLoginRenew(req, empty_login_fd) if err != nil { t.Fatal(err) } @@ -823,7 +804,7 @@ func Test_Renew(t *testing.T) { t.Fatal(err) } - resp, err = b.pathLoginRenew(req, nil) + resp, err = b.pathLoginRenew(req, empty_login_fd) if err != nil { t.Fatal(err) } diff --git a/builtin/credential/cert/path_certs.go b/builtin/credential/cert/path_certs.go index 285c58cfaa0b..9f467ed9f7e6 100644 --- a/builtin/credential/cert/path_certs.go +++ b/builtin/credential/cert/path_certs.go @@ -39,14 +39,9 @@ func pathCerts(b *backend) *framework.Path { Must be x509 PEM encoded.`, }, - "cn_prefix": &framework.FieldSchema{ + "required_name": &framework.FieldSchema{ Type: framework.TypeString, - Description: `A required prefix for the Common Name in the client certificate.`, - }, - - "cn_suffix": &framework.FieldSchema{ - Type: framework.TypeString, - Description: `A required suffix for the Common Name in the client certificate.`, + Description: `A name that must exist in either the Common Name or SANs. Supports globbing.`, }, "display_name": &framework.FieldSchema{ @@ -149,8 +144,7 @@ func (b *backend) pathCertWrite( certificate := d.Get("certificate").(string) displayName := d.Get("display_name").(string) policies := policyutil.ParsePolicies(d.Get("policies").(string)) - cnPrefix := d.Get("cn_prefix").(string) - cnSuffix := d.Get("cn_suffix").(string) + requiredName := d.Get("required_name").(string) // Default the display name to the certificate name if not given if displayName == "" { @@ -177,12 +171,11 @@ func (b *backend) pathCertWrite( } certEntry := &CertEntry{ - Name: name, - Certificate: certificate, - DisplayName: displayName, - Policies: policies, - CNPrefix: cnPrefix, - CNSuffix: cnSuffix, + Name: name, + Certificate: certificate, + DisplayName: displayName, + Policies: policies, + RequiredName: requiredName, } // Parse the lease duration or default to backend/system default @@ -210,13 +203,12 @@ func (b *backend) pathCertWrite( } type CertEntry struct { - Name string - Certificate string - DisplayName string - Policies []string - TTL time.Duration - CNPrefix string - CNSuffix string + Name string + Certificate string + DisplayName string + Policies []string + TTL time.Duration + RequiredName string } const pathCertHelpSyn = ` diff --git a/builtin/credential/cert/path_login.go b/builtin/credential/cert/path_login.go index 34cfe8e82be0..870a4f4df767 100644 --- a/builtin/credential/cert/path_login.go +++ b/builtin/credential/cert/path_login.go @@ -14,6 +14,8 @@ import ( "github.com/hashicorp/vault/helper/policyutil" "github.com/hashicorp/vault/logical" "github.com/hashicorp/vault/logical/framework" + + "github.com/ryanuber/go-glob" ) // ParsedCert is a certificate that has been configured as trusted @@ -159,7 +161,7 @@ func (b *backend) verifyCredentials(req *logical.Request) (*ParsedCert, *logical // Check for client cert being explicitly listed in the config (and matching other constraints) if tCert.SerialNumber.Cmp(clientCert.SerialNumber) == 0 && bytes.Equal(tCert.AuthorityKeyId, clientCert.AuthorityKeyId) && - b.matchPolicy(connState.PeerCertificates[0], trustedNonCA.Certificates, trustedNonCA) { + b.matchesConstraints(connState.PeerCertificates[0], trustedNonCA.Certificates, trustedNonCA) { return trustedNonCA, nil, nil } } @@ -183,7 +185,7 @@ func (b *backend) verifyCredentials(req *logical.Request) (*ParsedCert, *logical for _, chain := range trustedChains { // For each root chain that we matched for _, cCert := range chain { // For each cert in the matched chain if tCert.Equal(cCert) && // ParsedCert intersects with matched chain - b.matchPolicy(connState.PeerCertificates[0], chain, trust) { // validate client cert + matched chain against the config + b.matchesConstraints(connState.PeerCertificates[0], chain, trust) { // validate client cert + matched chain against the config // Add the match to the list matches = append(matches, trust) } @@ -201,10 +203,27 @@ func (b *backend) verifyCredentials(req *logical.Request) (*ParsedCert, *logical return matches[0], nil, nil } -func (b *backend) matchPolicy(clientCert *x509.Certificate, trustedChain []*x509.Certificate, config *ParsedCert) bool { - return !b.checkForChainInCRLs(trustedChain) && - strings.HasPrefix(clientCert.Subject.CommonName, config.Entry.CNPrefix) && - strings.HasSuffix(clientCert.Subject.CommonName, config.Entry.CNSuffix) +func (b *backend) matchesConstraints(clientCert *x509.Certificate, trustedChain []*x509.Certificate, config *ParsedCert) bool { + nameMatched := (config.Entry.RequiredName == "") + if config.Entry.RequiredName != "" { + if glob.Glob(config.Entry.RequiredName, clientCert.Subject.CommonName) { + nameMatched = true + } + + for _, name := range clientCert.DNSNames { + if glob.Glob(config.Entry.RequiredName, name) { + nameMatched = true + } + } + + for _, name := range clientCert.EmailAddresses { + if glob.Glob(config.Entry.RequiredName, name) { + nameMatched = true + } + } + } + + return !b.checkForChainInCRLs(trustedChain) && nameMatched } // loadTrustedCerts is used to load all the trusted certificates from the backend From eef123e8aed39fa02001601557dc53915bc7fa36 Mon Sep 17 00:00:00 2001 From: Michael Ansel Date: Sat, 15 Apr 2017 16:03:18 -0700 Subject: [PATCH 06/13] Allow authenticating against a single cert --- builtin/credential/cert/backend_test.go | 40 +++++++++++++++++++++++++ builtin/credential/cert/path_login.go | 24 +++++++++++---- 2 files changed, 58 insertions(+), 6 deletions(-) diff --git a/builtin/credential/cert/backend_test.go b/builtin/credential/cert/backend_test.go index 27286340997f..906111d8a9d2 100644 --- a/builtin/credential/cert/backend_test.go +++ b/builtin/credential/cert/backend_test.go @@ -430,6 +430,28 @@ func TestBackend_basic_singleCert(t *testing.T) { }) } +// Test against a collection of matching and non-matching rules +func TestBackend_mixed_constraints(t *testing.T) { + connState := testConnState(t, "test-fixtures/keys/cert.pem", + "test-fixtures/keys/key.pem", "test-fixtures/root/rootcacert.pem") + 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, "1unconstrained", ca, "foo", "", false), + testAccStepCert(t, "2matching", ca, "foo", "*.example.com", 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"), + testAccStepLoginWithNameInvalid(t, connState, "3invalid"), + }, + }) +} + // Test an untrusted client func TestBackend_untrusted(t *testing.T) { connState := testConnState(t, "test-fixtures/keys/cert.pem", @@ -484,6 +506,10 @@ func testAccStepDeleteCRL(t *testing.T, connState tls.ConnectionState) logicalte } func testAccStepLogin(t *testing.T, connState tls.ConnectionState) logicaltest.TestStep { + return testAccStepLoginWithName(t, connState, "") +} + +func testAccStepLoginWithName(t *testing.T, connState tls.ConnectionState, certName string) logicaltest.TestStep { return logicaltest.TestStep{ Operation: logical.UpdateOperation, Path: "login", @@ -494,9 +520,16 @@ func testAccStepLogin(t *testing.T, connState tls.ConnectionState) logicaltest.T t.Fatalf("bad lease length: %#v", resp.Auth) } + if certName != "" && resp.Auth.DisplayName != ("mnt-"+certName) { + t.Fatalf("matched the wrong cert: %#v", resp.Auth.DisplayName) + } + fn := logicaltest.TestCheckAuth([]string{"default", "foo"}) return fn(resp) }, + Data: map[string]interface{}{ + "name": certName, + }, } } @@ -518,6 +551,10 @@ func testAccStepLoginDefaultLease(t *testing.T, connState tls.ConnectionState) l } func testAccStepLoginInvalid(t *testing.T, connState tls.ConnectionState) logicaltest.TestStep { + return testAccStepLoginWithNameInvalid(t, connState, "") +} + +func testAccStepLoginWithNameInvalid(t *testing.T, connState tls.ConnectionState, certName string) logicaltest.TestStep { return logicaltest.TestStep{ Operation: logical.UpdateOperation, Path: "login", @@ -529,6 +566,9 @@ func testAccStepLoginInvalid(t *testing.T, connState tls.ConnectionState) logica } return nil }, + Data: map[string]interface{}{ + "name": certName, + }, ErrorOk: true, } } diff --git a/builtin/credential/cert/path_login.go b/builtin/credential/cert/path_login.go index 870a4f4df767..73d182ea1b08 100644 --- a/builtin/credential/cert/path_login.go +++ b/builtin/credential/cert/path_login.go @@ -27,7 +27,12 @@ type ParsedCert struct { func pathLogin(b *backend) *framework.Path { return &framework.Path{ Pattern: "login", - Fields: map[string]*framework.FieldSchema{}, + Fields: map[string]*framework.FieldSchema{ + "name": &framework.FieldSchema{ + Type: framework.TypeString, + Description: "The name of the certificate to authenticate against.", + }, + }, Callbacks: map[logical.Operation]framework.OperationFunc{ logical.UpdateOperation: b.pathLogin, }, @@ -38,7 +43,7 @@ func (b *backend) pathLogin( req *logical.Request, data *framework.FieldData) (*logical.Response, error) { var matched *ParsedCert - if verifyResp, resp, err := b.verifyCredentials(req); err != nil { + if verifyResp, resp, err := b.verifyCredentials(req, data); err != nil { return nil, err } else if resp != nil { return resp, nil @@ -95,7 +100,7 @@ func (b *backend) pathLoginRenew( if !config.DisableBinding { var matched *ParsedCert - if verifyResp, resp, err := b.verifyCredentials(req); err != nil { + if verifyResp, resp, err := b.verifyCredentials(req, d); err != nil { return nil, err } else if resp != nil { return resp, nil @@ -138,7 +143,7 @@ func (b *backend) pathLoginRenew( return framework.LeaseExtend(cert.TTL, 0, b.System())(req, d) } -func (b *backend) verifyCredentials(req *logical.Request) (*ParsedCert, *logical.Response, error) { +func (b *backend) verifyCredentials(req *logical.Request, d *framework.FieldData) (*ParsedCert, *logical.Response, error) { // Get the connection state if req.Connection == nil || req.Connection.ConnState == nil { return nil, logical.ErrorResponse("tls connection required"), nil @@ -149,8 +154,11 @@ func (b *backend) verifyCredentials(req *logical.Request) (*ParsedCert, *logical return nil, logical.ErrorResponse("client certificate must be supplied"), nil } + // Allow constraining the login request to a single CertEntry + certName := d.Get("name").(string) + // Load the trusted certificates - roots, trusted, trustedNonCAs := b.loadTrustedCerts(req.Storage) + roots, trusted, trustedNonCAs := b.loadTrustedCerts(req.Storage, certName) // If trustedNonCAs is not empty it means that client had registered a non-CA cert // with the backend. @@ -227,7 +235,7 @@ func (b *backend) matchesConstraints(clientCert *x509.Certificate, trustedChain } // loadTrustedCerts is used to load all the trusted certificates from the backend -func (b *backend) loadTrustedCerts(store logical.Storage) (pool *x509.CertPool, trusted []*ParsedCert, trustedNonCAs []*ParsedCert) { +func (b *backend) loadTrustedCerts(store logical.Storage, certName string) (pool *x509.CertPool, trusted []*ParsedCert, trustedNonCAs []*ParsedCert) { pool = x509.NewCertPool() trusted = make([]*ParsedCert, 0) trustedNonCAs = make([]*ParsedCert, 0) @@ -237,6 +245,10 @@ func (b *backend) loadTrustedCerts(store logical.Storage) (pool *x509.CertPool, return } for _, name := range names { + // If we are trying to select a single CertEntry and this isn't it + if certName != "" && name != certName { + continue + } entry, err := b.Cert(store, strings.TrimPrefix(name, "cert/")) if err != nil { b.Logger().Error("cert: failed to load trusted cert", "name", name, "error", err) From 24b87c4c2353255b1798d9a00bd8da0f2ce9265e Mon Sep 17 00:00:00 2001 From: Michael Ansel Date: Mon, 17 Apr 2017 10:20:44 -0700 Subject: [PATCH 07/13] Add CLI support for new param --- builtin/credential/cert/cli.go | 9 ++++++++- builtin/credential/cert/path_login.go | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/builtin/credential/cert/cli.go b/builtin/credential/cert/cli.go index 4afc1eadc567..66809c2e3a8f 100644 --- a/builtin/credential/cert/cli.go +++ b/builtin/credential/cert/cli.go @@ -13,6 +13,7 @@ type CLIHandler struct{} func (h *CLIHandler) Auth(c *api.Client, m map[string]string) (string, error) { var data struct { Mount string `mapstructure:"mount"` + Name string `mapstructure:"name"` } if err := mapstructure.WeakDecode(m, &data); err != nil { return "", err @@ -22,8 +23,11 @@ func (h *CLIHandler) Auth(c *api.Client, m map[string]string) (string, error) { data.Mount = "cert" } + options := map[string]interface{}{ + "name": data.Name, + } path := fmt.Sprintf("auth/%s/login", data.Mount) - secret, err := c.Logical().Write(path, nil) + secret, err := c.Logical().Write(path, options) if err != nil { return "", err } @@ -38,10 +42,13 @@ func (h *CLIHandler) Help() string { help := ` The "cert" credential provider allows you to authenticate with a client certificate. No other authentication materials are needed. +Optionally, you may specify the specific certificate role to +authenticate against with the "name" parameter. Example: vault auth -method=cert \ -client-cert=/path/to/cert.pem \ -client-key=/path/to/key.pem + name=cert1 ` diff --git a/builtin/credential/cert/path_login.go b/builtin/credential/cert/path_login.go index 73d182ea1b08..fae01b383370 100644 --- a/builtin/credential/cert/path_login.go +++ b/builtin/credential/cert/path_login.go @@ -30,7 +30,7 @@ func pathLogin(b *backend) *framework.Path { Fields: map[string]*framework.FieldSchema{ "name": &framework.FieldSchema{ Type: framework.TypeString, - Description: "The name of the certificate to authenticate against.", + Description: "The name of the certificate role to authenticate against.", }, }, Callbacks: map[logical.Operation]framework.OperationFunc{ From 75011475988a87f7186224e8823835f1ed5c2caf Mon Sep 17 00:00:00 2001 From: Michael Ansel Date: Mon, 17 Apr 2017 10:20:18 -0700 Subject: [PATCH 08/13] Add new params to documentation --- website/source/docs/auth/cert.html.md | 36 ++++++++++++++++++++++----- 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/website/source/docs/auth/cert.html.md b/website/source/docs/auth/cert.html.md index a9e50c6c030d..f2a185e5f7df 100644 --- a/website/source/docs/auth/cert.html.md +++ b/website/source/docs/auth/cert.html.md @@ -60,18 +60,25 @@ it is up to the administrator to remove it from the backend. ## Authentication ### Via the CLI +The below requires Vault to present a certificate signed by `ca.pem` and +presents `cert.pem` (using `key.pem`) to authenticate against the `web` cert +role. If a certificate role name is not specified, the auth backend will try to +authenticate against all trusted certificates. + ``` $ vault auth -method=cert \ - -ca-cert=ca.pem -client-cert=cert.pem -client-key=key.pem + -ca-cert=ca.pem -client-cert=cert.pem -client-key=key.pem \ + name=web ``` ### Via the API The endpoint for the login is `/login`. The client simply connects with their TLS certificate and when the login endpoint is hit, the auth backend will determine -if there is a matching trusted certificate to authenticate the client. +if there is a matching trusted certificate to authenticate the client. Optionally, +you may specify a single certificate role to authenticate against. ``` -$ curl --cacert ca.pem --cert cert.pem --key key.pem \ +$ curl --cacert ca.pem --cert cert.pem --key key.pem -d name=web \ $VAULT_ADDR/v1/auth/cert/login -XPOST ``` @@ -175,6 +182,7 @@ of the header should be "X-Vault-Token" and the value should be the token. "certificate": "-----BEGIN CERTIFICATE-----\nMIIEtzCCA5+.......ZRtAfQ6r\nwlW975rYa1ZqEdA=\n-----END CERTIFICATE-----", "display_name": "test", "policies": "", + "required_name": "", "ttl": 2764800 }, "warnings": null, @@ -245,6 +253,14 @@ of the header should be "X-Vault-Token" and the value should be the token. required The PEM-format CA certificate. +
  • + required_name + optional + Constrain the Common and Alternative Names in the client certificate + with a [globbed pattern](https://github.com/ryanuber/go-glob/blob/master/README.md#example). + Authentication requires at least one Name matching the pattern. + If not set, defaults to allowing all names. +
  • policies optional @@ -382,8 +398,8 @@ of the header should be "X-Vault-Token" and the value should be the token.
    Description
    - Log in and fetch a token. If there is a valid chain to a CA configured in the backend, - a token will be issued. + Log in and fetch a token. If there is a valid chain to a CA configured in + the backend and all role constraints are matched, a token will be issued.
    Method
    @@ -394,7 +410,15 @@ of the header should be "X-Vault-Token" and the value should be the token.
    Parameters
    - None. +
      +
    • + name + optional + Authenticate against only the named certificate role, returning its + policy list if successful. If not set, defaults to trying all + certificate roles and returning any one that matches. +
      • +
    Returns
    From 92c2dc6a8145ccaa995c6e38b7867fe69dec15f2 Mon Sep 17 00:00:00 2001 From: Michael Ansel Date: Tue, 25 Apr 2017 14:47:28 -0700 Subject: [PATCH 09/13] Refactor for style --- builtin/credential/cert/path_login.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/builtin/credential/cert/path_login.go b/builtin/credential/cert/path_login.go index fae01b383370..827ce4417031 100644 --- a/builtin/credential/cert/path_login.go +++ b/builtin/credential/cert/path_login.go @@ -153,6 +153,7 @@ func (b *backend) verifyCredentials(req *logical.Request, d *framework.FieldData if connState.PeerCertificates == nil || len(connState.PeerCertificates) == 0 { return nil, logical.ErrorResponse("client certificate must be supplied"), nil } + clientCert := connState.PeerCertificates[0] // Allow constraining the login request to a single CertEntry certName := d.Get("name").(string) @@ -163,13 +164,12 @@ func (b *backend) verifyCredentials(req *logical.Request, d *framework.FieldData // If trustedNonCAs is not empty it means that client had registered a non-CA cert // with the backend. if len(trustedNonCAs) != 0 { - clientCert := connState.PeerCertificates[0] for _, trustedNonCA := range trustedNonCAs { tCert := trustedNonCA.Certificates[0] // Check for client cert being explicitly listed in the config (and matching other constraints) if tCert.SerialNumber.Cmp(clientCert.SerialNumber) == 0 && bytes.Equal(tCert.AuthorityKeyId, clientCert.AuthorityKeyId) && - b.matchesConstraints(connState.PeerCertificates[0], trustedNonCA.Certificates, trustedNonCA) { + b.matchesConstraints(clientCert, trustedNonCA.Certificates, trustedNonCA) { return trustedNonCA, nil, nil } } @@ -193,7 +193,7 @@ func (b *backend) verifyCredentials(req *logical.Request, d *framework.FieldData for _, chain := range trustedChains { // For each root chain that we matched for _, cCert := range chain { // For each cert in the matched chain if tCert.Equal(cCert) && // ParsedCert intersects with matched chain - b.matchesConstraints(connState.PeerCertificates[0], chain, trust) { // validate client cert + matched chain against the config + b.matchesConstraints(clientCert, chain, trust) { // validate client cert + matched chain against the config // Add the match to the list matches = append(matches, trust) } From 5aff4dd50880fb70611b3fa323e8a8d8ab4f4ae5 Mon Sep 17 00:00:00 2001 From: Michael Ansel Date: Tue, 25 Apr 2017 15:02:38 -0700 Subject: [PATCH 10/13] Support multiple (ORed) name patterns --- builtin/credential/cert/backend_test.go | 14 ++++++------- builtin/credential/cert/path_certs.go | 28 ++++++++++++------------- builtin/credential/cert/path_login.go | 27 +++++++++++++----------- website/source/docs/auth/cert.html.md | 7 ++++--- 4 files changed, 40 insertions(+), 36 deletions(-) diff --git a/builtin/credential/cert/backend_test.go b/builtin/credential/cert/backend_test.go index 906111d8a9d2..ab7dfde69327 100644 --- a/builtin/credential/cert/backend_test.go +++ b/builtin/credential/cert/backend_test.go @@ -442,7 +442,7 @@ func TestBackend_mixed_constraints(t *testing.T) { Backend: testFactory(t), Steps: []logicaltest.TestStep{ testAccStepCert(t, "1unconstrained", ca, "foo", "", false), - testAccStepCert(t, "2matching", ca, "foo", "*.example.com", 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 @@ -620,17 +620,17 @@ func testAccStepListCerts( } func testAccStepCert( - t *testing.T, name string, cert []byte, policies string, requiredName string, expectError bool) logicaltest.TestStep { + t *testing.T, name string, cert []byte, policies string, requiredNames 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, - "required_name": requiredName, - "lease": 1000, + "certificate": string(cert), + "policies": policies, + "display_name": name, + "required_names": requiredNames, + "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 9f467ed9f7e6..4ea763fb941f 100644 --- a/builtin/credential/cert/path_certs.go +++ b/builtin/credential/cert/path_certs.go @@ -39,9 +39,9 @@ func pathCerts(b *backend) *framework.Path { Must be x509 PEM encoded.`, }, - "required_name": &framework.FieldSchema{ + "required_names": &framework.FieldSchema{ Type: framework.TypeString, - Description: `A name that must exist in either the Common Name or SANs. Supports globbing.`, + Description: `A comma-separated list of names. At least one must exist in either the Common Name or SANs. Supports globbing.`, }, "display_name": &framework.FieldSchema{ @@ -144,7 +144,7 @@ func (b *backend) pathCertWrite( certificate := d.Get("certificate").(string) displayName := d.Get("display_name").(string) policies := policyutil.ParsePolicies(d.Get("policies").(string)) - requiredName := d.Get("required_name").(string) + requiredNames := d.Get("required_names").(string) // Default the display name to the certificate name if not given if displayName == "" { @@ -171,11 +171,11 @@ func (b *backend) pathCertWrite( } certEntry := &CertEntry{ - Name: name, - Certificate: certificate, - DisplayName: displayName, - Policies: policies, - RequiredName: requiredName, + Name: name, + Certificate: certificate, + DisplayName: displayName, + Policies: policies, + RequiredNames: requiredNames, } // Parse the lease duration or default to backend/system default @@ -203,12 +203,12 @@ func (b *backend) pathCertWrite( } type CertEntry struct { - Name string - Certificate string - DisplayName string - Policies []string - TTL time.Duration - RequiredName string + Name string + Certificate string + DisplayName string + Policies []string + TTL time.Duration + RequiredNames string } const pathCertHelpSyn = ` diff --git a/builtin/credential/cert/path_login.go b/builtin/credential/cert/path_login.go index 827ce4417031..5335f0406795 100644 --- a/builtin/credential/cert/path_login.go +++ b/builtin/credential/cert/path_login.go @@ -212,21 +212,24 @@ func (b *backend) verifyCredentials(req *logical.Request, d *framework.FieldData } func (b *backend) matchesConstraints(clientCert *x509.Certificate, trustedChain []*x509.Certificate, config *ParsedCert) bool { - nameMatched := (config.Entry.RequiredName == "") - if config.Entry.RequiredName != "" { - if glob.Glob(config.Entry.RequiredName, clientCert.Subject.CommonName) { - nameMatched = true - } - - for _, name := range clientCert.DNSNames { - if glob.Glob(config.Entry.RequiredName, name) { + nameMatched := (config.Entry.RequiredNames == "") + if config.Entry.RequiredNames != "" { + // At least one pattern must match at least one name if any patterns are specified + for _, requiredName := range strings.Split(config.Entry.RequiredNames, ",") { + if glob.Glob(requiredName, clientCert.Subject.CommonName) { nameMatched = true } - } - for _, name := range clientCert.EmailAddresses { - if glob.Glob(config.Entry.RequiredName, name) { - nameMatched = true + for _, name := range clientCert.DNSNames { + if glob.Glob(requiredName, name) { + nameMatched = true + } + } + + for _, name := range clientCert.EmailAddresses { + if glob.Glob(requiredName, name) { + nameMatched = true + } } } } diff --git a/website/source/docs/auth/cert.html.md b/website/source/docs/auth/cert.html.md index f2a185e5f7df..d3fe28c8e340 100644 --- a/website/source/docs/auth/cert.html.md +++ b/website/source/docs/auth/cert.html.md @@ -182,7 +182,7 @@ of the header should be "X-Vault-Token" and the value should be the token. "certificate": "-----BEGIN CERTIFICATE-----\nMIIEtzCCA5+.......ZRtAfQ6r\nwlW975rYa1ZqEdA=\n-----END CERTIFICATE-----", "display_name": "test", "policies": "", - "required_name": "", + "required_names": "", "ttl": 2764800 }, "warnings": null, @@ -254,11 +254,12 @@ of the header should be "X-Vault-Token" and the value should be the token. The PEM-format CA certificate.
  • - required_name + required_names optional Constrain the Common and Alternative Names in the client certificate with a [globbed pattern](https://github.com/ryanuber/go-glob/blob/master/README.md#example). - Authentication requires at least one Name matching the pattern. + 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.
  • From b87b13e59c3e94ff6b3c0f87a70ba3b913e380b1 Mon Sep 17 00:00:00 2001 From: Michael Ansel Date: Fri, 28 Apr 2017 10:20:00 -0700 Subject: [PATCH 11/13] Rename required_names to allowed_names --- builtin/credential/cert/backend_test.go | 12 ++++++------ builtin/credential/cert/path_certs.go | 26 ++++++++++++------------- builtin/credential/cert/path_login.go | 12 ++++++------ 3 files changed, 25 insertions(+), 25 deletions(-) diff --git a/builtin/credential/cert/backend_test.go b/builtin/credential/cert/backend_test.go index ab7dfde69327..f96c9cb868c8 100644 --- a/builtin/credential/cert/backend_test.go +++ b/builtin/credential/cert/backend_test.go @@ -620,17 +620,17 @@ func testAccStepListCerts( } func testAccStepCert( - t *testing.T, name string, cert []byte, policies string, requiredNames string, expectError bool) logicaltest.TestStep { + t *testing.T, name string, cert []byte, policies string, allowedNames 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, - "required_names": requiredNames, - "lease": 1000, + "certificate": string(cert), + "policies": policies, + "display_name": name, + "allowed_names": allowedNames, + "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 4ea763fb941f..c7d5d8ae2291 100644 --- a/builtin/credential/cert/path_certs.go +++ b/builtin/credential/cert/path_certs.go @@ -39,7 +39,7 @@ func pathCerts(b *backend) *framework.Path { Must be x509 PEM encoded.`, }, - "required_names": &framework.FieldSchema{ + "allowed_names": &framework.FieldSchema{ Type: framework.TypeString, Description: `A comma-separated list of names. At least one must exist in either the Common Name or SANs. Supports globbing.`, }, @@ -144,7 +144,7 @@ func (b *backend) pathCertWrite( certificate := d.Get("certificate").(string) displayName := d.Get("display_name").(string) policies := policyutil.ParsePolicies(d.Get("policies").(string)) - requiredNames := d.Get("required_names").(string) + allowedNames := d.Get("allowed_names").(string) // Default the display name to the certificate name if not given if displayName == "" { @@ -171,11 +171,11 @@ func (b *backend) pathCertWrite( } certEntry := &CertEntry{ - Name: name, - Certificate: certificate, - DisplayName: displayName, - Policies: policies, - RequiredNames: requiredNames, + Name: name, + Certificate: certificate, + DisplayName: displayName, + Policies: policies, + AllowedNames: allowedNames, } // Parse the lease duration or default to backend/system default @@ -203,12 +203,12 @@ func (b *backend) pathCertWrite( } type CertEntry struct { - Name string - Certificate string - DisplayName string - Policies []string - TTL time.Duration - RequiredNames string + Name string + Certificate string + DisplayName string + Policies []string + TTL time.Duration + AllowedNames string } const pathCertHelpSyn = ` diff --git a/builtin/credential/cert/path_login.go b/builtin/credential/cert/path_login.go index 5335f0406795..6eccf35c1b16 100644 --- a/builtin/credential/cert/path_login.go +++ b/builtin/credential/cert/path_login.go @@ -212,22 +212,22 @@ func (b *backend) verifyCredentials(req *logical.Request, d *framework.FieldData } func (b *backend) matchesConstraints(clientCert *x509.Certificate, trustedChain []*x509.Certificate, config *ParsedCert) bool { - nameMatched := (config.Entry.RequiredNames == "") - if config.Entry.RequiredNames != "" { + nameMatched := (config.Entry.AllowedNames == "") + if config.Entry.AllowedNames != "" { // At least one pattern must match at least one name if any patterns are specified - for _, requiredName := range strings.Split(config.Entry.RequiredNames, ",") { - if glob.Glob(requiredName, clientCert.Subject.CommonName) { + for _, allowedName := range strings.Split(config.Entry.AllowedNames, ",") { + if glob.Glob(allowedName, clientCert.Subject.CommonName) { nameMatched = true } for _, name := range clientCert.DNSNames { - if glob.Glob(requiredName, name) { + if glob.Glob(allowedName, name) { nameMatched = true } } for _, name := range clientCert.EmailAddresses { - if glob.Glob(requiredName, name) { + if glob.Glob(allowedName, name) { nameMatched = true } } From f150f3066d5d2895d2d9b7c8e4f03e7a2371f237 Mon Sep 17 00:00:00 2001 From: Michael Ansel Date: Fri, 28 Apr 2017 10:42:38 -0700 Subject: [PATCH 12/13] Update docs for parameter rename --- website/source/docs/auth/cert.html.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/website/source/docs/auth/cert.html.md b/website/source/docs/auth/cert.html.md index d3fe28c8e340..4f094053e29d 100644 --- a/website/source/docs/auth/cert.html.md +++ b/website/source/docs/auth/cert.html.md @@ -182,7 +182,7 @@ of the header should be "X-Vault-Token" and the value should be the token. "certificate": "-----BEGIN CERTIFICATE-----\nMIIEtzCCA5+.......ZRtAfQ6r\nwlW975rYa1ZqEdA=\n-----END CERTIFICATE-----", "display_name": "test", "policies": "", - "required_names": "", + "allowed_names": "", "ttl": 2764800 }, "warnings": null, @@ -254,7 +254,7 @@ of the header should be "X-Vault-Token" and the value should be the token. The PEM-format CA certificate.
  • - required_names + allowed_names optional Constrain the Common and Alternative Names in the client certificate with a [globbed pattern](https://github.com/ryanuber/go-glob/blob/master/README.md#example). From 0697d16bb7aa921f7833727097c3a6600f646f34 Mon Sep 17 00:00:00 2001 From: Michael Ansel Date: Fri, 28 Apr 2017 10:22:22 -0700 Subject: [PATCH 13/13] Use the new TypeCommaStringSlice --- builtin/credential/cert/path_certs.go | 9 +++++---- builtin/credential/cert/path_login.go | 29 +++++++++++++-------------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/builtin/credential/cert/path_certs.go b/builtin/credential/cert/path_certs.go index c7d5d8ae2291..2c002f6e3f3f 100644 --- a/builtin/credential/cert/path_certs.go +++ b/builtin/credential/cert/path_certs.go @@ -40,8 +40,9 @@ Must be x509 PEM encoded.`, }, "allowed_names": &framework.FieldSchema{ - Type: framework.TypeString, - Description: `A comma-separated list of names. At least one must exist in either the Common Name or SANs. Supports globbing.`, + Type: framework.TypeCommaStringSlice, + Description: `A comma-separated list of names. +At least one must exist in either the Common Name or SANs. Supports globbing.`, }, "display_name": &framework.FieldSchema{ @@ -144,7 +145,7 @@ func (b *backend) pathCertWrite( certificate := d.Get("certificate").(string) displayName := d.Get("display_name").(string) policies := policyutil.ParsePolicies(d.Get("policies").(string)) - allowedNames := d.Get("allowed_names").(string) + allowedNames := d.Get("allowed_names").([]string) // Default the display name to the certificate name if not given if displayName == "" { @@ -208,7 +209,7 @@ type CertEntry struct { DisplayName string Policies []string TTL time.Duration - AllowedNames string + AllowedNames []string } const pathCertHelpSyn = ` diff --git a/builtin/credential/cert/path_login.go b/builtin/credential/cert/path_login.go index 6eccf35c1b16..164bbe7de7b6 100644 --- a/builtin/credential/cert/path_login.go +++ b/builtin/credential/cert/path_login.go @@ -212,24 +212,23 @@ func (b *backend) verifyCredentials(req *logical.Request, d *framework.FieldData } func (b *backend) matchesConstraints(clientCert *x509.Certificate, trustedChain []*x509.Certificate, config *ParsedCert) bool { - nameMatched := (config.Entry.AllowedNames == "") - if config.Entry.AllowedNames != "" { - // At least one pattern must match at least one name if any patterns are specified - for _, allowedName := range strings.Split(config.Entry.AllowedNames, ",") { - if glob.Glob(allowedName, clientCert.Subject.CommonName) { - nameMatched = true - } + // Default behavior (no names) is to allow all names + nameMatched := len(config.Entry.AllowedNames) == 0 + // 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 + } - for _, name := range clientCert.DNSNames { - if glob.Glob(allowedName, name) { - nameMatched = true - } + for _, name := range clientCert.DNSNames { + if glob.Glob(allowedName, name) { + nameMatched = true } + } - for _, name := range clientCert.EmailAddresses { - if glob.Glob(allowedName, name) { - nameMatched = true - } + for _, name := range clientCert.EmailAddresses { + if glob.Glob(allowedName, name) { + nameMatched = true } } }