From 52e8652ac568da9f56651a4f5e56d79bb26574c0 Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Wed, 12 Sep 2018 19:52:24 -0700 Subject: [PATCH] connect/ca: add intermediate functions to Consul CA provider --- agent/connect/ca/provider.go | 13 ++ agent/connect/ca/provider_consul.go | 252 +++++++++++++++++++++-- agent/connect/ca/provider_consul_test.go | 63 ++++++ agent/connect/ca/provider_vault.go | 12 ++ agent/connect/uri_signing.go | 2 +- 5 files changed, 327 insertions(+), 15 deletions(-) diff --git a/agent/connect/ca/provider.go b/agent/connect/ca/provider.go index a812d64113..04c1317855 100644 --- a/agent/connect/ca/provider.go +++ b/agent/connect/ca/provider.go @@ -22,6 +22,13 @@ type Provider interface { // ActiveIntermediate() ActiveRoot() (string, error) + // GenerateIntermediateCSR generates a CSR for an intermediate CA + // certificate, to be signed by the root of another datacenter. If isRoot is + // true, calling this is an error. + GenerateIntermediateCSR() (string, error) + + SetIntermediate(intermediatePEM, rootPEM string) error + // ActiveIntermediate returns the current signing cert used by this provider // for generating SPIFFE leaf certs. Note that this must not change except // when Consul requests the change via GenerateIntermediate. Changing the @@ -41,6 +48,12 @@ type Provider interface { // intemediate and any cross-signed intermediates managed by Consul. Sign(*x509.CertificateRequest) (string, error) + // SignIntermediate will validate the CSR to ensure the trust domain in the + // URI SAN matches the local one and that basic constraints for a CA certificate + // are met. It should return a signed CA certificate with a path length constraint + // of 0 to ensure that the certificate cannot be used to generate further CA certs. + SignIntermediate(*x509.CertificateRequest) (string, error) + // 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 diff --git a/agent/connect/ca/provider_consul.go b/agent/connect/ca/provider_consul.go index 803ce2ac18..6a7cc5b577 100644 --- a/agent/connect/ca/provider_consul.go +++ b/agent/connect/ca/provider_consul.go @@ -25,9 +25,12 @@ var ErrNotInitialized = errors.New("provider not initialized") type ConsulProvider struct { Delegate ConsulProviderStateDelegate - config *structs.ConsulCAProviderConfig - id string - isRoot bool + config *structs.ConsulCAProviderConfig + id string + clusterID string + isRoot bool + spiffeID *connect.SpiffeIDSigning + sync.RWMutex } @@ -44,9 +47,11 @@ func (c *ConsulProvider) Configure(clusterID string, isRoot bool, rawConfig map[ return err } c.config = config - c.isRoot = isRoot hash := sha256.Sum256([]byte(fmt.Sprintf("%s,%s,%v", config.PrivateKey, config.RootCert, isRoot))) c.id = strings.Replace(fmt.Sprintf("% x", hash), " ", ":", -1) + c.clusterID = clusterID + c.isRoot = isRoot + c.spiffeID = connect.SpiffeIDSigningForCluster(&structs.CAConfiguration{ClusterID: clusterID}) // Exit early if the state store has an entry for this provider's config. _, providerState, err := c.Delegate.State().CAProviderState(c.id) @@ -107,8 +112,7 @@ func (c *ConsulProvider) Configure(clusterID string, isRoot bool, rawConfig map[ // ActiveRoot returns the active root CA certificate. func (c *ConsulProvider) ActiveRoot() (string, error) { - state := c.Delegate.State() - _, providerState, err := state.CAProviderState(c.id) + _, providerState, err := c.getState() if err != nil { return "", err } @@ -119,15 +123,11 @@ func (c *ConsulProvider) ActiveRoot() (string, error) { // GenerateRoot initializes a new root certificate and private key // if needed. func (c *ConsulProvider) GenerateRoot() error { - state := c.Delegate.State() - idx, providerState, err := state.CAProviderState(c.id) + idx, providerState, err := c.getState() if err != nil { return err } - if providerState == nil { - return ErrNotInitialized - } if !c.isRoot { return fmt.Errorf("provider is not the root certificate authority") } @@ -170,10 +170,124 @@ func (c *ConsulProvider) GenerateRoot() error { return nil } +// GenerateIntermediateCSR creates a private key and generates a CSR +// for another datacenter's root to sign. +func (c *ConsulProvider) GenerateIntermediateCSR() (string, error) { + _, providerState, err := c.getState() + if err != nil { + return "", err + } + + if c.isRoot { + return "", fmt.Errorf("provider is the root certificate authority, " + + "cannot generate an intermediate CSR") + } + + // Create a new private key and CSR. + signer, pk, err := connect.GeneratePrivateKey() + if err != nil { + return "", err + } + + csr, err := connect.CreateCSR(c.spiffeID, signer) + if err != nil { + return "", err + } + + // Write the new provider state to the store. + newState := *providerState + newState.PrivateKey = pk + args := &structs.CARequest{ + Op: structs.CAOpSetProviderState, + ProviderState: &newState, + } + if err := c.Delegate.ApplyCARequest(args); err != nil { + return "", err + } + + return csr, nil +} + +func (c *ConsulProvider) SetIntermediate(intermediatePEM, rootPEM string) error { + _, providerState, err := c.getState() + if err != nil { + return err + } + + if c.isRoot { + return fmt.Errorf("cannot set an intermediate using another root in the primary datacenter") + } + + // Get the key from the incoming intermediate cert so we can compare it + // to the currently stored key. + intermediate, err := connect.ParseCert(intermediatePEM) + if err != nil { + return fmt.Errorf("error parsing intermediate PEM: %v", err) + } + privKey, err := connect.ParseSigner(providerState.PrivateKey) + if err != nil { + return err + } + + // Compare the two keys to make sure they match. + b1, err := x509.MarshalPKIXPublicKey(intermediate.PublicKey) + if err != nil { + return err + } + b2, err := x509.MarshalPKIXPublicKey(privKey.Public()) + if err != nil { + return err + } + if !bytes.Equal(b1, b2) { + return fmt.Errorf("intermediate cert is for a different private key") + } + + // Validate the remaining fields and make sure the intermediate validates against + // the given root cert. + if !intermediate.IsCA { + return fmt.Errorf("intermediate is not a CA certificate") + } + if got, want := intermediate.URIs[0].String(), c.spiffeID.URI().String(); got != want { + return fmt.Errorf("incoming cert URI %q does not match current URI: %q", got, want) + } + + pool := x509.NewCertPool() + pool.AppendCertsFromPEM([]byte(rootPEM)) + _, err = intermediate.Verify(x509.VerifyOptions{ + Roots: pool, + }) + if err != nil { + return fmt.Errorf("could not verify intermediate cert against root: %v", err) + } + + // Update the state + newState := *providerState + newState.IntermediateCert = intermediatePEM + newState.RootCert = rootPEM + args := &structs.CARequest{ + Op: structs.CAOpSetProviderState, + ProviderState: &newState, + } + if err := c.Delegate.ApplyCARequest(args); err != nil { + return err + } + + return nil +} + // We aren't maintaining separate root/intermediate CAs for the builtin // provider, so just return the root. func (c *ConsulProvider) ActiveIntermediate() (string, error) { - return c.ActiveRoot() + if c.isRoot { + return c.ActiveRoot() + } + + _, providerState, err := c.getState() + if err != nil { + return "", err + } + + return providerState.IntermediateCert, nil } // We aren't maintaining separate root/intermediate CAs for the builtin @@ -216,7 +330,7 @@ func (c *ConsulProvider) Sign(csr *x509.CertificateRequest) (string, error) { return "", err } if signer == nil { - return "", fmt.Errorf("error signing cert: Consul CA not initialized yet") + return "", ErrNotInitialized } keyId, err := connect.KeyId(signer.Public()) if err != nil { @@ -234,7 +348,11 @@ func (c *ConsulProvider) Sign(csr *x509.CertificateRequest) (string, error) { } // Parse the CA cert - caCert, err := connect.ParseCert(providerState.RootCert) + certPEM, err := c.ActiveIntermediate() + if err != nil { + return "", err + } + caCert, err := connect.ParseCert(certPEM) if err != nil { return "", fmt.Errorf("error parsing CA cert: %s", err) } @@ -290,6 +408,96 @@ func (c *ConsulProvider) Sign(csr *x509.CertificateRequest) (string, error) { return buf.String(), nil } +// SignIntermediate will validate the CSR to ensure the trust domain in the +// URI SAN matches the local one and that basic constraints for a CA certificate +// are met. It should return a signed CA certificate with a path length constraint +// of 0 to ensure that the certificate cannot be used to generate further CA certs. +func (c *ConsulProvider) SignIntermediate(csr *x509.CertificateRequest) (string, error) { + idx, providerState, err := c.getState() + if err != nil { + return "", err + } + + if len(csr.URIs) < 1 { + return "", fmt.Errorf("intermediate CSR has no URIs") + } + certURI, err := connect.ParseCertURI(csr.URIs[0]) + if err != nil { + return "", err + } + + // Verify that the trust domain is valid + if !c.spiffeID.CanSign(certURI) { + return "", fmt.Errorf("incoming CSR domain %q is not valid for our domain %q", + certURI.URI().String(), c.spiffeID.URI().String()) + } + + // Get the signing private key. + signer, err := connect.ParseSigner(providerState.PrivateKey) + if err != nil { + return "", err + } + if signer == nil { + return "", ErrNotInitialized + } + subjectKeyId, err := connect.KeyId(csr.PublicKey) + if err != nil { + return "", err + } + + // Parse the CA cert + caCert, err := connect.ParseCert(providerState.RootCert) + if err != nil { + return "", fmt.Errorf("error parsing CA cert: %s", err) + } + + // Cert template for generation + sn := &big.Int{} + sn.SetUint64(idx + 1) + // 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 + // cluster. A minute is more than enough for typical DC clock drift. + effectiveNow := time.Now().Add(-1 * time.Minute) + template := x509.Certificate{ + SerialNumber: sn, + Subject: csr.Subject, + URIs: csr.URIs, + Signature: csr.Signature, + SignatureAlgorithm: csr.SignatureAlgorithm, + PublicKeyAlgorithm: csr.PublicKeyAlgorithm, + PublicKey: csr.PublicKey, + BasicConstraintsValid: true, + KeyUsage: x509.KeyUsageCertSign | + x509.KeyUsageCRLSign | + x509.KeyUsageDigitalSignature, + IsCA: true, + MaxPathLenZero: true, + NotAfter: effectiveNow.Add(365 * 24 * time.Hour), + NotBefore: effectiveNow, + SubjectKeyId: subjectKeyId, + } + + // Create the certificate, PEM encode it and return that value. + var buf bytes.Buffer + bs, err := x509.CreateCertificate( + rand.Reader, &template, caCert, csr.PublicKey, signer) + if err != nil { + return "", fmt.Errorf("error generating certificate: %s", err) + } + err = pem.Encode(&buf, &pem.Block{Type: "CERTIFICATE", Bytes: bs}) + if err != nil { + return "", fmt.Errorf("error encoding certificate: %s", err) + } + + err = c.incrementProviderIndex(providerState) + if err != nil { + return "", err + } + + // Set the response + return buf.String(), nil +} + // CrossSignCA returns the given CA cert signed by the current active root. func (c *ConsulProvider) CrossSignCA(cert *x509.Certificate) (string, error) { c.Lock() @@ -356,6 +564,22 @@ func (c *ConsulProvider) CrossSignCA(cert *x509.Certificate) (string, error) { return buf.String(), nil } +// getState returns the current provider state from the state delegate, and returns +// ErrNotInitialized if no entry is found. +func (c *ConsulProvider) getState() (uint64, *structs.CAConsulProviderState, error) { + state := c.Delegate.State() + idx, providerState, err := state.CAProviderState(c.id) + if err != nil { + return 0, nil, err + } + + if providerState == nil { + return 0, nil, ErrNotInitialized + } + + return idx, providerState, nil +} + // incrementProviderIndex does a write to increment the provider state store table index // used for serial numbers when generating certificates. func (c *ConsulProvider) incrementProviderIndex(providerState *structs.CAConsulProviderState) error { diff --git a/agent/connect/ca/provider_consul_test.go b/agent/connect/ca/provider_consul_test.go index 26f044e30a..46d6bd78d6 100644 --- a/agent/connect/ca/provider_consul_test.go +++ b/agent/connect/ca/provider_consul_test.go @@ -275,6 +275,69 @@ func testCrossSignProviders(t *testing.T, provider1, provider2 Provider) { } } +func TestConsulProvider_SignIntermediate(t *testing.T) { + t.Parallel() + require := require.New(t) + + conf1 := testConsulCAConfig() + delegate1 := newMockDelegate(t, conf1) + provider1 := &ConsulProvider{Delegate: delegate1} + require.NoError(provider1.Configure(conf1.ClusterID, true, conf1.Config)) + require.NoError(provider1.GenerateRoot()) + + conf2 := testConsulCAConfig() + conf2.CreateIndex = 10 + delegate2 := newMockDelegate(t, conf2) + provider2 := &ConsulProvider{Delegate: delegate2} + require.NoError(provider2.Configure(conf2.ClusterID, false, conf2.Config)) + + // Get the intermediate CSR from provider2. + csrPEM, err := provider2.GenerateIntermediateCSR() + require.NoError(err) + csr, err := connect.ParseCSR(csrPEM) + require.NoError(err) + + // Sign the CSR with provider1. + intermediatePEM, err := provider1.SignIntermediate(csr) + require.NoError(err) + rootPEM, err := provider1.ActiveRoot() + require.NoError(err) + + // Give the new intermediate to provider2 to use. + require.NoError(provider2.SetIntermediate(intermediatePEM, rootPEM)) + + // Have provider2 sign a leaf cert and make sure the chain is correct. + 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) + + // 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.AppendCertsFromPEM([]byte(intermediatePEM)) + rootPool := x509.NewCertPool() + rootPool.AppendCertsFromPEM([]byte(rootPEM)) + + _, err = cert.Verify(x509.VerifyOptions{ + Intermediates: intermediatePool, + Roots: rootPool, + }) + require.NoError(err) +} + func TestConsulCAProvider_MigrateOldID(t *testing.T) { t.Parallel() diff --git a/agent/connect/ca/provider_vault.go b/agent/connect/ca/provider_vault.go index cbbccd7f8b..00adbc3ca1 100644 --- a/agent/connect/ca/provider_vault.go +++ b/agent/connect/ca/provider_vault.go @@ -102,6 +102,14 @@ func (v *VaultProvider) GenerateRoot() error { return nil } +func (v *VaultProvider) GenerateIntermediateCSR() (string, error) { + return "", nil +} + +func (v *VaultProvider) SetIntermediate(intermediatePEM, rootPEM string) error { + return nil +} + // ActiveIntermediate returns the current intermediate certificate. func (v *VaultProvider) ActiveIntermediate() (string, error) { return v.getCA(v.config.IntermediatePKIPath) @@ -249,6 +257,10 @@ func (v *VaultProvider) Sign(csr *x509.CertificateRequest) (string, error) { return fmt.Sprintf("%s\n%s", cert, ca), nil } +func (v *VaultProvider) SignIntermediate(*x509.CertificateRequest) (string, error) { + return "", nil +} + // 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) { diff --git a/agent/connect/uri_signing.go b/agent/connect/uri_signing.go index d934360ebe..1b4a4c2391 100644 --- a/agent/connect/uri_signing.go +++ b/agent/connect/uri_signing.go @@ -49,7 +49,7 @@ func (id *SpiffeIDSigning) CanSign(cu CertURI) bool { // that we could open this up later for example to support external // federation of roots and cross-signing external roots that have different // URI structure but it's simpler to start off restrictive. - return id == other + return id.URI().String() == other.URI().String() case *SpiffeIDService: // The host component of the service must be an exact match for now under // ascii case folding (since hostnames are case-insensitive). Later we might