From 444cdeb8fb0f19b501c4bd5a9c2ac5f80ad116f8 Mon Sep 17 00:00:00 2001 From: Hans Hasselberg Date: Wed, 27 Jan 2021 08:52:15 +0100 Subject: [PATCH] Add flags to support CA generation for Connect (#9585) --- .changelog/9585.txt | 3 + agent/consul/server_test.go | 22 +++---- agent/pool/peek_test.go | 15 ++--- agent/routine-leak-checker/leak_test.go | 21 +++---- agent/testagent.go | 21 +++---- command/tls/ca/create/tls_ca_create.go | 17 ++---- command/tls/ca/create/tls_ca_create_test.go | 35 ++++++++++- tlsutil/generate.go | 64 +++++++++++++++++---- tlsutil/generate_test.go | 28 ++++----- 9 files changed, 130 insertions(+), 96 deletions(-) create mode 100644 .changelog/9585.txt diff --git a/.changelog/9585.txt b/.changelog/9585.txt new file mode 100644 index 0000000000..04ee920f3e --- /dev/null +++ b/.changelog/9585.txt @@ -0,0 +1,3 @@ +```release-note:improvement +cli: Add new `-cluster-id` and `common-name` to `consul tls ca create` to support creating a CA for Consul Connect. +``` diff --git a/agent/consul/server_test.go b/agent/consul/server_test.go index 5fd40b06f4..dc6c77f513 100644 --- a/agent/consul/server_test.go +++ b/agent/consul/server_test.go @@ -2,9 +2,6 @@ package consul import ( "bytes" - "crypto/ecdsa" - "crypto/elliptic" - "crypto/rand" "crypto/x509" "fmt" "net" @@ -46,22 +43,17 @@ const ( // testTLSCertificates Generates a TLS CA and server key/cert and returns them // in PEM encoded form. func testTLSCertificates(serverName string) (cert string, key string, cacert string, err error) { - // generate CA - serial, err := tlsutil.GenerateSerialNumber() - if err != nil { - return "", "", "", err - } - signer, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) - if err != nil { - return "", "", "", err - } - ca, err := tlsutil.GenerateCA(signer, serial, 365, nil) + signer, _, err := tlsutil.GeneratePrivateKey() if err != nil { return "", "", "", err } - // generate leaf - serial, err = tlsutil.GenerateSerialNumber() + ca, _, err := tlsutil.GenerateCA(tlsutil.CAOpts{Signer: signer}) + if err != nil { + return "", "", "", err + } + + serial, err := tlsutil.GenerateSerialNumber() if err != nil { return "", "", "", err } diff --git a/agent/pool/peek_test.go b/agent/pool/peek_test.go index 8458726c74..ab830fc070 100644 --- a/agent/pool/peek_test.go +++ b/agent/pool/peek_test.go @@ -1,9 +1,6 @@ package pool import ( - "crypto/ecdsa" - "crypto/elliptic" - "crypto/rand" "crypto/tls" "crypto/x509" "errors" @@ -194,22 +191,18 @@ func deadlineNetPipe(deadline time.Time) (net.Conn, net.Conn, error) { } func generateTestCert(serverName string) (cert tls.Certificate, caPEM []byte, err error) { - // generate CA - serial, err := tlsutil.GenerateSerialNumber() + signer, _, err := tlsutil.GeneratePrivateKey() if err != nil { return tls.Certificate{}, nil, err } - signer, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) - if err != nil { - return tls.Certificate{}, nil, err - } - ca, err := tlsutil.GenerateCA(signer, serial, 365, nil) + + ca, _, err := tlsutil.GenerateCA(tlsutil.CAOpts{Signer: signer}) if err != nil { return tls.Certificate{}, nil, err } // generate leaf - serial, err = tlsutil.GenerateSerialNumber() + serial, err := tlsutil.GenerateSerialNumber() if err != nil { return tls.Certificate{}, nil, err } diff --git a/agent/routine-leak-checker/leak_test.go b/agent/routine-leak-checker/leak_test.go index 53cb7e7170..60ac2de39e 100644 --- a/agent/routine-leak-checker/leak_test.go +++ b/agent/routine-leak-checker/leak_test.go @@ -1,9 +1,6 @@ package leakcheck import ( - "crypto/ecdsa" - "crypto/elliptic" - "crypto/rand" "crypto/x509" "io/ioutil" "path/filepath" @@ -18,22 +15,18 @@ import ( ) func testTLSCertificates(serverName string) (cert string, key string, cacert string, err error) { - // generate CA - serial, err := tlsutil.GenerateSerialNumber() - if err != nil { - return "", "", "", err - } - signer, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) - if err != nil { - return "", "", "", err - } - ca, err := tlsutil.GenerateCA(signer, serial, 365, nil) + ca, _, err := tlsutil.GenerateCA(tlsutil.CAOpts{}) if err != nil { return "", "", "", err } // generate leaf - serial, err = tlsutil.GenerateSerialNumber() + serial, err := tlsutil.GenerateSerialNumber() + if err != nil { + return "", "", "", err + } + + signer, _, err := tlsutil.GeneratePrivateKey() if err != nil { return "", "", "", err } diff --git a/agent/testagent.go b/agent/testagent.go index 082f058571..9ceaddfd66 100644 --- a/agent/testagent.go +++ b/agent/testagent.go @@ -3,8 +3,6 @@ package agent import ( "bytes" "context" - "crypto/ecdsa" - "crypto/elliptic" "crypto/x509" "fmt" "io" @@ -569,22 +567,17 @@ func TestACLConfigWithParams(params *TestACLConfigParams) string { // testTLSCertificates Generates a TLS CA and server key/cert and returns them // in PEM encoded form. func testTLSCertificates(serverName string) (cert string, key string, cacert string, err error) { - // generate CA - serial, err := tlsutil.GenerateSerialNumber() - if err != nil { - return "", "", "", err - } - signer, err := ecdsa.GenerateKey(elliptic.P256(), rand.New(rand.NewSource(99))) - if err != nil { - return "", "", "", err - } - ca, err := tlsutil.GenerateCA(signer, serial, 365, nil) + signer, _, err := tlsutil.GeneratePrivateKey() if err != nil { return "", "", "", err } - // generate leaf - serial, err = tlsutil.GenerateSerialNumber() + ca, _, err := tlsutil.GenerateCA(tlsutil.CAOpts{Signer: signer}) + if err != nil { + return "", "", "", err + } + + serial, err := tlsutil.GenerateSerialNumber() if err != nil { return "", "", "", err } diff --git a/command/tls/ca/create/tls_ca_create.go b/command/tls/ca/create/tls_ca_create.go index 80f74ce587..ceef70b376 100644 --- a/command/tls/ca/create/tls_ca_create.go +++ b/command/tls/ca/create/tls_ca_create.go @@ -23,7 +23,9 @@ type cmd struct { help string days int domain string + clusterID string constraint bool + commonName string additionalConstraints flags.AppendSliceValue } @@ -36,6 +38,8 @@ func (c *cmd) init() { "DNS. If the UI is going to be served over HTTPS its DNS has to be added with -additional-constraint. It is not "+ "possible to add that after the fact! Defaults to false.") c.flags.StringVar(&c.domain, "domain", "consul", "Domain of consul cluster. Only used in combination with -name-constraint. Defaults to consul.") + c.flags.StringVar(&c.clusterID, "cluster-id", "", "ClusterID of the consul cluster, requires -domain to be set as well. When used will set URIs with spiffeid.") + c.flags.StringVar(&c.commonName, "common-name", "", "Common Name of CA. Defaults to Consul Agent CA.") c.flags.Var(&c.additionalConstraints, "additional-name-constraint", "Add name constraints for the CA. Results in rejecting certificates "+ "for other DNS than specified. Can be used multiple times. Only used in combination with -name-constraint.") c.help = flags.Usage(help, c.flags) @@ -62,23 +66,12 @@ func (c *cmd) Run(args []string) int { return 1 } - sn, err := tlsutil.GenerateSerialNumber() - if err != nil { - c.UI.Error(err.Error()) - return 1 - } - s, pk, err := tlsutil.GeneratePrivateKey() - if err != nil { - c.UI.Error(err.Error()) - return 1 - } - constraints := []string{} if c.constraint { constraints = append(c.additionalConstraints, []string{c.domain, "localhost"}...) } - ca, err := tlsutil.GenerateCA(s, sn, c.days, constraints) + ca, pk, err := tlsutil.GenerateCA(tlsutil.CAOpts{Name: c.commonName, Days: c.days, Domain: c.domain, PermittedDNSDomains: constraints, ClusterID: c.clusterID}) if err != nil { c.UI.Error(err.Error()) return 1 diff --git a/command/tls/ca/create/tls_ca_create_test.go b/command/tls/ca/create/tls_ca_create_test.go index f2677e081d..5689589598 100644 --- a/command/tls/ca/create/tls_ca_create_test.go +++ b/command/tls/ca/create/tls_ca_create_test.go @@ -62,22 +62,53 @@ func TestCACreateCommand(t *testing.T) { require.ElementsMatch(t, cert.PermittedDNSDomains, []string{"foo", "localhost", "bar"}) }, }, + {"with cluster-id", + []string{ + "-domain=foo", + "-cluster-id=uuid", + }, + "foo-agent-ca.pem", + "foo-agent-ca-key.pem", + func(t *testing.T, cert *x509.Certificate) { + require.Len(t, cert.URIs, 1) + require.Equal(t, cert.URIs[0].String(), "spiffe://uuid.foo") + }, + }, + {"with common-name", + []string{ + "-common-name=foo", + }, + "consul-agent-ca.pem", + "consul-agent-ca-key.pem", + func(t *testing.T, cert *x509.Certificate) { + require.Equal(t, cert.Subject.CommonName, "foo") + }, + }, + {"without common-name", + []string{}, + "consul-agent-ca.pem", + "consul-agent-ca-key.pem", + func(t *testing.T, cert *x509.Certificate) { + require.True(t, strings.HasPrefix(cert.Subject.CommonName, "Consul Agent CA")) + }, + }, } for _, tc := range cases { tc := tc require.True(t, t.Run(tc.name, func(t *testing.T) { ui := cli.NewMockUi() cmd := New(ui) - require.Equal(t, 0, cmd.Run(tc.args)) + require.Equal(t, 0, cmd.Run(tc.args), ui.ErrorWriter.String()) require.Equal(t, "", ui.ErrorWriter.String()) cert, _ := expectFiles(t, tc.caPath, tc.keyPath) - require.Contains(t, cert.Subject.CommonName, "Consul Agent CA") require.True(t, cert.BasicConstraintsValid) require.Equal(t, x509.KeyUsageCertSign|x509.KeyUsageCRLSign|x509.KeyUsageDigitalSignature, cert.KeyUsage) require.True(t, cert.IsCA) require.Equal(t, cert.AuthorityKeyId, cert.SubjectKeyId) tc.extraCheck(t, cert) + require.NoError(t, os.Remove(tc.caPath)) + require.NoError(t, os.Remove(tc.keyPath)) })) } diff --git a/tlsutil/generate.go b/tlsutil/generate.go index 2157386890..61fbf2b77f 100644 --- a/tlsutil/generate.go +++ b/tlsutil/generate.go @@ -15,6 +15,7 @@ import ( "time" "github.com/hashicorp/consul/agent/connect" + "net/url" ) // GenerateSerialNumber returns random bigint generated with crypto/rand @@ -32,18 +33,61 @@ func GeneratePrivateKey() (crypto.Signer, string, error) { return connect.GeneratePrivateKey() } +type CAOpts struct { + Signer crypto.Signer + Serial *big.Int + ClusterID string + Days int + PermittedDNSDomains []string + Domain string + Name string +} + // GenerateCA generates a new CA for agent TLS (not to be confused with Connect TLS) -func GenerateCA(signer crypto.Signer, sn *big.Int, days int, constraints []string) (string, error) { - id, err := keyID(signer.Public()) - if err != nil { - return "", err +func GenerateCA(opts CAOpts) (string, string, error) { + signer := opts.Signer + var pk string + if signer == nil { + var err error + signer, pk, err = GeneratePrivateKey() + if err != nil { + return "", "", err + } } - name := fmt.Sprintf("Consul Agent CA %d", sn) + id, err := keyID(signer.Public()) + if err != nil { + return "", "", err + } + + sn := opts.Serial + if sn == nil { + var err error + sn, err = GenerateSerialNumber() + if err != nil { + return "", "", err + } + } + name := opts.Name + if name == "" { + name = fmt.Sprintf("Consul Agent CA %d", sn) + } + + days := opts.Days + if opts.Days == 0 { + days = 365 + } + + var uris []*url.URL + if opts.ClusterID != "" { + spiffeID := connect.SpiffeIDSigning{ClusterID: opts.ClusterID, Domain: opts.Domain} + uris = []*url.URL{spiffeID.URI()} + } // Create the CA cert template := x509.Certificate{ SerialNumber: sn, + URIs: uris, Subject: pkix.Name{ Country: []string{"US"}, PostalCode: []string{"94105"}, @@ -62,23 +106,23 @@ func GenerateCA(signer crypto.Signer, sn *big.Int, days int, constraints []strin SubjectKeyId: id, } - if len(constraints) > 0 { + if len(opts.PermittedDNSDomains) > 0 { template.PermittedDNSDomainsCritical = true - template.PermittedDNSDomains = constraints + template.PermittedDNSDomains = opts.PermittedDNSDomains } bs, err := x509.CreateCertificate( rand.Reader, &template, &template, signer.Public(), signer) if err != nil { - return "", fmt.Errorf("error generating CA certificate: %s", err) + return "", "", fmt.Errorf("error generating CA certificate: %s", err) } var buf bytes.Buffer err = pem.Encode(&buf, &pem.Block{Type: "CERTIFICATE", Bytes: bs}) if err != nil { - return "", fmt.Errorf("error encoding private key: %s", err) + return "", "", fmt.Errorf("error encoding private key: %s", err) } - return buf.String(), nil + return buf.String(), pk, nil } // GenerateCert generates a new certificate for agent TLS (not to be confused with Connect TLS) diff --git a/tlsutil/generate_test.go b/tlsutil/generate_test.go index e8957ba2a8..b0a23d7c0b 100644 --- a/tlsutil/generate_test.go +++ b/tlsutil/generate_test.go @@ -8,13 +8,13 @@ import ( "crypto/rsa" "crypto/x509" "encoding/pem" - "fmt" "io" "net" "testing" "time" "github.com/stretchr/testify/require" + "strings" ) func TestSerialNumber(t *testing.T) { @@ -62,32 +62,26 @@ func (s *TestSigner) Sign(rand io.Reader, digest []byte, opts crypto.SignerOpts) func TestGenerateCA(t *testing.T) { t.Parallel() - sn, err := GenerateSerialNumber() - require.Nil(t, err) - var s crypto.Signer - - // test what happens without key - s = &TestSigner{} - ca, err := GenerateCA(s, sn, 0, nil) + ca, pk, err := GenerateCA(CAOpts{Signer: &TestSigner{}}) require.Error(t, err) require.Empty(t, ca) + require.Empty(t, pk) // test what happens with wrong key - s = &TestSigner{public: &rsa.PublicKey{}} - ca, err = GenerateCA(s, sn, 0, nil) + ca, pk, err = GenerateCA(CAOpts{Signer: &TestSigner{public: &rsa.PublicKey{}}}) require.Error(t, err) require.Empty(t, ca) + require.Empty(t, pk) // test what happens with correct key - s, err = ecdsa.GenerateKey(elliptic.P256(), rand.Reader) - require.Nil(t, err) - ca, err = GenerateCA(s, sn, 365, nil) + ca, pk, err = GenerateCA(CAOpts{}) require.Nil(t, err) require.NotEmpty(t, ca) + require.NotEmpty(t, pk) cert, err := parseCert(ca) require.Nil(t, err) - require.Equal(t, fmt.Sprintf("Consul Agent CA %d", sn), cert.Subject.CommonName) + require.True(t, strings.HasPrefix(cert.Subject.CommonName, "Consul Agent CA")) require.Equal(t, true, cert.IsCA) require.Equal(t, true, cert.BasicConstraintsValid) @@ -99,14 +93,12 @@ func TestGenerateCA(t *testing.T) { func TestGenerateCert(t *testing.T) { t.Parallel() - sn, err := GenerateSerialNumber() - require.Nil(t, err) signer, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) require.Nil(t, err) - ca, err := GenerateCA(signer, sn, 365, nil) + ca, _, err := GenerateCA(CAOpts{Signer: signer}) require.Nil(t, err) - sn, err = GenerateSerialNumber() + sn, err := GenerateSerialNumber() require.Nil(t, err) DNSNames := []string{"server.dc1.consul"} IPAddresses := []net.IP{net.ParseIP("123.234.243.213")}