From a1835a2c2ca1108e58f03fabccda52ba67bf15ec Mon Sep 17 00:00:00 2001 From: Denis Subbotin Date: Mon, 28 Oct 2019 19:31:56 +0300 Subject: [PATCH] Don't allow duplicate SAN names in PKI-issued certs (#7605) * fix https://github.com/hashicorp/vault/issues/6571 * fix test TestBackend_OID_SANs because now SANs are alphabetic sorted --- builtin/logical/pki/backend_test.go | 30 ++++++++++++++++------------- builtin/logical/pki/cert_util.go | 4 ++-- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/builtin/logical/pki/backend_test.go b/builtin/logical/pki/backend_test.go index 7a05ce434fc8..a636db5ee6ea 100644 --- a/builtin/logical/pki/backend_test.go +++ b/builtin/logical/pki/backend_test.go @@ -2170,7 +2170,7 @@ func TestBackend_OID_SANs(t *testing.T) { resp, err = client.Logical().Write("root/issue/test", map[string]interface{}{ "common_name": "foobar.com", "ip_sans": "1.2.3.4", - "alt_names": "foo.foobar.com,bar.foobar.com", + "alt_names": "foobar.com,foo.foobar.com,bar.foobar.com", "ttl": "1h", }) if err != nil { @@ -2185,9 +2185,10 @@ func TestBackend_OID_SANs(t *testing.T) { if cert.IPAddresses[0].String() != "1.2.3.4" { t.Fatalf("unexpected IP SAN %q", cert.IPAddresses[0].String()) } - if cert.DNSNames[0] != "foobar.com" || - cert.DNSNames[1] != "bar.foobar.com" || - cert.DNSNames[2] != "foo.foobar.com" { + if len(cert.DNSNames) != 3 || + cert.DNSNames[0] != "bar.foobar.com" || + cert.DNSNames[1] != "foo.foobar.com" || + cert.DNSNames[2] != "foobar.com" { t.Fatalf("unexpected DNS SANs %v", cert.DNSNames) } @@ -2272,9 +2273,10 @@ func TestBackend_OID_SANs(t *testing.T) { if cert.IPAddresses[0].String() != "1.2.3.4" { t.Fatalf("unexpected IP SAN %q", cert.IPAddresses[0].String()) } - if cert.DNSNames[0] != "foobar.com" || - cert.DNSNames[1] != "bar.foobar.com" || - cert.DNSNames[2] != "foo.foobar.com" { + if len(cert.DNSNames) != 3 || + cert.DNSNames[0] != "bar.foobar.com" || + cert.DNSNames[1] != "foo.foobar.com" || + cert.DNSNames[2] != "foobar.com" { t.Fatalf("unexpected DNS SANs %v", cert.DNSNames) } t.Logf("certificate 1 to check:\n%s", certStr) @@ -2299,9 +2301,10 @@ func TestBackend_OID_SANs(t *testing.T) { if cert.IPAddresses[0].String() != "1.2.3.4" { t.Fatalf("unexpected IP SAN %q", cert.IPAddresses[0].String()) } - if cert.DNSNames[0] != "foobar.com" || - cert.DNSNames[1] != "bar.foobar.com" || - cert.DNSNames[2] != "foo.foobar.com" { + if len(cert.DNSNames) != 3 || + cert.DNSNames[0] != "bar.foobar.com" || + cert.DNSNames[1] != "foo.foobar.com" || + cert.DNSNames[2] != "foobar.com" { t.Fatalf("unexpected DNS SANs %v", cert.DNSNames) } t.Logf("certificate 2 to check:\n%s", certStr) @@ -2326,9 +2329,10 @@ func TestBackend_OID_SANs(t *testing.T) { if cert.IPAddresses[0].String() != "1.2.3.4" { t.Fatalf("unexpected IP SAN %q", cert.IPAddresses[0].String()) } - if cert.DNSNames[0] != "foobar.com" || - cert.DNSNames[1] != "bar.foobar.com" || - cert.DNSNames[2] != "foo.foobar.com" { + if len(cert.DNSNames) != 3 || + cert.DNSNames[0] != "bar.foobar.com" || + cert.DNSNames[1] != "foo.foobar.com" || + cert.DNSNames[2] != "foobar.com" { t.Fatalf("unexpected DNS SANs %v", cert.DNSNames) } t.Logf("certificate 3 to check:\n%s", certStr) diff --git a/builtin/logical/pki/cert_util.go b/builtin/logical/pki/cert_util.go index b82c28539f8d..94030f7f85cf 100644 --- a/builtin/logical/pki/cert_util.go +++ b/builtin/logical/pki/cert_util.go @@ -906,8 +906,8 @@ func generateCreationBundle(b *backend, data *inputBundle, caSign *certutil.CAIn creation := &certutil.CreationBundle{ Params: &certutil.CreationParameters{ Subject: subject, - DNSNames: dnsNames, - EmailAddresses: emailAddresses, + DNSNames: strutil.RemoveDuplicates(dnsNames, false), + EmailAddresses: strutil.RemoveDuplicates(emailAddresses, false), IPAddresses: ipAddresses, URIs: URIs, OtherSANs: otherSANs,