From c1c1580bf8110c08e2f82feba26e9eccd4f11e6e Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Mon, 13 Dec 2021 14:49:32 -0500 Subject: [PATCH 1/9] ca: only return the leaf cert from Sign in vault provider The interface is documented as 'Sign will only return the leaf', and the other providers only return the leaf. It seems like this was added during the initial implementation, so is likely just something we missed. It doesn't break anything , but it does cause confusing cert chains in the API response which could break something in the future. --- agent/connect/ca/provider_vault.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/agent/connect/ca/provider_vault.go b/agent/connect/ca/provider_vault.go index d837fe1a85..4dd0e44a7a 100644 --- a/agent/connect/ca/provider_vault.go +++ b/agent/connect/ca/provider_vault.go @@ -529,12 +529,7 @@ func (v *VaultProvider) Sign(csr *x509.CertificateRequest) (string, error) { if !ok { return "", fmt.Errorf("certificate was not a string") } - ca, ok := response.Data["issuing_ca"].(string) - if !ok { - return "", fmt.Errorf("issuing_ca was not a string") - } - - return EnsureTrailingNewline(cert) + EnsureTrailingNewline(ca), nil + return EnsureTrailingNewline(cert), nil } // SignIntermediate returns a signed CA certificate with a path length constraint From 86994812ed235cb280328a508de5cbb3ab13e6f1 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 5 Jan 2022 19:08:26 -0500 Subject: [PATCH 2/9] ca: cleanup validateSetIntermediate --- agent/connect/ca/common.go | 52 +++++++++++++++-------------- agent/connect/ca/provider_consul.go | 10 +++--- agent/connect/ca/provider_vault.go | 3 +- 3 files changed, 32 insertions(+), 33 deletions(-) diff --git a/agent/connect/ca/common.go b/agent/connect/ca/common.go index d0a17b4569..cef412bd3e 100644 --- a/agent/connect/ca/common.go +++ b/agent/connect/ca/common.go @@ -9,11 +9,7 @@ import ( "github.com/hashicorp/consul/agent/connect" ) -func validateSetIntermediate( - intermediatePEM, rootPEM string, - currentPrivateKey string, // optional - spiffeID *connect.SpiffeIDSigning, -) error { +func validateSetIntermediate(intermediatePEM, rootPEM string, spiffeID *connect.SpiffeIDSigning) error { // Get the key from the incoming intermediate cert so we can compare it // to the currently stored key. intermediate, err := connect.ParseCert(intermediatePEM) @@ -21,26 +17,6 @@ func validateSetIntermediate( return fmt.Errorf("error parsing intermediate PEM: %v", err) } - if currentPrivateKey != "" { - privKey, err := connect.ParseSigner(currentPrivateKey) - 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 { @@ -65,6 +41,32 @@ func validateSetIntermediate( return nil } +func validateIntermediateSignedByPrivateKey(intermediatePEM string, privateKey string) error { + intermediate, err := connect.ParseCert(intermediatePEM) + if err != nil { + return fmt.Errorf("error parsing intermediate PEM: %v", err) + } + + privKey, err := connect.ParseSigner(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") + } + return nil +} + func validateSignIntermediate(csr *x509.CertificateRequest, spiffeID *connect.SpiffeIDSigning) error { // We explicitly _don't_ require that the CSR has a valid SPIFFE signing URI // SAN because AWS PCA doesn't let us set one :(. We need to relax it here diff --git a/agent/connect/ca/provider_consul.go b/agent/connect/ca/provider_consul.go index a12f70a1a0..40eb2f4457 100644 --- a/agent/connect/ca/provider_consul.go +++ b/agent/connect/ca/provider_consul.go @@ -253,12 +253,10 @@ func (c *ConsulProvider) SetIntermediate(intermediatePEM, rootPEM string) error return fmt.Errorf("cannot set an intermediate using another root in the primary datacenter") } - err = validateSetIntermediate( - intermediatePEM, rootPEM, - providerState.PrivateKey, - c.spiffeID, - ) - if err != nil { + if err = validateSetIntermediate(intermediatePEM, rootPEM, c.spiffeID); err != nil { + return err + } + if err := validateIntermediateSignedByPrivateKey(intermediatePEM, providerState.PrivateKey); err != nil { return err } diff --git a/agent/connect/ca/provider_vault.go b/agent/connect/ca/provider_vault.go index 4dd0e44a7a..9320be89c7 100644 --- a/agent/connect/ca/provider_vault.go +++ b/agent/connect/ca/provider_vault.go @@ -402,8 +402,7 @@ func (v *VaultProvider) SetIntermediate(intermediatePEM, rootPEM string) error { return fmt.Errorf("cannot set an intermediate using another root in the primary datacenter") } - // the private key is in vault, so we can't use it in this validation - err := validateSetIntermediate(intermediatePEM, rootPEM, "", v.spiffeID) + err := validateSetIntermediate(intermediatePEM, rootPEM, v.spiffeID) if err != nil { return err } From 71f3ae04e2914c5197fb6115afda3ef27413978f Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 1 Dec 2021 18:32:12 -0500 Subject: [PATCH 3/9] ca: small docs improvements --- agent/connect/ca/provider.go | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/agent/connect/ca/provider.go b/agent/connect/ca/provider.go index 59431fdbd7..8daa62dd1c 100644 --- a/agent/connect/ca/provider.go +++ b/agent/connect/ca/provider.go @@ -171,14 +171,21 @@ type PrimaryProvider interface { } type SecondaryProvider interface { - // GenerateIntermediateCSR generates a CSR for an intermediate CA - // certificate, to be signed by the root of another datacenter. If IsPrimary was - // set to true with Configure(), calling this is an error. + // GenerateIntermediateCSR should return a CSR for an intermediate CA + // certificate. The intermediate CA will be signed by the primary CA and + // should be used by the provider to sign leaf certificates in the local + // datacenter. + // + // After the certificate is signed, SecondaryProvider.SetIntermediate will + // be called to store the intermediate CA. GenerateIntermediateCSR() (string, error) - // SetIntermediate sets the provider to use the given intermediate certificate - // as well as the root it was signed by. This completes the initialization for - // a provider where IsPrimary was set to false in Configure(). + // SetIntermediate is called to store a newly signed leaf signing certificate and + // the chain of certificates back to the root CA certificate. + // + // The provider should save the certificates and use them to + // Provider.Sign leaf certificates. + // TODO: document exactly how the chain is passed. probably in intermediatePEM SetIntermediate(intermediatePEM, rootPEM string) error } From 42ec34d1017c68217106d2241d6f39be5d35b76c Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 5 Jan 2022 18:21:04 -0500 Subject: [PATCH 4/9] ca: examine the full chain in newCARoot make TestNewCARoot much more strict compare the full result instead of only a few fields. add a test case with 2 and 3 certificates in the pem --- agent/connect/ca/provider.go | 8 +- agent/connect/ca/provider_vault.go | 49 ++++- agent/connect/generate.go | 10 +- agent/connect/parsing.go | 34 ++-- agent/connect/testing_ca.go | 5 +- agent/consul/leader_connect_ca.go | 37 ++-- agent/consul/leader_connect_ca_test.go | 89 +++++++++- agent/consul/leader_connect_test.go | 168 ++++++++++++------ .../consul/testdata/cert-with-ec-256-key.pem | 2 +- .../consul/testdata/cert-with-ec-384-key.pem | 2 +- .../testdata/cert-with-rsa-4096-key.pem | 2 +- .../consul/testdata/pem-with-three-certs.pem | 48 +++++ agent/consul/testdata/pem-with-two-certs.pem | 34 ++++ agent/structs/connect_ca.go | 12 +- 14 files changed, 376 insertions(+), 124 deletions(-) create mode 100644 agent/consul/testdata/pem-with-three-certs.pem create mode 100644 agent/consul/testdata/pem-with-two-certs.pem diff --git a/agent/connect/ca/provider.go b/agent/connect/ca/provider.go index 8daa62dd1c..2775a498ea 100644 --- a/agent/connect/ca/provider.go +++ b/agent/connect/ca/provider.go @@ -135,6 +135,7 @@ type PrimaryProvider interface { // the active intermediate. If multiple intermediates are needed to complete // the chain from the signing certificate back to the active root, they should // all by bundled here. + // TODO: replace with GenerateLeafSigningCert GenerateIntermediate() (string, error) // SignIntermediate will validate the CSR to ensure the trust domain in the @@ -193,7 +194,12 @@ type SecondaryProvider interface { // // TODO: rename this struct type RootResult struct { - // PEM encoded certificate that will be used as the primary CA. + // PEM encoded bundle of CA certificates. The first certificate must be the + // primary CA used to sign intermediates for secondary datacenters, and the + // last certificate must be the trusted CA. + // + // If there is only a single certificate in the bundle then it will be used + // as both the primary CA and the trusted CA. PEM string } diff --git a/agent/connect/ca/provider_vault.go b/agent/connect/ca/provider_vault.go index 9320be89c7..986813dbb6 100644 --- a/agent/connect/ca/provider_vault.go +++ b/agent/connect/ca/provider_vault.go @@ -279,7 +279,7 @@ func (v *VaultProvider) GenerateRoot() (RootResult, error) { if err != nil { return RootResult{}, err } - _, err = v.client.Logical().Write(v.config.RootPKIPath+"root/generate/internal", map[string]interface{}{ + resp, err := v.client.Logical().Write(v.config.RootPKIPath+"root/generate/internal", map[string]interface{}{ "common_name": connect.CACN("vault", uid, v.clusterID, v.isPrimary), "uri_sans": v.spiffeID.URI().String(), "key_type": v.config.PrivateKeyType, @@ -288,12 +288,10 @@ func (v *VaultProvider) GenerateRoot() (RootResult, error) { if err != nil { return RootResult{}, err } - - // retrieve the newly generated cert so that we can return it - // TODO: is this already available from the Local().Write() above? - rootPEM, err = v.getCA(v.config.RootPKIPath) - if err != nil { - return RootResult{}, err + var ok bool + rootPEM, ok = resp.Data["certificate"].(string) + if !ok { + return RootResult{}, fmt.Errorf("unexpected response from Vault: %v", resp.Data["certificate"]) } default: @@ -302,7 +300,18 @@ func (v *VaultProvider) GenerateRoot() (RootResult, error) { } } - return RootResult{PEM: rootPEM}, nil + rootChain, err := v.getCAChain(v.config.RootPKIPath) + if err != nil { + return RootResult{}, err + } + + // Workaround for a bug in the Vault PKI API. + // See https://github.com/hashicorp/vault/issues/13489 + if rootChain == "" { + rootChain = rootPEM + } + + return RootResult{PEM: rootChain}, nil } // GenerateIntermediateCSR creates a private key and generates a CSR @@ -467,6 +476,29 @@ func (v *VaultProvider) getCA(path string) (string, error) { return root, nil } +// TODO: refactor to remove duplication with getCA +func (v *VaultProvider) getCAChain(path string) (string, error) { + req := v.client.NewRequest("GET", "/v1/"+path+"/ca_chain") + resp, err := v.client.RawRequest(req) + if resp != nil { + defer resp.Body.Close() + } + if resp != nil && resp.StatusCode == http.StatusNotFound { + return "", ErrBackendNotMounted + } + if err != nil { + return "", err + } + + raw, err := ioutil.ReadAll(resp.Body) + if err != nil { + return "", err + } + + root := EnsureTrailingNewline(string(raw)) + return root, nil +} + // GenerateIntermediate mounts the configured intermediate PKI backend if // necessary, then generates and signs a new CA CSR using the root PKI backend // and updates the intermediate backend to use that new certificate. @@ -571,6 +603,7 @@ func (v *VaultProvider) SignIntermediate(csr *x509.CertificateRequest) (string, // 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) { + // TODO: is this necessary? Doesn't vault check this for us? rootPEM, err := v.getCA(v.config.RootPKIPath) if err != nil { return "", err diff --git a/agent/connect/generate.go b/agent/connect/generate.go index 04b4f2900a..8c40faf9a6 100644 --- a/agent/connect/generate.go +++ b/agent/connect/generate.go @@ -20,11 +20,11 @@ const ( DefaultIntermediateCertTTL = 24 * 365 * time.Hour ) -func pemEncodeKey(key []byte, blockType string) (string, error) { +func pemEncode(value []byte, blockType string) (string, error) { var buf bytes.Buffer - if err := pem.Encode(&buf, &pem.Block{Type: blockType, Bytes: key}); err != nil { - return "", fmt.Errorf("error encoding private key: %s", err) + if err := pem.Encode(&buf, &pem.Block{Type: blockType, Bytes: value}); err != nil { + return "", fmt.Errorf("error encoding value %v: %s", blockType, err) } return buf.String(), nil } @@ -38,7 +38,7 @@ func generateRSAKey(keyBits int) (crypto.Signer, string, error) { } bs := x509.MarshalPKCS1PrivateKey(pk) - pemBlock, err := pemEncodeKey(bs, "RSA PRIVATE KEY") + pemBlock, err := pemEncode(bs, "RSA PRIVATE KEY") if err != nil { return nil, "", err } @@ -73,7 +73,7 @@ func generateECDSAKey(keyBits int) (crypto.Signer, string, error) { return nil, "", fmt.Errorf("error marshaling ECDSA private key: %s", err) } - pemBlock, err := pemEncodeKey(bs, "EC PRIVATE KEY") + pemBlock, err := pemEncode(bs, "EC PRIVATE KEY") if err != nil { return nil, "", err } diff --git a/agent/connect/parsing.go b/agent/connect/parsing.go index 5ee6f805fa..d591926503 100644 --- a/agent/connect/parsing.go +++ b/agent/connect/parsing.go @@ -56,6 +56,21 @@ func ParseLeafCerts(pemValue string) (*x509.Certificate, *x509.CertPool, error) return leaf, intermediates, nil } +// CertSubjects can be used in debugging to return the subject of each +// certificate in the PEM bundle. Each subject is separated by a newline. +func CertSubjects(pem string) string { + certs, err := parseCerts(pem) + if err != nil { + return err.Error() + } + var buf strings.Builder + for _, cert := range certs { + buf.WriteString(cert.Subject.String()) + buf.WriteString("\n") + } + return buf.String() +} + // ParseCerts parses the all x509 certificates from a PEM-encoded value. // The first returned cert is a leaf cert and any other ones are intermediates. // @@ -90,21 +105,10 @@ func parseCerts(pemValue string) ([]*x509.Certificate, error) { return out, nil } -// CalculateCertFingerprint parses the x509 certificate from a PEM-encoded value -// and calculates the SHA-1 fingerprint. -func CalculateCertFingerprint(pemValue string) (string, error) { - // The _ result below is not an error but the remaining PEM bytes. - block, _ := pem.Decode([]byte(pemValue)) - if block == nil { - return "", fmt.Errorf("no PEM-encoded data found") - } - - if block.Type != "CERTIFICATE" { - return "", fmt.Errorf("first PEM-block should be CERTIFICATE type") - } - - hash := sha1.Sum(block.Bytes) - return HexString(hash[:]), nil +// CalculateCertFingerprint calculates the SHA-1 fingerprint from the cert bytes. +func CalculateCertFingerprint(cert []byte) string { + hash := sha1.Sum(cert) + return HexString(hash[:]) } // ParseSigner parses a crypto.Signer from a PEM-encoded key. The private key diff --git a/agent/connect/testing_ca.go b/agent/connect/testing_ca.go index ae3ae819c4..1bbfdc18c4 100644 --- a/agent/connect/testing_ca.go +++ b/agent/connect/testing_ca.go @@ -112,10 +112,7 @@ func testCA(t testing.T, xc *structs.CARoot, keyType string, keyBits int, ttl ti t.Fatalf("error encoding private key: %s", err) } result.RootCert = buf.String() - result.ID, err = CalculateCertFingerprint(result.RootCert) - if err != nil { - t.Fatalf("error generating CA ID fingerprint: %s", err) - } + result.ID = CalculateCertFingerprint(bs) result.SerialNumber = uint64(sn.Int64()) result.NotBefore = template.NotBefore.UTC() result.NotAfter = template.NotAfter.UTC() diff --git a/agent/consul/leader_connect_ca.go b/agent/consul/leader_connect_ca.go index 47b5bafcee..71b21729fc 100644 --- a/agent/consul/leader_connect_ca.go +++ b/agent/consul/leader_connect_ca.go @@ -253,28 +253,24 @@ func (c *CAManager) initializeCAConfig() (*structs.CAConfiguration, error) { return config, nil } -// parseCARoot returns a filled-in structs.CARoot from a raw PEM value. -func parseCARoot(pemValue, provider, clusterID string) (*structs.CARoot, error) { - id, err := connect.CalculateCertFingerprint(pemValue) +// newCARoot returns a filled-in structs.CARoot from a raw PEM value. +func newCARoot(pemValue, provider, clusterID string) (*structs.CARoot, error) { + primaryCert, err := connect.ParseCert(pemValue) if err != nil { - return nil, fmt.Errorf("error parsing root fingerprint: %v", err) + return nil, err } - rootCert, err := connect.ParseCert(pemValue) - if err != nil { - return nil, fmt.Errorf("error parsing root cert: %v", err) - } - keyType, keyBits, err := connect.KeyInfoFromCert(rootCert) + keyType, keyBits, err := connect.KeyInfoFromCert(primaryCert) if err != nil { return nil, fmt.Errorf("error extracting root key info: %v", err) } return &structs.CARoot{ - ID: id, - Name: fmt.Sprintf("%s CA Root Cert", strings.Title(provider)), - SerialNumber: rootCert.SerialNumber.Uint64(), - SigningKeyID: connect.EncodeSigningKeyID(rootCert.SubjectKeyId), + ID: connect.CalculateCertFingerprint(primaryCert.Raw), + Name: fmt.Sprintf("%s CA Primary Cert", strings.Title(provider)), + SerialNumber: primaryCert.SerialNumber.Uint64(), + SigningKeyID: connect.EncodeSigningKeyID(primaryCert.SubjectKeyId), ExternalTrustDomain: clusterID, - NotBefore: rootCert.NotBefore, - NotAfter: rootCert.NotAfter, + NotBefore: primaryCert.NotBefore, + NotAfter: primaryCert.NotAfter, RootCert: pemValue, PrivateKeyType: keyType, PrivateKeyBits: keyBits, @@ -435,7 +431,7 @@ func (c *CAManager) secondaryInitialize(provider ca.Provider, conf *structs.CACo } var roots structs.IndexedCARoots if err := c.delegate.forwardDC("ConnectCA.Roots", c.serverConf.PrimaryDatacenter, &args, &roots); err != nil { - return err + return fmt.Errorf("failed to get CA roots from primary DC: %w", err) } c.secondarySetPrimaryRoots(roots) @@ -487,12 +483,12 @@ func (c *CAManager) primaryInitialize(provider ca.Provider, conf *structs.CAConf return fmt.Errorf("error generating CA root certificate: %v", err) } - rootCA, err := parseCARoot(root.PEM, conf.Provider, conf.ClusterID) + rootCA, err := newCARoot(root.PEM, conf.Provider, conf.ClusterID) if err != nil { return err } - // Also create the intermediate CA, which is the one that actually signs leaf certs + // TODO: delete this interPEM, err := provider.GenerateIntermediate() if err != nil { return fmt.Errorf("error generating intermediate cert: %v", err) @@ -887,7 +883,7 @@ func (c *CAManager) primaryUpdateRootCA(newProvider ca.Provider, args *structs.C } newRootPEM := providerRoot.PEM - newActiveRoot, err := parseCARoot(newRootPEM, args.Config.Provider, args.Config.ClusterID) + newActiveRoot, err := newCARoot(newRootPEM, args.Config.Provider, args.Config.ClusterID) if err != nil { return err } @@ -940,7 +936,7 @@ func (c *CAManager) primaryUpdateRootCA(newProvider ca.Provider, args *structs.C // 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. - // TODO: this cert is already parsed once in parseCARoot, could we remove the second parse? + // TODO: this cert is already parsed once in newCARoot, could we remove the second parse? newRoot, err := connect.ParseCert(newRootPEM) if err != nil { return err @@ -980,6 +976,7 @@ func (c *CAManager) primaryUpdateRootCA(newProvider ca.Provider, args *structs.C } } + // TODO: delete this intermediate, err := newProvider.GenerateIntermediate() if err != nil { return err diff --git a/agent/consul/leader_connect_ca_test.go b/agent/consul/leader_connect_ca_test.go index ead1a763a7..44bbfa8b57 100644 --- a/agent/consul/leader_connect_ca_test.go +++ b/agent/consul/leader_connect_ca_test.go @@ -618,7 +618,7 @@ func TestCAManager_Initialize_Vault_WithIntermediateAsPrimaryCA(t *testing.T) { generateExternalRootCA(t, vclient) meshRootPath := "pki-root" - primaryCert := setupPrimaryCA(t, vclient, meshRootPath) + primaryCert := setupPrimaryCA(t, vclient, meshRootPath, "") _, s1 := testServerWithConfig(t, func(c *Config) { c.CAConfig = &structs.CAConfiguration{ @@ -635,7 +635,6 @@ func TestCAManager_Initialize_Vault_WithIntermediateAsPrimaryCA(t *testing.T) { }, } }) - defer s1.Shutdown() runStep(t, "check primary DC", func(t *testing.T) { testrpc.WaitForTestAgent(t, s1.RPC, "dc1") @@ -704,10 +703,90 @@ func getLeafCert(t *testing.T, codec rpc.ClientCodec, trustDomain string, dc str cert := structs.IssuedCert{} err = msgpackrpc.CallWithCodec(codec, "ConnectCA.Sign", &req, &cert) require.NoError(t, err) - return cert.CertPEM } +func TestCAManager_Initialize_Vault_WithExternalTrustedCA(t *testing.T) { + if testing.Short() { + t.Skip("too slow for testing.Short") + } + ca.SkipIfVaultNotPresent(t) + + vault := ca.NewTestVaultServer(t) + vclient := vault.Client() + rootPEM := generateExternalRootCA(t, vclient) + + primaryCAPath := "pki-primary" + primaryCert := setupPrimaryCA(t, vclient, primaryCAPath, rootPEM) + + _, s1 := testServerWithConfig(t, func(c *Config) { + c.CAConfig = &structs.CAConfiguration{ + Provider: "vault", + Config: map[string]interface{}{ + "Address": vault.Addr, + "Token": vault.RootToken, + "RootPKIPath": primaryCAPath, + "IntermediatePKIPath": "pki-intermediate/", + // TODO: there are failures to init the CA system if these are not set + // to the values of the already initialized CA. + "PrivateKeyType": "ec", + "PrivateKeyBits": 256, + }, + } + }) + + runStep(t, "check primary DC", func(t *testing.T) { + testrpc.WaitForTestAgent(t, s1.RPC, "dc1") + + codec := rpcClient(t, s1) + roots := structs.IndexedCARoots{} + err := msgpackrpc.CallWithCodec(codec, "ConnectCA.Roots", &structs.DCSpecificRequest{}, &roots) + require.NoError(t, err) + require.Len(t, roots.Roots, 1) + require.Equal(t, primaryCert, roots.Roots[0].RootCert) + + leafCertPEM := getLeafCert(t, codec, roots.TrustDomain, "dc1") + verifyLeafCert(t, roots.Roots[0], leafCertPEM) + }) + + // TODO: renew primary leaf signing cert + // TODO: rotate root + + runStep(t, "run secondary DC", func(t *testing.T) { + _, sDC2 := testServerWithConfig(t, func(c *Config) { + c.Datacenter = "dc2" + c.PrimaryDatacenter = "dc1" + c.CAConfig = &structs.CAConfiguration{ + Provider: "vault", + Config: map[string]interface{}{ + "Address": vault.Addr, + "Token": vault.RootToken, + "RootPKIPath": primaryCAPath, + "IntermediatePKIPath": "pki-secondary/", + // TODO: there are failures to init the CA system if these are not set + // to the values of the already initialized CA. + "PrivateKeyType": "ec", + "PrivateKeyBits": 256, + }, + } + }) + defer sDC2.Shutdown() + joinWAN(t, sDC2, s1) + testrpc.WaitForActiveCARoot(t, sDC2.RPC, "dc2", nil) + + codec := rpcClient(t, sDC2) + roots := structs.IndexedCARoots{} + err := msgpackrpc.CallWithCodec(codec, "ConnectCA.Roots", &structs.DCSpecificRequest{}, &roots) + require.NoError(t, err) + require.Len(t, roots.Roots, 1) + + leafCertPEM := getLeafCert(t, codec, roots.TrustDomain, "dc2") + verifyLeafCert(t, roots.Roots[0], leafCertPEM) + + // TODO: renew secondary leaf signing cert + }) +} + func generateExternalRootCA(t *testing.T, client *vaultapi.Client) string { t.Helper() err := client.Sys().Mount("corp", &vaultapi.MountInput{ @@ -725,10 +804,10 @@ func generateExternalRootCA(t *testing.T, client *vaultapi.Client) string { "ttl": "2400h", }) require.NoError(t, err, "failed to generate root") - return resp.Data["certificate"].(string) + return ca.EnsureTrailingNewline(resp.Data["certificate"].(string)) } -func setupPrimaryCA(t *testing.T, client *vaultapi.Client, path string) string { +func setupPrimaryCA(t *testing.T, client *vaultapi.Client, path string, rootPEM string) string { t.Helper() err := client.Sys().Mount(path, &vaultapi.MountInput{ Type: "pki", diff --git a/agent/consul/leader_connect_test.go b/agent/consul/leader_connect_test.go index 50ad18c604..901154eee5 100644 --- a/agent/consul/leader_connect_test.go +++ b/agent/consul/leader_connect_test.go @@ -15,11 +15,13 @@ import ( msgpackrpc "github.com/hashicorp/consul-net-rpc/net-rpc-msgpackrpc" uuid "github.com/hashicorp/go-uuid" "github.com/stretchr/testify/require" + "gotest.tools/v3/assert" "github.com/hashicorp/consul/agent/connect" "github.com/hashicorp/consul/agent/connect/ca" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/agent/token" + "github.com/hashicorp/consul/sdk/testutil" "github.com/hashicorp/consul/sdk/testutil/retry" "github.com/hashicorp/consul/testrpc" ) @@ -1246,74 +1248,122 @@ func TestConnectCA_ConfigurationSet_PersistsRoots(t *testing.T) { }) } -func TestParseCARoot(t *testing.T) { - type test struct { - name string - pem string - wantSerial uint64 - wantSigningKeyID string - wantKeyType string - wantKeyBits int - wantErr bool +func TestNewCARoot(t *testing.T) { + type testCase struct { + name string + pem string + expected *structs.CARoot + expectedErr string } - // Test certs generated with + + run := func(t *testing.T, tc testCase) { + root, err := newCARoot(tc.pem, "provider-name", "cluster-id") + if tc.expectedErr != "" { + testutil.RequireErrorContains(t, err, tc.expectedErr) + return + } + require.NoError(t, err) + assert.DeepEqual(t, root, tc.expected) + } + + // Test certs can be generated with // go run connect/certgen/certgen.go -out-dir /tmp/connect-certs -key-type ec -key-bits 384 - // for various key types. This does limit the exposure to formats that might - // exist in external certificates which can be used as Connect CAs. - // Specifically many other certs will have serial numbers that don't fit into - // 64 bits but for reasons we truncate down to 64 bits which means our - // `SerialNumber` will not match the one reported by openssl. We should - // probably fix that at some point as it seems like a big footgun but it would - // be a breaking API change to change the type to not be a JSON number and - // JSON numbers don't even support the full range of a uint64... - tests := []test{ - {"no cert", "", 0, "", "", 0, true}, + // serial generated with: + // openssl x509 -noout -text + testCases := []testCase{ { - name: "default cert", - // Watchout for indentations they will break PEM format - pem: readTestData(t, "cert-with-ec-256-key.pem"), - // Based on `openssl x509 -noout -text` report from the cert - wantSerial: 8341954965092507701, - wantSigningKeyID: "97:4D:17:81:64:F8:B4:AF:05:E8:6C:79:C5:40:3B:0E:3E:8B:C0:AE:38:51:54:8A:2F:05:DB:E3:E8:E4:24:EC", - wantKeyType: "ec", - wantKeyBits: 256, - wantErr: false, + name: "no cert", + expectedErr: "no PEM-encoded data found", }, { - name: "ec 384 cert", - // Watchout for indentations they will break PEM format - pem: readTestData(t, "cert-with-ec-384-key.pem"), - // Based on `openssl x509 -noout -text` report from the cert - wantSerial: 2935109425518279965, - wantSigningKeyID: "0B:A0:88:9B:DC:95:31:51:2E:3D:D4:F9:42:D0:6A:A0:62:46:82:D2:7C:22:E7:29:A9:AA:E8:A5:8C:CF:C7:42", - wantKeyType: "ec", - wantKeyBits: 384, - wantErr: false, + name: "type=ec bits=256", + pem: readTestData(t, "cert-with-ec-256-key.pem"), + expected: &structs.CARoot{ + ID: "c9:1b:24:e0:89:63:1a:ba:22:01:f4:cf:bc:f1:c0:36:b2:6b:6c:3d", + Name: "Provider-Name CA Primary Cert", + SerialNumber: 8341954965092507701, + SigningKeyID: "97:4d:17:81:64:f8:b4:af:05:e8:6c:79:c5:40:3b:0e:3e:8b:c0:ae:38:51:54:8a:2f:05:db:e3:e8:e4:24:ec", + ExternalTrustDomain: "cluster-id", + NotBefore: time.Date(2019, 10, 17, 11, 46, 29, 0, time.UTC), + NotAfter: time.Date(2029, 10, 17, 11, 46, 29, 0, time.UTC), + RootCert: readTestData(t, "cert-with-ec-256-key.pem"), + Active: true, + PrivateKeyType: "ec", + PrivateKeyBits: 256, + }, }, { - name: "rsa 4096 cert", - // Watchout for indentations they will break PEM format - pem: readTestData(t, "cert-with-rsa-4096-key.pem"), - // Based on `openssl x509 -noout -text` report from the cert - wantSerial: 5186695743100577491, - wantSigningKeyID: "92:FA:CC:97:57:1E:31:84:A2:33:DD:9B:6A:A8:7C:FC:BE:E2:94:CA:AC:B3:33:17:39:3B:B8:67:9B:DC:C1:08", - wantKeyType: "rsa", - wantKeyBits: 4096, - wantErr: false, + name: "type=ec bits=384", + pem: readTestData(t, "cert-with-ec-384-key.pem"), + expected: &structs.CARoot{ + ID: "29:69:c4:0f:aa:8f:bd:07:31:0d:51:3b:45:62:3d:c0:b2:fc:c6:3f", + Name: "Provider-Name CA Primary Cert", + SerialNumber: 2935109425518279965, + SigningKeyID: "0b:a0:88:9b:dc:95:31:51:2e:3d:d4:f9:42:d0:6a:a0:62:46:82:d2:7c:22:e7:29:a9:aa:e8:a5:8c:cf:c7:42", + ExternalTrustDomain: "cluster-id", + NotBefore: time.Date(2019, 10, 17, 11, 55, 18, 0, time.UTC), + NotAfter: time.Date(2029, 10, 17, 11, 55, 18, 0, time.UTC), + RootCert: readTestData(t, "cert-with-ec-384-key.pem"), + Active: true, + PrivateKeyType: "ec", + PrivateKeyBits: 384, + }, + }, + { + name: "type=rsa bits=4096", + pem: readTestData(t, "cert-with-rsa-4096-key.pem"), + expected: &structs.CARoot{ + ID: "3a:6a:e3:e2:2d:44:85:5a:e9:44:3b:ef:d2:90:78:83:7f:61:a2:84", + Name: "Provider-Name CA Primary Cert", + SerialNumber: 5186695743100577491, + SigningKeyID: "92:fa:cc:97:57:1e:31:84:a2:33:dd:9b:6a:a8:7c:fc:be:e2:94:ca:ac:b3:33:17:39:3b:b8:67:9b:dc:c1:08", + ExternalTrustDomain: "cluster-id", + NotBefore: time.Date(2019, 10, 17, 11, 53, 15, 0, time.UTC), + NotAfter: time.Date(2029, 10, 17, 11, 53, 15, 0, time.UTC), + RootCert: readTestData(t, "cert-with-rsa-4096-key.pem"), + Active: true, + PrivateKeyType: "rsa", + PrivateKeyBits: 4096, + }, + }, + { + name: "two certs in pem", + pem: readTestData(t, "pem-with-two-certs.pem"), + expected: &structs.CARoot{ + ID: "42:43:10:1f:71:6b:21:21:d1:10:49:d1:f0:41:78:8c:0a:77:ef:c0", + Name: "Provider-Name CA Primary Cert", + SerialNumber: 17692800288680335732, + SigningKeyID: "9d:5c:27:43:ce:58:7b:ca:3e:7d:c4:fb:b6:2e:b7:13:e9:a1:68:3e", + ExternalTrustDomain: "cluster-id", + NotBefore: time.Date(2022, 1, 5, 23, 22, 12, 0, time.UTC), + NotAfter: time.Date(2022, 4, 7, 15, 22, 42, 0, time.UTC), + RootCert: readTestData(t, "pem-with-two-certs.pem"), + Active: true, + PrivateKeyType: "ec", + PrivateKeyBits: 256, + }, + }, + { + name: "three certs in pem", + pem: readTestData(t, "pem-with-three-certs.pem"), + expected: &structs.CARoot{ + ID: "42:43:10:1f:71:6b:21:21:d1:10:49:d1:f0:41:78:8c:0a:77:ef:c0", + Name: "Provider-Name CA Primary Cert", + SerialNumber: 17692800288680335732, + SigningKeyID: "9d:5c:27:43:ce:58:7b:ca:3e:7d:c4:fb:b6:2e:b7:13:e9:a1:68:3e", + ExternalTrustDomain: "cluster-id", + NotBefore: time.Date(2022, 1, 5, 23, 22, 12, 0, time.UTC), + NotAfter: time.Date(2022, 4, 7, 15, 22, 42, 0, time.UTC), + RootCert: readTestData(t, "pem-with-three-certs.pem"), + Active: true, + PrivateKeyType: "ec", + PrivateKeyBits: 256, + }, }, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - root, err := parseCARoot(tt.pem, "consul", "cluster") - if tt.wantErr { - require.Error(t, err) - return - } - require.NoError(t, err) - require.Equal(t, tt.wantSerial, root.SerialNumber) - require.Equal(t, strings.ToLower(tt.wantSigningKeyID), root.SigningKeyID) - require.Equal(t, tt.wantKeyType, root.PrivateKeyType) - require.Equal(t, tt.wantKeyBits, root.PrivateKeyBits) + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + run(t, tc) }) } } diff --git a/agent/consul/testdata/cert-with-ec-256-key.pem b/agent/consul/testdata/cert-with-ec-256-key.pem index e60234bb5f..b18aa174d8 100644 --- a/agent/consul/testdata/cert-with-ec-256-key.pem +++ b/agent/consul/testdata/cert-with-ec-256-key.pem @@ -9,4 +9,4 @@ VR0jBCQwIoAgl00XgWT4tK8F6Gx5xUA7Dj6LwK44UVSKLwXb4+jkJOwwPwYDVR0R BDgwNoY0c3BpZmZlOi8vMTExMTExMTEtMjIyMi0zMzMzLTQ0NDQtNTU1NTU1NTU1 NTU1LmNvbnN1bDAKBggqhkjOPQQDAgNIADBFAiEA/x2MeYU5vCk2hwP7zlrv7bx3 9zx5YSbn04sgP6sNK30CIEPfjxDGy6K2dPDckATboYkZVQ4CJpPd6WrgwQaHpWC9 ------END CERTIFICATE----- \ No newline at end of file +-----END CERTIFICATE----- diff --git a/agent/consul/testdata/cert-with-ec-384-key.pem b/agent/consul/testdata/cert-with-ec-384-key.pem index cd7aad5564..ae77cfb593 100644 --- a/agent/consul/testdata/cert-with-ec-384-key.pem +++ b/agent/consul/testdata/cert-with-ec-384-key.pem @@ -11,4 +11,4 @@ MTExMS0yMjIyLTMzMzMtNDQ0NC01NTU1NTU1NTU1NTUuY29uc3VsMAoGCCqGSM49 BAMDA2gAMGUCMBT0orKHSATvulb6nRxVHq3OWOfmVgHu8VUCq9yuyAu1AAy/przY /U0ury3g8T4jhwIxAIoCqYwWSJMFb13DZAR3XY+aFssVP5+vzhlaulqtg+YqjpKP KzuCBpS3yUyAwWDphg== ------END CERTIFICATE----- \ No newline at end of file +-----END CERTIFICATE----- diff --git a/agent/consul/testdata/cert-with-rsa-4096-key.pem b/agent/consul/testdata/cert-with-rsa-4096-key.pem index c5f41ed260..d0dd7580dc 100644 --- a/agent/consul/testdata/cert-with-rsa-4096-key.pem +++ b/agent/consul/testdata/cert-with-rsa-4096-key.pem @@ -28,4 +28,4 @@ RPw2YW6oqaMmZ9Uehxym8RDWqyyFPg9S0C73MTK7FitIROLW88hWKSpDDhFck/32 FVaRL8cC0KVlMCFByL/o6u0AsRNCOux1q3BJEdmAh7VI84+SPgztHFkptR4VnlHZ kKTj2Mj/OylHHwhe6AU9pbtAGM6DtcqSjmd4wrkRX8WJDd/F3RlYZ8WhOToOj9gP ra4mUhGz/OlDg6vN9TSeVlb5Ap7c38KoCmmt2n+F/KUpe6V4L1QA5yfz0S8= ------END CERTIFICATE----- \ No newline at end of file +-----END CERTIFICATE----- diff --git a/agent/consul/testdata/pem-with-three-certs.pem b/agent/consul/testdata/pem-with-three-certs.pem new file mode 100644 index 0000000000..f075d2ec6c --- /dev/null +++ b/agent/consul/testdata/pem-with-three-certs.pem @@ -0,0 +1,48 @@ +-----BEGIN CERTIFICATE----- +MIICUjCCATqgAwIBAgIUQjOIDzaM7bGW8bU69Yl0H0C4WXQwDQYJKoZIhvcNAQEL +BQAwFzEVMBMGA1UEAxMMY29ycG9yYXRlIENBMB4XDTIyMDEwNTIzMjIxMloXDTIy +MDQwNzE1MjI0MlowFTETMBEGA1UEAxMKcHJpbWFyeSBDQTBZMBMGByqGSM49AgEG +CCqGSM49AwEHA0IABEIcOmVSobge9pLDGh6rfyFg2+ilTFmo2ICv5vrgUfIZhi8O +fwYz5WGb7qBPRdMw9kP8BWH/lCrn2W3Ax3x2E+2jYzBhMA4GA1UdDwEB/wQEAwIB +BjAPBgNVHRMBAf8EBTADAQH/MB0GA1UdDgQWBBSdXCdDzlh7yj59xPu2LrcT6aFo +PjAfBgNVHSMEGDAWgBS+x+IFMFb+hCJy1OQzcdzJuwDVhDANBgkqhkiG9w0BAQsF +AAOCAQEAWWBBoygbjUEtoueGuC+jHAlr+VOBwiQPJLQA+xtbCvWSn8yIx/M1RyhY +0/6WLMzhYA1lQAIze8CgKzqoGXXIcHif3PRZ3mRUMNdV/qGUv0oHZBzTKZVySOIm +MLIoq7WvyVdVNxyvRalhHxiQA1Hrh+zQKjXhVPM6dpG0duTNYit9kJCCeNDzRjWc +a/GgFyeeYMTheU3eBR6Vp2A8hy2h5xw82ul8YLwX0bCtcP12XAUzj3jFqwt6RLxW +Wc7rvsLfgimEfulQwo2WLPWZw8bJdnPvNcUFX8f2Zvqy0Jg6fELnxO+AdHnAnI9J +WtJr0ImA95Hw8gGTzmXOddYVGHuGLA== +-----END CERTIFICATE----- +-----BEGIN CERTIFICATE----- +MIIDHzCCAgegAwIBAgIUDaEOI5nsEt9abNBbJibbQt+VZQIwDQYJKoZIhvcNAQEL +BQAwFzEVMBMGA1UEAxMMY29ycG9yYXRlIENBMB4XDTIyMDEwNTIzMjIxMloXDTIy +MDQxNTIzMjI0MlowFzEVMBMGA1UEAxMMY29ycG9yYXRlIENBMIIBIjANBgkqhkiG +9w0BAQEFAAOCAQ8AMIIBCgKCAQEAut/Gbr3MvypzEmRDTl7HGaSoVIydNEZNPqDD +jh1lqMFywB4DujTmkWLYcPJJ0RTT2NsSakteti/e1DHCuBSU0t3Q3K1paTh8aVLx +eK0IKNlCWqX5d1aYzCNZsRjJuQgPX6p/xcNGS+RS27jmRWPpvm6n1JfMvYRa7fF+ +HnKhGNO+hDbhkQO4s0V1U+unNhshKDhTW3mBLmAEb2OHLOEaUZtYSbqr1E9tYXgU +DiYRkeWUpQXJ6pE91fmcaZFG0SxkqWnhe7GUa6wbb/vROWph4A1ZVHympBtOYwoJ +eibcJjBZLrugZdix8kl8NDI7SuIM/P0x0m9WkNfhJ9vSgQXlaQIDAQABo2MwYTAO +BgNVHQ8BAf8EBAMCAQYwDwYDVR0TAQH/BAUwAwEB/zAdBgNVHQ4EFgQUvsfiBTBW +/oQictTkM3HcybsA1YQwHwYDVR0jBBgwFoAUvsfiBTBW/oQictTkM3HcybsA1YQw +DQYJKoZIhvcNAQELBQADggEBALUJWitOV4xAvfNB8Z20AQ+/hdXkWVgj1VBbd++v ++X88q1TnueKAExU5o87MCjh9jMxalZqSVN9MUbQ4Xa+tmkjayizdpFaw6TbbaMIB +Tgqq5ATXMnOdZd46QC764Po9R9+k9hk4dNIr5gk1ifXZDMy/7jSOVARvpwzr0cTx +flRCTgZbcK10freoU7a74/YjEpG0wggGlR4aRWfm90Im9JM3aI55zAYQFzduf56c +HXJDLgBtbOx/ceqVrkPdvYwP9Q34tKAMiheQ0G3tTxP3Xc87gh4UEDV02oHhcbqw +WSm+8zTfGUlchowPRdqKE66urWTep+BA9c8zUqDdoq5lE9s= +-----END CERTIFICATE----- +-----BEGIN CERTIFICATE----- +MIICGTCCAZ+gAwIBAgIIJhC6ZZyZ/lQwCgYIKoZIzj0EAwMwFDESMBAGA1UEAxMJ +VGVzdCBDQSAxMB4XDTIyMDEwNTIzNDMyNVoXDTMyMDEwNTIzNDMyNVowFDESMBAG +A1UEAxMJVGVzdCBDQSAxMHYwEAYHKoZIzj0CAQYFK4EEACIDYgAETEyAhuLLOcxy +z2UHI7ePcB5AXL1o6mLwVfzyeaGfqUevzrFcLQ7WPiypZJW1KhOW5Q2bRgcjE8y3 +fN+B8D+KT4fPtaRLtUVX6aZ0LCROFdgWjVo2DCvCq5VQnCGjW8r0o4G9MIG6MA4G +A1UdDwEB/wQEAwIBhjAPBgNVHRMBAf8EBTADAQH/MCkGA1UdDgQiBCBU/reewmUW +iduB8xxfW5clyUmrMewrWwtJuWPA/tFvTTArBgNVHSMEJDAigCBU/reewmUWiduB +8xxfW5clyUmrMewrWwtJuWPA/tFvTTA/BgNVHREEODA2hjRzcGlmZmU6Ly8xMTEx +MTExMS0yMjIyLTMzMzMtNDQ0NC01NTU1NTU1NTU1NTUuY29uc3VsMAoGCCqGSM49 +BAMDA2gAMGUCMA4V/Iemelne4ZB+0glmxoKV6OPQ64oKkkrcy+vo1t1RZ+7jntRx +mxAnY3S2m35boQIxAOARpY+qfR3U3JM+vMW9KO0/KqM+y1/uvIaOA0bQex2w8bfN +V+QjFUDmjTT1dLpc7A== +-----END CERTIFICATE----- diff --git a/agent/consul/testdata/pem-with-two-certs.pem b/agent/consul/testdata/pem-with-two-certs.pem new file mode 100644 index 0000000000..0e1cb65265 --- /dev/null +++ b/agent/consul/testdata/pem-with-two-certs.pem @@ -0,0 +1,34 @@ +-----BEGIN CERTIFICATE----- +MIICUjCCATqgAwIBAgIUQjOIDzaM7bGW8bU69Yl0H0C4WXQwDQYJKoZIhvcNAQEL +BQAwFzEVMBMGA1UEAxMMY29ycG9yYXRlIENBMB4XDTIyMDEwNTIzMjIxMloXDTIy +MDQwNzE1MjI0MlowFTETMBEGA1UEAxMKcHJpbWFyeSBDQTBZMBMGByqGSM49AgEG +CCqGSM49AwEHA0IABEIcOmVSobge9pLDGh6rfyFg2+ilTFmo2ICv5vrgUfIZhi8O +fwYz5WGb7qBPRdMw9kP8BWH/lCrn2W3Ax3x2E+2jYzBhMA4GA1UdDwEB/wQEAwIB +BjAPBgNVHRMBAf8EBTADAQH/MB0GA1UdDgQWBBSdXCdDzlh7yj59xPu2LrcT6aFo +PjAfBgNVHSMEGDAWgBS+x+IFMFb+hCJy1OQzcdzJuwDVhDANBgkqhkiG9w0BAQsF +AAOCAQEAWWBBoygbjUEtoueGuC+jHAlr+VOBwiQPJLQA+xtbCvWSn8yIx/M1RyhY +0/6WLMzhYA1lQAIze8CgKzqoGXXIcHif3PRZ3mRUMNdV/qGUv0oHZBzTKZVySOIm +MLIoq7WvyVdVNxyvRalhHxiQA1Hrh+zQKjXhVPM6dpG0duTNYit9kJCCeNDzRjWc +a/GgFyeeYMTheU3eBR6Vp2A8hy2h5xw82ul8YLwX0bCtcP12XAUzj3jFqwt6RLxW +Wc7rvsLfgimEfulQwo2WLPWZw8bJdnPvNcUFX8f2Zvqy0Jg6fELnxO+AdHnAnI9J +WtJr0ImA95Hw8gGTzmXOddYVGHuGLA== +-----END CERTIFICATE----- +-----BEGIN CERTIFICATE----- +MIIDHzCCAgegAwIBAgIUDaEOI5nsEt9abNBbJibbQt+VZQIwDQYJKoZIhvcNAQEL +BQAwFzEVMBMGA1UEAxMMY29ycG9yYXRlIENBMB4XDTIyMDEwNTIzMjIxMloXDTIy +MDQxNTIzMjI0MlowFzEVMBMGA1UEAxMMY29ycG9yYXRlIENBMIIBIjANBgkqhkiG +9w0BAQEFAAOCAQ8AMIIBCgKCAQEAut/Gbr3MvypzEmRDTl7HGaSoVIydNEZNPqDD +jh1lqMFywB4DujTmkWLYcPJJ0RTT2NsSakteti/e1DHCuBSU0t3Q3K1paTh8aVLx +eK0IKNlCWqX5d1aYzCNZsRjJuQgPX6p/xcNGS+RS27jmRWPpvm6n1JfMvYRa7fF+ +HnKhGNO+hDbhkQO4s0V1U+unNhshKDhTW3mBLmAEb2OHLOEaUZtYSbqr1E9tYXgU +DiYRkeWUpQXJ6pE91fmcaZFG0SxkqWnhe7GUa6wbb/vROWph4A1ZVHympBtOYwoJ +eibcJjBZLrugZdix8kl8NDI7SuIM/P0x0m9WkNfhJ9vSgQXlaQIDAQABo2MwYTAO +BgNVHQ8BAf8EBAMCAQYwDwYDVR0TAQH/BAUwAwEB/zAdBgNVHQ4EFgQUvsfiBTBW +/oQictTkM3HcybsA1YQwHwYDVR0jBBgwFoAUvsfiBTBW/oQictTkM3HcybsA1YQw +DQYJKoZIhvcNAQELBQADggEBALUJWitOV4xAvfNB8Z20AQ+/hdXkWVgj1VBbd++v ++X88q1TnueKAExU5o87MCjh9jMxalZqSVN9MUbQ4Xa+tmkjayizdpFaw6TbbaMIB +Tgqq5ATXMnOdZd46QC764Po9R9+k9hk4dNIr5gk1ifXZDMy/7jSOVARvpwzr0cTx +flRCTgZbcK10freoU7a74/YjEpG0wggGlR4aRWfm90Im9JM3aI55zAYQFzduf56c +HXJDLgBtbOx/ceqVrkPdvYwP9Q34tKAMiheQ0G3tTxP3Xc87gh4UEDV02oHhcbqw +WSm+8zTfGUlchowPRdqKE66urWTep+BA9c8zUqDdoq5lE9s= +-----END CERTIFICATE----- diff --git a/agent/structs/connect_ca.go b/agent/structs/connect_ca.go index 4b4549b7ae..9d3f00d1cc 100644 --- a/agent/structs/connect_ca.go +++ b/agent/structs/connect_ca.go @@ -66,14 +66,15 @@ func (r IndexedCARoots) Active() *CARoot { // CARoot represents a root CA certificate that is trusted. type CARoot struct { - // ID is a globally unique ID (UUID) representing this CA root. + // ID is a globally unique ID (UUID) representing this CA chain. It is + // calculated from the SHA1 of the primary CA certificate. ID string // Name is a human-friendly name for this CA root. This value is // opaque to Consul and is not used for anything internally. Name string - // SerialNumber is the x509 serial number of the certificate. + // SerialNumber is the x509 serial number of the primary CA certificate. SerialNumber uint64 // SigningKeyID is the connect.HexString encoded id of the public key that @@ -96,9 +97,12 @@ type CARoot struct { // future flexibility. ExternalTrustDomain string - // Time validity bounds. + // NotBefore is the x509.Certificate.NotBefore value of the primary CA + // certificate. This value should generally be a time in the past. NotBefore time.Time - NotAfter time.Time + // NotAfter is the x509.Certificate.NotAfter value of the primary CA + // certificate. This is the time when the certificate will expire. + NotAfter time.Time // RootCert is the PEM-encoded public certificate for the root CA. The // certificate is the same for all federated clusters. From 5e8ea2a039254e86325bf5c2723d504b5bf52207 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 5 Jan 2022 19:03:42 -0500 Subject: [PATCH 5/9] ca: add a test for secondary with external CA --- agent/connect/ca/provider.go | 1 - agent/consul/leader_connect_ca_test.go | 139 ++++++++++++++++++------- 2 files changed, 102 insertions(+), 38 deletions(-) diff --git a/agent/connect/ca/provider.go b/agent/connect/ca/provider.go index 2775a498ea..614a4456cf 100644 --- a/agent/connect/ca/provider.go +++ b/agent/connect/ca/provider.go @@ -186,7 +186,6 @@ type SecondaryProvider interface { // // The provider should save the certificates and use them to // Provider.Sign leaf certificates. - // TODO: document exactly how the chain is passed. probably in intermediatePEM SetIntermediate(intermediatePEM, rootPEM string) error } diff --git a/agent/consul/leader_connect_ca_test.go b/agent/consul/leader_connect_ca_test.go index 44bbfa8b57..f0aabb823f 100644 --- a/agent/consul/leader_connect_ca_test.go +++ b/agent/consul/leader_connect_ca_test.go @@ -12,6 +12,7 @@ import ( "fmt" "math/big" "net/url" + "strings" "testing" "time" @@ -82,7 +83,6 @@ func TestCAManager_Initialize_Vault_Secondary_SharedVault(t *testing.T) { }, } }) - defer serverDC2.Shutdown() joinWAN(t, serverDC2, serverDC1) testrpc.WaitForActiveCARoot(t, serverDC2.RPC, "dc2", nil) @@ -719,7 +719,7 @@ func TestCAManager_Initialize_Vault_WithExternalTrustedCA(t *testing.T) { primaryCAPath := "pki-primary" primaryCert := setupPrimaryCA(t, vclient, primaryCAPath, rootPEM) - _, s1 := testServerWithConfig(t, func(c *Config) { + _, serverDC1 := testServerWithConfig(t, func(c *Config) { c.CAConfig = &structs.CAConfiguration{ Provider: "vault", Config: map[string]interface{}{ @@ -734,57 +734,118 @@ func TestCAManager_Initialize_Vault_WithExternalTrustedCA(t *testing.T) { }, } }) + testrpc.WaitForTestAgent(t, serverDC1.RPC, "dc1") - runStep(t, "check primary DC", func(t *testing.T) { - testrpc.WaitForTestAgent(t, s1.RPC, "dc1") - - codec := rpcClient(t, s1) - roots := structs.IndexedCARoots{} + var origLeaf string + roots := structs.IndexedCARoots{} + runStep(t, "verify primary DC", func(t *testing.T) { + codec := rpcClient(t, serverDC1) err := msgpackrpc.CallWithCodec(codec, "ConnectCA.Roots", &structs.DCSpecificRequest{}, &roots) require.NoError(t, err) require.Len(t, roots.Roots, 1) require.Equal(t, primaryCert, roots.Roots[0].RootCert) + require.Contains(t, roots.Roots[0].RootCert, rootPEM) - leafCertPEM := getLeafCert(t, codec, roots.TrustDomain, "dc1") - verifyLeafCert(t, roots.Roots[0], leafCertPEM) + leafCert := getLeafCert(t, codec, roots.TrustDomain, "dc1") + verifyLeafCert(t, roots.Active(), leafCert) + origLeaf = leafCert + }) + + _, serverDC2 := testServerWithConfig(t, func(c *Config) { + c.Datacenter = "dc2" + c.PrimaryDatacenter = "dc1" + c.CAConfig = &structs.CAConfiguration{ + Provider: "vault", + Config: map[string]interface{}{ + "Address": vault.Addr, + "Token": vault.RootToken, + "RootPKIPath": "should-be-ignored", + "IntermediatePKIPath": "pki-secondary/", + }, + } }) // TODO: renew primary leaf signing cert // TODO: rotate root - runStep(t, "run secondary DC", func(t *testing.T) { - _, sDC2 := testServerWithConfig(t, func(c *Config) { - c.Datacenter = "dc2" - c.PrimaryDatacenter = "dc1" - c.CAConfig = &structs.CAConfiguration{ - Provider: "vault", - Config: map[string]interface{}{ - "Address": vault.Addr, - "Token": vault.RootToken, - "RootPKIPath": primaryCAPath, - "IntermediatePKIPath": "pki-secondary/", - // TODO: there are failures to init the CA system if these are not set - // to the values of the already initialized CA. - "PrivateKeyType": "ec", - "PrivateKeyBits": 256, - }, - } - }) - defer sDC2.Shutdown() - joinWAN(t, sDC2, s1) - testrpc.WaitForActiveCARoot(t, sDC2.RPC, "dc2", nil) + runStep(t, "start secondary DC", func(t *testing.T) { + joinWAN(t, serverDC2, serverDC1) + testrpc.WaitForActiveCARoot(t, serverDC2.RPC, "dc2", nil) - codec := rpcClient(t, sDC2) + codec := rpcClient(t, serverDC2) roots := structs.IndexedCARoots{} err := msgpackrpc.CallWithCodec(codec, "ConnectCA.Roots", &structs.DCSpecificRequest{}, &roots) require.NoError(t, err) require.Len(t, roots.Roots, 1) - leafCertPEM := getLeafCert(t, codec, roots.TrustDomain, "dc2") - verifyLeafCert(t, roots.Roots[0], leafCertPEM) - - // TODO: renew secondary leaf signing cert + leafPEM := getLeafCert(t, codec, roots.TrustDomain, "dc2") + verifyLeafCert(t, roots.Roots[0], leafPEM) }) + + runStep(t, "renew leaf signing CA in primary", func(t *testing.T) { + previous := serverDC1.caManager.getLeafSigningCertFromRoot(roots.Active()) + + renewLeafSigningCert(t, serverDC1.caManager, serverDC1.caManager.primaryRenewIntermediate) + + codec := rpcClient(t, serverDC1) + err := msgpackrpc.CallWithCodec(codec, "ConnectCA.Roots", &structs.DCSpecificRequest{}, &roots) + require.NoError(t, err) + require.Len(t, roots.Roots, 1) + require.Len(t, roots.Roots[0].IntermediateCerts, 2) + + newCert := serverDC1.caManager.getLeafSigningCertFromRoot(roots.Active()) + require.NotEqual(t, previous, newCert) + + leafPEM := getLeafCert(t, codec, roots.TrustDomain, "dc1") + verifyLeafCert(t, roots.Roots[0], leafPEM) + + // original certs from old signing cert should still verify + verifyLeafCert(t, roots.Roots[0], origLeaf) + }) + + runStep(t, "renew leaf signing CA in secondary", func(t *testing.T) { + previous := serverDC2.caManager.getLeafSigningCertFromRoot(roots.Active()) + + renewLeafSigningCert(t, serverDC2.caManager, serverDC2.caManager.secondaryRequestNewSigningCert) + + codec := rpcClient(t, serverDC2) + err := msgpackrpc.CallWithCodec(codec, "ConnectCA.Roots", &structs.DCSpecificRequest{}, &roots) + require.NoError(t, err) + require.Len(t, roots.Roots, 1) + // one intermediate from primary, two from secondary + require.Len(t, roots.Roots[0].IntermediateCerts, 3) + + newCert := serverDC1.caManager.getLeafSigningCertFromRoot(roots.Active()) + require.NotEqual(t, previous, newCert) + + leafPEM := getLeafCert(t, codec, roots.TrustDomain, "dc2") + verifyLeafCert(t, roots.Roots[0], leafPEM) + + // original certs from old signing cert should still verify + verifyLeafCert(t, roots.Roots[0], origLeaf) + }) +} + +// renewLeafSigningCert mimics RenewIntermediate. This is unfortunate, but +// necessary for now as there is no easy way to invoke that logic unconditionally. +// Currently, it requires patching values and polling for the operation to +// complete, which adds a lot of distractions to a test case. +// With this function we can instead unconditionally rotate the leaf signing cert +// synchronously. +func renewLeafSigningCert(t *testing.T, manager *CAManager, fn func(ca.Provider, *structs.CARoot) error) { + t.Helper() + provider, _ := manager.getCAProvider() + + store := manager.delegate.State() + _, root, err := store.CARootActive(nil) + require.NoError(t, err) + + activeRoot := root.Clone() + err = fn(provider, activeRoot) + require.NoError(t, err) + err = manager.persistNewRootAndConfig(provider, activeRoot, nil) + require.NoError(t, err) + manager.setCAProvider(provider, activeRoot) } func generateExternalRootCA(t *testing.T, client *vaultapi.Client) string { @@ -835,9 +896,13 @@ func setupPrimaryCA(t *testing.T, client *vaultapi.Client, path string, rootPEM }) require.NoError(t, err, "failed to sign intermediate") + var buf strings.Builder + buf.WriteString(ca.EnsureTrailingNewline(intermediate.Data["certificate"].(string))) + buf.WriteString(ca.EnsureTrailingNewline(rootPEM)) + _, err = client.Logical().Write(path+"/intermediate/set-signed", map[string]interface{}{ - "certificate": intermediate.Data["certificate"], + "certificate": buf.String(), }) require.NoError(t, err, "failed to set signed intermediate") - return ca.EnsureTrailingNewline(intermediate.Data["certificate"].(string)) + return ca.EnsureTrailingNewline(buf.String()) } From 1853a32df6fe63c70e5e17bd8345e20257712dcf Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 2 Feb 2022 18:51:59 -0500 Subject: [PATCH 6/9] ca: add test cases for rotating external trusted CA --- .changelog/11910.txt | 3 + agent/consul/leader_connect_ca_test.go | 108 +++++++++++++++++++------ 2 files changed, 88 insertions(+), 23 deletions(-) create mode 100644 .changelog/11910.txt diff --git a/.changelog/11910.txt b/.changelog/11910.txt new file mode 100644 index 0000000000..8bae2b3f8b --- /dev/null +++ b/.changelog/11910.txt @@ -0,0 +1,3 @@ +```release-note:feature +ca: support using an external root CA with the vault CA provider +``` diff --git a/agent/consul/leader_connect_ca_test.go b/agent/consul/leader_connect_ca_test.go index f0aabb823f..9bdfc07401 100644 --- a/agent/consul/leader_connect_ca_test.go +++ b/agent/consul/leader_connect_ca_test.go @@ -98,14 +98,25 @@ func TestCAManager_Initialize_Vault_Secondary_SharedVault(t *testing.T) { } func verifyLeafCert(t *testing.T, root *structs.CARoot, leafCertPEM string) { + t.Helper() + roots := structs.IndexedCARoots{ + ActiveRootID: root.ID, + Roots: []*structs.CARoot{root}, + } + verifyLeafCertWithRoots(t, roots, leafCertPEM) +} + +func verifyLeafCertWithRoots(t *testing.T, roots structs.IndexedCARoots, leafCertPEM string) { t.Helper() leaf, intermediates, err := connect.ParseLeafCerts(leafCertPEM) require.NoError(t, err) pool := x509.NewCertPool() - ok := pool.AppendCertsFromPEM([]byte(root.RootCert)) - if !ok { - t.Fatalf("Failed to add root CA PEM to cert pool") + for _, r := range roots.Roots { + ok := pool.AppendCertsFromPEM([]byte(r.RootCert)) + if !ok { + t.Fatalf("Failed to add root CA PEM to cert pool") + } } // verify with intermediates from leaf CertPEM @@ -118,10 +129,12 @@ func verifyLeafCert(t *testing.T, root *structs.CARoot, leafCertPEM string) { // verify with intermediates from the CARoot intermediates = x509.NewCertPool() - for _, intermediate := range root.IntermediateCerts { - c, err := connect.ParseCert(intermediate) - require.NoError(t, err) - intermediates.AddCert(c) + for _, r := range roots.Roots { + for _, intermediate := range r.IntermediateCerts { + c, err := connect.ParseCert(intermediate) + require.NoError(t, err) + intermediates.AddCert(c) + } } _, err = leaf.Verify(x509.VerifyOptions{ @@ -628,10 +641,6 @@ func TestCAManager_Initialize_Vault_WithIntermediateAsPrimaryCA(t *testing.T) { "Token": vault.RootToken, "RootPKIPath": meshRootPath, "IntermediatePKIPath": "pki-intermediate/", - // TODO: there are failures to init the CA system if these are not set - // to the values of the already initialized CA. - "PrivateKeyType": "ec", - "PrivateKeyBits": 256, }, } }) @@ -664,10 +673,6 @@ func TestCAManager_Initialize_Vault_WithIntermediateAsPrimaryCA(t *testing.T) { "Token": vault.RootToken, "RootPKIPath": meshRootPath, "IntermediatePKIPath": "pki-secondary/", - // TODO: there are failures to init the CA system if these are not set - // to the values of the already initialized CA. - "PrivateKeyType": "ec", - "PrivateKeyBits": 256, }, } }) @@ -727,10 +732,6 @@ func TestCAManager_Initialize_Vault_WithExternalTrustedCA(t *testing.T) { "Token": vault.RootToken, "RootPKIPath": primaryCAPath, "IntermediatePKIPath": "pki-intermediate/", - // TODO: there are failures to init the CA system if these are not set - // to the values of the already initialized CA. - "PrivateKeyType": "ec", - "PrivateKeyBits": 256, }, } }) @@ -765,15 +766,12 @@ func TestCAManager_Initialize_Vault_WithExternalTrustedCA(t *testing.T) { } }) - // TODO: renew primary leaf signing cert - // TODO: rotate root - runStep(t, "start secondary DC", func(t *testing.T) { joinWAN(t, serverDC2, serverDC1) testrpc.WaitForActiveCARoot(t, serverDC2.RPC, "dc2", nil) codec := rpcClient(t, serverDC2) - roots := structs.IndexedCARoots{} + roots = structs.IndexedCARoots{} err := msgpackrpc.CallWithCodec(codec, "ConnectCA.Roots", &structs.DCSpecificRequest{}, &roots) require.NoError(t, err) require.Len(t, roots.Roots, 1) @@ -788,6 +786,7 @@ func TestCAManager_Initialize_Vault_WithExternalTrustedCA(t *testing.T) { renewLeafSigningCert(t, serverDC1.caManager, serverDC1.caManager.primaryRenewIntermediate) codec := rpcClient(t, serverDC1) + roots = structs.IndexedCARoots{} err := msgpackrpc.CallWithCodec(codec, "ConnectCA.Roots", &structs.DCSpecificRequest{}, &roots) require.NoError(t, err) require.Len(t, roots.Roots, 1) @@ -809,6 +808,7 @@ func TestCAManager_Initialize_Vault_WithExternalTrustedCA(t *testing.T) { renewLeafSigningCert(t, serverDC2.caManager, serverDC2.caManager.secondaryRequestNewSigningCert) codec := rpcClient(t, serverDC2) + roots = structs.IndexedCARoots{} err := msgpackrpc.CallWithCodec(codec, "ConnectCA.Roots", &structs.DCSpecificRequest{}, &roots) require.NoError(t, err) require.Len(t, roots.Roots, 1) @@ -824,6 +824,68 @@ func TestCAManager_Initialize_Vault_WithExternalTrustedCA(t *testing.T) { // original certs from old signing cert should still verify verifyLeafCert(t, roots.Roots[0], origLeaf) }) + + runStep(t, "rotate root by changing the provider", func(t *testing.T) { + codec := rpcClient(t, serverDC1) + req := &structs.CARequest{ + Op: structs.CAOpSetConfig, + Config: &structs.CAConfiguration{ + Provider: "consul", + }, + } + var resp error + err := msgpackrpc.CallWithCodec(codec, "ConnectCA.ConfigurationSet", req, &resp) + require.NoError(t, err) + require.Nil(t, resp) + + roots = structs.IndexedCARoots{} + err = msgpackrpc.CallWithCodec(codec, "ConnectCA.Roots", &structs.DCSpecificRequest{}, &roots) + require.NoError(t, err) + require.Len(t, roots.Roots, 2) + active := roots.Active() + require.Len(t, active.IntermediateCerts, 1) + + leafPEM := getLeafCert(t, codec, roots.TrustDomain, "dc1") + verifyLeafCert(t, roots.Active(), leafPEM) + + // original certs from old root cert should still verify + verifyLeafCertWithRoots(t, roots, origLeaf) + }) + + runStep(t, "rotate to a different external root", func(t *testing.T) { + setupPrimaryCA(t, vclient, "pki-primary-2/", rootPEM) + + codec := rpcClient(t, serverDC1) + req := &structs.CARequest{ + Op: structs.CAOpSetConfig, + Config: &structs.CAConfiguration{ + Provider: "vault", + Config: map[string]interface{}{ + "Address": vault.Addr, + "Token": vault.RootToken, + "RootPKIPath": "pki-primary-2/", + "IntermediatePKIPath": "pki-intermediate-2/", + }, + }, + } + var resp error + err := msgpackrpc.CallWithCodec(codec, "ConnectCA.ConfigurationSet", req, &resp) + require.NoError(t, err) + require.Nil(t, resp) + + roots = structs.IndexedCARoots{} + err = msgpackrpc.CallWithCodec(codec, "ConnectCA.Roots", &structs.DCSpecificRequest{}, &roots) + require.NoError(t, err) + require.Len(t, roots.Roots, 3) + active := roots.Active() + require.Len(t, active.IntermediateCerts, 2) + + leafPEM := getLeafCert(t, codec, roots.TrustDomain, "dc1") + verifyLeafCert(t, roots.Active(), leafPEM) + + // original certs from old root cert should still verify + verifyLeafCertWithRoots(t, roots, origLeaf) + }) } // renewLeafSigningCert mimics RenewIntermediate. This is unfortunate, but From 12f12d577ad84110b297040848237b3628172308 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 11 Feb 2022 16:34:34 -0500 Subject: [PATCH 7/9] docs: add docs for using an external CA --- website/content/docs/connect/ca/vault.mdx | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/website/content/docs/connect/ca/vault.mdx b/website/content/docs/connect/ca/vault.mdx index 3929c43ee0..0b2d1061bf 100644 --- a/website/content/docs/connect/ca/vault.mdx +++ b/website/content/docs/connect/ca/vault.mdx @@ -116,16 +116,25 @@ The configuration options are listed below. - `RootPKIPath` / `root_pki_path` (`string: `) - The path to - a PKI secrets engine for the root certificate. If the path does not + a PKI secrets engine for the root certificate. + + If the path does not exist, Consul will mount a new PKI secrets engine at the specified path with the `RootCertTTL` value as the root certificate's TTL. If the `RootCertTTL` is not set, a [`max_lease_ttl`](https://www.vaultproject.io/api/system/mounts#max_lease_ttl) of 87600 hours, or 10 years is applied by default as of Consul 1.11 and later. Prior to Consul 1.11, the root certificate TTL was set to 8760 hour, or 1 year, and was not configurable. The root certificate will expire at the end of the specified period. - + When WAN Federation is enabled, each secondary datacenter must use the same Vault cluster and share the same `root_pki_path` - with the primary datacenter. + with the primary datacenter. + + To use an intermediate certificate as the primary CA in Consul initialize the + `RootPKIPath` in Vault with a PEM bundle. The first certificate in the bundle + must be the intermediate certificate that Consul will use as the primary CA. + The last certificate in the bundle must be a root certificate. The bundle + must contain a valid chain, where each certificate is followed by the certificate + that authorized it. - `IntermediatePKIPath` / `intermediate_pki_path` (`string: `) - The path to a PKI secrets engine for the generated intermediate certificate. From 6b679aa9d4e1bbd0bd44ddec0ce262de7dd3d975 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Thu, 17 Feb 2022 17:44:02 -0500 Subject: [PATCH 8/9] Update TODOs to reference an issue with more details And remove a no longer needed TODO --- agent/connect/ca/provider.go | 2 +- agent/connect/ca/provider_vault.go | 1 - agent/consul/leader_connect_ca.go | 4 ++-- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/agent/connect/ca/provider.go b/agent/connect/ca/provider.go index 614a4456cf..439c848ebb 100644 --- a/agent/connect/ca/provider.go +++ b/agent/connect/ca/provider.go @@ -135,7 +135,7 @@ type PrimaryProvider interface { // the active intermediate. If multiple intermediates are needed to complete // the chain from the signing certificate back to the active root, they should // all by bundled here. - // TODO: replace with GenerateLeafSigningCert + // TODO: replace with GenerateLeafSigningCert (https://github.com/hashicorp/consul/issues/12386) GenerateIntermediate() (string, error) // SignIntermediate will validate the CSR to ensure the trust domain in the diff --git a/agent/connect/ca/provider_vault.go b/agent/connect/ca/provider_vault.go index 986813dbb6..91b92528cf 100644 --- a/agent/connect/ca/provider_vault.go +++ b/agent/connect/ca/provider_vault.go @@ -603,7 +603,6 @@ func (v *VaultProvider) SignIntermediate(csr *x509.CertificateRequest) (string, // 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) { - // TODO: is this necessary? Doesn't vault check this for us? rootPEM, err := v.getCA(v.config.RootPKIPath) if err != nil { return "", err diff --git a/agent/consul/leader_connect_ca.go b/agent/consul/leader_connect_ca.go index 71b21729fc..35b4f343eb 100644 --- a/agent/consul/leader_connect_ca.go +++ b/agent/consul/leader_connect_ca.go @@ -488,7 +488,7 @@ func (c *CAManager) primaryInitialize(provider ca.Provider, conf *structs.CAConf return err } - // TODO: delete this + // TODO: https://github.com/hashicorp/consul/issues/12386 interPEM, err := provider.GenerateIntermediate() if err != nil { return fmt.Errorf("error generating intermediate cert: %v", err) @@ -976,7 +976,7 @@ func (c *CAManager) primaryUpdateRootCA(newProvider ca.Provider, args *structs.C } } - // TODO: delete this + // TODO: https://github.com/hashicorp/consul/issues/12386 intermediate, err := newProvider.GenerateIntermediate() if err != nil { return err From 6021105dfcf1760d1003e6ab1cf898aca1847e79 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Thu, 17 Feb 2022 18:45:08 -0500 Subject: [PATCH 9/9] ca: test that original certs from secondary still verify There's a chance this could flake if the secondary hasn't received the update yet, but running this test many times doesn't show any flakes yet. --- agent/consul/leader_connect_ca_test.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/agent/consul/leader_connect_ca_test.go b/agent/consul/leader_connect_ca_test.go index 9bdfc07401..73787700aa 100644 --- a/agent/consul/leader_connect_ca_test.go +++ b/agent/consul/leader_connect_ca_test.go @@ -766,6 +766,7 @@ func TestCAManager_Initialize_Vault_WithExternalTrustedCA(t *testing.T) { } }) + var origLeafSecondary string runStep(t, "start secondary DC", func(t *testing.T) { joinWAN(t, serverDC2, serverDC1) testrpc.WaitForActiveCARoot(t, serverDC2.RPC, "dc2", nil) @@ -778,6 +779,7 @@ func TestCAManager_Initialize_Vault_WithExternalTrustedCA(t *testing.T) { leafPEM := getLeafCert(t, codec, roots.TrustDomain, "dc2") verifyLeafCert(t, roots.Roots[0], leafPEM) + origLeafSecondary = leafPEM }) runStep(t, "renew leaf signing CA in primary", func(t *testing.T) { @@ -850,6 +852,13 @@ func TestCAManager_Initialize_Vault_WithExternalTrustedCA(t *testing.T) { // original certs from old root cert should still verify verifyLeafCertWithRoots(t, roots, origLeaf) + + // original certs from secondary should still verify + rootsSecondary := structs.IndexedCARoots{} + r := &structs.DCSpecificRequest{Datacenter: "dc2"} + err = msgpackrpc.CallWithCodec(codec, "ConnectCA.Roots", r, &rootsSecondary) + require.NoError(t, err) + verifyLeafCertWithRoots(t, rootsSecondary, origLeafSecondary) }) runStep(t, "rotate to a different external root", func(t *testing.T) { @@ -885,6 +894,13 @@ func TestCAManager_Initialize_Vault_WithExternalTrustedCA(t *testing.T) { // original certs from old root cert should still verify verifyLeafCertWithRoots(t, roots, origLeaf) + + // original certs from secondary should still verify + rootsSecondary := structs.IndexedCARoots{} + r := &structs.DCSpecificRequest{Datacenter: "dc2"} + err = msgpackrpc.CallWithCodec(codec, "ConnectCA.Roots", r, &rootsSecondary) + require.NoError(t, err) + verifyLeafCertWithRoots(t, rootsSecondary, origLeafSecondary) }) }