diff --git a/agent/config/builder.go b/agent/config/builder.go index 9d3a1cdc79..031268d45f 100644 --- a/agent/config/builder.go +++ b/agent/config/builder.go @@ -935,6 +935,7 @@ func (b *Builder) Validate(rt RuntimeConfig) error { // Validate the given Connect CA provider config validCAProviders := map[string]bool{ + "": true, structs.ConsulCAProvider: true, structs.VaultCAProvider: true, } diff --git a/agent/config/default.go b/agent/config/default.go index 99082d09c6..c95a0c6434 100644 --- a/agent/config/default.go +++ b/agent/config/default.go @@ -113,7 +113,6 @@ func DevSource() Source { connect = { enabled = true - ca_provider = "consul" } performance = { raft_multiplier = 1 diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go index 8ecf397b01..f6c07accd8 100644 --- a/agent/config/runtime_test.go +++ b/agent/config/runtime_test.go @@ -2484,10 +2484,9 @@ func TestFullConfig(t *testing.T) { "check_update_interval": "16507s", "client_addr": "93.83.18.19", "connect": { - "ca_provider": "b8j4ynx9", + "ca_provider": "consul", "ca_config": { - "g4cvJyys": "IRLXE9Ds", - "hyMy9Oxn": "XeBp4Sis" + "RotationPeriod": "90h" }, "enabled": true, "proxy_defaults": { @@ -2946,10 +2945,9 @@ func TestFullConfig(t *testing.T) { check_update_interval = "16507s" client_addr = "93.83.18.19" connect { - ca_provider = "b8j4ynx9" + ca_provider = "consul" ca_config { - "g4cvJyys" = "IRLXE9Ds" - "hyMy9Oxn" = "XeBp4Sis" + "RotationPeriod" = "90h" } enabled = true proxy_defaults { @@ -3550,10 +3548,9 @@ func TestFullConfig(t *testing.T) { ConnectEnabled: true, ConnectProxyBindMinPort: 2000, ConnectProxyBindMaxPort: 3000, - ConnectCAProvider: "b8j4ynx9", + ConnectCAProvider: "consul", ConnectCAConfig: map[string]interface{}{ - "g4cvJyys": "IRLXE9Ds", - "hyMy9Oxn": "XeBp4Sis", + "RotationPeriod": "90h", }, ConnectProxyAllowManagedRoot: false, ConnectProxyAllowManagedAPIRegistration: false, diff --git a/agent/connect/ca/provider_consul.go b/agent/connect/ca/provider_consul.go index d800215ffb..b9add03968 100644 --- a/agent/connect/ca/provider_consul.go +++ b/agent/connect/ca/provider_consul.go @@ -242,8 +242,53 @@ func (c *ConsulProvider) Sign(csr *x509.CertificateRequest) (string, error) { return buf.String(), nil } +// GetCrossSigningCSR creates a CSR from our root CA certificate to be signed +// by another CA provider. +func (c *ConsulProvider) GetCrossSigningCSR() (*x509.CertificateRequest, error) { + c.RLock() + defer c.RUnlock() + + // Get the provider state + state := c.delegate.State() + _, providerState, err := state.CAProviderState(c.id) + if err != nil { + return nil, err + } + privKey, err := connect.ParseSigner(providerState.PrivateKey) + if err != nil { + return nil, err + } + rootCA, err := connect.ParseCert(providerState.RootCert) + if err != nil { + return nil, err + } + + // Create the CSR + template := &x509.CertificateRequest{ + DNSNames: rootCA.DNSNames, + EmailAddresses: rootCA.EmailAddresses, + IPAddresses: rootCA.IPAddresses, + URIs: rootCA.URIs, + SignatureAlgorithm: rootCA.SignatureAlgorithm, + Subject: rootCA.Subject, + Extensions: rootCA.Extensions, + ExtraExtensions: rootCA.ExtraExtensions, + } + + bs, err := x509.CreateCertificateRequest(rand.Reader, template, privKey) + if err != nil { + return nil, err + } + csr, err := x509.ParseCertificateRequest(bs) + if err != nil { + return nil, err + } + + return csr, nil +} + // CrossSignCA returns the given intermediate CA cert signed by the current active root. -func (c *ConsulProvider) CrossSignCA(cert *x509.Certificate) (string, error) { +func (c *ConsulProvider) CrossSignCA(csr *x509.CertificateRequest) (string, error) { c.Lock() defer c.Unlock() @@ -264,7 +309,12 @@ func (c *ConsulProvider) CrossSignCA(cert *x509.Certificate) (string, error) { return "", err } - keyId, err := connect.KeyId(privKey.Public()) + authKeyId, err := connect.KeyId(privKey.Public()) + if err != nil { + return "", err + } + + subjKeyId, err := connect.KeyId(csr.PublicKey) if err != nil { return "", err } @@ -272,11 +322,26 @@ func (c *ConsulProvider) CrossSignCA(cert *x509.Certificate) (string, error) { // Create the cross-signing template from the existing root CA serialNum := &big.Int{} serialNum.SetUint64(idx + 1) - template := *cert - template.SerialNumber = serialNum - template.SignatureAlgorithm = rootCA.SignatureAlgorithm - template.SubjectKeyId = keyId - template.AuthorityKeyId = keyId + template := &x509.Certificate{ + SerialNumber: serialNum, + SignatureAlgorithm: rootCA.SignatureAlgorithm, + DNSNames: csr.DNSNames, + EmailAddresses: csr.EmailAddresses, + IPAddresses: csr.IPAddresses, + URIs: csr.URIs, + SubjectKeyId: subjKeyId, + AuthorityKeyId: authKeyId, + Subject: csr.Subject, + Extensions: csr.Extensions, + ExtraExtensions: csr.ExtraExtensions, + BasicConstraintsValid: true, + KeyUsage: x509.KeyUsageCertSign | + x509.KeyUsageCRLSign | + x509.KeyUsageDigitalSignature, + IsCA: true, + NotAfter: time.Now().Add(7 * 24 * time.Hour), + NotBefore: time.Now(), + } // Sign the certificate valid from 1 minute in the past, this helps it be // accepted right away even when nodes are not in close time sync accross the @@ -290,7 +355,7 @@ func (c *ConsulProvider) CrossSignCA(cert *x509.Certificate) (string, error) { template.NotAfter = effectiveNow.Add(7 * 24 * time.Hour) bs, err := x509.CreateCertificate( - rand.Reader, &template, rootCA, cert.PublicKey, privKey) + rand.Reader, template, rootCA, csr.PublicKey, privKey) if err != nil { return "", fmt.Errorf("error generating CA certificate: %s", err) } diff --git a/agent/connect/ca/provider_consul_test.go b/agent/connect/ca/provider_consul_test.go index 60166be36f..6ac12f42ec 100644 --- a/agent/connect/ca/provider_consul_test.go +++ b/agent/connect/ca/provider_consul_test.go @@ -9,6 +9,7 @@ import ( "github.com/hashicorp/consul/agent/consul/state" "github.com/hashicorp/consul/agent/structs" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) type consulCAMockDelegate struct { @@ -52,7 +53,7 @@ func newMockDelegate(t *testing.T, conf *structs.CAConfiguration) *consulCAMockD if s == nil { t.Fatalf("missing state store") } - if err := s.CASetConfig(0, conf); err != nil { + if err := s.CASetConfig(conf.RaftIndex.CreateIndex, conf); err != nil { t.Fatalf("err: %s", err) } @@ -177,37 +178,44 @@ func TestConsulCAProvider_SignLeaf(t *testing.T) { func TestConsulCAProvider_CrossSignCA(t *testing.T) { t.Parallel() - assert := assert.New(t) - conf := testConsulCAConfig() - delegate := newMockDelegate(t, conf) - provider, err := NewConsulProvider(conf.Config, delegate) - assert.NoError(err) + require := require.New(t) - // Make a new CA cert to get cross-signed. - rootCA := connect.TestCA(t, nil) - rootPEM, err := provider.ActiveRoot() - assert.NoError(err) - root, err := connect.ParseCert(rootPEM) - assert.NoError(err) + conf1 := testConsulCAConfig() + delegate1 := newMockDelegate(t, conf1) + provider1, err := NewConsulProvider(conf1.Config, delegate1) + + conf2 := testConsulCAConfig() + conf2.CreateIndex = 10 + delegate2 := newMockDelegate(t, conf2) + provider2, err := NewConsulProvider(conf2.Config, delegate2) + require.NoError(err) + + require.NoError(err) + + // Have provider2 generate a cross-signing CSR + csr, err := provider2.GetCrossSigningCSR() + require.NoError(err) + oldSubject := csr.Subject.CommonName // Have the provider cross sign our new CA cert. - cert, err := connect.ParseCert(rootCA.RootCert) - assert.NoError(err) - oldSubject := cert.Subject.CommonName - xcPEM, err := provider.CrossSignCA(cert) - assert.NoError(err) - + xcPEM, err := provider1.CrossSignCA(csr) + require.NoError(err) xc, err := connect.ParseCert(xcPEM) - assert.NoError(err) + require.NoError(err) - // AuthorityKeyID and SubjectKeyID should be the signing root's. - assert.Equal(root.AuthorityKeyId, xc.AuthorityKeyId) - assert.Equal(root.SubjectKeyId, xc.SubjectKeyId) + rootPEM, err := provider1.ActiveRoot() + require.NoError(err) + root, err := connect.ParseCert(rootPEM) + require.NoError(err) + + // AuthorityKeyID should be the signing root's, SubjectKeyId should be different. + require.Equal(root.AuthorityKeyId, xc.AuthorityKeyId) + require.NotEqual(root.SubjectKeyId, xc.SubjectKeyId) // Subject name should not have changed. - assert.NotEqual(root.Subject.CommonName, xc.Subject.CommonName) - assert.Equal(oldSubject, xc.Subject.CommonName) + require.NotEqual(root.Subject.CommonName, xc.Subject.CommonName) + require.Equal(oldSubject, xc.Subject.CommonName) // Issuer should be the signing root. - assert.Equal(root.Issuer.CommonName, xc.Issuer.CommonName) + require.Equal(root.Issuer.CommonName, xc.Issuer.CommonName) } diff --git a/agent/connect/parsing.go b/agent/connect/parsing.go index ff0f0813db..163f36083e 100644 --- a/agent/connect/parsing.go +++ b/agent/connect/parsing.go @@ -3,7 +3,6 @@ package connect import ( "crypto" "crypto/ecdsa" - "crypto/rsa" "crypto/sha1" "crypto/sha256" "crypto/x509" @@ -98,7 +97,6 @@ func ParseCSR(pemValue string) (*x509.CertificateRequest, error) { func KeyId(raw interface{}) ([]byte, error) { switch raw.(type) { case *ecdsa.PublicKey: - case *rsa.PublicKey: default: return nil, fmt.Errorf("invalid key type: %T", raw) } diff --git a/agent/connect_ca_endpoint.go b/agent/connect_ca_endpoint.go index 5fbc2679fd..e3efde1a55 100644 --- a/agent/connect_ca_endpoint.go +++ b/agent/connect_ca_endpoint.go @@ -82,15 +82,15 @@ func fixupConfig(conf *structs.CAConfiguration) { for k, v := range conf.Config { if raw, ok := v.([]uint8); ok { conf.Config[k] = ca.Uint8ToString(raw) - } - switch conf.Provider { - case structs.ConsulCAProvider: - if v, ok := conf.Config["PrivateKey"]; ok && v != "" { - conf.Config["PrivateKey"] = "hidden" - } - case structs.VaultCAProvider: - if v, ok := conf.Config["Token"]; ok && v != "" { - conf.Config["Token"] = "hidden" + switch conf.Provider { + case structs.ConsulCAProvider: + if k == "PrivateKey" && ca.Uint8ToString(raw) != "" { + conf.Config["PrivateKey"] = "hidden" + } + case structs.VaultCAProvider: + if k == "Token" && ca.Uint8ToString(raw) != "" { + conf.Config["Token"] = "hidden" + } } } } diff --git a/agent/consul/connect_ca_endpoint.go b/agent/consul/connect_ca_endpoint.go index 4d0e1c95c8..6ebe251dc3 100644 --- a/agent/consul/connect_ca_endpoint.go +++ b/agent/consul/connect_ca_endpoint.go @@ -140,22 +140,14 @@ func (s *ConnectCA) ConfigurationSet( // to get the cross-signed intermediate // 3. Get the active root for the new provider, append the intermediate from step 3 // to its list of intermediates - intermediatePEM, err := newProvider.GenerateIntermediate() - if err != nil { - return err - } - - intermediateCA, err := connect.ParseCert(intermediatePEM) - if err != nil { - return err - } + csr, err := newProvider.GetCrossSigningCSR() // Have the old provider cross-sign the new intermediate oldProvider, _ := s.srv.getCAProvider() if oldProvider == nil { return fmt.Errorf("internal error: CA provider is nil") } - xcCert, err := oldProvider.CrossSignCA(intermediateCA) + xcCert, err := oldProvider.CrossSignCA(csr) if err != nil { return err } diff --git a/agent/consul/connect_ca_endpoint_test.go b/agent/consul/connect_ca_endpoint_test.go index a56c2bc897..bcd706903d 100644 --- a/agent/consul/connect_ca_endpoint_test.go +++ b/agent/consul/connect_ca_endpoint_test.go @@ -210,10 +210,10 @@ func TestConnectCAConfig_TriggerRotation(t *testing.T) { oldRootCert := testParseCert(t, oldRoot.RootCert) newRootCert := testParseCert(t, r.RootCert) - // Should have the authority/subject key IDs and signature algo of the + // Should have the authority key ID and signature algo of the // (old) signing CA. assert.Equal(xc.AuthorityKeyId, oldRootCert.AuthorityKeyId) - assert.Equal(xc.SubjectKeyId, oldRootCert.SubjectKeyId) + assert.NotEqual(xc.SubjectKeyId, oldRootCert.SubjectKeyId) assert.Equal(xc.SignatureAlgorithm, oldRootCert.SignatureAlgorithm) // The common name and SAN should not have changed. diff --git a/api/connect_ca_test.go b/api/connect_ca_test.go index 6e93ba927a..77d047e953 100644 --- a/api/connect_ca_test.go +++ b/api/connect_ca_test.go @@ -80,6 +80,7 @@ func TestAPI_ConnectCAConfig_get_set(t *testing.T) { verify.Values(r, "", parsed, expected) // Change a config value and update + conf.Config["PrivateKey"] = "" conf.Config["RotationPeriod"] = 120 * 24 * time.Hour _, err = connect.CASetConfig(conf, nil) r.Check(err)