diff --git a/agent/connect/ca/provider.go b/agent/connect/ca/provider.go index 2d92baf80..f7daa5fc7 100644 --- a/agent/connect/ca/provider.go +++ b/agent/connect/ca/provider.go @@ -32,12 +32,7 @@ type Provider interface { // intemediate and any cross-signed intermediates managed by Consul. Sign(*x509.CertificateRequest) (string, error) - // GetCrossSigningCSR returns a CSR that can be signed by another root - // to create an intermediate cert that forms a chain of trust back to - // that root CA. - GetCrossSigningCSR() (*x509.CertificateRequest, error) - - // CrossSignCA must accept a CA CSR signed by another CA's key + // CrossSignCA must accept a CA certificate from another CA provider // and cross sign it exactly as it is such that it forms a chain back the the // CAProvider's current root. Specifically, the Distinguished Name, Subject // Alternative Name, SubjectKeyID and other relevant extensions must be kept. @@ -45,7 +40,7 @@ type Provider interface { // AuthorityKeyID set to the CAProvider's current signing key as well as the // Issuer related fields changed as necessary. The resulting certificate is // returned as a PEM formatted string. - CrossSignCA(*x509.CertificateRequest) (string, error) + CrossSignCA(*x509.Certificate) (string, error) // Cleanup performs any necessary cleanup that should happen when the provider // is shut down permanently, such as removing a temporary PKI backend in Vault diff --git a/agent/connect/ca/provider_consul.go b/agent/connect/ca/provider_consul.go index b9add0396..99bc3158a 100644 --- a/agent/connect/ca/provider_consul.go +++ b/agent/connect/ca/provider_consul.go @@ -242,53 +242,8 @@ 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(csr *x509.CertificateRequest) (string, error) { +// CrossSignCA returns the given CA cert signed by the current active root. +func (c *ConsulProvider) CrossSignCA(cert *x509.Certificate) (string, error) { c.Lock() defer c.Unlock() @@ -309,12 +264,7 @@ func (c *ConsulProvider) CrossSignCA(csr *x509.CertificateRequest) (string, erro return "", err } - authKeyId, err := connect.KeyId(privKey.Public()) - if err != nil { - return "", err - } - - subjKeyId, err := connect.KeyId(csr.PublicKey) + keyId, err := connect.KeyId(privKey.Public()) if err != nil { return "", err } @@ -322,26 +272,10 @@ func (c *ConsulProvider) CrossSignCA(csr *x509.CertificateRequest) (string, erro // Create the cross-signing template from the existing root CA serialNum := &big.Int{} serialNum.SetUint64(idx + 1) - 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(), - } + template := *cert + template.SerialNumber = serialNum + template.SignatureAlgorithm = rootCA.SignatureAlgorithm + template.AuthorityKeyId = keyId // 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 @@ -355,7 +289,7 @@ func (c *ConsulProvider) CrossSignCA(csr *x509.CertificateRequest) (string, erro template.NotAfter = effectiveNow.Add(7 * 24 * time.Hour) bs, err := x509.CreateCertificate( - rand.Reader, template, rootCA, csr.PublicKey, privKey) + rand.Reader, &template, rootCA, cert.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 3263a66eb..7c510fff1 100644 --- a/agent/connect/ca/provider_consul_test.go +++ b/agent/connect/ca/provider_consul_test.go @@ -179,46 +179,55 @@ func TestConsulCAProvider_SignLeaf(t *testing.T) { func TestConsulCAProvider_CrossSignCA(t *testing.T) { t.Parallel() - require := require.New(t) - conf1 := testConsulCAConfig() delegate1 := newMockDelegate(t, conf1) provider1, err := NewConsulProvider(conf1.Config, delegate1) + require.NoError(t, err) conf2 := testConsulCAConfig() conf2.CreateIndex = 10 delegate2 := newMockDelegate(t, conf2) provider2, err := NewConsulProvider(conf2.Config, delegate2) + require.NoError(t, err) + + testCrossSignProviders(t, provider1, provider2) +} + +func testCrossSignProviders(t *testing.T, provider1, provider2 Provider) { + require := require.New(t) + + // Get the root from the new provider to be cross-signed. + newRootPEM, err := provider2.ActiveRoot() + require.NoError(err) + newRoot, err := connect.ParseCert(newRootPEM) + require.NoError(err) + oldSubject := newRoot.Subject.CommonName + + newInterPEM, err := provider2.ActiveIntermediate() + require.NoError(err) + newIntermediate, err := connect.ParseCert(newInterPEM) 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 provider1 cross sign our new CA cert. - xcPEM, err := provider1.CrossSignCA(csr) + // Have provider1 cross sign our new root cert. + xcPEM, err := provider1.CrossSignCA(newRoot) require.NoError(err) xc, err := connect.ParseCert(xcPEM) require.NoError(err) - rootPEM, err := provider1.ActiveRoot() + oldRootPEM, err := provider1.ActiveRoot() require.NoError(err) - root, err := connect.ParseCert(rootPEM) + oldRoot, err := connect.ParseCert(oldRootPEM) 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) + // AuthorityKeyID should now be the signing root's, SubjectKeyId should be kept. + require.Equal(oldRoot.AuthorityKeyId, xc.AuthorityKeyId) + require.Equal(newRoot.SubjectKeyId, xc.SubjectKeyId) // Subject name should not have changed. - require.NotEqual(root.Subject.CommonName, xc.Subject.CommonName) require.Equal(oldSubject, xc.Subject.CommonName) // Issuer should be the signing root. - require.Equal(root.Issuer.CommonName, xc.Issuer.CommonName) + require.Equal(oldRoot.Issuer.CommonName, xc.Issuer.CommonName) // Get a leaf cert so we can verify against the cross-signed cert. spiffeService := &connect.SpiffeIDService{ @@ -238,18 +247,20 @@ func TestConsulCAProvider_CrossSignCA(t *testing.T) { cert, err := connect.ParseCert(leafPEM) require.NoError(err) + // Check that the leaf signed by the new cert can be verified by either root + // certificate by using the new intermediate + cross-signed cert. intermediatePool := x509.NewCertPool() + intermediatePool.AddCert(newIntermediate) intermediatePool.AddCert(xc) - rootPool := x509.NewCertPool() - rootPool.AddCert(root) + for _, root := range []*x509.Certificate{oldRoot, newRoot} { + rootPool := x509.NewCertPool() + rootPool.AddCert(root) - // Check that the leaf signed by the new cert can be verified by the - // chain of cross-signed cert + old root (as would be the case on any - // proxies that haven't received the new root yet) for backwards compatibility. - _, err = cert.Verify(x509.VerifyOptions{ - Intermediates: intermediatePool, - Roots: rootPool, - }) - require.NoError(err) + _, err = cert.Verify(x509.VerifyOptions{ + Intermediates: intermediatePool, + Roots: rootPool, + }) + require.NoError(err) + } } diff --git a/agent/connect/ca/provider_vault.go b/agent/connect/ca/provider_vault.go index 5cd6203b7..d2099eabc 100644 --- a/agent/connect/ca/provider_vault.go +++ b/agent/connect/ca/provider_vault.go @@ -11,6 +11,7 @@ import ( "github.com/hashicorp/consul/agent/connect" "github.com/hashicorp/consul/agent/structs" + "github.com/hashicorp/go-uuid" vaultapi "github.com/hashicorp/vault/api" "github.com/mitchellh/mapstructure" ) @@ -72,8 +73,13 @@ func NewVaultProvider(rawConfig map[string]interface{}, clusterId string) (*Vaul fallthrough case ErrBackendNotInitialized: spiffeID := connect.SpiffeIDSigning{ClusterID: clusterId, Domain: "consul"} - _, err := client.Logical().Write(conf.RootPKIPath+"root/generate/internal", map[string]interface{}{ - "uri_sans": spiffeID.URI().String(), + uuid, err := uuid.GenerateUUID() + if err != nil { + return nil, err + } + _, err = client.Logical().Write(conf.RootPKIPath+"root/generate/internal", map[string]interface{}{ + "common_name": fmt.Sprintf("Vault CA Root Authority %s", uuid), + "uri_sans": spiffeID.URI().String(), }) if err != nil { return nil, err @@ -167,6 +173,7 @@ func (v *VaultProvider) GenerateIntermediate() (string, error) { "allowed_uri_sans": "spiffe://*", "key_type": "any", "max_ttl": "72h", + "require_cn": false, }) if err != nil { return "", err @@ -240,48 +247,18 @@ func (v *VaultProvider) Sign(csr *x509.CertificateRequest) (string, error) { return fmt.Sprintf("%s\n%s", cert, ca), nil } -// GetCrossSigningCSR creates a CSR for an intermediate CA certificate to be signed -// by another CA provider. -func (v *VaultProvider) GetCrossSigningCSR() (*x509.CertificateRequest, error) { - // Generate a new intermediate CSR for the root to sign. - spiffeID := connect.SpiffeIDSigning{ClusterID: v.clusterId, Domain: "consul"} - csr, err := v.client.Logical().Write(v.config.IntermediatePKIPath+"intermediate/generate/internal", map[string]interface{}{ - "common_name": "Vault CA Intermediate Authority", - "uri_sans": spiffeID.URI().String(), - }) - if err != nil { - return nil, err - } - if csr == nil || csr.Data["csr"] == "" { - return nil, fmt.Errorf("got empty value when generating intermediate CSR") - } - - // Return the parsed CSR. - pem, ok := csr.Data["csr"].(string) - if !ok { - return nil, fmt.Errorf("CSR was not a string") - } - result, err := connect.ParseCSR(pem) - if err != nil { - return nil, err - } - - return result, nil -} - -// CrossSignCA creates a CSR from the given CA certificate and uses the -// configured root PKI backend to issue a cert from the request. -func (v *VaultProvider) CrossSignCA(cert *x509.CertificateRequest) (string, error) { - var csrBuf bytes.Buffer - err := pem.Encode(&csrBuf, &pem.Block{Type: "CERTIFICATE REQUEST", Bytes: cert.Raw}) +// CrossSignCA takes a CA certificate and cross-signs it to form a trust chain +// back to our active root. +func (v *VaultProvider) CrossSignCA(cert *x509.Certificate) (string, error) { + var pemBuf bytes.Buffer + err := pem.Encode(&pemBuf, &pem.Block{Type: "CERTIFICATE", Bytes: cert.Raw}) if err != nil { return "", err } - // Have the root PKI backend sign this CSR. - response, err := v.client.Logical().Write(v.config.RootPKIPath+"root/sign-intermediate", map[string]interface{}{ - "csr": csrBuf.String(), - "use_csr_values": true, + // Have the root PKI backend sign this cert. + response, err := v.client.Logical().Write(v.config.RootPKIPath+"root/sign-self-issued", map[string]interface{}{ + "certificate": pemBuf.String(), }) if err != nil { return "", fmt.Errorf("error having Vault cross-sign cert: %v", err) diff --git a/agent/connect/ca/provider_vault_test.go b/agent/connect/ca/provider_vault_test.go index 6941afe19..37d686549 100644 --- a/agent/connect/ca/provider_vault_test.go +++ b/agent/connect/ca/provider_vault_test.go @@ -1,7 +1,6 @@ package ca import ( - "crypto/x509" "fmt" "io/ioutil" "net" @@ -118,7 +117,6 @@ func TestVaultCAProvider_SignLeaf(t *testing.T) { parsed, err := connect.ParseCert(cert) require.NoError(err) require.Equal(parsed.URIs[0], spiffeService.URI()) - require.Equal(parsed.Subject.CommonName, "foo") firstSerial = parsed.SerialNumber.Uint64() // Ensure the cert is valid now and expires within the correct limit. @@ -141,7 +139,6 @@ func TestVaultCAProvider_SignLeaf(t *testing.T) { parsed, err := connect.ParseCert(cert) require.NoError(err) require.Equal(parsed.URIs[0], spiffeService.URI()) - require.Equal(parsed.Subject.CommonName, "bar") require.NotEqual(firstSerial, parsed.SerialNumber.Uint64()) // Ensure the cert is valid now and expires within the correct limit. @@ -153,7 +150,6 @@ func TestVaultCAProvider_SignLeaf(t *testing.T) { func TestVaultCAProvider_CrossSignCA(t *testing.T) { t.Parallel() - require := require.New(t) provider1, core1, listener1 := testVaultCluster(t) defer core1.Shutdown() defer listener1.Close() @@ -162,65 +158,5 @@ func TestVaultCAProvider_CrossSignCA(t *testing.T) { defer core2.Shutdown() defer listener2.Close() - // Have provider2 generate a cross-signing CSR - csr, err := provider2.GetCrossSigningCSR() - require.NoError(err) - oldSubject := csr.Subject.CommonName - _, err = provider2.GenerateIntermediate() - require.NoError(err) - - // Have provider1 cross sign our new CA cert. - xcPEM, err := provider1.CrossSignCA(csr) - require.NoError(err) - xc, err := connect.ParseCert(xcPEM) - require.NoError(err) - - 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. - require.NotEqual(root.Subject.CommonName, xc.Subject.CommonName) - require.Equal(oldSubject, xc.Subject.CommonName) - - // Issuer should be the signing root. - require.Equal(root.Issuer.CommonName, xc.Issuer.CommonName) - - // Get a leaf cert so we can verify against the cross-signed cert. - spiffeService := &connect.SpiffeIDService{ - Host: "node1", - Namespace: "default", - Datacenter: "dc1", - Service: "foo", - } - raw, _ := connect.TestCSR(t, spiffeService) - - leafCsr, err := connect.ParseCSR(raw) - require.NoError(err) - - leafPEM, err := provider2.Sign(leafCsr) - require.NoError(err) - - cert, err := connect.ParseCert(leafPEM) - require.NoError(err) - - intermediatePool := x509.NewCertPool() - intermediatePool.AddCert(xc) - - rootPool := x509.NewCertPool() - rootPool.AddCert(root) - - // Check that the leaf signed by the new cert can be verified by the - // chain of cross-signed cert + old root (as would be the case on any - // proxies that haven't received the new root yet) for backwards compatibility. - _, err = cert.Verify(x509.VerifyOptions{ - Intermediates: intermediatePool, - Roots: rootPool, - }) - require.NoError(err) + testCrossSignProviders(t, provider1, provider2) } diff --git a/agent/connect/csr.go b/agent/connect/csr.go index 44ee53d18..16a46af3f 100644 --- a/agent/connect/csr.go +++ b/agent/connect/csr.go @@ -5,24 +5,16 @@ import ( "crypto" "crypto/rand" "crypto/x509" - "crypto/x509/pkix" "encoding/pem" - "fmt" "net/url" ) // CreateCSR returns a CSR to sign the given service along with the PEM-encoded // private key for this certificate. func CreateCSR(uri CertURI, privateKey crypto.Signer) (string, error) { - serviceId, ok := uri.(*SpiffeIDService) - if !ok { - return "", fmt.Errorf("SPIFFE ID in CSR must be a service ID") - } - template := &x509.CertificateRequest{ URIs: []*url.URL{uri.URI()}, SignatureAlgorithm: x509.ECDSAWithSHA256, - Subject: pkix.Name{CommonName: serviceId.Service}, } // Create the CSR itself diff --git a/agent/connect/testing_ca.go b/agent/connect/testing_ca.go index 93cfca13a..cc015af81 100644 --- a/agent/connect/testing_ca.go +++ b/agent/connect/testing_ca.go @@ -201,15 +201,9 @@ func TestLeaf(t testing.T, service string, root *structs.CARoot) (string, string // TestCSR returns a CSR to sign the given service along with the PEM-encoded // private key for this certificate. func TestCSR(t testing.T, uri CertURI) (string, string) { - serviceId, ok := uri.(*SpiffeIDService) - if !ok { - t.Fatalf("SPIFFE ID in CSR must be a service ID") - } - template := &x509.CertificateRequest{ URIs: []*url.URL{uri.URI()}, SignatureAlgorithm: x509.ECDSAWithSHA256, - Subject: pkix.Name{CommonName: serviceId.Service}, } // Create the private key we'll use diff --git a/agent/consul/connect_ca_endpoint.go b/agent/consul/connect_ca_endpoint.go index 6ebe251dc..9739bfeda 100644 --- a/agent/consul/connect_ca_endpoint.go +++ b/agent/consul/connect_ca_endpoint.go @@ -135,25 +135,35 @@ func (s *ConnectCA) ConfigurationSet( // to use a different root certificate. // If it's a config change that would trigger a rotation (different provider/root): - // 1. Get the intermediate from the new provider - // 2. Generate a CSR for the new intermediate, call SignCA on the old/current provider - // 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 - csr, err := newProvider.GetCrossSigningCSR() + // 1. Get the root from the new provider. + // 2. Call CrossSignCA on the old provider to sign the new root with the old one to + // get a cross-signed certificate. + // 3. Take the active root for the new provider and append the intermediate from step 2 + // to its list of intermediates. + newRoot, err := connect.ParseCert(newRootPEM) + if err != nil { + return err + } // 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(csr) + xcCert, err := oldProvider.CrossSignCA(newRoot) if err != nil { return err } - // Add the cross signed cert to the new root's intermediates + // Add the cross signed cert to the new root's intermediates. newActiveRoot.IntermediateCerts = []string{xcCert} + intermediate, err := newProvider.GenerateIntermediate() + if err != nil { + return err + } + if intermediate != newRootPEM { + newActiveRoot.IntermediateCerts = append(newActiveRoot.IntermediateCerts, intermediate) + } // Update the roots and CA config in the state store at the same time idx, roots, err := state.CARoots(nil) diff --git a/agent/dns.go b/agent/dns.go index e014c1330..d61ac7e70 100644 --- a/agent/dns.go +++ b/agent/dns.go @@ -790,8 +790,8 @@ func (d *DNSServer) trimTCPResponse(req, resp *dns.Msg) (trimmed bool) { originalNumRecords := len(resp.Answer) // It is not possible to return more than 4k records even with compression - // Since we are performing binary search it is not a big deal, but it - // improves a bit performance, even with binary search + // Since we are performing binary search it is not a big deal, but it + // improves a bit performance, even with binary search truncateAt := 4096 if req.Question[0].Qtype == dns.TypeSRV { // More than 1024 SRV records do not fit in 64k