From 3eac8fe5135462a379c986afa5524f75306a1b76 Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Tue, 13 Feb 2018 13:06:27 -0500 Subject: [PATCH 1/8] Verify DNS SANs if PermittedDNSDomains is set --- builtin/credential/cert/path_login.go | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/builtin/credential/cert/path_login.go b/builtin/credential/cert/path_login.go index d37e6c62300a..8d6a27268391 100644 --- a/builtin/credential/cert/path_login.go +++ b/builtin/credential/cert/path_login.go @@ -439,12 +439,29 @@ func validateConnState(roots *x509.CertPool, cs *tls.ConnectionState) ([][]*x509 } } - chains, err := certs[0].Verify(opts) - if err != nil { - if _, ok := err.(x509.UnknownAuthorityError); ok { - return nil, nil + var chains [][]*x509.Certificate + var err error + switch { + case len(certs[0].PermittedDNSDomains) > 0: + for _, dnsName := range certs[0].DNSNames { + opts.DNSName = dnsName + chains, err = certs[0].Verify(opts) + if err != nil { + if _, ok := err.(x509.UnknownAuthorityError); ok { + return nil, nil + } + return nil, errors.New("failed to verify client's certificate: " + err.Error()) + } + } + default: + chains, err = certs[0].Verify(opts) + if err != nil { + if _, ok := err.(x509.UnknownAuthorityError); ok { + return nil, nil + } + return nil, errors.New("failed to verify client's certificate: " + err.Error()) } - return nil, errors.New("failed to verify client's certificate: " + err.Error()) } + return chains, nil } From 5293379c8ec033c743764aa7169f434b22bda317 Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Wed, 14 Feb 2018 16:53:58 -0500 Subject: [PATCH 2/8] Use DNSNames check and not PermittedDNSDomains on leaf certificate --- builtin/credential/cert/path_login.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/credential/cert/path_login.go b/builtin/credential/cert/path_login.go index 8d6a27268391..51f7b59fb526 100644 --- a/builtin/credential/cert/path_login.go +++ b/builtin/credential/cert/path_login.go @@ -442,7 +442,7 @@ func validateConnState(roots *x509.CertPool, cs *tls.ConnectionState) ([][]*x509 var chains [][]*x509.Certificate var err error switch { - case len(certs[0].PermittedDNSDomains) > 0: + case len(certs[0].DNSNames) > 0: for _, dnsName := range certs[0].DNSNames { opts.DNSName = dnsName chains, err = certs[0].Verify(opts) From 80700db14bccbb561839cab098824f0c59061b0e Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Wed, 14 Feb 2018 17:59:12 -0500 Subject: [PATCH 3/8] Document the check --- website/source/api/auth/cert/index.html.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/website/source/api/auth/cert/index.html.md b/website/source/api/auth/cert/index.html.md index 7bba0ba32467..1b29b73f4f65 100644 --- a/website/source/api/auth/cert/index.html.md +++ b/website/source/api/auth/cert/index.html.md @@ -299,7 +299,10 @@ $ curl \ ## Login with TLS Certificate Method Log in and fetch a token. If there is a valid chain to a CA configured in the -method and all role constraints are matched, a token will be issued. +method and all role constraints are matched, a token will be issued. If the +certificate has DNS SANs in it, each of those will be verified. If Common Name +is required to be verified, then it should be a fully qualified DNS domain name +and must be duplicated as a DNS SAN. | Method | Path | Produces | | :------- | :--------------------------- | :--------------------- | From da537d978e110649f5213b2b93d2809422553af6 Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Wed, 14 Feb 2018 20:12:24 -0500 Subject: [PATCH 4/8] Add RFC link --- website/source/api/auth/cert/index.html.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/website/source/api/auth/cert/index.html.md b/website/source/api/auth/cert/index.html.md index 1b29b73f4f65..f1c6e4e998e6 100644 --- a/website/source/api/auth/cert/index.html.md +++ b/website/source/api/auth/cert/index.html.md @@ -302,7 +302,8 @@ Log in and fetch a token. If there is a valid chain to a CA configured in the method and all role constraints are matched, a token will be issued. If the certificate has DNS SANs in it, each of those will be verified. If Common Name is required to be verified, then it should be a fully qualified DNS domain name -and must be duplicated as a DNS SAN. +and must be duplicated as a DNS SAN (see +https://tools.ietf.org/html/rfc6125#section-2.3) | Method | Path | Produces | | :------- | :--------------------------- | :--------------------- | From a2e52fc6dca396a3f8010b9b4edccf28657243ff Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Thu, 15 Feb 2018 18:24:32 -0500 Subject: [PATCH 5/8] Test for success case --- builtin/credential/cert/backend_test.go | 227 ++++++++++++++++++++++++ 1 file changed, 227 insertions(+) diff --git a/builtin/credential/cert/backend_test.go b/builtin/credential/cert/backend_test.go index 62f1b4646081..7ee692c787ba 100644 --- a/builtin/credential/cert/backend_test.go +++ b/builtin/credential/cert/backend_test.go @@ -3,6 +3,10 @@ package cert import ( "context" "crypto/rand" + "net/http" + + "golang.org/x/net/http2" + "crypto/rsa" "crypto/tls" "crypto/x509" @@ -17,11 +21,19 @@ import ( "testing" "time" + logxi "github.com/mgutz/logxi/v1" + + cleanhttp "github.com/hashicorp/go-cleanhttp" + "github.com/hashicorp/vault/api" + vaulthttp "github.com/hashicorp/vault/http" + "github.com/hashicorp/go-rootcerts" + "github.com/hashicorp/vault/builtin/logical/pki" "github.com/hashicorp/vault/helper/certutil" "github.com/hashicorp/vault/logical" "github.com/hashicorp/vault/logical/framework" logicaltest "github.com/hashicorp/vault/logical/testing" + "github.com/hashicorp/vault/vault" "github.com/mitchellh/mapstructure" ) @@ -142,6 +154,221 @@ func connectionState(serverCAPath, serverCertPath, serverKeyPath, clientCertPath return <-connState, nil } +func TestBackend_DNSNameVerification(t *testing.T) { + // Enable PKI secret engine and Cert auth method + coreConfig := &vault.CoreConfig{ + DisableMlock: true, + DisableCache: true, + Logger: logxi.NullLog, + CredentialBackends: map[string]logical.Factory{ + "cert": Factory, + }, + LogicalBackends: map[string]logical.Factory{ + "pki": pki.Factory, + }, + } + cluster := vault.NewTestCluster(t, coreConfig, &vault.TestClusterOptions{ + HandlerFunc: vaulthttp.Handler, + }) + cluster.Start() + defer cluster.Cleanup() + cores := cluster.Cores + vault.TestWaitActive(t, cores[0].Core) + client := cores[0].Client + + var err error + + // Mount /pki as a root CA + err = client.Sys().Mount("pki", &api.MountInput{ + Type: "pki", + Config: api.MountConfigInput{ + DefaultLeaseTTL: "16h", + MaxLeaseTTL: "32h", + }, + }) + if err != nil { + t.Fatal(err) + } + + // Set the cluster's certificate as the root CA in /pki + pemBundleRootCA := string(cluster.CACertPEM) + string(cluster.CAKeyPEM) + _, err = client.Logical().Write("pki/config/ca", map[string]interface{}{ + "pem_bundle": pemBundleRootCA, + }) + if err != nil { + t.Fatal(err) + } + + // Mount /pki2 to operate as an intermediate CA + err = client.Sys().Mount("pki2", &api.MountInput{ + Type: "pki", + Config: api.MountConfigInput{ + DefaultLeaseTTL: "16h", + MaxLeaseTTL: "32h", + }, + }) + if err != nil { + t.Fatal(err) + } + + // Create a CSR for the intermediate CA + secret, err := client.Logical().Write("pki2/intermediate/generate/internal", nil) + if err != nil { + t.Fatal(err) + } + intermediateCSR := secret.Data["csr"].(string) + + // Sign the intermediate CSR using /pki + secret, err = client.Logical().Write("pki/root/sign-intermediate", map[string]interface{}{ + "permitted_dns_names": "myvault.com", + "csr": intermediateCSR, + }) + if err != nil { + t.Fatal(err) + } + intermediateCertPEM := secret.Data["certificate"].(string) + + // Configure the intermediate cert as the CA in /pki2 + _, err = client.Logical().Write("pki2/intermediate/set-signed", map[string]interface{}{ + "certificate": intermediateCertPEM, + }) + if err != nil { + t.Fatal(err) + } + + // Create a role on the intermediate CA mount + _, err = client.Logical().Write("pki2/roles/myvault-dot-com", map[string]interface{}{ + "allowed_domains": "myvault.com", + "allow_subdomains": "true", + "max_ttl": "5m", + }) + if err != nil { + t.Fatal(err) + } + + // Issue a leaf cert using the intermediate CA + secret, err = client.Logical().Write("pki2/issue/myvault-dot-com", map[string]interface{}{ + "common_name": "cert.myvault.com", + "format": "pem", + "ip_sans": "127.0.0.1", + }) + if err != nil { + t.Fatal(err) + } + leafCertPEM := secret.Data["certificate"].(string) + leafCertKeyPEM := secret.Data["private_key"].(string) + + // Enable the cert auth method + err = client.Sys().EnableAuthWithOptions("cert", &api.EnableAuthOptions{ + Type: "cert", + }) + if err != nil { + t.Fatal(err) + } + + // Set the intermediate CA cert as a trusted certificate in the backend + _, err = client.Logical().Write("auth/cert/certs/myvault-dot-com", map[string]interface{}{ + "display_name": "myvault.com", + "policies": "default", + "certificate": intermediateCertPEM, + }) + if err != nil { + t.Fatal(err) + } + + // Create temporary files for CA cert, client cert and client cert key. + // This is used to configure TLS in the api client. + caCertFile, err := ioutil.TempFile("", "caCert") + if err != nil { + t.Fatal(err) + } + defer os.Remove(caCertFile.Name()) + if _, err := caCertFile.Write([]byte(cluster.CACertPEM)); err != nil { + t.Fatal(err) + } + if err := caCertFile.Close(); err != nil { + t.Fatal(err) + } + + leafCertFile, err := ioutil.TempFile("", "leafCert") + if err != nil { + t.Fatal(err) + } + defer os.Remove(leafCertFile.Name()) + if _, err := leafCertFile.Write([]byte(leafCertPEM)); err != nil { + t.Fatal(err) + } + if err := leafCertFile.Close(); err != nil { + t.Fatal(err) + } + + leafCertKeyFile, err := ioutil.TempFile("", "leafCertKey") + if err != nil { + t.Fatal(err) + } + defer os.Remove(leafCertKeyFile.Name()) + if _, err := leafCertKeyFile.Write([]byte(leafCertKeyPEM)); err != nil { + t.Fatal(err) + } + if err := leafCertKeyFile.Close(); err != nil { + t.Fatal(err) + } + + // This function is a copy-pasta from the NewTestCluster, with the + // modification to reconfigure the TLS on the api client with the leaf + // certificates generated above. + getAPIClient := func(port int, tlsConfig *tls.Config) *api.Client { + transport := cleanhttp.DefaultPooledTransport() + transport.TLSClientConfig = tlsConfig.Clone() + if err := http2.ConfigureTransport(transport); err != nil { + t.Fatal(err) + } + client := &http.Client{ + Transport: transport, + CheckRedirect: func(*http.Request, []*http.Request) error { + // This can of course be overridden per-test by using its own client + return fmt.Errorf("redirects not allowed in these tests") + }, + } + config := api.DefaultConfig() + if config.Error != nil { + t.Fatal(config.Error) + } + config.Address = fmt.Sprintf("https://127.0.0.1:%d", port) + config.HttpClient = client + + // Set the above issued certificates as the client certificates + config.ConfigureTLS(&api.TLSConfig{ + CACert: caCertFile.Name(), + ClientCert: leafCertFile.Name(), + ClientKey: leafCertKeyFile.Name(), + }) + + apiClient, err := api.NewClient(config) + if err != nil { + t.Fatal(err) + } + return apiClient + } + + // Create a new api client with the desired TLS configuration + newClient := getAPIClient(cores[0].Listeners[0].Address.Port, cores[0].TLSConfig) + + // Copy the token from the old client + //newClient.SetToken(client.Token()) + + // Set the intermediate CA cert as a trusted certificate in the backend + secret, err = newClient.Logical().Write("auth/cert/login", map[string]interface{}{ + "name": "myvault-dot-com", + }) + if err != nil { + t.Fatal(err) + } + if secret.Auth == nil || secret.Auth.ClientToken == "" { + t.Fatalf("expected a successful authentication") + } +} + func TestBackend_NonCAExpiry(t *testing.T) { var resp *logical.Response var err error From a15330f1d40a310b334596b895ebc1743b8fd564 Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Thu, 15 Feb 2018 18:55:22 -0500 Subject: [PATCH 6/8] fix the parameter 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 7ee692c787ba..6b78a625716a 100644 --- a/builtin/credential/cert/backend_test.go +++ b/builtin/credential/cert/backend_test.go @@ -220,7 +220,7 @@ func TestBackend_DNSNameVerification(t *testing.T) { // Sign the intermediate CSR using /pki secret, err = client.Logical().Write("pki/root/sign-intermediate", map[string]interface{}{ - "permitted_dns_names": "myvault.com", + "permitted_dns_domains": ".myvault.com", "csr": intermediateCSR, }) if err != nil { From 74615df30ff1329395f579b0dd50d28762847cef Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Thu, 15 Feb 2018 19:01:28 -0500 Subject: [PATCH 7/8] rename the test --- 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 6b78a625716a..c399b332bafd 100644 --- a/builtin/credential/cert/backend_test.go +++ b/builtin/credential/cert/backend_test.go @@ -154,7 +154,7 @@ func connectionState(serverCAPath, serverCertPath, serverKeyPath, clientCertPath return <-connState, nil } -func TestBackend_DNSNameVerification(t *testing.T) { +func TestBackend_PermittedDNSDomainsIntermediateCA(t *testing.T) { // Enable PKI secret engine and Cert auth method coreConfig := &vault.CoreConfig{ DisableMlock: true, From fb18fdac2bd24b04ff4450a5963adb7aabaeb128 Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Fri, 16 Feb 2018 11:30:01 -0500 Subject: [PATCH 8/8] remove unneeded commented code --- builtin/credential/cert/backend_test.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/builtin/credential/cert/backend_test.go b/builtin/credential/cert/backend_test.go index c399b332bafd..1e23ee007588 100644 --- a/builtin/credential/cert/backend_test.go +++ b/builtin/credential/cert/backend_test.go @@ -316,7 +316,7 @@ func TestBackend_PermittedDNSDomainsIntermediateCA(t *testing.T) { // This function is a copy-pasta from the NewTestCluster, with the // modification to reconfigure the TLS on the api client with the leaf - // certificates generated above. + // certificate generated above. getAPIClient := func(port int, tlsConfig *tls.Config) *api.Client { transport := cleanhttp.DefaultPooledTransport() transport.TLSClientConfig = tlsConfig.Clone() @@ -354,9 +354,6 @@ func TestBackend_PermittedDNSDomainsIntermediateCA(t *testing.T) { // Create a new api client with the desired TLS configuration newClient := getAPIClient(cores[0].Listeners[0].Address.Port, cores[0].TLSConfig) - // Copy the token from the old client - //newClient.SetToken(client.Token()) - // Set the intermediate CA cert as a trusted certificate in the backend secret, err = newClient.Logical().Write("auth/cert/login", map[string]interface{}{ "name": "myvault-dot-com",