status-im
/
consul
mirror of https://github.com/status-im/consul.git
2
0
Fork 0
Code Issues Projects Releases Wiki Activity

Fix bug with Vault CA provider (#18112) 

Updating RootPKIPath but not IntermediatePKIPath would not update 
leaf signing certs with the new root. Unsure if this happens in practice 
but manual testing showed it is a bug that would break mesh and agent 
connections once the old root is pruned.
This commit is contained in:
Chris S. Kim 2023-07-14 15:58:33 -04:00 committed by GitHub
parent 5208ea90e4
commit 747a4c73c1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 229 additions and 138 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
.changelog/18112.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:bug
ca: Fixes a Vault CA provider bug where updating RootPKIPath but not IntermediatePKIPath would not renew leaf signing certificates
```

48
agent/connect/ca/mock_Provider.go
View File

@ -89,6 +89,30 @@ func (_m *MockProvider) CrossSignCA(_a0 *x509.Certificate) (string, error) {
return r0, r1
}
// GenerateCAChain provides a mock function with given fields:
func (_m *MockProvider) GenerateCAChain() (string, error) {
ret := _m.Called()
var r0 string
var r1 error
if rf, ok := ret.Get(0).(func() (string, error)); ok {
return rf()
}
if rf, ok := ret.Get(0).(func() string); ok {
r0 = rf()
} else {
r0 = ret.Get(0).(string)
}
if rf, ok := ret.Get(1).(func() error); ok {
r1 = rf()
} else {
r1 = ret.Error(1)
}
return r0, r1
}
// GenerateIntermediateCSR provides a mock function with given fields:
func (_m *MockProvider) GenerateIntermediateCSR() (string, string, error) {
ret := _m.Called()
@ -120,30 +144,6 @@ func (_m *MockProvider) GenerateIntermediateCSR() (string, string, error) {
return r0, r1, r2
}
// GenerateCAChain provides a mock function with given fields:
func (_m *MockProvider) GenerateCAChain() (CAChainResult, error) {
ret := _m.Called()
var r0 CAChainResult
var r1 error
if rf, ok := ret.Get(0).(func() (CAChainResult, error)); ok {
return rf()
}
if rf, ok := ret.Get(0).(func() CAChainResult); ok {
r0 = rf()
} else {
r0 = ret.Get(0).(CAChainResult)
}
if rf, ok := ret.Get(1).(func() error); ok {
r1 = rf()
} else {
r1 = ret.Error(1)
}
return r0, r1
}
// SetIntermediate provides a mock function with given fields: intermediatePEM, rootPEM, opaque
func (_m *MockProvider) SetIntermediate(intermediatePEM string, rootPEM string, opaque string) error {
ret := _m.Called(intermediatePEM, rootPEM, opaque)

9
agent/connect/ca/provider.go
View File

@ -135,9 +135,12 @@ type PrimaryProvider interface {
// provider.
//
// Depending on the provider and its configuration, GenerateCAChain may return
// a single root certificate or a chain of certs. The provider should return an
// existing CA chain if one exists or generate a new one and return it.
GenerateCAChain() (CAChainResult, error)
// a single root certificate or a chain of certs.
// The first certificate must be the primary CA used to sign intermediates for
// secondary datacenters, and the last certificate must be the trusted CA.
// The provider should return an existing CA chain if one exists or generate a
// new one and return it.
GenerateCAChain() (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

10
agent/connect/ca/provider_aws.go
View File

@ -140,19 +140,19 @@ func (a *AWSProvider) State() (map[string]string, error) {
}
// GenerateCAChain implements Provider
func (a *AWSProvider) GenerateCAChain() (CAChainResult, error) {
func (a *AWSProvider) GenerateCAChain() (string, error) {
if !a.isPrimary {
return CAChainResult{}, fmt.Errorf("provider is not the root certificate authority")
return "", fmt.Errorf("provider is not the root certificate authority")
}
if err := a.ensureCA(); err != nil {
return CAChainResult{}, err
return "", err
}
if a.rootPEM == "" {
return CAChainResult{}, fmt.Errorf("AWS CA provider not fully Initialized")
return "", fmt.Errorf("AWS CA provider not fully Initialized")
}
return CAChainResult{PEM: a.rootPEM}, nil
return a.rootPEM, nil
}
// ensureCA loads the CA resource to check it exists if configured by User or in

18
agent/connect/ca/provider_aws_test.go
View File

@ -49,9 +49,8 @@ func TestAWSBootstrapAndSignPrimary(t *testing.T) {
provider := testAWSProvider(t, testProviderConfigPrimary(t, cfg))
defer provider.Cleanup(true, nil)
root, err := provider.GenerateCAChain()
rootPEM, err := provider.GenerateCAChain()
require.NoError(t, err)
rootPEM := root.PEM
// Ensure they use the right key type
rootCert, err := connect.ParseCert(rootPEM)
@ -76,9 +75,8 @@ func TestAWSBootstrapAndSignPrimary(t *testing.T) {
provider := testAWSProvider(t, testProviderConfigPrimary(t, nil))
defer provider.Cleanup(true, nil)
root, err := provider.GenerateCAChain()
rootPEM, err := provider.GenerateCAChain()
require.NoError(t, err)
rootPEM := root.PEM
// Ensure they use the right key type
rootCert, err := connect.ParseCert(rootPEM)
@ -111,9 +109,8 @@ func TestAWSBootstrapAndSignSecondary(t *testing.T) {
p1 := testAWSProvider(t, testProviderConfigPrimary(t, nil))
defer p1.Cleanup(true, nil)
root, err := p1.GenerateCAChain()
rootPEM, err := p1.GenerateCAChain()
require.NoError(t, err)
rootPEM := root.PEM
p2 := testAWSProvider(t, testProviderConfigSecondary(t, nil))
defer p2.Cleanup(true, nil)
@ -140,9 +137,8 @@ func TestAWSBootstrapAndSignSecondary(t *testing.T) {
cfg1 := testProviderConfigPrimary(t, nil)
cfg1.State = p1State
p1 = testAWSProvider(t, cfg1)
root, err := p1.GenerateCAChain()
newRootPEM, err := p1.GenerateCAChain()
require.NoError(t, err)
newRootPEM := root.PEM
cfg2 := testProviderConfigPrimary(t, nil)
cfg2.State = p2State
@ -174,9 +170,8 @@ func TestAWSBootstrapAndSignSecondary(t *testing.T) {
"ExistingARN": p1State[AWSStateCAARNKey],
})
p1 = testAWSProvider(t, cfg1)
root, err := p1.GenerateCAChain()
newRootPEM, err := p1.GenerateCAChain()
require.NoError(t, err)
newRootPEM := root.PEM
cfg2 := testProviderConfigPrimary(t, map[string]interface{}{
"ExistingARN": p2State[AWSStateCAARNKey],
@ -213,9 +208,8 @@ func TestAWSBootstrapAndSignSecondary(t *testing.T) {
p2 = testAWSProvider(t, cfg2)
require.NoError(t, p2.SetIntermediate(newIntPEM, newRootPEM, ""))
root, err = p1.GenerateCAChain()
newRootPEM, err = p1.GenerateCAChain()
require.NoError(t, err)
newRootPEM = root.PEM
newIntPEM, err = p2.ActiveLeafSigningCert()
require.NoError(t, err)

18
agent/connect/ca/provider_consul.go
View File

@ -155,17 +155,17 @@ func (c *ConsulProvider) State() (map[string]string, error) {
}
// GenerateCAChain initializes a new root certificate and private key if needed.
func (c *ConsulProvider) GenerateCAChain() (CAChainResult, error) {
func (c *ConsulProvider) GenerateCAChain() (string, error) {
providerState, err := c.getState()
if err != nil {
return CAChainResult{}, err
return "", err
}
if !c.isPrimary {
return CAChainResult{}, fmt.Errorf("provider is not the root certificate authority")
return "", fmt.Errorf("provider is not the root certificate authority")
}
if providerState.RootCert != "" {
return CAChainResult{PEM: providerState.RootCert}, nil
return providerState.RootCert, nil
}
// Generate a private key if needed
@ -173,7 +173,7 @@ func (c *ConsulProvider) GenerateCAChain() (CAChainResult, error) {
if c.config.PrivateKey == "" {
_, pk, err := connect.GeneratePrivateKeyWithConfig(c.config.PrivateKeyType, c.config.PrivateKeyBits)
if err != nil {
return CAChainResult{}, err
return "", err
}
newState.PrivateKey = pk
} else {
@ -184,12 +184,12 @@ func (c *ConsulProvider) GenerateCAChain() (CAChainResult, error) {
if c.config.RootCert == "" {
nextSerial, err := c.incrementAndGetNextSerialNumber()
if err != nil {
return CAChainResult{}, fmt.Errorf("error computing next serial number: %v", err)
return "", fmt.Errorf("error computing next serial number: %v", err)
}
ca, err := c.generateCA(newState.PrivateKey, nextSerial, c.config.RootCertTTL)
if err != nil {
return CAChainResult{}, fmt.Errorf("error generating CA: %v", err)
return "", fmt.Errorf("error generating CA: %v", err)
}
newState.RootCert = ca
} else {
@ -202,10 +202,10 @@ func (c *ConsulProvider) GenerateCAChain() (CAChainResult, error) {
ProviderState: &newState,
}
if _, err := c.Delegate.ApplyCARequest(args); err != nil {
return CAChainResult{}, err
return "", err
}
return CAChainResult{PEM: newState.RootCert}, nil
return newState.RootCert, nil
}
// GenerateIntermediateCSR creates a private key and generates a CSR

16
agent/connect/ca/provider_consul_test.go
View File

@ -93,10 +93,10 @@ func TestConsulCAProvider_Bootstrap(t *testing.T) {
// Intermediate should be the same cert.
inter, err := provider.ActiveLeafSigningCert()
require.NoError(t, err)
require.Equal(t, root.PEM, inter)
require.Equal(t, root, inter)
// Should be a valid cert
parsed, err := connect.ParseCert(root.PEM)
parsed, err := connect.ParseCert(root)
require.NoError(t, err)
require.Equal(t, parsed.URIs[0].String(), fmt.Sprintf("spiffe://%s.consul", conf.ClusterID))
requireNotEncoded(t, parsed.SubjectKeyId)
@ -128,10 +128,10 @@ func TestConsulCAProvider_Bootstrap_WithCert(t *testing.T) {
root, err := provider.GenerateCAChain()
require.NoError(t, err)
require.Equal(t, root.PEM, rootCA.RootCert)
require.Equal(t, root, rootCA.RootCert)
// Should be a valid cert
parsed, err := connect.ParseCert(root.PEM)
parsed, err := connect.ParseCert(root)
require.NoError(t, err)
// test that the default root cert ttl was not applied to the provided cert
@ -298,7 +298,7 @@ func testCrossSignProviders(t *testing.T, provider1, provider2 Provider) {
root, err := provider2.GenerateCAChain()
require.NoError(t, err)
newRoot, err := connect.ParseCert(root.PEM)
newRoot, err := connect.ParseCert(root)
require.NoError(t, err)
oldSubject := newRoot.Subject.CommonName
requireNotEncoded(t, newRoot.SubjectKeyId)
@ -321,7 +321,7 @@ func testCrossSignProviders(t *testing.T, provider1, provider2 Provider) {
p1Root, err := provider1.GenerateCAChain()
require.NoError(t, err)
oldRoot, err := connect.ParseCert(p1Root.PEM)
oldRoot, err := connect.ParseCert(p1Root)
require.NoError(t, err)
requireNotEncoded(t, oldRoot.SubjectKeyId)
requireNotEncoded(t, oldRoot.AuthorityKeyId)
@ -385,7 +385,7 @@ func testCrossSignProvidersShouldFail(t *testing.T, provider1, provider2 Provide
root, err := provider2.GenerateCAChain()
require.NoError(t, err)
newRoot, err := connect.ParseCert(root.PEM)
newRoot, err := connect.ParseCert(root)
require.NoError(t, err)
requireNotEncoded(t, newRoot.SubjectKeyId)
requireNotEncoded(t, newRoot.AuthorityKeyId)
@ -454,7 +454,7 @@ func testSignIntermediateCrossDC(t *testing.T, provider1, provider2 Provider) {
require.NoError(t, err)
root, err := provider1.GenerateCAChain()
require.NoError(t, err)
rootPEM := root.PEM
rootPEM := root
// Give the new intermediate to provider2 to use.
require.NoError(t, provider2.SetIntermediate(intermediatePEM, rootPEM, opaque))

31
agent/connect/ca/provider_vault.go
View File

@ -280,9 +280,9 @@ func (v *VaultProvider) State() (map[string]string, error) {
}
// GenerateCAChain mounts and initializes a new root PKI backend if needed.
func (v *VaultProvider) GenerateCAChain() (CAChainResult, error) {
func (v *VaultProvider) GenerateCAChain() (string, error) {
if !v.isPrimary {
return CAChainResult{}, fmt.Errorf("provider is not the root certificate authority")
return "", fmt.Errorf("provider is not the root certificate authority")
}
// Set up the root PKI backend if necessary.
@ -302,7 +302,7 @@ func (v *VaultProvider) GenerateCAChain() (CAChainResult, error) {
},
})
if err != nil {
return CAChainResult{}, fmt.Errorf("failed to mount root CA backend: %w", err)
return "", fmt.Errorf("failed to mount root CA backend: %w", err)
}
// We want to initialize afterwards
@ -310,7 +310,7 @@ func (v *VaultProvider) GenerateCAChain() (CAChainResult, error) {
case ErrBackendNotInitialized:
uid, err := connect.CompactUID()
if err != nil {
return CAChainResult{}, err
return "", err
}
resp, err := v.writeNamespaced(v.config.RootPKINamespace, v.config.RootPKIPath+"root/generate/internal", map[string]interface{}{
"common_name": connect.CACN("vault", uid, v.clusterID, v.isPrimary),
@ -319,23 +319,23 @@ func (v *VaultProvider) GenerateCAChain() (CAChainResult, error) {
"key_bits": v.config.PrivateKeyBits,
})
if err != nil {
return CAChainResult{}, fmt.Errorf("failed to initialize root CA: %w", err)
return "", fmt.Errorf("failed to initialize root CA: %w", err)
}
var ok bool
rootPEM, ok = resp.Data["certificate"].(string)
if !ok {
return CAChainResult{}, fmt.Errorf("unexpected response from Vault: %v", resp.Data["certificate"])
return "", fmt.Errorf("unexpected response from Vault: %v", resp.Data["certificate"])
}
default:
if err != nil {
return CAChainResult{}, fmt.Errorf("unexpected error while setting root PKI backend: %w", err)
return "", fmt.Errorf("unexpected error while setting root PKI backend: %w", err)
}
}
rootChain, err := v.getCAChain(v.config.RootPKINamespace, v.config.RootPKIPath)
if err != nil {
return CAChainResult{}, err
return "", err
}
// Workaround for a bug in the Vault PKI API.
@ -344,18 +344,7 @@ func (v *VaultProvider) GenerateCAChain() (CAChainResult, error) {
rootChain = rootPEM
}
intermediate, err := v.ActiveLeafSigningCert()
if err != nil {
return CAChainResult{}, fmt.Errorf("error fetching active intermediate: %w", err)
}
if intermediate == "" {
intermediate, err = v.GenerateLeafSigningCert()
if err != nil {
return CAChainResult{}, fmt.Errorf("error generating intermediate: %w", err)
}
}
return CAChainResult{PEM: rootChain, IntermediatePEM: intermediate}, nil
return rootChain, nil
}
// GenerateIntermediateCSR creates a private key and generates a CSR
@ -582,7 +571,7 @@ func (v *VaultProvider) getCAChain(namespace, path string) (string, error) {
return root, nil
}
// GenerateIntermediate mounts the configured intermediate PKI backend if
// GenerateLeafSigningCert 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.
func (v *VaultProvider) GenerateLeafSigningCert() (string, error) {

25
agent/connect/ca/provider_vault_test.go
View File

@ -420,7 +420,7 @@ func TestVaultCAProvider_Bootstrap(t *testing.T) {
},
certFunc: func(provider *VaultProvider) (string, error) {
root, err := provider.GenerateCAChain()
return root.PEM, err
return root, err
},
backendPath: "pki-root/",
rootCaCreation: true,
@ -497,9 +497,8 @@ func TestVaultCAProvider_SignLeaf(t *testing.T) {
Service: "foo",
}
root, err := provider.GenerateCAChain()
rootPEM, err := provider.GenerateCAChain()
require.NoError(t, err)
rootPEM := root.PEM
assertCorrectKeyType(t, tc.KeyType, rootPEM)
intPEM, err := provider.ActiveLeafSigningCert()
@ -600,9 +599,9 @@ func TestVaultCAProvider_CrossSignCA(t *testing.T) {
})
testutil.RunStep(t, "init", func(t *testing.T) {
root, err := provider1.GenerateCAChain()
rootPEM, err := provider1.GenerateCAChain()
require.NoError(t, err)
assertCorrectKeyType(t, tc.SigningKeyType, root.PEM)
assertCorrectKeyType(t, tc.SigningKeyType, rootPEM)
intPEM, err := provider1.ActiveLeafSigningCert()
require.NoError(t, err)
@ -628,9 +627,9 @@ func TestVaultCAProvider_CrossSignCA(t *testing.T) {
})
testutil.RunStep(t, "swap", func(t *testing.T) {
root, err := provider2.GenerateCAChain()
rootPEM, err := provider2.GenerateCAChain()
require.NoError(t, err)
assertCorrectKeyType(t, tc.CSRKeyType, root.PEM)
assertCorrectKeyType(t, tc.CSRKeyType, rootPEM)
intPEM, err := provider2.ActiveLeafSigningCert()
require.NoError(t, err)
@ -1147,14 +1146,14 @@ func TestVaultCAProvider_GenerateIntermediate(t *testing.T) {
// This test was created to ensure that our calls to Vault
// returns a new Intermediate certificate and further calls
// to ActiveLeafSigningCert return the same new cert.
new, err := provider.GenerateLeafSigningCert()
newLeaf, err := provider.GenerateLeafSigningCert()
require.NoError(t, err)
newActive, err := provider.ActiveLeafSigningCert()
require.NoError(t, err)
require.Equal(t, new, newActive)
require.NotEqual(t, orig, new)
require.Equal(t, newLeaf, newActive)
require.NotEqual(t, orig, newLeaf)
}
func TestVaultCAProvider_AutoTidyExpiredIssuers(t *testing.T) {
@ -1240,9 +1239,8 @@ func TestVaultCAProvider_GenerateIntermediate_inSecondary(t *testing.T) {
// Sign the CSR with primaryProvider.
intermediatePEM, err := primaryProvider.SignIntermediate(csr)
require.NoError(t, err)
root, err := primaryProvider.GenerateCAChain()
rootPEM, err := primaryProvider.GenerateCAChain()
require.NoError(t, err)
rootPEM := root.PEM
// Give the new intermediate to provider to use.
require.NoError(t, provider.SetIntermediate(intermediatePEM, rootPEM, issuerID))
@ -1261,9 +1259,8 @@ func TestVaultCAProvider_GenerateIntermediate_inSecondary(t *testing.T) {
// Sign the CSR with primaryProvider.
intermediatePEM, err := primaryProvider.SignIntermediate(csr)
require.NoError(t, err)
root, err := primaryProvider.GenerateCAChain()
rootPEM, err := primaryProvider.GenerateCAChain()
require.NoError(t, err)
rootPEM := root.PEM
// Give the new intermediate to provider to use.
require.NoError(t, provider.SetIntermediate(intermediatePEM, rootPEM, issuerID))

70
agent/consul/leader_connect_ca.go
View File

@ -258,8 +258,8 @@ func (c *CAManager) initializeCAConfig() (*structs.CAConfiguration, error) {
}
// newCARoot returns a filled-in structs.CARoot from a raw PEM value.
func newCARoot(rootResult ca.CAChainResult, provider, clusterID string) (*structs.CARoot, error) {
primaryCert, err := connect.ParseCert(rootResult.PEM)
func newCARoot(caPem, provider, clusterID string) (*structs.CARoot, error) {
primaryCert, err := connect.ParseCert(caPem)
if err != nil {
return nil, err
}
@ -275,17 +275,12 @@ func newCARoot(rootResult ca.CAChainResult, provider, clusterID string) (*struct
ExternalTrustDomain: clusterID,
NotBefore: primaryCert.NotBefore,
NotAfter: primaryCert.NotAfter,
RootCert: lib.EnsureTrailingNewline(rootResult.PEM),
RootCert: lib.EnsureTrailingNewline(caPem),
PrivateKeyType: keyType,
PrivateKeyBits: keyBits,
Active: true,
}
if rootResult.IntermediatePEM == "" {
return caRoot, nil
}
if err := setLeafSigningCert(caRoot, rootResult.IntermediatePEM); err != nil {
return nil, fmt.Errorf("error setting leaf signing cert: %w", err)
}
return caRoot, nil
}
@ -518,6 +513,19 @@ func (c *CAManager) primaryInitialize(provider ca.Provider, conf *structs.CAConf
return err
}
// provider may use intermediates for leaf signing in which case
// we need to generate a leaf signing CA.
if usesIntermediate, ok := provider.(ca.PrimaryUsesIntermediate); ok {
leafPem, err := usesIntermediate.GenerateLeafSigningCert()
if err != nil {
return fmt.Errorf("error generating new leaf signing cert: %w", err)
}
if err := setLeafSigningCert(rootCA, leafPem); err != nil {
return fmt.Errorf("error setting leaf signing cert: %w", err)
}
}
var rootUpdateRequired bool
if len(rootCA.IntermediateCerts) > 0 {
rootUpdateRequired = true
@ -764,7 +772,6 @@ func (c *CAManager) UpdateConfiguration(args *structs.CARequest) (reterr error)
return err
}
// Exit early if it's a no-op change
state := c.delegate.State()
_, config, err := state.CAConfig(nil)
if err != nil {
@ -780,6 +787,8 @@ func (c *CAManager) UpdateConfiguration(args *structs.CARequest) (reterr error)
// Don't allow users to change the ClusterID.
args.Config.ClusterID = config.ClusterID
// Exit early if it's a no-op change
if args.Config.Provider == config.Provider && reflect.DeepEqual(args.Config.Config, config.Config) {
return nil
}
@ -866,26 +875,53 @@ func (c *CAManager) primaryUpdateRootCA(newProvider ca.Provider, args *structs.C
}
args.Config.State = pState
providerRoot, err := newProvider.GenerateCAChain()
caPEM, err := newProvider.GenerateCAChain()
if err != nil {
return fmt.Errorf("error generating CA root certificate: %v", err)
}
newRootPEM := providerRoot.PEM
newActiveRoot, err := newCARoot(providerRoot, args.Config.Provider, args.Config.ClusterID)
newActiveRoot, err := newCARoot(caPEM, args.Config.Provider, args.Config.ClusterID)
if err != nil {
return err
}
// Fetch the existing root CA to compare with the current one.
state := c.delegate.State()
// Compare the new provider's root CA ID to the current one. If they
// match, just update the existing provider with the new config.
// If they don't match, begin the root rotation process.
_, root, err := state.CARootActive(nil)
if err != nil {
return err
}
// provider may use intermediates for leaf signing in which case
// we may need to generate a leaf signing CA if the root has changed.
if usesIntermediate, ok := newProvider.(ca.PrimaryUsesIntermediate); ok {
var leafPemFunc func() (string, error)
if root != nil && root.ID == newActiveRoot.ID {
// If Root ID is the same, we can reuse the existing leaf signing cert
leafPemFunc = newProvider.ActiveLeafSigningCert
} else {
// If Root ID is different, we need to generate a new leaf signing cert
// else the trust chain will break when the old root expires.
leafPemFunc = usesIntermediate.GenerateLeafSigningCert
}
leafPem, err := leafPemFunc()
if err != nil {
return fmt.Errorf("error fetching leaf signing cert: %w", err)
}
// newProvider.ActiveLeafSigningCert may return a blank leafPem so we
// fall back to generating a new one just in case.
if leafPem == "" {
leafPem, err = usesIntermediate.GenerateLeafSigningCert()
if err != nil {
return fmt.Errorf("error generating new leaf signing cert: %w", err)
}
}
if err := setLeafSigningCert(newActiveRoot, leafPem); err != nil {
return fmt.Errorf("error setting leaf signing cert: %w", err)
}
}
// If the root didn't change, just update the config and return.
if root != nil && root.ID == newActiveRoot.ID {
args.Op = structs.CAOpSetConfig
@ -919,7 +955,7 @@ func (c *CAManager) primaryUpdateRootCA(newProvider ca.Provider, args *structs.C
// 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 newCARoot, could we remove the second parse?
newRoot, err := connect.ParseCert(newRootPEM)
newRoot, err := connect.ParseCert(caPEM)
if err != nil {
return err
}

109
agent/consul/leader_connect_ca_test.go
View File

@ -259,8 +259,8 @@ type mockCAProvider struct {
func (m *mockCAProvider) Configure(cfg ca.ProviderConfig) error { return nil }
func (m *mockCAProvider) State() (map[string]string, error) { return nil, nil }
func (m *mockCAProvider) GenerateCAChain() (ca.CAChainResult, error) {
return ca.CAChainResult{PEM: m.rootPEM}, nil
func (m *mockCAProvider) GenerateCAChain() (string, error) {
return m.rootPEM, nil
}
func (m *mockCAProvider) GenerateIntermediateCSR() (string, string, error) {
m.callbackCh <- "provider/GenerateIntermediateCSR"
@ -624,7 +624,7 @@ func TestCAManager_UpdateConfiguration_Vault_Primary(t *testing.T) {
require.Equal(t, connect.HexString(cert.SubjectKeyId), origRoot.SigningKeyID)
t.Run("update config without changing root", func(t *testing.T) {
err = s1.caManager.UpdateConfiguration(&structs.CARequest{
require.NoError(t, s1.caManager.UpdateConfiguration(&structs.CARequest{
Config: &structs.CAConfiguration{
Provider: "vault",
Config: map[string]interface{}{
@ -635,26 +635,81 @@ func TestCAManager_UpdateConfiguration_Vault_Primary(t *testing.T) {
"CSRMaxPerSecond": 100,
},
},
})
require.NoError(t, err)
_, sameRoot, err := s1.fsm.State().CARootActive(nil)
require.NoError(t, err)
require.Len(t, sameRoot.IntermediateCerts, 1)
sameRoot.CreateIndex = s1.caManager.providerRoot.CreateIndex
sameRoot.ModifyIndex = s1.caManager.providerRoot.ModifyIndex
}))
cert, err := connect.ParseCert(s1.caManager.getLeafSigningCertFromRoot(sameRoot))
_, newRoot, err := s1.fsm.State().CARootActive(nil)
require.NoError(t, err)
require.Equal(t, connect.HexString(cert.SubjectKeyId), sameRoot.SigningKeyID)
require.Len(t, newRoot.IntermediateCerts, 1)
newRoot.CreateIndex = s1.caManager.providerRoot.CreateIndex
newRoot.ModifyIndex = s1.caManager.providerRoot.ModifyIndex
require.Equal(t, origRoot, sameRoot)
require.Equal(t, sameRoot, s1.caManager.providerRoot)
orig, err := connect.ParseCert(s1.caManager.getLeafSigningCertFromRoot(newRoot))
require.NoError(t, err)
require.Equal(t, connect.HexString(orig.SubjectKeyId), newRoot.SigningKeyID)
require.Equal(t, origRoot, newRoot)
require.Equal(t, newRoot, s1.caManager.providerRoot)
})
t.Run("update config and change root", func(t *testing.T) {
t.Run("update config and change root only", func(t *testing.T) {
// Read the active leaf CA
provider, _ := s1.caManager.getCAProvider()
before, err := provider.ActiveLeafSigningCert()
require.NoError(t, err)
vaultToken2 := ca.CreateVaultTokenWithAttrs(t, vault.Client(), &ca.VaultTokenAttributes{
RootPath: "pki-root-2",
IntermediatePath: "pki-intermediate-2",
IntermediatePath: "pki-intermediate",
ConsulManaged: true,
WithSudo: true,
})
require.NoError(t, s1.caManager.UpdateConfiguration(&structs.CARequest{
Config: &structs.CAConfiguration{
Provider: "vault",
Config: map[string]interface{}{
"Address": vault.Addr,
"Token": vaultToken2,
"RootPKIPath": "pki-root-2/",
"IntermediatePKIPath": "pki-intermediate/",
},
},
}))
// fetch the new root from the state store to check that
// raft apply has occurred.
_, newRoot, err := s1.fsm.State().CARootActive(nil)
require.NoError(t, err)
require.Len(t, newRoot.IntermediateCerts, 2,
"expected one cross-sign cert and one local leaf sign cert")
// Refresh provider
provider, _ = s1.caManager.getCAProvider()
// Leaf signing cert should have been updated
after, err := provider.ActiveLeafSigningCert()
require.NoError(t, err)
require.NotEqual(t, before, after,
"expected leaf signing cert to be changed after RootPKIPath was changed")
cert, err = connect.ParseCert(after)
require.NoError(t, err)
require.Equal(t, connect.HexString(cert.SubjectKeyId), newRoot.SigningKeyID)
})
t.Run("update config, change root and intermediate", func(t *testing.T) {
// Read the active leaf CA
provider, _ := s1.caManager.getCAProvider()
before, err := provider.ActiveLeafSigningCert()
require.NoError(t, err)
vaultToken3 := ca.CreateVaultTokenWithAttrs(t, vault.Client(), &ca.VaultTokenAttributes{
RootPath: "pki-root-3",
IntermediatePath: "pki-intermediate-3",
ConsulManaged: true,
})
@ -663,22 +718,34 @@ func TestCAManager_UpdateConfiguration_Vault_Primary(t *testing.T) {
Provider: "vault",
Config: map[string]interface{}{
"Address": vault.Addr,
"Token": vaultToken2,
"RootPKIPath": "pki-root-2/",
"IntermediatePKIPath": "pki-intermediate-2/",
"Token": vaultToken3,
"RootPKIPath": "pki-root-3/",
"IntermediatePKIPath": "pki-intermediate-3/",
},
},
})
require.NoError(t, err)
// fetch the new root from the state store to check that
// raft apply has occurred.
_, newRoot, err := s1.fsm.State().CARootActive(nil)
require.NoError(t, err)
require.Len(t, newRoot.IntermediateCerts, 2,
"expected one cross-sign cert and one local leaf sign cert")
require.NotEqual(t, origRoot.ID, newRoot.ID)
cert, err = connect.ParseCert(s1.caManager.getLeafSigningCertFromRoot(newRoot))
// Refresh provider
provider, _ = s1.caManager.getCAProvider()
// Leaf signing cert should have been updated
after, err := provider.ActiveLeafSigningCert()
require.NoError(t, err)
require.NotEqual(t, before, after,
"expected leaf signing cert to be changed after RootPKIPath and IntermediatePKIPath were changed")
cert, err = connect.ParseCert(after)
require.NoError(t, err)
require.Equal(t, connect.HexString(cert.SubjectKeyId), newRoot.SigningKeyID)
})
}

10
agent/consul/leader_connect_test.go
View File

@ -1362,10 +1362,12 @@ func TestNewCARoot(t *testing.T) {
}
run := func(t *testing.T, tc testCase) {
root, err := newCARoot(ca.CAChainResult{
PEM: tc.pem,
IntermediatePEM: tc.intermediatePem,
}, "provider-name", "cluster-id")
root, err := newCARoot(
tc.pem,
"provider-name", "cluster-id")
if tc.intermediatePem != "" {
setLeafSigningCert(root, tc.intermediatePem)
}
if tc.expectedErr != "" {
testutil.RequireErrorContains(t, err, tc.expectedErr)
return