connect/ca: ensure edits to the key type/bits for the connect builtin CA will regenerate the roots (#10330)

progress on #9572
This commit is contained in:
R.B. Boyer 2021-07-13 11:12:07 -05:00 committed by GitHub
parent 0537922c6c
commit ae8b526be8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 398 additions and 180 deletions

3
.changelog/10330.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:bug
connect/ca: ensure edits to the key type/bits for the connect builtin CA will regenerate the roots
```

View File

@ -60,6 +60,11 @@ type ConsulProviderStateDelegate interface {
ApplyCARequest(*structs.CARequest) (interface{}, error) ApplyCARequest(*structs.CARequest) (interface{}, error)
} }
func hexStringHash(input string) string {
hash := sha256.Sum256([]byte(input))
return connect.HexString(hash[:])
}
// Configure sets up the provider using the given configuration. // Configure sets up the provider using the given configuration.
func (c *ConsulProvider) Configure(cfg ProviderConfig) error { func (c *ConsulProvider) Configure(cfg ProviderConfig) error {
// Parse the raw config and update our ID. // Parse the raw config and update our ID.
@ -68,8 +73,7 @@ func (c *ConsulProvider) Configure(cfg ProviderConfig) error {
return err return err
} }
c.config = config c.config = config
hash := sha256.Sum256([]byte(fmt.Sprintf("%s,%s,%v", config.PrivateKey, config.RootCert, cfg.IsPrimary))) c.id = hexStringHash(fmt.Sprintf("%s,%s,%s,%d,%v", config.PrivateKey, config.RootCert, config.PrivateKeyType, config.PrivateKeyBits, cfg.IsPrimary))
c.id = connect.HexString(hash[:])
c.clusterID = cfg.ClusterID c.clusterID = cfg.ClusterID
c.isPrimary = cfg.IsPrimary c.isPrimary = cfg.IsPrimary
c.spiffeID = connect.SpiffeIDSigningForCluster(&structs.CAConfiguration{ClusterID: c.clusterID}) c.spiffeID = connect.SpiffeIDSigningForCluster(&structs.CAConfiguration{ClusterID: c.clusterID})
@ -87,35 +91,41 @@ func (c *ConsulProvider) Configure(cfg ProviderConfig) error {
return nil return nil
} }
// Check if there's an entry with the old ID scheme. oldIDs := []string{
oldID := fmt.Sprintf("%s,%s", config.PrivateKey, config.RootCert) hexStringHash(fmt.Sprintf("%s,%s,%v", config.PrivateKey, config.RootCert, cfg.IsPrimary)),
_, providerState, err = c.Delegate.State().CAProviderState(oldID) fmt.Sprintf("%s,%s", config.PrivateKey, config.RootCert),
if err != nil {
return err
} }
// Found an entry with the old ID, so update it to the new ID and // Check if there any entries with old ID schemes.
// delete the old entry. for _, oldID := range oldIDs {
if providerState != nil { _, providerState, err = c.Delegate.State().CAProviderState(oldID)
newState := *providerState if err != nil {
newState.ID = c.id
createReq := &structs.CARequest{
Op: structs.CAOpSetProviderState,
ProviderState: &newState,
}
if _, err := c.Delegate.ApplyCARequest(createReq); err != nil {
return err return err
} }
deleteReq := &structs.CARequest{ // Found an entry with the old ID, so update it to the new ID and
Op: structs.CAOpDeleteProviderState, // delete the old entry.
ProviderState: providerState, if providerState != nil {
} newState := *providerState
if _, err := c.Delegate.ApplyCARequest(deleteReq); err != nil { newState.ID = c.id
return err createReq := &structs.CARequest{
} Op: structs.CAOpSetProviderState,
ProviderState: &newState,
}
if _, err := c.Delegate.ApplyCARequest(createReq); err != nil {
return err
}
return nil deleteReq := &structs.CARequest{
Op: structs.CAOpDeleteProviderState,
ProviderState: providerState,
}
if _, err := c.Delegate.ApplyCARequest(deleteReq); err != nil {
return err
}
return nil
}
} }
args := &structs.CARequest{ args := &structs.CARequest{

View File

@ -438,30 +438,45 @@ func testSignIntermediateCrossDC(t *testing.T, provider1, provider2 Provider) {
} }
func TestConsulCAProvider_MigrateOldID(t *testing.T) { func TestConsulCAProvider_MigrateOldID(t *testing.T) {
t.Parallel() cases := []struct {
name string
require := require.New(t) oldID string
conf := testConsulCAConfig() }{
delegate := newMockDelegate(t, conf) {
name: "original-unhashed",
// Create an entry with an old-style ID. oldID: ",",
_, err := delegate.ApplyCARequest(&structs.CARequest{
Op: structs.CAOpSetProviderState,
ProviderState: &structs.CAConsulProviderState{
ID: ",",
}, },
}) {
require.NoError(err) name: "hash-v1",
_, providerState, err := delegate.state.CAProviderState(",") oldID: hexStringHash(",,true"),
require.NoError(err) },
require.NotNil(providerState) }
provider := TestConsulProvider(t, delegate) for _, tc := range cases {
require.NoError(provider.Configure(testProviderConfig(conf))) t.Run(tc.name, func(t *testing.T) {
require.NoError(provider.GenerateRoot()) conf := testConsulCAConfig()
delegate := newMockDelegate(t, conf)
// After running Configure, the old ID entry should be gone. // Create an entry with an old-style ID.
_, providerState, err = delegate.state.CAProviderState(",") _, err := delegate.ApplyCARequest(&structs.CARequest{
require.NoError(err) Op: structs.CAOpSetProviderState,
require.Nil(providerState) ProviderState: &structs.CAConsulProviderState{
ID: tc.oldID,
},
})
require.NoError(t, err)
_, providerState, err := delegate.state.CAProviderState(tc.oldID)
require.NoError(t, err)
require.NotNil(t, providerState)
provider := TestConsulProvider(t, delegate)
require.NoError(t, provider.Configure(testProviderConfig(conf)))
require.NoError(t, provider.GenerateRoot())
// After running Configure, the old ID entry should be gone.
_, providerState, err = delegate.state.CAProviderState(tc.oldID)
require.NoError(t, err)
require.Nil(t, providerState)
})
}
} }

View File

@ -361,154 +361,194 @@ func TestConnectCAConfig_TriggerRotation(t *testing.T) {
t.Parallel() t.Parallel()
assert := assert.New(t) cases := []struct {
require := require.New(t) name string
dir1, s1 := testServer(t) configFn func() (*structs.CAConfiguration, error)
defer os.RemoveAll(dir1) }{
defer s1.Shutdown() {
codec := rpcClient(t, s1) name: "new private key provided",
defer codec.Close() configFn: func() (*structs.CAConfiguration, error) {
// Update the provider config to use a new private key, which should
// cause a rotation.
_, newKey, err := connect.GeneratePrivateKey()
if err != nil {
return nil, err
}
testrpc.WaitForTestAgent(t, s1.RPC, "dc1") return &structs.CAConfiguration{
Provider: "consul",
// Store the current root Config: map[string]interface{}{
rootReq := &structs.DCSpecificRequest{ "PrivateKey": newKey,
Datacenter: "dc1", "RootCert": "",
} },
var rootList structs.IndexedCARoots }, nil
require.Nil(msgpackrpc.CallWithCodec(codec, "ConnectCA.Roots", rootReq, &rootList)) },
assert.Len(rootList.Roots, 1) },
oldRoot := rootList.Roots[0] {
name: "update private key bits",
// Update the provider config to use a new private key, which should configFn: func() (*structs.CAConfiguration, error) {
// cause a rotation. return &structs.CAConfiguration{
_, newKey, err := connect.GeneratePrivateKey() Provider: "consul",
assert.NoError(err) Config: map[string]interface{}{
newConfig := &structs.CAConfiguration{ "PrivateKeyType": "ec",
Provider: "consul", "PrivateKeyBits": 384,
Config: map[string]interface{}{ },
"PrivateKey": newKey, }, nil
"RootCert": "", },
},
{
name: "update private key type",
configFn: func() (*structs.CAConfiguration, error) {
return &structs.CAConfiguration{
Provider: "consul",
Config: map[string]interface{}{
"PrivateKeyType": "rsa",
"PrivateKeyBits": "2048",
},
}, nil
},
}, },
} }
{
args := &structs.CARequest{
Datacenter: "dc1",
Config: newConfig,
}
var reply interface{}
require.NoError(msgpackrpc.CallWithCodec(codec, "ConnectCA.ConfigurationSet", args, &reply)) for _, tc := range cases {
} t.Run(tc.name, func(t *testing.T) {
dir1, s1 := testServer(t)
defer os.RemoveAll(dir1)
defer s1.Shutdown()
codec := rpcClient(t, s1)
defer codec.Close()
// Make sure the new root has been added along with an intermediate testrpc.WaitForTestAgent(t, s1.RPC, "dc1")
// cross-signed by the old root.
var newRootPEM string
{
args := &structs.DCSpecificRequest{
Datacenter: "dc1",
}
var reply structs.IndexedCARoots
require.Nil(msgpackrpc.CallWithCodec(codec, "ConnectCA.Roots", args, &reply))
assert.Len(reply.Roots, 2)
for _, r := range reply.Roots { // Store the current root
if r.ID == oldRoot.ID { rootReq := &structs.DCSpecificRequest{
// The old root should no longer be marked as the active root, Datacenter: "dc1",
// and none of its other fields should have changed.
assert.False(r.Active)
assert.Equal(r.Name, oldRoot.Name)
assert.Equal(r.RootCert, oldRoot.RootCert)
assert.Equal(r.SigningCert, oldRoot.SigningCert)
assert.Equal(r.IntermediateCerts, oldRoot.IntermediateCerts)
} else {
newRootPEM = r.RootCert
// The new root should have a valid cross-signed cert from the old
// root as an intermediate.
assert.True(r.Active)
assert.Len(r.IntermediateCerts, 1)
xc := testParseCert(t, r.IntermediateCerts[0])
oldRootCert := testParseCert(t, oldRoot.RootCert)
newRootCert := testParseCert(t, r.RootCert)
// Should have the authority key ID and signature algo of the
// (old) signing CA.
assert.Equal(xc.AuthorityKeyId, oldRootCert.AuthorityKeyId)
assert.NotEqual(xc.SubjectKeyId, oldRootCert.SubjectKeyId)
assert.Equal(xc.SignatureAlgorithm, oldRootCert.SignatureAlgorithm)
// The common name and SAN should not have changed.
assert.Equal(xc.Subject.CommonName, newRootCert.Subject.CommonName)
assert.Equal(xc.URIs, newRootCert.URIs)
} }
} var rootList structs.IndexedCARoots
} require.Nil(t, msgpackrpc.CallWithCodec(codec, "ConnectCA.Roots", rootReq, &rootList))
assert.Len(t, rootList.Roots, 1)
oldRoot := rootList.Roots[0]
// Verify the new config was set. newConfig, err := tc.configFn()
{ require.NoError(t, err)
args := &structs.DCSpecificRequest{
Datacenter: "dc1",
}
var reply structs.CAConfiguration
require.NoError(msgpackrpc.CallWithCodec(codec, "ConnectCA.ConfigurationGet", args, &reply))
actual, err := ca.ParseConsulCAConfig(reply.Config) {
require.NoError(err) args := &structs.CARequest{
expected, err := ca.ParseConsulCAConfig(newConfig.Config) Datacenter: "dc1",
require.NoError(err) Config: newConfig,
assert.Equal(reply.Provider, newConfig.Provider) }
assert.Equal(actual, expected) var reply interface{}
}
// Verify that new leaf certs get the cross-signed intermediate bundled require.NoError(t, msgpackrpc.CallWithCodec(codec, "ConnectCA.ConfigurationSet", args, &reply))
{ }
// Generate a CSR and request signing
spiffeId := connect.TestSpiffeIDService(t, "web")
csr, _ := connect.TestCSR(t, spiffeId)
args := &structs.CASignRequest{
Datacenter: "dc1",
CSR: csr,
}
var reply structs.IssuedCert
require.NoError(msgpackrpc.CallWithCodec(codec, "ConnectCA.Sign", args, &reply))
// Verify that the cert is signed by the new CA // Make sure the new root has been added along with an intermediate
{ // cross-signed by the old root.
roots := x509.NewCertPool() var newRootPEM string
require.True(roots.AppendCertsFromPEM([]byte(newRootPEM))) runStep(t, "ensure roots look correct", func(t *testing.T) {
leaf, err := connect.ParseCert(reply.CertPEM) args := &structs.DCSpecificRequest{
require.NoError(err) Datacenter: "dc1",
_, err = leaf.Verify(x509.VerifyOptions{ }
Roots: roots, var reply structs.IndexedCARoots
require.Nil(t, msgpackrpc.CallWithCodec(codec, "ConnectCA.Roots", args, &reply))
assert.Len(t, reply.Roots, 2)
for _, r := range reply.Roots {
if r.ID == oldRoot.ID {
// The old root should no longer be marked as the active root,
// and none of its other fields should have changed.
assert.False(t, r.Active)
assert.Equal(t, r.Name, oldRoot.Name)
assert.Equal(t, r.RootCert, oldRoot.RootCert)
assert.Equal(t, r.SigningCert, oldRoot.SigningCert)
assert.Equal(t, r.IntermediateCerts, oldRoot.IntermediateCerts)
} else {
newRootPEM = r.RootCert
// The new root should have a valid cross-signed cert from the old
// root as an intermediate.
assert.True(t, r.Active)
assert.Len(t, r.IntermediateCerts, 1)
xc := testParseCert(t, r.IntermediateCerts[0])
oldRootCert := testParseCert(t, oldRoot.RootCert)
newRootCert := testParseCert(t, r.RootCert)
// Should have the authority key ID and signature algo of the
// (old) signing CA.
assert.Equal(t, xc.AuthorityKeyId, oldRootCert.AuthorityKeyId)
assert.NotEqual(t, xc.SubjectKeyId, oldRootCert.SubjectKeyId)
assert.Equal(t, xc.SignatureAlgorithm, oldRootCert.SignatureAlgorithm)
// The common name and SAN should not have changed.
assert.Equal(t, xc.Subject.CommonName, newRootCert.Subject.CommonName)
assert.Equal(t, xc.URIs, newRootCert.URIs)
}
}
}) })
require.NoError(err)
}
// And that it validates via the intermediate runStep(t, "verify the new config was set", func(t *testing.T) {
{ args := &structs.DCSpecificRequest{
roots := x509.NewCertPool() Datacenter: "dc1",
assert.True(roots.AppendCertsFromPEM([]byte(oldRoot.RootCert))) }
leaf, err := connect.ParseCert(reply.CertPEM) var reply structs.CAConfiguration
require.NoError(err) require.NoError(t, msgpackrpc.CallWithCodec(codec, "ConnectCA.ConfigurationGet", args, &reply))
// Make sure the intermediate was returned as well as leaf actual, err := ca.ParseConsulCAConfig(reply.Config)
_, rest := pem.Decode([]byte(reply.CertPEM)) require.NoError(t, err)
require.NotEmpty(rest) expected, err := ca.ParseConsulCAConfig(newConfig.Config)
require.NoError(t, err)
intermediates := x509.NewCertPool() assert.Equal(t, reply.Provider, newConfig.Provider)
require.True(intermediates.AppendCertsFromPEM(rest)) assert.Equal(t, actual, expected)
_, err = leaf.Verify(x509.VerifyOptions{
Roots: roots,
Intermediates: intermediates,
}) })
require.NoError(err)
}
// Verify other fields runStep(t, "verify that new leaf certs get the cross-signed intermediate bundled", func(t *testing.T) {
assert.Equal("web", reply.Service) // Generate a CSR and request signing
assert.Equal(spiffeId.URI().String(), reply.ServiceURI) spiffeId := connect.TestSpiffeIDService(t, "web")
csr, _ := connect.TestCSR(t, spiffeId)
args := &structs.CASignRequest{
Datacenter: "dc1",
CSR: csr,
}
var reply structs.IssuedCert
require.NoError(t, msgpackrpc.CallWithCodec(codec, "ConnectCA.Sign", args, &reply))
runStep(t, "verify that the cert is signed by the new CA", func(t *testing.T) {
roots := x509.NewCertPool()
require.True(t, roots.AppendCertsFromPEM([]byte(newRootPEM)))
leaf, err := connect.ParseCert(reply.CertPEM)
require.NoError(t, err)
_, err = leaf.Verify(x509.VerifyOptions{
Roots: roots,
})
require.NoError(t, err)
})
runStep(t, "and that it validates via the intermediate", func(t *testing.T) {
roots := x509.NewCertPool()
assert.True(t, roots.AppendCertsFromPEM([]byte(oldRoot.RootCert)))
leaf, err := connect.ParseCert(reply.CertPEM)
require.NoError(t, err)
// Make sure the intermediate was returned as well as leaf
_, rest := pem.Decode([]byte(reply.CertPEM))
require.NotEmpty(t, rest)
intermediates := x509.NewCertPool()
require.True(t, intermediates.AppendCertsFromPEM(rest))
_, err = leaf.Verify(x509.VerifyOptions{
Roots: roots,
Intermediates: intermediates,
})
require.NoError(t, err)
})
runStep(t, "verify other fields", func(t *testing.T) {
assert.Equal(t, "web", reply.Service)
assert.Equal(t, spiffeId.URI().String(), reply.ServiceURI)
})
})
})
} }
} }

View File

@ -24,6 +24,156 @@ import (
"github.com/hashicorp/consul/testrpc" "github.com/hashicorp/consul/testrpc"
) )
func TestLeader_Builtin_PrimaryCA_ChangeKeyConfig(t *testing.T) {
if testing.Short() {
t.Skip("too slow for testing.Short")
}
types := []struct {
keyType string
keyBits int
}{
{connect.DefaultPrivateKeyType, connect.DefaultPrivateKeyBits},
{"ec", 256},
{"ec", 384},
{"rsa", 2048},
{"rsa", 4096},
}
for _, src := range types {
for _, dst := range types {
if src == dst {
continue // skip
}
src := src
dst := dst
t.Run(fmt.Sprintf("%s-%d to %s-%d", src.keyType, src.keyBits, dst.keyType, dst.keyBits), func(t *testing.T) {
t.Parallel()
providerState := map[string]string{"foo": "dc1-value"}
// Initialize primary as the primary DC
dir1, srv := testServerWithConfig(t, func(c *Config) {
c.Datacenter = "dc1"
c.Build = "1.6.0"
c.CAConfig.Config["PrivateKeyType"] = src.keyType
c.CAConfig.Config["PrivateKeyBits"] = src.keyBits
c.CAConfig.Config["test_state"] = providerState
})
defer os.RemoveAll(dir1)
defer srv.Shutdown()
codec := rpcClient(t, srv)
defer codec.Close()
testrpc.WaitForLeader(t, srv.RPC, "dc1")
testrpc.WaitForActiveCARoot(t, srv.RPC, "dc1", nil)
var (
provider ca.Provider
caRoot *structs.CARoot
)
retry.Run(t, func(r *retry.R) {
provider, caRoot = getCAProviderWithLock(srv)
require.NotNil(r, caRoot)
// Sanity check CA is using the correct key type
require.Equal(r, src.keyType, caRoot.PrivateKeyType)
require.Equal(r, src.keyBits, caRoot.PrivateKeyBits)
})
runStep(t, "sign leaf cert and make sure chain is correct", func(t *testing.T) {
spiffeService := &connect.SpiffeIDService{
Host: "node1",
Namespace: "default",
Datacenter: "dc1",
Service: "foo",
}
raw, _ := connect.TestCSR(t, spiffeService)
leafCsr, err := connect.ParseCSR(raw)
require.NoError(t, err)
leafPEM, err := provider.Sign(leafCsr)
require.NoError(t, err)
// Check that the leaf signed by the new cert can be verified using the
// returned cert chain
require.NoError(t, connect.ValidateLeaf(caRoot.RootCert, leafPEM, []string{}))
})
runStep(t, "verify persisted state is correct", func(t *testing.T) {
state := srv.fsm.State()
_, caConfig, err := state.CAConfig(nil)
require.NoError(t, err)
require.Equal(t, providerState, caConfig.State)
})
runStep(t, "change roots", func(t *testing.T) {
// Update a config value
newConfig := &structs.CAConfiguration{
Provider: "consul",
Config: map[string]interface{}{
"PrivateKey": "",
"RootCert": "",
"PrivateKeyType": dst.keyType,
"PrivateKeyBits": dst.keyBits,
// This verifies the state persistence for providers although Consul
// provider doesn't actually use that mechanism outside of tests.
"test_state": providerState,
},
}
args := &structs.CARequest{
Datacenter: "dc1",
Config: newConfig,
}
var reply interface{}
require.NoError(t, msgpackrpc.CallWithCodec(codec, "ConnectCA.ConfigurationSet", args, &reply))
})
var (
newProvider ca.Provider
newCaRoot *structs.CARoot
)
retry.Run(t, func(r *retry.R) {
newProvider, newCaRoot = getCAProviderWithLock(srv)
require.NotNil(r, newCaRoot)
// Sanity check CA is using the correct key type
require.Equal(r, dst.keyType, newCaRoot.PrivateKeyType)
require.Equal(r, dst.keyBits, newCaRoot.PrivateKeyBits)
})
runStep(t, "sign leaf cert and make sure NEW chain is correct", func(t *testing.T) {
spiffeService := &connect.SpiffeIDService{
Host: "node1",
Namespace: "default",
Datacenter: "dc1",
Service: "foo",
}
raw, _ := connect.TestCSR(t, spiffeService)
leafCsr, err := connect.ParseCSR(raw)
require.NoError(t, err)
leafPEM, err := newProvider.Sign(leafCsr)
require.NoError(t, err)
// Check that the leaf signed by the new cert can be verified using the
// returned cert chain
require.NoError(t, connect.ValidateLeaf(newCaRoot.RootCert, leafPEM, []string{}))
})
runStep(t, "verify persisted state is still correct", func(t *testing.T) {
state := srv.fsm.State()
_, caConfig, err := state.CAConfig(nil)
require.NoError(t, err)
require.Equal(t, providerState, caConfig.State)
})
})
}
}
}
func TestLeader_SecondaryCA_Initialize(t *testing.T) { func TestLeader_SecondaryCA_Initialize(t *testing.T) {
if testing.Short() { if testing.Short() {
t.Skip("too slow for testing.Short") t.Skip("too slow for testing.Short")