From b28e11fdd318c7754a625b291a2f9f8997d416f1 Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Sun, 29 Apr 2018 20:44:40 -0700 Subject: [PATCH] Fill out connect CA rpc endpoint tests --- agent/consul/connect_ca_endpoint.go | 4 +- agent/consul/connect_ca_endpoint_test.go | 207 +++++++++++++++++++++-- agent/consul/connect_ca_provider.go | 4 +- agent/testagent.go | 3 + 4 files changed, 196 insertions(+), 22 deletions(-) diff --git a/agent/consul/connect_ca_endpoint.go b/agent/consul/connect_ca_endpoint.go index 72ac2adbc..35dbe46e8 100644 --- a/agent/consul/connect_ca_endpoint.go +++ b/agent/consul/connect_ca_endpoint.go @@ -130,7 +130,7 @@ func (s *ConnectCA) ConfigurationSet( // If the config has been committed, update the local provider instance s.srv.setCAProvider(newProvider) - s.srv.logger.Printf("[INFO] connect: provider config updated") + s.srv.logger.Printf("[INFO] connect: CA provider config updated") return nil } @@ -295,7 +295,7 @@ func (s *ConnectCA) Sign( } // Set the response - reply = &structs.IssuedCert{ + *reply = structs.IssuedCert{ SerialNumber: connect.HexString(cert.SerialNumber.Bytes()), CertPEM: pem, Service: serviceId.Service, diff --git a/agent/consul/connect_ca_endpoint_test.go b/agent/consul/connect_ca_endpoint_test.go index f2404eb4c..321bcfcb4 100644 --- a/agent/consul/connect_ca_endpoint_test.go +++ b/agent/consul/connect_ca_endpoint_test.go @@ -4,6 +4,7 @@ import ( "crypto/x509" "os" "testing" + "time" "github.com/hashicorp/consul/agent/connect" "github.com/hashicorp/consul/agent/structs" @@ -12,6 +13,14 @@ import ( "github.com/stretchr/testify/assert" ) +func testParseCert(t *testing.T, pemValue string) *x509.Certificate { + cert, err := connect.ParseCert(pemValue) + if err != nil { + t.Fatal(err) + } + return cert +} + // Test listing root CAs. func TestConnectCARoots(t *testing.T) { t.Parallel() @@ -30,16 +39,18 @@ func TestConnectCARoots(t *testing.T) { ca1 := connect.TestCA(t, nil) ca2 := connect.TestCA(t, nil) ca2.Active = false - ok, err := state.CARootSetCAS(1, 0, []*structs.CARoot{ca1, ca2}) + idx, _, err := state.CARoots(nil) + assert.NoError(err) + ok, err := state.CARootSetCAS(idx, idx, []*structs.CARoot{ca1, ca2}) assert.True(ok) - assert.Nil(err) + assert.NoError(err) // Request args := &structs.DCSpecificRequest{ Datacenter: "dc1", } var reply structs.IndexedCARoots - assert.Nil(msgpackrpc.CallWithCodec(codec, "ConnectCA.Roots", args, &reply)) + assert.NoError(msgpackrpc.CallWithCodec(codec, "ConnectCA.Roots", args, &reply)) // Verify assert.Equal(ca1.ID, reply.ActiveRootID) @@ -51,11 +62,173 @@ func TestConnectCARoots(t *testing.T) { } } +func TestConnectCAConfig_GetSet(t *testing.T) { + t.Parallel() + + assert := assert.New(t) + dir1, s1 := testServer(t) + defer os.RemoveAll(dir1) + defer s1.Shutdown() + codec := rpcClient(t, s1) + defer codec.Close() + + testrpc.WaitForLeader(t, s1.RPC, "dc1") + + // Get the starting config + { + args := &structs.DCSpecificRequest{ + Datacenter: "dc1", + } + var reply structs.CAConfiguration + assert.NoError(msgpackrpc.CallWithCodec(codec, "ConnectCA.ConfigurationGet", args, &reply)) + + actual, err := ParseConsulCAConfig(reply.Config) + assert.NoError(err) + expected, err := ParseConsulCAConfig(s1.config.CAConfig.Config) + assert.NoError(err) + assert.Equal(reply.Provider, s1.config.CAConfig.Provider) + assert.Equal(actual, expected) + } + + // Update a config value + newConfig := &structs.CAConfiguration{ + Provider: "consul", + Config: map[string]interface{}{ + "PrivateKey": "", + "RootCert": "", + "RotationPeriod": 180 * 24 * time.Hour, + }, + } + { + args := &structs.CARequest{ + Datacenter: "dc1", + Config: newConfig, + } + var reply interface{} + + assert.NoError(msgpackrpc.CallWithCodec(codec, "ConnectCA.ConfigurationSet", args, &reply)) + } + + // Verify the new config was set + { + args := &structs.DCSpecificRequest{ + Datacenter: "dc1", + } + var reply structs.CAConfiguration + assert.NoError(msgpackrpc.CallWithCodec(codec, "ConnectCA.ConfigurationGet", args, &reply)) + + actual, err := ParseConsulCAConfig(reply.Config) + assert.NoError(err) + expected, err := ParseConsulCAConfig(newConfig.Config) + assert.NoError(err) + assert.Equal(reply.Provider, newConfig.Provider) + assert.Equal(actual, expected) + } +} + +func TestConnectCAConfig_TriggerRotation(t *testing.T) { + t.Parallel() + + assert := assert.New(t) + dir1, s1 := testServer(t) + defer os.RemoveAll(dir1) + defer s1.Shutdown() + codec := rpcClient(t, s1) + defer codec.Close() + + testrpc.WaitForLeader(t, s1.RPC, "dc1") + + // Store the current root + rootReq := &structs.DCSpecificRequest{ + Datacenter: "dc1", + } + var rootList structs.IndexedCARoots + assert.Nil(msgpackrpc.CallWithCodec(codec, "ConnectCA.Roots", rootReq, &rootList)) + assert.Len(rootList.Roots, 1) + oldRoot := rootList.Roots[0] + + // Update the provider config to use a new private key, which should + // cause a rotation. + newKey, err := generatePrivateKey() + assert.NoError(err) + newConfig := &structs.CAConfiguration{ + Provider: "consul", + Config: map[string]interface{}{ + "PrivateKey": newKey, + "RootCert": "", + "RotationPeriod": 90 * 24 * time.Hour, + }, + } + { + args := &structs.CARequest{ + Datacenter: "dc1", + Config: newConfig, + } + var reply interface{} + + assert.NoError(msgpackrpc.CallWithCodec(codec, "ConnectCA.ConfigurationSet", args, &reply)) + } + + // Make sure the new root has been added along with an intermediate + // cross-signed by the old root. + { + args := &structs.DCSpecificRequest{ + Datacenter: "dc1", + } + var reply structs.IndexedCARoots + assert.Nil(msgpackrpc.CallWithCodec(codec, "ConnectCA.Roots", args, &reply)) + assert.Len(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(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 { + // 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/subject key IDs and signature algo of the + // (old) signing CA. + assert.Equal(xc.AuthorityKeyId, oldRootCert.AuthorityKeyId) + assert.Equal(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) + } + } + } + + // Verify the new config was set. + { + args := &structs.DCSpecificRequest{ + Datacenter: "dc1", + } + var reply structs.CAConfiguration + assert.NoError(msgpackrpc.CallWithCodec(codec, "ConnectCA.ConfigurationGet", args, &reply)) + + actual, err := ParseConsulCAConfig(reply.Config) + assert.NoError(err) + expected, err := ParseConsulCAConfig(newConfig.Config) + assert.NoError(err) + assert.Equal(reply.Provider, newConfig.Provider) + assert.Equal(actual, expected) + } +} + // Test CA signing -// -// NOTE(mitchellh): Just testing the happy path and not all the other validation -// issues because the internals of this method will probably be gutted for the -// CA plugins then we can just test mocks. func TestConnectCASign(t *testing.T) { t.Parallel() @@ -68,32 +241,30 @@ func TestConnectCASign(t *testing.T) { testrpc.WaitForLeader(t, s1.RPC, "dc1") - // Insert a CA - state := s1.fsm.State() - ca := connect.TestCA(t, nil) - ok, err := state.CARootSetCAS(1, 0, []*structs.CARoot{ca}) - assert.True(ok) - assert.Nil(err) - // Generate a CSR and request signing spiffeId := connect.TestSpiffeIDService(t, "web") csr, _ := connect.TestCSR(t, spiffeId) args := &structs.CASignRequest{ - Datacenter: "dc01", + Datacenter: "dc1", CSR: csr, } var reply structs.IssuedCert - assert.Nil(msgpackrpc.CallWithCodec(codec, "ConnectCA.Sign", args, &reply)) + assert.NoError(msgpackrpc.CallWithCodec(codec, "ConnectCA.Sign", args, &reply)) + + // Get the current CA + state := s1.fsm.State() + _, ca, err := state.CARootActive(nil) + assert.NoError(err) // Verify that the cert is signed by the CA roots := x509.NewCertPool() assert.True(roots.AppendCertsFromPEM([]byte(ca.RootCert))) leaf, err := connect.ParseCert(reply.CertPEM) - assert.Nil(err) + assert.NoError(err) _, err = leaf.Verify(x509.VerifyOptions{ Roots: roots, }) - assert.Nil(err) + assert.NoError(err) // Verify other fields assert.Equal("web", reply.Service) diff --git a/agent/consul/connect_ca_provider.go b/agent/consul/connect_ca_provider.go index 1c509e2b0..cb2bcad57 100644 --- a/agent/consul/connect_ca_provider.go +++ b/agent/consul/connect_ca_provider.go @@ -30,7 +30,7 @@ type ConsulCAProvider struct { // NewConsulCAProvider returns a new instance of the Consul CA provider, // bootstrapping its state in the state store necessary func NewConsulCAProvider(rawConfig map[string]interface{}, srv *Server) (*ConsulCAProvider, error) { - conf, err := decodeConfig(rawConfig) + conf, err := ParseConsulCAConfig(rawConfig) if err != nil { return nil, err } @@ -116,7 +116,7 @@ func NewConsulCAProvider(rawConfig map[string]interface{}, srv *Server) (*Consul return provider, nil } -func decodeConfig(raw map[string]interface{}) (*structs.ConsulCAProviderConfig, error) { +func ParseConsulCAConfig(raw map[string]interface{}) (*structs.ConsulCAProviderConfig, error) { var config *structs.ConsulCAProviderConfig if err := mapstructure.WeakDecode(raw, &config); err != nil { return nil, fmt.Errorf("error decoding config: %s", err) diff --git a/agent/testagent.go b/agent/testagent.go index 581143016..c2e4ddf01 100644 --- a/agent/testagent.go +++ b/agent/testagent.go @@ -334,6 +334,9 @@ func TestConfig(sources ...config.Source) *config.RuntimeConfig { server = true node_id = "` + nodeID + `" node_name = "Node ` + nodeID + `" + connect { + enabled = true + } performance { raft_multiplier = 1 }