From c1f2025d96e867193da1d20bc3549c5502473d0f Mon Sep 17 00:00:00 2001 From: Paul Banks Date: Tue, 8 May 2018 14:23:44 +0100 Subject: [PATCH] Return TrustDomain from CARoots RPC --- agent/connect/ca/provider_consul.go | 2 +- agent/connect/uri_signing.go | 12 ++++++++++++ agent/connect/uri_signing_test.go | 11 +++++++++++ agent/consul/connect_ca_endpoint.go | 23 +++++++++++++++++++++++ agent/consul/connect_ca_endpoint_test.go | 13 ++++++++++--- agent/structs/connect_ca.go | 15 +++++++++++++++ 6 files changed, 72 insertions(+), 4 deletions(-) diff --git a/agent/connect/ca/provider_consul.go b/agent/connect/ca/provider_consul.go index 8fa1fb3d30..d88a58bfc3 100644 --- a/agent/connect/ca/provider_consul.go +++ b/agent/connect/ca/provider_consul.go @@ -346,7 +346,7 @@ func (c *ConsulProvider) generateCA(privateKey string, sn uint64) (string, error name := fmt.Sprintf("Consul CA %d", sn) // The URI (SPIFFE compatible) for the cert - id := &connect.SpiffeIDSigning{ClusterID: config.ClusterID, Domain: "consul"} + id := connect.SpiffeIDSigningForCluster(config) keyId, err := connect.KeyId(privKey.Public()) if err != nil { return "", err diff --git a/agent/connect/uri_signing.go b/agent/connect/uri_signing.go index 213f744d1f..b43971ed7d 100644 --- a/agent/connect/uri_signing.go +++ b/agent/connect/uri_signing.go @@ -27,3 +27,15 @@ func (id *SpiffeIDSigning) Authorize(ixn *structs.Intention) (bool, bool) { // Never authorize as a client. return false, true } + +// SpiffeIDSigningForCluster returns the SPIFFE signing identifier (trust +// domain) representation of the given CA config. +// +// NOTE(banks): we intentionally fix the tld `.consul` for now rather than tie +// this to the `domain` config used for DNS because changing DNS domain can't +// break all certificate validation. That does mean that DNS prefix might not +// match the identity URIs and so the trust domain might not actually resolve +// which we would like but don't actually need. +func SpiffeIDSigningForCluster(config *structs.CAConfiguration) *SpiffeIDSigning { + return &SpiffeIDSigning{ClusterID: config.ClusterID, Domain: "consul"} +} diff --git a/agent/connect/uri_signing_test.go b/agent/connect/uri_signing_test.go index a9be3c5e2b..98babbc2d9 100644 --- a/agent/connect/uri_signing_test.go +++ b/agent/connect/uri_signing_test.go @@ -3,6 +3,8 @@ package connect import ( "testing" + "github.com/hashicorp/consul/agent/structs" + "github.com/stretchr/testify/assert" ) @@ -13,3 +15,12 @@ func TestSpiffeIDSigningAuthorize(t *testing.T) { assert.False(t, auth) assert.True(t, ok) } + +func TestSpiffeIDSigningForCluster(t *testing.T) { + // For now it should just append .consul to the ID. + config := &structs.CAConfiguration{ + ClusterID: testClusterID, + } + id := SpiffeIDSigningForCluster(config) + assert.Equal(t, id.URI().String(), "spiffe://"+testClusterID+".consul") +} diff --git a/agent/consul/connect_ca_endpoint.go b/agent/consul/connect_ca_endpoint.go index 8acaa4ed09..f70c0d3a3e 100644 --- a/agent/consul/connect_ca_endpoint.go +++ b/agent/consul/connect_ca_endpoint.go @@ -211,6 +211,29 @@ func (s *ConnectCA) Roots( return err } + // Load the ClusterID to generate TrustDomain. We do this outside the loop + // since by definition this value should be immutable once set for lifetime of + // the cluster so we don't need to look it up more than once. We also don't + // have to worry about non-atomicity between the config fetch transaction and + // the CARoots transaction below since this field must remain immutable. Do + // not re-use this state/config for other logic that might care about changes + // of config during the blocking query below. + { + state := s.srv.fsm.State() + _, config, err := state.CAConfig() + if err != nil { + return err + } + // Build TrustDomain based on the ClusterID stored. + spiffeID := connect.SpiffeIDSigningForCluster(config) + uri := spiffeID.URI() + if uri == nil { + // Impossible(tm) but let's not panic + return errors.New("no trust domain found") + } + reply.TrustDomain = uri.Host + } + return s.srv.blockingQuery( &args.QueryOptions, &reply.QueryMeta, func(ws memdb.WatchSet, state *state.Store) error { diff --git a/agent/consul/connect_ca_endpoint_test.go b/agent/consul/connect_ca_endpoint_test.go index 4609934ada..655b1d7f4f 100644 --- a/agent/consul/connect_ca_endpoint_test.go +++ b/agent/consul/connect_ca_endpoint_test.go @@ -2,10 +2,13 @@ package consul import ( "crypto/x509" + "fmt" "os" "testing" "time" + "github.com/stretchr/testify/require" + "github.com/hashicorp/consul/agent/connect" ca "github.com/hashicorp/consul/agent/connect/ca" "github.com/hashicorp/consul/agent/structs" @@ -27,6 +30,7 @@ func TestConnectCARoots(t *testing.T) { t.Parallel() assert := assert.New(t) + require := require.New(t) dir1, s1 := testServer(t) defer os.RemoveAll(dir1) defer s1.Shutdown() @@ -41,17 +45,19 @@ func TestConnectCARoots(t *testing.T) { ca2 := connect.TestCA(t, nil) ca2.Active = false idx, _, err := state.CARoots(nil) - assert.NoError(err) + require.NoError(err) ok, err := state.CARootSetCAS(idx, idx, []*structs.CARoot{ca1, ca2}) assert.True(ok) - assert.NoError(err) + require.NoError(err) + _, caCfg, err := state.CAConfig() + require.NoError(err) // Request args := &structs.DCSpecificRequest{ Datacenter: "dc1", } var reply structs.IndexedCARoots - assert.NoError(msgpackrpc.CallWithCodec(codec, "ConnectCA.Roots", args, &reply)) + require.NoError(msgpackrpc.CallWithCodec(codec, "ConnectCA.Roots", args, &reply)) // Verify assert.Equal(ca1.ID, reply.ActiveRootID) @@ -61,6 +67,7 @@ func TestConnectCARoots(t *testing.T) { assert.Equal("", r.SigningCert) assert.Equal("", r.SigningKey) } + assert.Equal(fmt.Sprintf("%s.consul", caCfg.ClusterID), reply.TrustDomain) } func TestConnectCAConfig_GetSet(t *testing.T) { diff --git a/agent/structs/connect_ca.go b/agent/structs/connect_ca.go index 3a4ca81313..fa273a3e47 100644 --- a/agent/structs/connect_ca.go +++ b/agent/structs/connect_ca.go @@ -11,6 +11,21 @@ type IndexedCARoots struct { // the process of being rotated out. ActiveRootID string + // TrustDomain is the identification root for this Consul cluster. All + // certificates signed by the cluster's CA must have their identifying URI in + // this domain. + // + // This does not include the protocol (currently spiffe://) since we may + // implement other protocols in future with equivalent semantics. It should be + // compared against the "authority" section of a URI (i.e. host:port). + // + // NOTE(banks): Later we may support explicitly trusting external domains + // which may be encoded into the CARoot struct or a separate list but this + // domain identifier should be immutable and cluster-wide so deserves to be at + // the root of this response rather than duplicated through all CARoots that + // are not externally trusted entities. + TrustDomain string + // Roots is a list of root CA certs to trust. Roots []*CARoot