From 26e65f6bfd9a3eb290d22785a1d02c5751d74eda Mon Sep 17 00:00:00 2001 From: Paul Banks Date: Thu, 29 Mar 2018 16:25:11 +0100 Subject: [PATCH] connect.Service based implementation after review feedback. --- agent/connect/testing_ca.go | 108 ++--- agent/connect/testing_ca_test.go | 15 +- connect/auth.go | 43 -- connect/certgen/certgen.go | 86 ++++ connect/client.go | 436 +++++++++--------- connect/client_test.go | 238 +++++----- connect/example_test.go | 53 +++ connect/resolver.go | 131 ++++++ connect/resolver_test.go | 164 +++++++ connect/service.go | 185 ++++++++ connect/service_test.go | 105 +++++ .../testdata/ca1-ca-consul-internal.cert.pem | 14 - .../testdata/ca1-ca-consul-internal.key.pem | 5 - connect/testdata/ca1-svc-cache.cert.pem | 14 - connect/testdata/ca1-svc-cache.key.pem | 5 - connect/testdata/ca1-svc-db.cert.pem | 13 - connect/testdata/ca1-svc-db.key.pem | 5 - connect/testdata/ca1-svc-web.cert.pem | 13 - connect/testdata/ca1-svc-web.key.pem | 5 - connect/testdata/ca2-ca-vault.cert.pem | 14 - connect/testdata/ca2-ca-vault.key.pem | 5 - connect/testdata/ca2-svc-cache.cert.pem | 13 - connect/testdata/ca2-svc-cache.key.pem | 5 - connect/testdata/ca2-svc-db.cert.pem | 13 - connect/testdata/ca2-svc-db.key.pem | 5 - connect/testdata/ca2-svc-web.cert.pem | 13 - connect/testdata/ca2-svc-web.key.pem | 5 - connect/testdata/ca2-xc-by-ca1.cert.pem | 14 - connect/testdata/mkcerts.go | 243 ---------- connect/testing.go | 162 ++++++- connect/tls.go | 120 +++-- connect/tls_test.go | 114 +++-- 32 files changed, 1417 insertions(+), 947 deletions(-) delete mode 100644 connect/auth.go create mode 100644 connect/certgen/certgen.go create mode 100644 connect/example_test.go create mode 100644 connect/resolver.go create mode 100644 connect/resolver_test.go create mode 100644 connect/service.go create mode 100644 connect/service_test.go delete mode 100644 connect/testdata/ca1-ca-consul-internal.cert.pem delete mode 100644 connect/testdata/ca1-ca-consul-internal.key.pem delete mode 100644 connect/testdata/ca1-svc-cache.cert.pem delete mode 100644 connect/testdata/ca1-svc-cache.key.pem delete mode 100644 connect/testdata/ca1-svc-db.cert.pem delete mode 100644 connect/testdata/ca1-svc-db.key.pem delete mode 100644 connect/testdata/ca1-svc-web.cert.pem delete mode 100644 connect/testdata/ca1-svc-web.key.pem delete mode 100644 connect/testdata/ca2-ca-vault.cert.pem delete mode 100644 connect/testdata/ca2-ca-vault.key.pem delete mode 100644 connect/testdata/ca2-svc-cache.cert.pem delete mode 100644 connect/testdata/ca2-svc-cache.key.pem delete mode 100644 connect/testdata/ca2-svc-db.cert.pem delete mode 100644 connect/testdata/ca2-svc-db.key.pem delete mode 100644 connect/testdata/ca2-svc-web.cert.pem delete mode 100644 connect/testdata/ca2-svc-web.key.pem delete mode 100644 connect/testdata/ca2-xc-by-ca1.cert.pem delete mode 100644 connect/testdata/mkcerts.go diff --git a/agent/connect/testing_ca.go b/agent/connect/testing_ca.go index a2f7117636..3fbcf2e021 100644 --- a/agent/connect/testing_ca.go +++ b/agent/connect/testing_ca.go @@ -29,7 +29,7 @@ const testClusterID = "11111111-2222-3333-4444-555555555555" // testCACounter is just an atomically incremented counter for creating // unique names for the CA certs. -var testCACounter uint64 = 0 +var testCACounter uint64 // TestCA creates a test CA certificate and signing key and returns it // in the CARoot structure format. The returned CA will be set as Active = true. @@ -44,7 +44,8 @@ func TestCA(t testing.T, xc *structs.CARoot) *structs.CARoot { result.Name = fmt.Sprintf("Test CA %d", atomic.AddUint64(&testCACounter, 1)) // Create the private key we'll use for this CA cert. - signer := testPrivateKey(t, &result) + signer, keyPEM := testPrivateKey(t) + result.SigningKey = keyPEM // The serial number for the cert sn, err := testSerialNumber() @@ -125,9 +126,9 @@ func TestCA(t testing.T, xc *structs.CARoot) *structs.CARoot { return &result } -// TestLeaf returns a valid leaf certificate for the named service with -// the given CA Root. -func TestLeaf(t testing.T, service string, root *structs.CARoot) string { +// TestLeaf returns a valid leaf certificate and it's private key for the named +// service with the given CA Root. +func TestLeaf(t testing.T, service string, root *structs.CARoot) (string, string) { // Parse the CA cert and signing key from the root cert := root.SigningCert if cert == "" { @@ -137,7 +138,7 @@ func TestLeaf(t testing.T, service string, root *structs.CARoot) string { if err != nil { t.Fatalf("error parsing CA cert: %s", err) } - signer, err := ParseSigner(root.SigningKey) + caSigner, err := ParseSigner(root.SigningKey) if err != nil { t.Fatalf("error parsing signing key: %s", err) } @@ -156,6 +157,9 @@ func TestLeaf(t testing.T, service string, root *structs.CARoot) string { t.Fatalf("error generating serial number: %s", err) } + // Genereate fresh private key + pkSigner, pkPEM := testPrivateKey(t) + // Cert template for generation template := x509.Certificate{ SerialNumber: sn, @@ -173,14 +177,14 @@ func TestLeaf(t testing.T, service string, root *structs.CARoot) string { }, NotAfter: time.Now().Add(10 * 365 * 24 * time.Hour), NotBefore: time.Now(), - AuthorityKeyId: testKeyID(t, signer.Public()), - SubjectKeyId: testKeyID(t, signer.Public()), + AuthorityKeyId: testKeyID(t, caSigner.Public()), + SubjectKeyId: testKeyID(t, pkSigner.Public()), } // Create the certificate, PEM encode it and return that value. var buf bytes.Buffer bs, err := x509.CreateCertificate( - rand.Reader, &template, caCert, signer.Public(), signer) + rand.Reader, &template, caCert, pkSigner.Public(), caSigner) if err != nil { t.Fatalf("error generating certificate: %s", err) } @@ -189,7 +193,7 @@ func TestLeaf(t testing.T, service string, root *structs.CARoot) string { t.Fatalf("error encoding private key: %s", err) } - return buf.String() + return buf.String(), pkPEM } // TestCSR returns a CSR to sign the given service along with the PEM-encoded @@ -200,39 +204,22 @@ func TestCSR(t testing.T, uri CertURI) (string, string) { SignatureAlgorithm: x509.ECDSAWithSHA256, } - // Result buffers - var csrBuf, pkBuf bytes.Buffer - // Create the private key we'll use - signer := testPrivateKey(t, nil) + signer, pkPEM := testPrivateKey(t) - { - // Create the private key PEM - bs, err := x509.MarshalECPrivateKey(signer.(*ecdsa.PrivateKey)) - if err != nil { - t.Fatalf("error marshalling PK: %s", err) - } - - err = pem.Encode(&pkBuf, &pem.Block{Type: "EC PRIVATE KEY", Bytes: bs}) - if err != nil { - t.Fatalf("error encoding PK: %s", err) - } + // Create the CSR itself + var csrBuf bytes.Buffer + bs, err := x509.CreateCertificateRequest(rand.Reader, template, signer) + if err != nil { + t.Fatalf("error creating CSR: %s", err) } - { - // Create the CSR itself - bs, err := x509.CreateCertificateRequest(rand.Reader, template, signer) - if err != nil { - t.Fatalf("error creating CSR: %s", err) - } - - err = pem.Encode(&csrBuf, &pem.Block{Type: "CERTIFICATE REQUEST", Bytes: bs}) - if err != nil { - t.Fatalf("error encoding CSR: %s", err) - } + err = pem.Encode(&csrBuf, &pem.Block{Type: "CERTIFICATE REQUEST", Bytes: bs}) + if err != nil { + t.Fatalf("error encoding CSR: %s", err) } - return csrBuf.String(), pkBuf.String() + return csrBuf.String(), pkPEM } // testKeyID returns a KeyID from the given public key. This just calls @@ -246,25 +233,26 @@ func testKeyID(t testing.T, raw interface{}) []byte { return result } -// testMemoizePK is the private key that we memoize once we generate it -// once so that our tests don't rely on too much system entropy. -var testMemoizePK atomic.Value - -// testPrivateKey creates an ECDSA based private key. -func testPrivateKey(t testing.T, ca *structs.CARoot) crypto.Signer { - // If we already generated a private key, use that - var pk *ecdsa.PrivateKey - if v := testMemoizePK.Load(); v != nil { - pk = v.(*ecdsa.PrivateKey) - } - - // If we have no key, then create a new one. - if pk == nil { - var err error - pk, err = ecdsa.GenerateKey(elliptic.P256(), rand.Reader) - if err != nil { - t.Fatalf("error generating private key: %s", err) - } +// testPrivateKey creates an ECDSA based private key. Both a crypto.Signer and +// the key in PEM form are returned. +// +// NOTE(banks): this was memoized to save entropy during tests but it turns out +// crypto/rand will never block and always reads from /dev/urandom on unix OSes +// which does not consume entropy. +// +// If we find by profiling it's taking a lot of cycles we could optimise/cache +// again but we at least need to use different keys for each distinct CA (when +// multiple CAs are generated at once e.g. to test cross-signing) and a +// different one again for the leafs otherwise we risk tests that have false +// positives since signatures from different logical cert's keys are +// indistinguishable, but worse we build validation chains using AuthorityKeyID +// which will be the same for multiple CAs/Leafs. Also note that our UUID +// generator also reads from crypto rand and is called far more often during +// tests than this will be. +func testPrivateKey(t testing.T) (crypto.Signer, string) { + pk, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("error generating private key: %s", err) } bs, err := x509.MarshalECPrivateKey(pk) @@ -277,14 +265,8 @@ func testPrivateKey(t testing.T, ca *structs.CARoot) crypto.Signer { if err != nil { t.Fatalf("error encoding private key: %s", err) } - if ca != nil { - ca.SigningKey = buf.String() - } - // Memoize the key - testMemoizePK.Store(pk) - - return pk + return pk, buf.String() } // testSerialNumber generates a serial number suitable for a certificate. diff --git a/agent/connect/testing_ca_test.go b/agent/connect/testing_ca_test.go index d07aac201a..193e532c37 100644 --- a/agent/connect/testing_ca_test.go +++ b/agent/connect/testing_ca_test.go @@ -29,7 +29,7 @@ func TestTestCAAndLeaf(t *testing.T) { // Create the certs ca := TestCA(t, nil) - leaf := TestLeaf(t, "web", ca) + leaf, _ := TestLeaf(t, "web", ca) // Create a temporary directory for storing the certs td, err := ioutil.TempDir("", "consul") @@ -62,8 +62,8 @@ func TestTestCAAndLeaf_xc(t *testing.T) { // Create the certs ca1 := TestCA(t, nil) ca2 := TestCA(t, ca1) - leaf1 := TestLeaf(t, "web", ca1) - leaf2 := TestLeaf(t, "web", ca2) + leaf1, _ := TestLeaf(t, "web", ca1) + leaf2, _ := TestLeaf(t, "web", ca2) // Create a temporary directory for storing the certs td, err := ioutil.TempDir("", "consul") @@ -98,12 +98,3 @@ func TestTestCAAndLeaf_xc(t *testing.T) { assert.Nil(err) } } - -// Test that the private key is memoized to preseve system entropy. -func TestTestPrivateKey_memoize(t *testing.T) { - ca1 := TestCA(t, nil) - ca2 := TestCA(t, nil) - if ca1.SigningKey != ca2.SigningKey { - t.Fatal("should have the same signing keys for tests") - } -} diff --git a/connect/auth.go b/connect/auth.go deleted file mode 100644 index 73c16f0bf5..0000000000 --- a/connect/auth.go +++ /dev/null @@ -1,43 +0,0 @@ -package connect - -import "crypto/x509" - -// Auther is the interface that provides both Authentication and Authorization -// for an mTLS connection. It's only method is compatible with -// tls.Config.VerifyPeerCertificate. -type Auther interface { - // Auth is called during tls Connection establishment to Authenticate and - // Authorize the presented peer. Note that verifiedChains must not be relied - // upon as we typically have to skip Go's internal verification so the - // implementation takes full responsibility to validating the certificate - // against known roots. It is also up to the user of the interface to ensure - // appropriate validation is performed for client or server end by arranging - // for an appropriate implementation to be hooked into the tls.Config used. - Auth(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error -} - -// ClientAuther is used to auth Clients connecting to a Server. -type ClientAuther struct{} - -// Auth implements Auther -func (a *ClientAuther) Auth(rawCerts [][]byte, - verifiedChains [][]*x509.Certificate) error { - - // TODO(banks): implement path validation and AuthZ - return nil -} - -// ServerAuther is used to auth the Server identify from a connecting Client. -type ServerAuther struct { - // TODO(banks): We'll need a way to pass the expected service identity (name, - // namespace, dc, cluster) here based on discovery result. -} - -// Auth implements Auther -func (a *ServerAuther) Auth(rawCerts [][]byte, - verifiedChains [][]*x509.Certificate) error { - - // TODO(banks): implement path validation and verify URI matches the target - // service we intended to connect to. - return nil -} diff --git a/connect/certgen/certgen.go b/connect/certgen/certgen.go new file mode 100644 index 0000000000..6fecf6ae1f --- /dev/null +++ b/connect/certgen/certgen.go @@ -0,0 +1,86 @@ +// certgen: a tool for generating test certificates on disk for use as +// test-fixtures and for end-to-end testing and local development. +// +// Example usage: +// +// $ go run connect/certgen/certgen.go -out-dir /tmp/connect-certs +// +// You can verify a given leaf with a given root using: +// +// $ openssl verify -verbose -CAfile ca2-ca.cert.pem ca1-svc-db.cert.pem +// +// Note that to verify via the cross-signed intermediate, openssl requires it to +// be bundled with the _root_ CA bundle and will ignore the cert if it's passed +// with the subject. You can do that with: +// +// $ openssl verify -verbose -CAfile \ +// <(cat ca1-ca.cert.pem ca2-xc-by-ca1.cert.pem) \ +// ca2-svc-db.cert.pem +// ca2-svc-db.cert.pem: OK +// +// Note that the same leaf and root without the intermediate should fail: +// +// $ openssl verify -verbose -CAfile ca1-ca.cert.pem ca2-svc-db.cert.pem +// ca2-svc-db.cert.pem: CN = db +// error 20 at 0 depth lookup:unable to get local issuer certificate +// +// NOTE: THIS IS A QUIRK OF OPENSSL; in Connect we distribute the roots alone +// and stable intermediates like the XC cert to the _leaf_. +package main // import "github.com/hashicorp/consul/connect/certgen" +import ( + "flag" + "fmt" + "io/ioutil" + "log" + "os" + + "github.com/hashicorp/consul/agent/connect" + "github.com/hashicorp/consul/agent/structs" + "github.com/mitchellh/go-testing-interface" +) + +func main() { + var numCAs = 2 + var services = []string{"web", "db", "cache"} + //var slugRe = regexp.MustCompile("[^a-zA-Z0-9]+") + var outDir string + + flag.StringVar(&outDir, "out-dir", "", + "REQUIRED: the dir to write certificates to") + flag.Parse() + + if outDir == "" { + flag.PrintDefaults() + os.Exit(1) + } + + // Create CA certs + var prevCA *structs.CARoot + for i := 1; i <= numCAs; i++ { + ca := connect.TestCA(&testing.RuntimeT{}, prevCA) + prefix := fmt.Sprintf("%s/ca%d-ca", outDir, i) + writeFile(prefix+".cert.pem", ca.RootCert) + writeFile(prefix+".key.pem", ca.SigningKey) + if prevCA != nil { + fname := fmt.Sprintf("%s/ca%d-xc-by-ca%d.cert.pem", outDir, i, i-1) + writeFile(fname, ca.SigningCert) + } + prevCA = ca + + // Create service certs for each CA + for _, svc := range services { + certPEM, keyPEM := connect.TestLeaf(&testing.RuntimeT{}, svc, ca) + prefix := fmt.Sprintf("%s/ca%d-svc-%s", outDir, i, svc) + writeFile(prefix+".cert.pem", certPEM) + writeFile(prefix+".key.pem", keyPEM) + } + } +} + +func writeFile(name, content string) { + fmt.Println("Writing ", name) + err := ioutil.WriteFile(name, []byte(content), 0600) + if err != nil { + log.Fatalf("failed writing file: %s", err) + } +} diff --git a/connect/client.go b/connect/client.go index 867bf0db54..18e43f4cb8 100644 --- a/connect/client.go +++ b/connect/client.go @@ -1,256 +1,256 @@ package connect -import ( - "context" - "crypto/tls" - "fmt" - "math/rand" - "net" +// import ( +// "context" +// "crypto/tls" +// "fmt" +// "math/rand" +// "net" - "github.com/hashicorp/consul/api" -) +// "github.com/hashicorp/consul/api" +// ) -// CertStatus indicates whether the Client currently has valid certificates for -// incoming and outgoing connections. -type CertStatus int +// // CertStatus indicates whether the Client currently has valid certificates for +// // incoming and outgoing connections. +// type CertStatus int -const ( - // CertStatusUnknown is the zero value for CertStatus which may be returned - // when a watch channel is closed on shutdown. It has no other meaning. - CertStatusUnknown CertStatus = iota +// const ( +// // CertStatusUnknown is the zero value for CertStatus which may be returned +// // when a watch channel is closed on shutdown. It has no other meaning. +// CertStatusUnknown CertStatus = iota - // CertStatusOK indicates the client has valid certificates and trust roots to - // Authenticate incoming and outgoing connections. - CertStatusOK +// // CertStatusOK indicates the client has valid certificates and trust roots to +// // Authenticate incoming and outgoing connections. +// CertStatusOK - // CertStatusPending indicates the client is waiting to be issued initial - // certificates, or that it's certificates have expired and it's waiting to be - // issued new ones. In this state all incoming and outgoing connections will - // fail. - CertStatusPending -) +// // CertStatusPending indicates the client is waiting to be issued initial +// // certificates, or that it's certificates have expired and it's waiting to be +// // issued new ones. In this state all incoming and outgoing connections will +// // fail. +// CertStatusPending +// ) -func (s CertStatus) String() string { - switch s { - case CertStatusOK: - return "OK" - case CertStatusPending: - return "pending" - case CertStatusUnknown: - fallthrough - default: - return "unknown" - } -} +// func (s CertStatus) String() string { +// switch s { +// case CertStatusOK: +// return "OK" +// case CertStatusPending: +// return "pending" +// case CertStatusUnknown: +// fallthrough +// default: +// return "unknown" +// } +// } -// Client is the interface a basic client implementation must support. -type Client interface { - // TODO(banks): build this and test it - // CertStatus returns the current status of the client's certificates. It can - // be used to determine if the Client is able to service requests at the - // current time. - //CertStatus() CertStatus +// // Client is the interface a basic client implementation must support. +// type Client interface { +// // TODO(banks): build this and test it +// // CertStatus returns the current status of the client's certificates. It can +// // be used to determine if the Client is able to service requests at the +// // current time. +// //CertStatus() CertStatus - // TODO(banks): build this and test it - // WatchCertStatus returns a channel that is notified on all status changes. - // Note that a message on the channel isn't guaranteed to be different so it's - // value should be inspected. During Client shutdown the channel will be - // closed returning a zero type which is equivalent to CertStatusUnknown. - //WatchCertStatus() <-chan CertStatus +// // TODO(banks): build this and test it +// // WatchCertStatus returns a channel that is notified on all status changes. +// // Note that a message on the channel isn't guaranteed to be different so it's +// // value should be inspected. During Client shutdown the channel will be +// // closed returning a zero type which is equivalent to CertStatusUnknown. +// //WatchCertStatus() <-chan CertStatus - // ServerTLSConfig returns the *tls.Config to be used when creating a TCP - // listener that should accept Connect connections. It is likely that at - // startup the tlsCfg returned will not be immediately usable since - // certificates are typically fetched from the agent asynchronously. In this - // case it's still safe to listen with the provided config, but auth failures - // will occur until initial certificate discovery is complete. In general at - // any time it is possible for certificates to expire before new replacements - // have been issued due to local network errors so the server may not actually - // have a working certificate configuration at any time, however as soon as - // valid certs can be issued it will automatically start working again so - // should take no action. - ServerTLSConfig() (*tls.Config, error) +// // ServerTLSConfig returns the *tls.Config to be used when creating a TCP +// // listener that should accept Connect connections. It is likely that at +// // startup the tlsCfg returned will not be immediately usable since +// // certificates are typically fetched from the agent asynchronously. In this +// // case it's still safe to listen with the provided config, but auth failures +// // will occur until initial certificate discovery is complete. In general at +// // any time it is possible for certificates to expire before new replacements +// // have been issued due to local network errors so the server may not actually +// // have a working certificate configuration at any time, however as soon as +// // valid certs can be issued it will automatically start working again so +// // should take no action. +// ServerTLSConfig() (*tls.Config, error) - // DialService opens a new connection to the named service registered in - // Consul. It will perform service discovery to find healthy instances. If - // there is an error during connection it is returned and the caller may call - // again. The client implementation makes a best effort to make consecutive - // Dials against different instances either by randomising the list and/or - // maintaining a local memory of which instances recently failed. If the - // context passed times out before connection is established and verified an - // error is returned. - DialService(ctx context.Context, namespace, name string) (net.Conn, error) +// // DialService opens a new connection to the named service registered in +// // Consul. It will perform service discovery to find healthy instances. If +// // there is an error during connection it is returned and the caller may call +// // again. The client implementation makes a best effort to make consecutive +// // Dials against different instances either by randomising the list and/or +// // maintaining a local memory of which instances recently failed. If the +// // context passed times out before connection is established and verified an +// // error is returned. +// DialService(ctx context.Context, namespace, name string) (net.Conn, error) - // DialPreparedQuery opens a new connection by executing the named Prepared - // Query against the local Consul agent, and picking one of the returned - // instances to connect to. It will perform service discovery with the same - // semantics as DialService. - DialPreparedQuery(ctx context.Context, namespace, name string) (net.Conn, error) -} +// // DialPreparedQuery opens a new connection by executing the named Prepared +// // Query against the local Consul agent, and picking one of the returned +// // instances to connect to. It will perform service discovery with the same +// // semantics as DialService. +// DialPreparedQuery(ctx context.Context, namespace, name string) (net.Conn, error) +// } -/* +// /* -Maybe also convenience wrappers for: - - listening TLS conn with right config - - http.ListenAndServeTLS equivalent +// Maybe also convenience wrappers for: +// - listening TLS conn with right config +// - http.ListenAndServeTLS equivalent -*/ +// */ -// AgentClient is the primary implementation of a connect.Client which -// communicates with the local Consul agent. -type AgentClient struct { - agent *api.Client - tlsCfg *ReloadableTLSConfig -} +// // AgentClient is the primary implementation of a connect.Client which +// // communicates with the local Consul agent. +// type AgentClient struct { +// agent *api.Client +// tlsCfg *ReloadableTLSConfig +// } -// NewClient returns an AgentClient to allow consuming and providing -// Connect-enabled network services. -func NewClient(agent *api.Client) Client { - // TODO(banks): hook up fetching certs from Agent and updating tlsCfg on cert - // delivery/change. Perhaps need to make - return &AgentClient{ - agent: agent, - tlsCfg: NewReloadableTLSConfig(defaultTLSConfig()), - } -} +// // NewClient returns an AgentClient to allow consuming and providing +// // Connect-enabled network services. +// func NewClient(agent *api.Client) Client { +// // TODO(banks): hook up fetching certs from Agent and updating tlsCfg on cert +// // delivery/change. Perhaps need to make +// return &AgentClient{ +// agent: agent, +// tlsCfg: NewReloadableTLSConfig(defaultTLSConfig()), +// } +// } -// NewInsecureDevClientWithLocalCerts returns an AgentClient that will still do -// service discovery via the local agent but will use externally provided -// certificates and skip authorization. This is intended just for development -// and must not be used ever in production. -func NewInsecureDevClientWithLocalCerts(agent *api.Client, caFile, certFile, - keyFile string) (Client, error) { +// // NewInsecureDevClientWithLocalCerts returns an AgentClient that will still do +// // service discovery via the local agent but will use externally provided +// // certificates and skip authorization. This is intended just for development +// // and must not be used ever in production. +// func NewInsecureDevClientWithLocalCerts(agent *api.Client, caFile, certFile, +// keyFile string) (Client, error) { - cfg, err := devTLSConfigFromFiles(caFile, certFile, keyFile) - if err != nil { - return nil, err - } +// cfg, err := devTLSConfigFromFiles(caFile, certFile, keyFile) +// if err != nil { +// return nil, err +// } - return &AgentClient{ - agent: agent, - tlsCfg: NewReloadableTLSConfig(cfg), - }, nil -} +// return &AgentClient{ +// agent: agent, +// tlsCfg: NewReloadableTLSConfig(cfg), +// }, nil +// } -// ServerTLSConfig implements Client -func (c *AgentClient) ServerTLSConfig() (*tls.Config, error) { - return c.tlsCfg.ServerTLSConfig(), nil -} +// // ServerTLSConfig implements Client +// func (c *AgentClient) ServerTLSConfig() (*tls.Config, error) { +// return c.tlsCfg.ServerTLSConfig(), nil +// } -// DialService implements Client -func (c *AgentClient) DialService(ctx context.Context, namespace, - name string) (net.Conn, error) { - return c.dial(ctx, "service", namespace, name) -} +// // DialService implements Client +// func (c *AgentClient) DialService(ctx context.Context, namespace, +// name string) (net.Conn, error) { +// return c.dial(ctx, "service", namespace, name) +// } -// DialPreparedQuery implements Client -func (c *AgentClient) DialPreparedQuery(ctx context.Context, namespace, - name string) (net.Conn, error) { - return c.dial(ctx, "prepared_query", namespace, name) -} +// // DialPreparedQuery implements Client +// func (c *AgentClient) DialPreparedQuery(ctx context.Context, namespace, +// name string) (net.Conn, error) { +// return c.dial(ctx, "prepared_query", namespace, name) +// } -func (c *AgentClient) dial(ctx context.Context, discoveryType, namespace, - name string) (net.Conn, error) { +// func (c *AgentClient) dial(ctx context.Context, discoveryType, namespace, +// name string) (net.Conn, error) { - svcs, err := c.discoverInstances(ctx, discoveryType, namespace, name) - if err != nil { - return nil, err - } +// svcs, err := c.discoverInstances(ctx, discoveryType, namespace, name) +// if err != nil { +// return nil, err +// } - svc, err := c.pickInstance(svcs) - if err != nil { - return nil, err - } - if svc == nil { - return nil, fmt.Errorf("no healthy services discovered") - } +// svc, err := c.pickInstance(svcs) +// if err != nil { +// return nil, err +// } +// if svc == nil { +// return nil, fmt.Errorf("no healthy services discovered") +// } - // OK we have a service we can dial! We need a ClientAuther that will validate - // the connection is legit. +// // OK we have a service we can dial! We need a ClientAuther that will validate +// // the connection is legit. - // TODO(banks): implement ClientAuther properly to actually verify connected - // cert matches the expected service/cluster etc. based on svc. - auther := &ClientAuther{} - tlsConfig := c.tlsCfg.TLSConfig(auther) +// // TODO(banks): implement ClientAuther properly to actually verify connected +// // cert matches the expected service/cluster etc. based on svc. +// auther := &ClientAuther{} +// tlsConfig := c.tlsCfg.TLSConfig(auther) - // Resolve address TODO(banks): I expected this to happen magically in the - // agent at registration time if I register with no explicit address but - // apparently doesn't. This is a quick hack to make it work for now, need to - // see if there is a better shared code path for doing this. - addr := svc.Service.Address - if addr == "" { - addr = svc.Node.Address - } - var dialer net.Dialer - tcpConn, err := dialer.DialContext(ctx, "tcp", - fmt.Sprintf("%s:%d", addr, svc.Service.Port)) - if err != nil { - return nil, err - } +// // Resolve address TODO(banks): I expected this to happen magically in the +// // agent at registration time if I register with no explicit address but +// // apparently doesn't. This is a quick hack to make it work for now, need to +// // see if there is a better shared code path for doing this. +// addr := svc.Service.Address +// if addr == "" { +// addr = svc.Node.Address +// } +// var dialer net.Dialer +// tcpConn, err := dialer.DialContext(ctx, "tcp", +// fmt.Sprintf("%s:%d", addr, svc.Service.Port)) +// if err != nil { +// return nil, err +// } - tlsConn := tls.Client(tcpConn, tlsConfig) - err = tlsConn.Handshake() - if err != nil { - tlsConn.Close() - return nil, err - } +// tlsConn := tls.Client(tcpConn, tlsConfig) +// err = tlsConn.Handshake() +// if err != nil { +// tlsConn.Close() +// return nil, err +// } - return tlsConn, nil -} +// return tlsConn, nil +// } -// pickInstance returns an instance from the given list to try to connect to. It -// may be made pluggable later, for now it just picks a random one regardless of -// whether the list is already shuffled. -func (c *AgentClient) pickInstance(svcs []*api.ServiceEntry) (*api.ServiceEntry, error) { - if len(svcs) < 1 { - return nil, nil - } - idx := rand.Intn(len(svcs)) - return svcs[idx], nil -} +// // pickInstance returns an instance from the given list to try to connect to. It +// // may be made pluggable later, for now it just picks a random one regardless of +// // whether the list is already shuffled. +// func (c *AgentClient) pickInstance(svcs []*api.ServiceEntry) (*api.ServiceEntry, error) { +// if len(svcs) < 1 { +// return nil, nil +// } +// idx := rand.Intn(len(svcs)) +// return svcs[idx], nil +// } -// discoverInstances returns all instances for the given discoveryType, -// namespace and name. The returned service entries may or may not be shuffled -func (c *AgentClient) discoverInstances(ctx context.Context, discoverType, - namespace, name string) ([]*api.ServiceEntry, error) { +// // discoverInstances returns all instances for the given discoveryType, +// // namespace and name. The returned service entries may or may not be shuffled +// func (c *AgentClient) discoverInstances(ctx context.Context, discoverType, +// namespace, name string) ([]*api.ServiceEntry, error) { - q := &api.QueryOptions{ - // TODO(banks): make this configurable? - AllowStale: true, - } - q = q.WithContext(ctx) +// q := &api.QueryOptions{ +// // TODO(banks): make this configurable? +// AllowStale: true, +// } +// q = q.WithContext(ctx) - switch discoverType { - case "service": - svcs, _, err := c.agent.Health().Connect(name, "", true, q) - if err != nil { - return nil, err - } - return svcs, err +// switch discoverType { +// case "service": +// svcs, _, err := c.agent.Health().Connect(name, "", true, q) +// if err != nil { +// return nil, err +// } +// return svcs, err - case "prepared_query": - // TODO(banks): it's not super clear to me how this should work eventually. - // How do we distinguise between a PreparedQuery for the actual services and - // one that should return the connect proxies where that differs? If we - // can't then we end up with a janky UX where user specifies a reasonable - // prepared query but we try to connect to non-connect services and fail - // with a confusing TLS error. Maybe just a way to filter PreparedQuery - // results by connect-enabled would be sufficient (or even metadata to do - // that ourselves in the response although less efficient). - resp, _, err := c.agent.PreparedQuery().Execute(name, q) - if err != nil { - return nil, err - } +// case "prepared_query": +// // TODO(banks): it's not super clear to me how this should work eventually. +// // How do we distinguise between a PreparedQuery for the actual services and +// // one that should return the connect proxies where that differs? If we +// // can't then we end up with a janky UX where user specifies a reasonable +// // prepared query but we try to connect to non-connect services and fail +// // with a confusing TLS error. Maybe just a way to filter PreparedQuery +// // results by connect-enabled would be sufficient (or even metadata to do +// // that ourselves in the response although less efficient). +// resp, _, err := c.agent.PreparedQuery().Execute(name, q) +// if err != nil { +// return nil, err +// } - // Awkward, we have a slice of api.ServiceEntry here but want a slice of - // *api.ServiceEntry for compat with Connect/Service APIs. Have to convert - // them to keep things type-happy. - svcs := make([]*api.ServiceEntry, len(resp.Nodes)) - for idx, se := range resp.Nodes { - svcs[idx] = &se - } - return svcs, err - default: - return nil, fmt.Errorf("unsupported discovery type: %s", discoverType) - } -} +// // Awkward, we have a slice of api.ServiceEntry here but want a slice of +// // *api.ServiceEntry for compat with Connect/Service APIs. Have to convert +// // them to keep things type-happy. +// svcs := make([]*api.ServiceEntry, len(resp.Nodes)) +// for idx, se := range resp.Nodes { +// svcs[idx] = &se +// } +// return svcs, err +// default: +// return nil, fmt.Errorf("unsupported discovery type: %s", discoverType) +// } +// } diff --git a/connect/client_test.go b/connect/client_test.go index fcb18e6000..045bc8fd60 100644 --- a/connect/client_test.go +++ b/connect/client_test.go @@ -1,148 +1,148 @@ package connect -import ( - "context" - "crypto/x509" - "crypto/x509/pkix" - "encoding/asn1" - "io/ioutil" - "net" - "net/http" - "net/http/httptest" - "net/url" - "strconv" - "testing" +// import ( +// "context" +// "crypto/x509" +// "crypto/x509/pkix" +// "encoding/asn1" +// "io/ioutil" +// "net" +// "net/http" +// "net/http/httptest" +// "net/url" +// "strconv" +// "testing" - "github.com/hashicorp/consul/api" - "github.com/hashicorp/consul/testutil" - "github.com/stretchr/testify/require" -) +// "github.com/hashicorp/consul/api" +// "github.com/hashicorp/consul/testutil" +// "github.com/stretchr/testify/require" +// ) -func TestNewInsecureDevClientWithLocalCerts(t *testing.T) { +// func TestNewInsecureDevClientWithLocalCerts(t *testing.T) { - agent, err := api.NewClient(api.DefaultConfig()) - require.Nil(t, err) +// agent, err := api.NewClient(api.DefaultConfig()) +// require.Nil(t, err) - got, err := NewInsecureDevClientWithLocalCerts(agent, - "testdata/ca1-ca-consul-internal.cert.pem", - "testdata/ca1-svc-web.cert.pem", - "testdata/ca1-svc-web.key.pem", - ) - require.Nil(t, err) +// got, err := NewInsecureDevClientWithLocalCerts(agent, +// "testdata/ca1-ca-consul-internal.cert.pem", +// "testdata/ca1-svc-web.cert.pem", +// "testdata/ca1-svc-web.key.pem", +// ) +// require.Nil(t, err) - // Sanity check correct certs were loaded - serverCfg, err := got.ServerTLSConfig() - require.Nil(t, err) - caSubjects := serverCfg.RootCAs.Subjects() - require.Len(t, caSubjects, 1) - caSubject, err := testNameFromRawDN(caSubjects[0]) - require.Nil(t, err) - require.Equal(t, "Consul Internal", caSubject.CommonName) +// // Sanity check correct certs were loaded +// serverCfg, err := got.ServerTLSConfig() +// require.Nil(t, err) +// caSubjects := serverCfg.RootCAs.Subjects() +// require.Len(t, caSubjects, 1) +// caSubject, err := testNameFromRawDN(caSubjects[0]) +// require.Nil(t, err) +// require.Equal(t, "Consul Internal", caSubject.CommonName) - require.Len(t, serverCfg.Certificates, 1) - cert, err := x509.ParseCertificate(serverCfg.Certificates[0].Certificate[0]) - require.Nil(t, err) - require.Equal(t, "web", cert.Subject.CommonName) -} +// require.Len(t, serverCfg.Certificates, 1) +// cert, err := x509.ParseCertificate(serverCfg.Certificates[0].Certificate[0]) +// require.Nil(t, err) +// require.Equal(t, "web", cert.Subject.CommonName) +// } -func testNameFromRawDN(raw []byte) (*pkix.Name, error) { - var seq pkix.RDNSequence - if _, err := asn1.Unmarshal(raw, &seq); err != nil { - return nil, err - } +// func testNameFromRawDN(raw []byte) (*pkix.Name, error) { +// var seq pkix.RDNSequence +// if _, err := asn1.Unmarshal(raw, &seq); err != nil { +// return nil, err +// } - var name pkix.Name - name.FillFromRDNSequence(&seq) - return &name, nil -} +// var name pkix.Name +// name.FillFromRDNSequence(&seq) +// return &name, nil +// } -func testAgent(t *testing.T) (*testutil.TestServer, *api.Client) { - t.Helper() +// func testAgent(t *testing.T) (*testutil.TestServer, *api.Client) { +// t.Helper() - // Make client config - conf := api.DefaultConfig() +// // Make client config +// conf := api.DefaultConfig() - // Create server - server, err := testutil.NewTestServerConfigT(t, nil) - require.Nil(t, err) +// // Create server +// server, err := testutil.NewTestServerConfigT(t, nil) +// require.Nil(t, err) - conf.Address = server.HTTPAddr +// conf.Address = server.HTTPAddr - // Create client - agent, err := api.NewClient(conf) - require.Nil(t, err) +// // Create client +// agent, err := api.NewClient(conf) +// require.Nil(t, err) - return server, agent -} +// return server, agent +// } -func testService(t *testing.T, ca, name string, client *api.Client) *httptest.Server { - t.Helper() +// func testService(t *testing.T, ca, name string, client *api.Client) *httptest.Server { +// t.Helper() - // Run a test service to discover - server := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Write([]byte("svc: " + name)) - })) - server.TLS = TestTLSConfig(t, ca, name) - server.StartTLS() +// // Run a test service to discover +// server := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { +// w.Write([]byte("svc: " + name)) +// })) +// server.TLS = TestTLSConfig(t, ca, name) +// server.StartTLS() - u, err := url.Parse(server.URL) - require.Nil(t, err) +// u, err := url.Parse(server.URL) +// require.Nil(t, err) - port, err := strconv.Atoi(u.Port()) - require.Nil(t, err) +// port, err := strconv.Atoi(u.Port()) +// require.Nil(t, err) - // If client is passed, register the test service instance - if client != nil { - svc := &api.AgentServiceRegistration{ - // TODO(banks): we don't really have a good way to represent - // connect-native apps yet so we have to pretend out little server is a - // proxy for now. - Kind: api.ServiceKindConnectProxy, - ProxyDestination: name, - Name: name + "-proxy", - Address: u.Hostname(), - Port: port, - } - err := client.Agent().ServiceRegister(svc) - require.Nil(t, err) - } +// // If client is passed, register the test service instance +// if client != nil { +// svc := &api.AgentServiceRegistration{ +// // TODO(banks): we don't really have a good way to represent +// // connect-native apps yet so we have to pretend out little server is a +// // proxy for now. +// Kind: api.ServiceKindConnectProxy, +// ProxyDestination: name, +// Name: name + "-proxy", +// Address: u.Hostname(), +// Port: port, +// } +// err := client.Agent().ServiceRegister(svc) +// require.Nil(t, err) +// } - return server -} +// return server +// } -func TestDialService(t *testing.T) { - consulServer, agent := testAgent(t) - defer consulServer.Stop() +// func TestDialService(t *testing.T) { +// consulServer, agent := testAgent(t) +// defer consulServer.Stop() - svc := testService(t, "ca1", "web", agent) - defer svc.Close() +// svc := testService(t, "ca1", "web", agent) +// defer svc.Close() - c, err := NewInsecureDevClientWithLocalCerts(agent, - "testdata/ca1-ca-consul-internal.cert.pem", - "testdata/ca1-svc-web.cert.pem", - "testdata/ca1-svc-web.key.pem", - ) - require.Nil(t, err) +// c, err := NewInsecureDevClientWithLocalCerts(agent, +// "testdata/ca1-ca-consul-internal.cert.pem", +// "testdata/ca1-svc-web.cert.pem", +// "testdata/ca1-svc-web.key.pem", +// ) +// require.Nil(t, err) - conn, err := c.DialService(context.Background(), "default", "web") - require.Nilf(t, err, "err: %s", err) +// conn, err := c.DialService(context.Background(), "default", "web") +// require.Nilf(t, err, "err: %s", err) - // Inject our conn into http.Transport - httpClient := &http.Client{ - Transport: &http.Transport{ - DialTLS: func(network, addr string) (net.Conn, error) { - return conn, nil - }, - }, - } +// // Inject our conn into http.Transport +// httpClient := &http.Client{ +// Transport: &http.Transport{ +// DialTLS: func(network, addr string) (net.Conn, error) { +// return conn, nil +// }, +// }, +// } - // Don't be fooled the hostname here is ignored since we did the dialling - // ourselves - resp, err := httpClient.Get("https://web.connect.consul/") - require.Nil(t, err) - defer resp.Body.Close() - body, err := ioutil.ReadAll(resp.Body) - require.Nil(t, err) +// // Don't be fooled the hostname here is ignored since we did the dialling +// // ourselves +// resp, err := httpClient.Get("https://web.connect.consul/") +// require.Nil(t, err) +// defer resp.Body.Close() +// body, err := ioutil.ReadAll(resp.Body) +// require.Nil(t, err) - require.Equal(t, "svc: web", string(body)) -} +// require.Equal(t, "svc: web", string(body)) +// } diff --git a/connect/example_test.go b/connect/example_test.go new file mode 100644 index 0000000000..eb66bdbc02 --- /dev/null +++ b/connect/example_test.go @@ -0,0 +1,53 @@ +package connect + +import ( + "crypto/tls" + "log" + "net" + "net/http" + + "github.com/hashicorp/consul/api" +) + +type apiHandler struct{} + +func (apiHandler) ServeHTTP(http.ResponseWriter, *http.Request) {} + +// Note: this assumes a suitable Consul ACL token with 'service:write' for +// service 'web' is set in CONSUL_HTTP_TOKEN ENV var. +func ExampleService_ServerTLSConfig_hTTP() { + client, _ := api.NewClient(api.DefaultConfig()) + svc, _ := NewService("web", client) + server := &http.Server{ + Addr: ":8080", + Handler: apiHandler{}, + TLSConfig: svc.ServerTLSConfig(), + } + // Cert and key files are blank since the tls.Config will handle providing + // those dynamically. + log.Fatal(server.ListenAndServeTLS("", "")) +} + +func acceptLoop(l net.Listener) {} + +// Note: this assumes a suitable Consul ACL token with 'service:write' for +// service 'web' is set in CONSUL_HTTP_TOKEN ENV var. +func ExampleService_ServerTLSConfig_tLS() { + client, _ := api.NewClient(api.DefaultConfig()) + svc, _ := NewService("web", client) + l, _ := tls.Listen("tcp", ":8080", svc.ServerTLSConfig()) + acceptLoop(l) +} + +func handleResponse(r *http.Response) {} + +// Note: this assumes a suitable Consul ACL token with 'service:write' for +// service 'web' is set in CONSUL_HTTP_TOKEN ENV var. +func ExampleService_HTTPClient() { + client, _ := api.NewClient(api.DefaultConfig()) + svc, _ := NewService("web", client) + + httpClient := svc.HTTPClient() + resp, _ := httpClient.Get("https://web.service.consul/foo/bar") + handleResponse(resp) +} diff --git a/connect/resolver.go b/connect/resolver.go new file mode 100644 index 0000000000..41dc70e82e --- /dev/null +++ b/connect/resolver.go @@ -0,0 +1,131 @@ +package connect + +import ( + "context" + "fmt" + "math/rand" + + "github.com/hashicorp/consul/agent/connect" + "github.com/hashicorp/consul/api" + testing "github.com/mitchellh/go-testing-interface" +) + +// Resolver is the interface implemented by a service discovery mechanism. +type Resolver interface { + // Resolve returns a single service instance to connect to. Implementations + // may attempt to ensure the instance returned is currently available. It is + // expected that a client will re-dial on a connection failure so making an + // effort to return a different service instance each time where available + // increases reliability. The context passed can be used to impose timeouts + // which may or may not be respected by implementations that make network + // calls to resolve the service. The addr returned is a string in any valid + // form for passing directly to `net.Dial("tcp", addr)`. + Resolve(ctx context.Context) (addr string, certURI connect.CertURI, err error) +} + +// StaticResolver is a statically defined resolver. This can be used to connect +// to an known-Connect endpoint without performing service discovery. +type StaticResolver struct { + // Addr is the network address (including port) of the instance. It must be + // the connect-enabled mTLS server and may be a proxy in front of the actual + // target service process. It is a string in any valid form for passing + // directly to `net.Dial("tcp", addr)`. + Addr string + + // CertURL is the _identity_ we expect the server to present in it's TLS + // certificate. It must be an exact match or the connection will be rejected. + CertURI connect.CertURI +} + +// Resolve implements Resolver by returning the static values. +func (sr *StaticResolver) Resolve(ctx context.Context) (string, connect.CertURI, error) { + return sr.Addr, sr.CertURI, nil +} + +const ( + // ConsulResolverTypeService indicates resolving healthy service nodes. + ConsulResolverTypeService int = iota + + // ConsulResolverTypePreparedQuery indicates resolving via prepared query. + ConsulResolverTypePreparedQuery +) + +// ConsulResolver queries Consul for a service instance. +type ConsulResolver struct { + // Client is the Consul API client to use. Must be non-nil or Resolve will + // panic. + Client *api.Client + + // Namespace of the query target + Namespace string + + // Name of the query target + Name string + + // Type of the query target, + Type int + + // Datacenter to resolve in, empty indicates agent's local DC. + Datacenter string +} + +// Resolve performs service discovery against the local Consul agent and returns +// the address and expected identity of a suitable service instance. +func (cr *ConsulResolver) Resolve(ctx context.Context) (string, connect.CertURI, error) { + switch cr.Type { + case ConsulResolverTypeService: + return cr.resolveService(ctx) + case ConsulResolverTypePreparedQuery: + // TODO(banks): we need to figure out what API changes are needed for + // prepared queries to become connect-aware. How do we signal that we want + // connect-enabled endpoints vs the direct ones for the responses? + return "", nil, fmt.Errorf("not implemented") + default: + return "", nil, fmt.Errorf("unknown resolver type") + } +} + +func (cr *ConsulResolver) resolveService(ctx context.Context) (string, connect.CertURI, error) { + health := cr.Client.Health() + + svcs, _, err := health.Connect(cr.Name, "", true, cr.queryOptions(ctx)) + if err != nil { + return "", nil, err + } + + if len(svcs) < 1 { + return "", nil, fmt.Errorf("no healthy instances found") + } + + // Services are not shuffled by HTTP API, pick one at (pseudo) random. + idx := 0 + if len(svcs) > 1 { + idx = rand.Intn(len(svcs)) + } + + addr := svcs[idx].Service.Address + if addr == "" { + addr = svcs[idx].Node.Address + } + port := svcs[idx].Service.Port + + // Generate the expected CertURI + + // TODO(banks): when we've figured out the CA story around generating and + // propagating these trust domains we need to actually fetch the trust domain + // somehow. We also need to implement namespaces. Use of test function here is + // temporary pending the work on trust domains. + certURI := connect.TestSpiffeIDService(&testing.RuntimeT{}, cr.Name) + + return fmt.Sprintf("%s:%d", addr, port), certURI, nil +} + +func (cr *ConsulResolver) queryOptions(ctx context.Context) *api.QueryOptions { + q := &api.QueryOptions{ + // We may make this configurable one day but we may also implement our own + // caching which is even more stale so... + AllowStale: true, + Datacenter: cr.Datacenter, + } + return q.WithContext(ctx) +} diff --git a/connect/resolver_test.go b/connect/resolver_test.go new file mode 100644 index 0000000000..29a40e3d32 --- /dev/null +++ b/connect/resolver_test.go @@ -0,0 +1,164 @@ +package connect + +import ( + "context" + "testing" + "time" + + "github.com/hashicorp/consul/agent" + "github.com/hashicorp/consul/agent/connect" + "github.com/hashicorp/consul/api" + "github.com/stretchr/testify/require" +) + +func TestStaticResolver_Resolve(t *testing.T) { + type fields struct { + Addr string + CertURI connect.CertURI + } + tests := []struct { + name string + fields fields + }{ + { + name: "simples", + fields: fields{"1.2.3.4:80", connect.TestSpiffeIDService(t, "foo")}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + sr := StaticResolver{ + Addr: tt.fields.Addr, + CertURI: tt.fields.CertURI, + } + addr, certURI, err := sr.Resolve(context.Background()) + require := require.New(t) + require.Nil(err) + require.Equal(sr.Addr, addr) + require.Equal(sr.CertURI, certURI) + }) + } +} + +func TestConsulResolver_Resolve(t *testing.T) { + + // Setup a local test agent to query + agent := agent.NewTestAgent("test-consul", "") + defer agent.Shutdown() + + cfg := api.DefaultConfig() + cfg.Address = agent.HTTPAddr() + client, err := api.NewClient(cfg) + require.Nil(t, err) + + // Setup a service with a connect proxy instance + regSrv := &api.AgentServiceRegistration{ + Name: "web", + Port: 8080, + } + err = client.Agent().ServiceRegister(regSrv) + require.Nil(t, err) + + regProxy := &api.AgentServiceRegistration{ + Kind: "connect-proxy", + Name: "web-proxy", + Port: 9090, + ProxyDestination: "web", + } + err = client.Agent().ServiceRegister(regProxy) + require.Nil(t, err) + + // And another proxy so we can test handling with multiple endpoints returned + regProxy.Port = 9091 + regProxy.ID = "web-proxy-2" + err = client.Agent().ServiceRegister(regProxy) + require.Nil(t, err) + + proxyAddrs := []string{ + agent.Config.AdvertiseAddrLAN.String() + ":9090", + agent.Config.AdvertiseAddrLAN.String() + ":9091", + } + + type fields struct { + Namespace string + Name string + Type int + Datacenter string + } + tests := []struct { + name string + fields fields + timeout time.Duration + wantAddr string + wantCertURI connect.CertURI + wantErr bool + }{ + { + name: "basic service discovery", + fields: fields{ + Namespace: "default", + Name: "web", + Type: ConsulResolverTypeService, + }, + wantCertURI: connect.TestSpiffeIDService(t, "web"), + wantErr: false, + }, + { + name: "Bad Type errors", + fields: fields{ + Namespace: "default", + Name: "web", + Type: 123, + }, + wantErr: true, + }, + { + name: "Non-existent service errors", + fields: fields{ + Namespace: "default", + Name: "foo", + Type: ConsulResolverTypeService, + }, + wantErr: true, + }, + { + name: "timeout errors", + fields: fields{ + Namespace: "default", + Name: "web", + Type: ConsulResolverTypeService, + }, + timeout: 1 * time.Nanosecond, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require := require.New(t) + cr := &ConsulResolver{ + Client: client, + Namespace: tt.fields.Namespace, + Name: tt.fields.Name, + Type: tt.fields.Type, + Datacenter: tt.fields.Datacenter, + } + // WithCancel just to have a cancel func in scope to assign in the if + // clause. + ctx, cancel := context.WithCancel(context.Background()) + if tt.timeout > 0 { + ctx, cancel = context.WithTimeout(ctx, tt.timeout) + } + defer cancel() + gotAddr, gotCertURI, err := cr.Resolve(ctx) + if tt.wantErr { + require.NotNil(err) + return + } + + require.Nil(err) + // Address should be either of the registered proxy ports so check both + require.Contains(proxyAddrs, gotAddr) + require.Equal(tt.wantCertURI, gotCertURI) + }) + } +} diff --git a/connect/service.go b/connect/service.go new file mode 100644 index 0000000000..db83ce5aad --- /dev/null +++ b/connect/service.go @@ -0,0 +1,185 @@ +package connect + +import ( + "context" + "crypto/tls" + "log" + "net" + "net/http" + "os" + "time" + + "github.com/hashicorp/consul/api" +) + +// Service represents a Consul service that accepts and/or connects via Connect. +// This can represent a service that only is a server, only is a client, or +// both. +// +// TODO(banks): API for monitoring status of certs from app +// +// TODO(banks): Agent implicit health checks based on knowing which certs are +// available should prevent clients being routed until the agent knows the +// service has been delivered valid certificates. Once built, document that here +// too. +type Service struct { + // serviceID is the unique ID for this service in the agent-local catalog. + // This is often but not always the service name. This is used to request + // Connect metadata. If the service with this ID doesn't exist on the local + // agent no error will be returned and the Service will retry periodically. + // This allows service startup and registration to happen in either order + // without coordination since they might be performed by separate processes. + serviceID string + + // client is the Consul API client. It must be configured with an appropriate + // Token that has `service:write` policy on the provided ServiceID. If an + // insufficient token is provided, the Service will abort further attempts to + // fetch certificates and print a loud error message. It will not Close() or + // kill the process since that could lead to a crash loop in every service if + // ACL token was revoked. All attempts to dial will error and any incoming + // connections will fail to verify. + client *api.Client + + // serverTLSCfg is the (reloadable) TLS config we use for serving. + serverTLSCfg *ReloadableTLSConfig + + // clientTLSCfg is the (reloadable) TLS config we use for dialling. + clientTLSCfg *ReloadableTLSConfig + + logger *log.Logger +} + +// NewService creates and starts a Service. The caller must close the returned +// service to free resources and allow the program to exit normally. This is +// typically called in a signal handler. +func NewService(serviceID string, client *api.Client) (*Service, error) { + return NewServiceWithLogger(serviceID, client, + log.New(os.Stderr, "", log.LstdFlags)) +} + +// NewServiceWithLogger starts the service with a specified log.Logger. +func NewServiceWithLogger(serviceID string, client *api.Client, + logger *log.Logger) (*Service, error) { + s := &Service{ + serviceID: serviceID, + client: client, + logger: logger, + } + s.serverTLSCfg = NewReloadableTLSConfig(defaultTLSConfig(serverVerifyCerts)) + s.clientTLSCfg = NewReloadableTLSConfig(defaultTLSConfig(clientVerifyCerts)) + + // TODO(banks) run the background certificate sync + return s, nil +} + +// NewDevServiceFromCertFiles creates a Service using certificate and key files +// passed instead of fetching them from the client. +func NewDevServiceFromCertFiles(serviceID string, client *api.Client, + logger *log.Logger, caFile, certFile, keyFile string) (*Service, error) { + s := &Service{ + serviceID: serviceID, + client: client, + logger: logger, + } + tlsCfg, err := devTLSConfigFromFiles(caFile, certFile, keyFile) + if err != nil { + return nil, err + } + + // Note that NewReloadableTLSConfig makes a copy so we can re-use the same + // base for both client and server with swapped verifiers. + tlsCfg.VerifyPeerCertificate = serverVerifyCerts + s.serverTLSCfg = NewReloadableTLSConfig(tlsCfg) + tlsCfg.VerifyPeerCertificate = clientVerifyCerts + s.clientTLSCfg = NewReloadableTLSConfig(tlsCfg) + return s, nil +} + +// ServerTLSConfig returns a *tls.Config that allows any TCP listener to accept +// and authorize incoming Connect clients. It will return a single static config +// with hooks to dynamically load certificates, and perform Connect +// authorization during verification. Service implementations do not need to +// reload this to get new certificates. +// +// At any time it may be possible that the Service instance does not have access +// to usable certificates due to not being initially setup yet or a prolonged +// error during renewal. The listener will be able to accept connections again +// once connectivity is restored provided the client's Token is valid. +func (s *Service) ServerTLSConfig() *tls.Config { + return s.serverTLSCfg.TLSConfig() +} + +// Dial connects to a remote Connect-enabled server. The passed Resolver is used +// to discover a single candidate instance which will be dialled and have it's +// TLS certificate verified against the expected identity. Failures are returned +// directly with no retries. Repeated dials may use different instances +// depending on the Resolver implementation. +// +// Timeout can be managed via the Context. +func (s *Service) Dial(ctx context.Context, resolver Resolver) (net.Conn, error) { + addr, certURI, err := resolver.Resolve(ctx) + if err != nil { + return nil, err + } + var dialer net.Dialer + tcpConn, err := dialer.DialContext(ctx, "tcp", addr) + if err != nil { + return nil, err + } + + tlsConn := tls.Client(tcpConn, s.clientTLSCfg.TLSConfig()) + // Set deadline for Handshake to complete. + deadline, ok := ctx.Deadline() + if ok { + tlsConn.SetDeadline(deadline) + } + err = tlsConn.Handshake() + if err != nil { + tlsConn.Close() + return nil, err + } + // Clear deadline since that was only for connection. Caller can set their own + // deadline later as necessary. + tlsConn.SetDeadline(time.Time{}) + + // Verify that the connect server's URI matches certURI + err = verifyServerCertMatchesURI(tlsConn.ConnectionState().PeerCertificates, + certURI) + if err != nil { + tlsConn.Close() + return nil, err + } + + return tlsConn, nil +} + +// HTTPDialContext is compatible with http.Transport.DialContext. It expects the +// addr hostname to be specified using Consul DNS query syntax, e.g. +// "web.service.consul". It converts that into the equivalent ConsulResolver and +// then call s.Dial with the resolver. This is low level, clients should +// typically use HTTPClient directly. +func (s *Service) HTTPDialContext(ctx context.Context, network, + addr string) (net.Conn, error) { + var r ConsulResolver + // TODO(banks): parse addr into ConsulResolver + return s.Dial(ctx, &r) +} + +// HTTPClient returns an *http.Client configured to dial remote Consul Connect +// HTTP services. The client will return an error if attempting to make requests +// to a non HTTPS hostname. It resolves the domain of the request with the same +// syntax as Consul DNS queries although it performs discovery directly via the +// API rather than just relying on Consul DNS. Hostnames that are not valid +// Consul DNS queries will fail. +func (s *Service) HTTPClient() *http.Client { + return &http.Client{ + Transport: &http.Transport{ + DialContext: s.HTTPDialContext, + }, + } +} + +// Close stops the service and frees resources. +func (s *Service) Close() { + // TODO(banks): stop background activity if started +} diff --git a/connect/service_test.go b/connect/service_test.go new file mode 100644 index 0000000000..a2adfe7f1f --- /dev/null +++ b/connect/service_test.go @@ -0,0 +1,105 @@ +package connect + +import ( + "context" + "fmt" + "testing" + "time" + + "github.com/hashicorp/consul/agent/connect" + "github.com/stretchr/testify/require" +) + +func TestService_Dial(t *testing.T) { + ca := connect.TestCA(t, nil) + + tests := []struct { + name string + accept bool + handshake bool + presentService string + wantErr string + }{ + { + name: "working", + accept: true, + handshake: true, + presentService: "db", + wantErr: "", + }, + { + name: "tcp connect fail", + accept: false, + handshake: false, + presentService: "db", + wantErr: "connection refused", + }, + { + name: "handshake timeout", + accept: true, + handshake: false, + presentService: "db", + wantErr: "i/o timeout", + }, + { + name: "bad cert", + accept: true, + handshake: true, + presentService: "web", + wantErr: "peer certificate mismatch", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require := require.New(t) + + s, err := NewService("web", nil) + require.Nil(err) + + // Force TLSConfig + s.clientTLSCfg = NewReloadableTLSConfig(TestTLSConfig(t, "web", ca)) + + ctx, cancel := context.WithTimeout(context.Background(), + 100*time.Millisecond) + defer cancel() + + testSvc := NewTestService(t, tt.presentService, ca) + testSvc.TimeoutHandshake = !tt.handshake + + if tt.accept { + go func() { + err := testSvc.Serve() + require.Nil(err) + }() + defer testSvc.Close() + } + + // Always expect to be connecting to a "DB" + resolver := &StaticResolver{ + Addr: testSvc.Addr, + CertURI: connect.TestSpiffeIDService(t, "db"), + } + + // All test runs should complete in under 500ms due to the timeout about. + // Don't wait for whole test run to get stuck. + testTimeout := 500 * time.Millisecond + testTimer := time.AfterFunc(testTimeout, func() { + panic(fmt.Sprintf("test timed out after %s", testTimeout)) + }) + + conn, err := s.Dial(ctx, resolver) + testTimer.Stop() + + if tt.wantErr == "" { + require.Nil(err) + } else { + require.NotNil(err) + require.Contains(err.Error(), tt.wantErr) + } + + if err == nil { + conn.Close() + } + }) + } +} diff --git a/connect/testdata/ca1-ca-consul-internal.cert.pem b/connect/testdata/ca1-ca-consul-internal.cert.pem deleted file mode 100644 index 6a557775f9..0000000000 --- a/connect/testdata/ca1-ca-consul-internal.cert.pem +++ /dev/null @@ -1,14 +0,0 @@ ------BEGIN CERTIFICATE----- -MIICIDCCAcagAwIBAgIBATAKBggqhkjOPQQDAjAaMRgwFgYDVQQDEw9Db25zdWwg -SW50ZXJuYWwwHhcNMTgwMzIzMjIwNDI1WhcNMjgwMzIwMjIwNDI1WjAaMRgwFgYD -VQQDEw9Db25zdWwgSW50ZXJuYWwwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAAT3 -IPiDHugKYEVaSpIzBjqU5lQrmirC6N1XHyOAhF2psGGxcxezpf8Vgy5Iv6XbmeHr -cttyzUYtUKhrFBhxkPYRo4H8MIH5MA4GA1UdDwEB/wQEAwIBhjAPBgNVHRMBAf8E -BTADAQH/MCkGA1UdDgQiBCCrnNQy2IQS73Co9WbrPXtq/YP9SvIBOJ8iYRWTOxjC -qTArBgNVHSMEJDAigCCrnNQy2IQS73Co9WbrPXtq/YP9SvIBOJ8iYRWTOxjCqTA/ -BgNVHREEODA2hjRzcGlmZmU6Ly8xMTExMTExMS0yMjIyLTMzMzMtNDQ0NC01NTU1 -NTU1NTU1NTUuY29uc3VsMD0GA1UdHgEB/wQzMDGgLzAtgisxMTExMTExMS0yMjIy -LTMzMzMtNDQ0NC01NTU1NTU1NTU1NTUuY29uc3VsMAoGCCqGSM49BAMCA0gAMEUC -IQDwWL6ZuszKrZjSJwDzdhRQtj1ppezJrKaDTJx+4F/tyQIgEaQCR935ztIqZzgO -Ka6ozcH2Ubd4j4cDC1XswVMW6zs= ------END CERTIFICATE----- diff --git a/connect/testdata/ca1-ca-consul-internal.key.pem b/connect/testdata/ca1-ca-consul-internal.key.pem deleted file mode 100644 index 8c40fd26bd..0000000000 --- a/connect/testdata/ca1-ca-consul-internal.key.pem +++ /dev/null @@ -1,5 +0,0 @@ ------BEGIN EC PRIVATE KEY----- -MHcCAQEEIDUDO3I7WKbLTTWkNKA4unB2RLq/RX+L+XIFssDE/AD7oAoGCCqGSM49 -AwEHoUQDQgAE9yD4gx7oCmBFWkqSMwY6lOZUK5oqwujdVx8jgIRdqbBhsXMXs6X/ -FYMuSL+l25nh63Lbcs1GLVCoaxQYcZD2EQ== ------END EC PRIVATE KEY----- diff --git a/connect/testdata/ca1-svc-cache.cert.pem b/connect/testdata/ca1-svc-cache.cert.pem deleted file mode 100644 index 097a2b6a6f..0000000000 --- a/connect/testdata/ca1-svc-cache.cert.pem +++ /dev/null @@ -1,14 +0,0 @@ ------BEGIN CERTIFICATE----- -MIICEDCCAbagAwIBAgIBBTAKBggqhkjOPQQDAjAaMRgwFgYDVQQDEw9Db25zdWwg -SW50ZXJuYWwwHhcNMTgwMzIzMjIwNDI1WhcNMjgwMzIwMjIwNDI1WjAQMQ4wDAYD -VQQDEwVjYWNoZTBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABOWw8369v4DHJAI6 -k061hU8rxaQs87mZFQ52JfleJjRoDUuZIPLhZHMFbvbI8pDWi7YdjluNbzNNh6nu -fAivylujgfYwgfMwDgYDVR0PAQH/BAQDAgO4MB0GA1UdJQQWMBQGCCsGAQUFBwMC -BggrBgEFBQcDATAMBgNVHRMBAf8EAjAAMCkGA1UdDgQiBCCHhMqV2/R8meSsXtwh -OLC9hP7WQfuvwJ6V6uwKZdEofTArBgNVHSMEJDAigCCrnNQy2IQS73Co9WbrPXtq -/YP9SvIBOJ8iYRWTOxjCqTBcBgNVHREEVTBThlFzcGlmZmU6Ly8xMTExMTExMS0y -MjIyLTMzMzMtNDQ0NC01NTU1NTU1NTU1NTUuY29uc3VsL25zL2RlZmF1bHQvZGMv -ZGMwMS9zdmMvY2FjaGUwCgYIKoZIzj0EAwIDSAAwRQIgPfekKBd/ltpVkdjnB0Hp -cV9HPwy12tXp4suR2nspSNkCIQD1Th/hvxuBKkRYy9Bl+jgTbrFdd4fLCWPeFbaM -sgLK7g== ------END CERTIFICATE----- diff --git a/connect/testdata/ca1-svc-cache.key.pem b/connect/testdata/ca1-svc-cache.key.pem deleted file mode 100644 index f780f63db8..0000000000 --- a/connect/testdata/ca1-svc-cache.key.pem +++ /dev/null @@ -1,5 +0,0 @@ ------BEGIN EC PRIVATE KEY----- -MHcCAQEEIPTSPV2cWNnO69f+vYyCg5frpoBtK6L+kZVLrGCv3TdnoAoGCCqGSM49 -AwEHoUQDQgAE5bDzfr2/gMckAjqTTrWFTyvFpCzzuZkVDnYl+V4mNGgNS5kg8uFk -cwVu9sjykNaLth2OW41vM02Hqe58CK/KWw== ------END EC PRIVATE KEY----- diff --git a/connect/testdata/ca1-svc-db.cert.pem b/connect/testdata/ca1-svc-db.cert.pem deleted file mode 100644 index d00a38ea08..0000000000 --- a/connect/testdata/ca1-svc-db.cert.pem +++ /dev/null @@ -1,13 +0,0 @@ ------BEGIN CERTIFICATE----- -MIICCjCCAbCgAwIBAgIBBDAKBggqhkjOPQQDAjAaMRgwFgYDVQQDEw9Db25zdWwg -SW50ZXJuYWwwHhcNMTgwMzIzMjIwNDI1WhcNMjgwMzIwMjIwNDI1WjANMQswCQYD -VQQDEwJkYjBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABEcTyr2l7yYWZuh++02M -usR20QrZtHdd7goKmYrIpQ3ekmHuLLgJWgTTaIhCj8fzbryep+s8oM7EiPhRQ14l -uSujgfMwgfAwDgYDVR0PAQH/BAQDAgO4MB0GA1UdJQQWMBQGCCsGAQUFBwMCBggr -BgEFBQcDATAMBgNVHRMBAf8EAjAAMCkGA1UdDgQiBCAy6jHCBBT2bii+aMJCDJ33 -bFJtR72bxDBUi5b+YWyWwDArBgNVHSMEJDAigCCrnNQy2IQS73Co9WbrPXtq/YP9 -SvIBOJ8iYRWTOxjCqTBZBgNVHREEUjBQhk5zcGlmZmU6Ly8xMTExMTExMS0yMjIy -LTMzMzMtNDQ0NC01NTU1NTU1NTU1NTUuY29uc3VsL25zL2RlZmF1bHQvZGMvZGMw -MS9zdmMvZGIwCgYIKoZIzj0EAwIDSAAwRQIhALCW4cOEpuYfLJ0NGwEmYG5Fko0N -WMccL0gEQzKUbIWrAiAIw8wkTSf1O8vTHeKdR1fCmdVoDRFRKB643PaofUzFxA== ------END CERTIFICATE----- diff --git a/connect/testdata/ca1-svc-db.key.pem b/connect/testdata/ca1-svc-db.key.pem deleted file mode 100644 index 3ec23a33b7..0000000000 --- a/connect/testdata/ca1-svc-db.key.pem +++ /dev/null @@ -1,5 +0,0 @@ ------BEGIN EC PRIVATE KEY----- -MHcCAQEEIMHv1pjt75IjKXzl8l4rBtEFS1pEuOM4WNgeHg5Qn1RroAoGCCqGSM49 -AwEHoUQDQgAERxPKvaXvJhZm6H77TYy6xHbRCtm0d13uCgqZisilDd6SYe4suAla -BNNoiEKPx/NuvJ6n6zygzsSI+FFDXiW5Kw== ------END EC PRIVATE KEY----- diff --git a/connect/testdata/ca1-svc-web.cert.pem b/connect/testdata/ca1-svc-web.cert.pem deleted file mode 100644 index a786a2c06a..0000000000 --- a/connect/testdata/ca1-svc-web.cert.pem +++ /dev/null @@ -1,13 +0,0 @@ ------BEGIN CERTIFICATE----- -MIICDDCCAbKgAwIBAgIBAzAKBggqhkjOPQQDAjAaMRgwFgYDVQQDEw9Db25zdWwg -SW50ZXJuYWwwHhcNMTgwMzIzMjIwNDI1WhcNMjgwMzIwMjIwNDI1WjAOMQwwCgYD -VQQDEwN3ZWIwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAARF47lERGXziNBC74Kh -U3W29/M7JO9LIUaJgK0LJbhgf0MuPxf7gX+PnxH5ImI5yfXRv0SSxeCq7377IkXP -XS6Fo4H0MIHxMA4GA1UdDwEB/wQEAwIDuDAdBgNVHSUEFjAUBggrBgEFBQcDAgYI -KwYBBQUHAwEwDAYDVR0TAQH/BAIwADApBgNVHQ4EIgQg26hfNYiVwYRm7CQJvdOd -NIOmG3t8vNwXCtktC782cf8wKwYDVR0jBCQwIoAgq5zUMtiEEu9wqPVm6z17av2D -/UryATifImEVkzsYwqkwWgYDVR0RBFMwUYZPc3BpZmZlOi8vMTExMTExMTEtMjIy -Mi0zMzMzLTQ0NDQtNTU1NTU1NTU1NTU1LmNvbnN1bC9ucy9kZWZhdWx0L2RjL2Rj -MDEvc3ZjL3dlYjAKBggqhkjOPQQDAgNIADBFAiAzi8uBs+ApPfAZZm5eO/hhVZiv -E8p84VKCqPeF3tFfoAIhANVkdSnp2AKU5T7SlJHmieu3DFNWCVpajlHJvf286J94 ------END CERTIFICATE----- diff --git a/connect/testdata/ca1-svc-web.key.pem b/connect/testdata/ca1-svc-web.key.pem deleted file mode 100644 index 8ed82c13c7..0000000000 --- a/connect/testdata/ca1-svc-web.key.pem +++ /dev/null @@ -1,5 +0,0 @@ ------BEGIN EC PRIVATE KEY----- -MHcCAQEEIPOIj4BFS0fknG+uAVKZIWRpnzp7O3OKpBDgEmuml7lcoAoGCCqGSM49 -AwEHoUQDQgAEReO5RERl84jQQu+CoVN1tvfzOyTvSyFGiYCtCyW4YH9DLj8X+4F/ -j58R+SJiOcn10b9EksXgqu9++yJFz10uhQ== ------END EC PRIVATE KEY----- diff --git a/connect/testdata/ca2-ca-vault.cert.pem b/connect/testdata/ca2-ca-vault.cert.pem deleted file mode 100644 index a7f6174686..0000000000 --- a/connect/testdata/ca2-ca-vault.cert.pem +++ /dev/null @@ -1,14 +0,0 @@ ------BEGIN CERTIFICATE----- -MIICDDCCAbKgAwIBAgIBAjAKBggqhkjOPQQDAjAQMQ4wDAYDVQQDEwVWYXVsdDAe -Fw0xODAzMjMyMjA0MjVaFw0yODAzMjAyMjA0MjVaMBAxDjAMBgNVBAMTBVZhdWx0 -MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEAjGVnRy/7Q2SU4ePbKbsurRAHKYA -CuA3r9QrowgZOr7yptF54shiobMIORpfKYkoYkhzL1lhWKI06BUJ4xuPd6OB/DCB -+TAOBgNVHQ8BAf8EBAMCAYYwDwYDVR0TAQH/BAUwAwEB/zApBgNVHQ4EIgQgqEc5 -ZrELD5ySxapbU+eRb+aEv1MEoCvjC0mCA1uJecMwKwYDVR0jBCQwIoAgqEc5ZrEL -D5ySxapbU+eRb+aEv1MEoCvjC0mCA1uJecMwPwYDVR0RBDgwNoY0c3BpZmZlOi8v -MTExMTExMTEtMjIyMi0zMzMzLTQ0NDQtNTU1NTU1NTU1NTU1LmNvbnN1bDA9BgNV -HR4BAf8EMzAxoC8wLYIrMTExMTExMTEtMjIyMi0zMzMzLTQ0NDQtNTU1NTU1NTU1 -NTU1LmNvbnN1bDAKBggqhkjOPQQDAgNIADBFAiEA6pBdeglhq//A7sYnYk85XL+3 -4IDrXrGN3KjC9qo3J9ICIDS9pEoTPWAWDfn1ccPafKVBrJm6KrmljcvymQ2QUDIZ ------END CERTIFICATE----- ----- diff --git a/connect/testdata/ca2-ca-vault.key.pem b/connect/testdata/ca2-ca-vault.key.pem deleted file mode 100644 index 43534b961b..0000000000 --- a/connect/testdata/ca2-ca-vault.key.pem +++ /dev/null @@ -1,5 +0,0 @@ ------BEGIN EC PRIVATE KEY----- -MHcCAQEEIKnuCctuvtyzf+M6B8jGqejG4T5o7NMRYjO2M3dZITCboAoGCCqGSM49 -AwEHoUQDQgAEAjGVnRy/7Q2SU4ePbKbsurRAHKYACuA3r9QrowgZOr7yptF54shi -obMIORpfKYkoYkhzL1lhWKI06BUJ4xuPdw== ------END EC PRIVATE KEY----- diff --git a/connect/testdata/ca2-svc-cache.cert.pem b/connect/testdata/ca2-svc-cache.cert.pem deleted file mode 100644 index 32110e232b..0000000000 --- a/connect/testdata/ca2-svc-cache.cert.pem +++ /dev/null @@ -1,13 +0,0 @@ ------BEGIN CERTIFICATE----- -MIICBzCCAaygAwIBAgIBCDAKBggqhkjOPQQDAjAQMQ4wDAYDVQQDEwVWYXVsdDAe -Fw0xODAzMjMyMjA0MjVaFw0yODAzMjAyMjA0MjVaMBAxDjAMBgNVBAMTBWNhY2hl -MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEyB6D+Eqi/71EhUrBWlcZOV2vjS9Y -xnUQ3jfH+QUZur7WOuGLnO7eArbAHcDbqKGyDWxlkZH04sGYOXaEW7UUd6OB9jCB -8zAOBgNVHQ8BAf8EBAMCA7gwHQYDVR0lBBYwFAYIKwYBBQUHAwIGCCsGAQUFBwMB -MAwGA1UdEwEB/wQCMAAwKQYDVR0OBCIEIGapiHFxlbYbNKFlwdPMpKhIypvNZXo8 -k/OZLki/vurQMCsGA1UdIwQkMCKAIKhHOWaxCw+cksWqW1PnkW/mhL9TBKAr4wtJ -ggNbiXnDMFwGA1UdEQRVMFOGUXNwaWZmZTovLzExMTExMTExLTIyMjItMzMzMy00 -NDQ0LTU1NTU1NTU1NTU1NS5jb25zdWwvbnMvZGVmYXVsdC9kYy9kYzAxL3N2Yy9j -YWNoZTAKBggqhkjOPQQDAgNJADBGAiEA/vRLXbkigS6l89MxFk0RFE7Zo4vorv7s -E1juCOsVJBICIQDXlpmYH9fPon6DYMyOxQttNjkuWbJgnPv7rPg+CllRyA== ------END CERTIFICATE----- diff --git a/connect/testdata/ca2-svc-cache.key.pem b/connect/testdata/ca2-svc-cache.key.pem deleted file mode 100644 index cabad8179d..0000000000 --- a/connect/testdata/ca2-svc-cache.key.pem +++ /dev/null @@ -1,5 +0,0 @@ ------BEGIN EC PRIVATE KEY----- -MHcCAQEEIEbQOv4odF2Tu8ZnJTJuytvOd2HOF9HxgGw5ei1pkP4moAoGCCqGSM49 -AwEHoUQDQgAEyB6D+Eqi/71EhUrBWlcZOV2vjS9YxnUQ3jfH+QUZur7WOuGLnO7e -ArbAHcDbqKGyDWxlkZH04sGYOXaEW7UUdw== ------END EC PRIVATE KEY----- diff --git a/connect/testdata/ca2-svc-db.cert.pem b/connect/testdata/ca2-svc-db.cert.pem deleted file mode 100644 index 33273058a4..0000000000 --- a/connect/testdata/ca2-svc-db.cert.pem +++ /dev/null @@ -1,13 +0,0 @@ ------BEGIN CERTIFICATE----- -MIICADCCAaagAwIBAgIBBzAKBggqhkjOPQQDAjAQMQ4wDAYDVQQDEwVWYXVsdDAe -Fw0xODAzMjMyMjA0MjVaFw0yODAzMjAyMjA0MjVaMA0xCzAJBgNVBAMTAmRiMFkw -EwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEFeB4DynO6IeKOE4zFLlBVFv+4HeWRvK3 -6cQ9L6v5uhLfdcYyqhT/QLbQ4R8ks1vUTTiq0XJsAGdkvkt71fiEl6OB8zCB8DAO -BgNVHQ8BAf8EBAMCA7gwHQYDVR0lBBYwFAYIKwYBBQUHAwIGCCsGAQUFBwMBMAwG -A1UdEwEB/wQCMAAwKQYDVR0OBCIEIKjVz8n91cej8q6WpDNd0hwSMAE2ddY056PH -hMfaBM6GMCsGA1UdIwQkMCKAIKhHOWaxCw+cksWqW1PnkW/mhL9TBKAr4wtJggNb -iXnDMFkGA1UdEQRSMFCGTnNwaWZmZTovLzExMTExMTExLTIyMjItMzMzMy00NDQ0 -LTU1NTU1NTU1NTU1NS5jb25zdWwvbnMvZGVmYXVsdC9kYy9kYzAxL3N2Yy9kYjAK -BggqhkjOPQQDAgNIADBFAiAdYkokbeZr7W32NhjcNoTMNwpz9CqJpK6Yzu4N6EJc -pAIhALHpRM57zdiMouDOlhGPX5XKzbSl2AnBjFvbPqgFV09Z ------END CERTIFICATE----- diff --git a/connect/testdata/ca2-svc-db.key.pem b/connect/testdata/ca2-svc-db.key.pem deleted file mode 100644 index 7f7ab9ff81..0000000000 --- a/connect/testdata/ca2-svc-db.key.pem +++ /dev/null @@ -1,5 +0,0 @@ ------BEGIN EC PRIVATE KEY----- -MHcCAQEEIHnzia+DNTFB7uYQEuWvLR2czGCuDfOTt1FfcBo1uBJioAoGCCqGSM49 -AwEHoUQDQgAEFeB4DynO6IeKOE4zFLlBVFv+4HeWRvK36cQ9L6v5uhLfdcYyqhT/ -QLbQ4R8ks1vUTTiq0XJsAGdkvkt71fiElw== ------END EC PRIVATE KEY----- diff --git a/connect/testdata/ca2-svc-web.cert.pem b/connect/testdata/ca2-svc-web.cert.pem deleted file mode 100644 index ae1e338f66..0000000000 --- a/connect/testdata/ca2-svc-web.cert.pem +++ /dev/null @@ -1,13 +0,0 @@ ------BEGIN CERTIFICATE----- -MIICAjCCAaigAwIBAgIBBjAKBggqhkjOPQQDAjAQMQ4wDAYDVQQDEwVWYXVsdDAe -Fw0xODAzMjMyMjA0MjVaFw0yODAzMjAyMjA0MjVaMA4xDDAKBgNVBAMTA3dlYjBZ -MBMGByqGSM49AgEGCCqGSM49AwEHA0IABM9XzxWFCa80uQDfJEGboUC15Yr+FwDp -OemThalQxFpkL7gQSIgpzgGULIx+jCiu+clJ0QhbWT2dnS8vFUKq35qjgfQwgfEw -DgYDVR0PAQH/BAQDAgO4MB0GA1UdJQQWMBQGCCsGAQUFBwMCBggrBgEFBQcDATAM -BgNVHRMBAf8EAjAAMCkGA1UdDgQiBCCN+TKHPCOr48hxRCx4rqbWQg5QHkCSNzjZ -qi1JGs13njArBgNVHSMEJDAigCCoRzlmsQsPnJLFqltT55Fv5oS/UwSgK+MLSYID -W4l5wzBaBgNVHREEUzBRhk9zcGlmZmU6Ly8xMTExMTExMS0yMjIyLTMzMzMtNDQ0 -NC01NTU1NTU1NTU1NTUuY29uc3VsL25zL2RlZmF1bHQvZGMvZGMwMS9zdmMvd2Vi -MAoGCCqGSM49BAMCA0gAMEUCIBd6gpL6E8rms5BU+cJeeyv0Rjc18edn2g3q2wLN -r1zAAiEAv16whKwR0DeKkldGLDQIu9nCNvfDZrEWgywIBYbzLxY= ------END CERTIFICATE----- diff --git a/connect/testdata/ca2-svc-web.key.pem b/connect/testdata/ca2-svc-web.key.pem deleted file mode 100644 index 65f0bc48e6..0000000000 --- a/connect/testdata/ca2-svc-web.key.pem +++ /dev/null @@ -1,5 +0,0 @@ ------BEGIN EC PRIVATE KEY----- -MHcCAQEEIOCMjjRexX3qHjixpRwLxggJd9yuskqUoPy8/MepafP+oAoGCCqGSM49 -AwEHoUQDQgAEz1fPFYUJrzS5AN8kQZuhQLXliv4XAOk56ZOFqVDEWmQvuBBIiCnO -AZQsjH6MKK75yUnRCFtZPZ2dLy8VQqrfmg== ------END EC PRIVATE KEY----- diff --git a/connect/testdata/ca2-xc-by-ca1.cert.pem b/connect/testdata/ca2-xc-by-ca1.cert.pem deleted file mode 100644 index e864f6c00f..0000000000 --- a/connect/testdata/ca2-xc-by-ca1.cert.pem +++ /dev/null @@ -1,14 +0,0 @@ ------BEGIN CERTIFICATE----- -MIICFjCCAbygAwIBAgIBAjAKBggqhkjOPQQDAjAaMRgwFgYDVQQDEw9Db25zdWwg -SW50ZXJuYWwwHhcNMTgwMzIzMjIwNDI1WhcNMjgwMzIwMjIwNDI1WjAQMQ4wDAYD -VQQDEwVWYXVsdDBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABAIxlZ0cv+0NklOH -j2ym7Lq0QBymAArgN6/UK6MIGTq+8qbReeLIYqGzCDkaXymJKGJIcy9ZYViiNOgV -CeMbj3ejgfwwgfkwDgYDVR0PAQH/BAQDAgGGMA8GA1UdEwEB/wQFMAMBAf8wKQYD -VR0OBCIEIKhHOWaxCw+cksWqW1PnkW/mhL9TBKAr4wtJggNbiXnDMCsGA1UdIwQk -MCKAIKuc1DLYhBLvcKj1Zus9e2r9g/1K8gE4nyJhFZM7GMKpMD8GA1UdEQQ4MDaG -NHNwaWZmZTovLzExMTExMTExLTIyMjItMzMzMy00NDQ0LTU1NTU1NTU1NTU1NS5j -b25zdWwwPQYDVR0eAQH/BDMwMaAvMC2CKzExMTExMTExLTIyMjItMzMzMy00NDQ0 -LTU1NTU1NTU1NTU1NS5jb25zdWwwCgYIKoZIzj0EAwIDSAAwRQIgWWWj8/6SaY2y -wzOtIphwZLewCuLMG6KG8uY4S7UsosgCIQDhCbT/LUKq/A21khQncBmM79ng9Gbx -/4Zw8zbVmnZJKg== ------END CERTIFICATE----- diff --git a/connect/testdata/mkcerts.go b/connect/testdata/mkcerts.go deleted file mode 100644 index 7fe82f53a6..0000000000 --- a/connect/testdata/mkcerts.go +++ /dev/null @@ -1,243 +0,0 @@ -package main - -import ( - "crypto/ecdsa" - "crypto/elliptic" - "crypto/rand" - "crypto/sha256" - "crypto/x509" - "crypto/x509/pkix" - "encoding/pem" - "fmt" - "log" - "math/big" - "net/url" - "os" - "regexp" - "strings" - "time" -) - -// You can verify a given leaf with a given root using: -// -// $ openssl verify -verbose -CAfile ca2-ca-vault.cert.pem ca1-svc-db.cert.pem -// -// Note that to verify via the cross-signed intermediate, openssl requires it to -// be bundled with the _root_ CA bundle and will ignore the cert if it's passed -// with the subject. You can do that with: -// -// $ openssl verify -verbose -CAfile \ -// <(cat ca1-ca-consul-internal.cert.pem ca2-xc-by-ca1.cert.pem) \ -// ca2-svc-db.cert.pem -// ca2-svc-db.cert.pem: OK -// -// Note that the same leaf and root without the intermediate should fail: -// -// $ openssl verify -verbose -CAfile ca1-ca-consul-internal.cert.pem ca2-svc-db.cert.pem -// ca2-svc-db.cert.pem: CN = db -// error 20 at 0 depth lookup:unable to get local issuer certificate -// -// NOTE: THIS IS A QUIRK OF OPENSSL; in Connect we will distribute the roots -// alone and stable intermediates like the XC cert to the _leaf_. - -var clusterID = "11111111-2222-3333-4444-555555555555" -var cAs = []string{"Consul Internal", "Vault"} -var services = []string{"web", "db", "cache"} -var slugRe = regexp.MustCompile("[^a-zA-Z0-9]+") -var serial int64 - -type caInfo struct { - id int - name string - slug string - uri *url.URL - pk *ecdsa.PrivateKey - cert *x509.Certificate -} - -func main() { - // Make CA certs - caInfos := make(map[string]caInfo) - var previousCA *caInfo - for idx, name := range cAs { - ca := caInfo{ - id: idx + 1, - name: name, - slug: strings.ToLower(slugRe.ReplaceAllString(name, "-")), - } - pk, err := makePK(fmt.Sprintf("ca%d-ca-%s.key.pem", ca.id, ca.slug)) - if err != nil { - log.Fatal(err) - } - ca.pk = pk - caURI, err := url.Parse(fmt.Sprintf("spiffe://%s.consul", clusterID)) - if err != nil { - log.Fatal(err) - } - ca.uri = caURI - cert, err := makeCACert(ca, previousCA) - if err != nil { - log.Fatal(err) - } - ca.cert = cert - caInfos[name] = ca - previousCA = &ca - } - - // For each CA, make a leaf cert for each service - for _, ca := range caInfos { - for _, svc := range services { - _, err := makeLeafCert(ca, svc) - if err != nil { - log.Fatal(err) - } - } - } -} - -func makePK(path string) (*ecdsa.PrivateKey, error) { - log.Printf("Writing PK file: %s", path) - priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) - if err != nil { - return nil, err - } - - bs, err := x509.MarshalECPrivateKey(priv) - if err != nil { - return nil, err - } - - err = writePEM(path, "EC PRIVATE KEY", bs) - return priv, nil -} - -func makeCACert(ca caInfo, previousCA *caInfo) (*x509.Certificate, error) { - path := fmt.Sprintf("ca%d-ca-%s.cert.pem", ca.id, ca.slug) - log.Printf("Writing CA cert file: %s", path) - serial++ - subj := pkix.Name{ - CommonName: ca.name, - } - template := x509.Certificate{ - SerialNumber: big.NewInt(serial), - Subject: subj, - // New in go 1.10 - URIs: []*url.URL{ca.uri}, - // Add DNS name constraint - PermittedDNSDomainsCritical: true, - PermittedDNSDomains: []string{ca.uri.Hostname()}, - SignatureAlgorithm: x509.ECDSAWithSHA256, - BasicConstraintsValid: true, - KeyUsage: x509.KeyUsageCertSign | x509.KeyUsageCRLSign | x509.KeyUsageDigitalSignature, - IsCA: true, - NotAfter: time.Now().Add(10 * 365 * 24 * time.Hour), - NotBefore: time.Now(), - AuthorityKeyId: keyID(&ca.pk.PublicKey), - SubjectKeyId: keyID(&ca.pk.PublicKey), - } - bs, err := x509.CreateCertificate(rand.Reader, &template, &template, - &ca.pk.PublicKey, ca.pk) - if err != nil { - return nil, err - } - - err = writePEM(path, "CERTIFICATE", bs) - if err != nil { - return nil, err - } - - cert, err := x509.ParseCertificate(bs) - if err != nil { - return nil, err - } - - if previousCA != nil { - // Also create cross-signed cert as we would use during rotation between - // previous CA and this one. - template.AuthorityKeyId = keyID(&previousCA.pk.PublicKey) - bs, err := x509.CreateCertificate(rand.Reader, &template, - previousCA.cert, &ca.pk.PublicKey, previousCA.pk) - if err != nil { - return nil, err - } - - path := fmt.Sprintf("ca%d-xc-by-ca%d.cert.pem", ca.id, previousCA.id) - err = writePEM(path, "CERTIFICATE", bs) - if err != nil { - return nil, err - } - } - - return cert, err -} - -func keyID(pub *ecdsa.PublicKey) []byte { - // This is not standard; RFC allows any unique identifier as long as they - // match in subject/authority chains but suggests specific hashing of DER - // bytes of public key including DER tags. I can't be bothered to do esp. - // since ECDSA keys don't have a handy way to marshal the publick key alone. - h := sha256.New() - h.Write(pub.X.Bytes()) - h.Write(pub.Y.Bytes()) - return h.Sum([]byte{}) -} - -func makeLeafCert(ca caInfo, svc string) (*x509.Certificate, error) { - svcURI := ca.uri - svcURI.Path = "/ns/default/dc/dc01/svc/" + svc - - keyPath := fmt.Sprintf("ca%d-svc-%s.key.pem", ca.id, svc) - cPath := fmt.Sprintf("ca%d-svc-%s.cert.pem", ca.id, svc) - - pk, err := makePK(keyPath) - if err != nil { - return nil, err - } - - log.Printf("Writing Service Cert: %s", cPath) - - serial++ - subj := pkix.Name{ - CommonName: svc, - } - template := x509.Certificate{ - SerialNumber: big.NewInt(serial), - Subject: subj, - // New in go 1.10 - URIs: []*url.URL{svcURI}, - SignatureAlgorithm: x509.ECDSAWithSHA256, - BasicConstraintsValid: true, - KeyUsage: x509.KeyUsageDataEncipherment | - x509.KeyUsageKeyAgreement | x509.KeyUsageDigitalSignature | - x509.KeyUsageKeyEncipherment, - ExtKeyUsage: []x509.ExtKeyUsage{ - x509.ExtKeyUsageClientAuth, - x509.ExtKeyUsageServerAuth, - }, - NotAfter: time.Now().Add(10 * 365 * 24 * time.Hour), - NotBefore: time.Now(), - AuthorityKeyId: keyID(&ca.pk.PublicKey), - SubjectKeyId: keyID(&pk.PublicKey), - } - bs, err := x509.CreateCertificate(rand.Reader, &template, ca.cert, - &pk.PublicKey, ca.pk) - if err != nil { - return nil, err - } - - err = writePEM(cPath, "CERTIFICATE", bs) - if err != nil { - return nil, err - } - - return x509.ParseCertificate(bs) -} - -func writePEM(name, typ string, bs []byte) error { - f, err := os.OpenFile(name, os.O_WRONLY|os.O_CREATE, 0600) - if err != nil { - return err - } - defer f.Close() - return pem.Encode(f, &pem.Block{Type: typ, Bytes: bs}) -} diff --git a/connect/testing.go b/connect/testing.go index 90db332a2a..7e1b9cdace 100644 --- a/connect/testing.go +++ b/connect/testing.go @@ -4,30 +4,155 @@ import ( "crypto/tls" "crypto/x509" "fmt" - "io/ioutil" - "path" - "path/filepath" - "runtime" + "io" + "net" + "sync/atomic" - "github.com/mitchellh/go-testing-interface" + "github.com/hashicorp/consul/agent/connect" + "github.com/hashicorp/consul/agent/structs" + "github.com/hashicorp/consul/lib/freeport" + testing "github.com/mitchellh/go-testing-interface" "github.com/stretchr/testify/require" ) -// testDataDir is a janky temporary hack to allow use of these methods from -// proxy package. We need to revisit where all this lives since it logically -// overlaps with consul/agent in Mitchell's PR and that one generates certs on -// the fly which will make this unecessary but I want to get things working for -// now with what I've got :). This wonderful heap kinda-sorta gets the path -// relative to _this_ file so it works even if the Test* method is being called -// from a test binary in another package dir. -func testDataDir() string { - _, filename, _, ok := runtime.Caller(0) - if !ok { - panic("no caller information") - } - return path.Dir(filename) + "/testdata" +// testVerifier creates a helper verifyFunc that can be set in a tls.Config and +// records calls made, passing back the certificates presented via the returned +// channel. The channel is buffered so up to 128 verification calls can be made +// without reading the chan before verification blocks. +func testVerifier(t testing.T, returnErr error) (verifyFunc, chan [][]byte) { + ch := make(chan [][]byte, 128) + return func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error { + ch <- rawCerts + return returnErr + }, ch } +// TestTLSConfig returns a *tls.Config suitable for use during tests. +func TestTLSConfig(t testing.T, service string, ca *structs.CARoot) *tls.Config { + t.Helper() + + // Insecure default (nil verifier) + cfg := defaultTLSConfig(nil) + cfg.Certificates = []tls.Certificate{TestSvcKeyPair(t, service, ca)} + cfg.RootCAs = TestCAPool(t, ca) + cfg.ClientCAs = TestCAPool(t, ca) + return cfg +} + +// TestCAPool returns an *x509.CertPool containing the passed CA's root(s) +func TestCAPool(t testing.T, cas ...*structs.CARoot) *x509.CertPool { + t.Helper() + pool := x509.NewCertPool() + for _, ca := range cas { + pool.AppendCertsFromPEM([]byte(ca.RootCert)) + } + return pool +} + +// TestSvcKeyPair returns an tls.Certificate containing both cert and private +// key for a given service under a given CA from the testdata dir. +func TestSvcKeyPair(t testing.T, service string, ca *structs.CARoot) tls.Certificate { + t.Helper() + certPEM, keyPEM := connect.TestLeaf(t, service, ca) + cert, err := tls.X509KeyPair([]byte(certPEM), []byte(keyPEM)) + require.Nil(t, err) + return cert +} + +// TestPeerCertificates returns a []*x509.Certificate as you'd get from +// tls.Conn.ConnectionState().PeerCertificates including the named certificate. +func TestPeerCertificates(t testing.T, service string, ca *structs.CARoot) []*x509.Certificate { + t.Helper() + certPEM, _ := connect.TestLeaf(t, service, ca) + cert, err := connect.ParseCert(certPEM) + require.Nil(t, err) + return []*x509.Certificate{cert} +} + +// TestService runs a service listener that can be used to test clients. It's +// behaviour can be controlled by the struct members. +type TestService struct { + // The service name to serve. + Service string + // The (test) CA to use for generating certs. + CA *structs.CARoot + // TimeoutHandshake controls whether the listening server will complete a TLS + // handshake quickly enough. + TimeoutHandshake bool + // TLSCfg is the tls.Config that will be used. By default it's set up from the + // service and ca set. + TLSCfg *tls.Config + // Addr is the listen address. It is set to a random free port on `localhost` + // by default. + Addr string + + l net.Listener + stopFlag int32 + stopChan chan struct{} +} + +// NewTestService returns a TestService. It should be closed when test is +// complete. +func NewTestService(t testing.T, service string, ca *structs.CARoot) *TestService { + ports := freeport.GetT(t, 1) + return &TestService{ + Service: service, + CA: ca, + stopChan: make(chan struct{}), + TLSCfg: TestTLSConfig(t, service, ca), + Addr: fmt.Sprintf("localhost:%d", ports[0]), + } +} + +// Serve runs a TestService and blocks until it is closed or errors. +func (s *TestService) Serve() error { + // Just accept TCP conn but so we can control timing of accept/handshake + l, err := net.Listen("tcp", s.Addr) + if err != nil { + return err + } + s.l = l + + for { + conn, err := s.l.Accept() + if err != nil { + if atomic.LoadInt32(&s.stopFlag) == 1 { + return nil + } + return err + } + + // Ignore the conn if we are not actively ha + if !s.TimeoutHandshake { + // Upgrade conn to TLS + conn = tls.Server(conn, s.TLSCfg) + + // Run an echo service + go io.Copy(conn, conn) + } + + // Close this conn when we stop + go func(c net.Conn) { + <-s.stopChan + c.Close() + }(conn) + } + + return nil +} + +// Close stops a TestService +func (s *TestService) Close() { + old := atomic.SwapInt32(&s.stopFlag, 1) + if old == 0 { + if s.l != nil { + s.l.Close() + } + close(s.stopChan) + } +} + +/* // TestCAPool returns an *x509.CertPool containing the named CA certs from the // testdata dir. func TestCAPool(t testing.T, caNames ...string) *x509.CertPool { @@ -86,3 +211,4 @@ func (a *TestAuther) Auth(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error { return a.Return } +*/ diff --git a/connect/tls.go b/connect/tls.go index af66d9c0c3..8d3bc3a94c 100644 --- a/connect/tls.go +++ b/connect/tls.go @@ -3,13 +3,18 @@ package connect import ( "crypto/tls" "crypto/x509" + "errors" "io/ioutil" "sync" + + "github.com/hashicorp/consul/agent/connect" ) -// defaultTLSConfig returns the standard config for connect clients and servers. -func defaultTLSConfig() *tls.Config { - serverAuther := &ServerAuther{} +// verifyFunc is the type of tls.Config.VerifyPeerCertificate for convenience. +type verifyFunc func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error + +// defaultTLSConfig returns the standard config. +func defaultTLSConfig(verify verifyFunc) *tls.Config { return &tls.Config{ MinVersion: tls.VersionTLS12, ClientAuth: tls.RequireAndVerifyClientCert, @@ -29,16 +34,18 @@ func defaultTLSConfig() *tls.Config { // We have to set this since otherwise Go will attempt to verify DNS names // match DNS SAN/CN which we don't want. We hook up VerifyPeerCertificate to // do our own path validation as well as Connect AuthZ. - InsecureSkipVerify: true, - // By default auth as if we are a server. Clients need to override this with - // an Auther that is performs correct validation of the server identity they - // intended to connect to. - VerifyPeerCertificate: serverAuther.Auth, + InsecureSkipVerify: true, + VerifyPeerCertificate: verify, + // Include h2 to allow connect http servers to automatically support http2. + // See: https://github.com/golang/go/blob/917c33fe8672116b04848cf11545296789cafd3b/src/net/http/server.go#L2724-L2731 + NextProtos: []string{"h2"}, } } // ReloadableTLSConfig exposes a tls.Config that can have it's certificates -// reloaded. This works by +// reloaded. On a server, this uses GetConfigForClient to pass the current +// tls.Config or client certificate for each acceptted connection. On a client, +// this uses GetClientCertificate to provide the current client certificate. type ReloadableTLSConfig struct { mu sync.Mutex @@ -46,52 +53,40 @@ type ReloadableTLSConfig struct { cfg *tls.Config } -// NewReloadableTLSConfig returns a reloadable config currently set to base. The -// Auther used to verify certificates for incoming connections on a Server will -// just be copied from the VerifyPeerCertificate passed. Clients will need to -// pass a specific Auther instance when they call TLSConfig that is configured -// to perform the necessary validation of the server's identity. +// NewReloadableTLSConfig returns a reloadable config currently set to base. func NewReloadableTLSConfig(base *tls.Config) *ReloadableTLSConfig { - return &ReloadableTLSConfig{cfg: base} + c := &ReloadableTLSConfig{} + c.SetTLSConfig(base) + return c } -// ServerTLSConfig returns a *tls.Config that will dynamically load certs for -// each inbound connection via the GetConfigForClient callback. -func (c *ReloadableTLSConfig) ServerTLSConfig() *tls.Config { - // Setup the basic one with current params even though we will be using - // different config for each new conn. - c.mu.Lock() - base := c.cfg - c.mu.Unlock() - - // Dynamically fetch the current config for each new inbound connection - base.GetConfigForClient = func(info *tls.ClientHelloInfo) (*tls.Config, error) { - return c.TLSConfig(nil), nil - } - - return base -} - -// TLSConfig returns the current value for the config. It is safe to call from -// any goroutine. The passed Auther is inserted into the config's -// VerifyPeerCertificate. Passing a nil Auther will leave the default one in the -// base config -func (c *ReloadableTLSConfig) TLSConfig(auther Auther) *tls.Config { +// TLSConfig returns a *tls.Config that will dynamically load certs. It's +// suitable for use in either a client or server. +func (c *ReloadableTLSConfig) TLSConfig() *tls.Config { c.mu.Lock() cfgCopy := c.cfg c.mu.Unlock() - if auther != nil { - cfgCopy.VerifyPeerCertificate = auther.Auth - } return cfgCopy } // SetTLSConfig sets the config used for future connections. It is safe to call // from any goroutine. func (c *ReloadableTLSConfig) SetTLSConfig(cfg *tls.Config) error { + copy := cfg.Clone() + copy.GetClientCertificate = func(*tls.CertificateRequestInfo) (*tls.Certificate, error) { + current := c.TLSConfig() + if len(current.Certificates) < 1 { + return nil, errors.New("tls: no certificates configured") + } + return ¤t.Certificates[0], nil + } + copy.GetConfigForClient = func(*tls.ClientHelloInfo) (*tls.Config, error) { + return c.TLSConfig(), nil + } + c.mu.Lock() defer c.mu.Unlock() - c.cfg = cfg + c.cfg = copy return nil } @@ -114,7 +109,8 @@ func devTLSConfigFromFiles(caFile, certFile, return nil, err } - cfg := defaultTLSConfig() + // Insecure no verification + cfg := defaultTLSConfig(nil) cfg.Certificates = []tls.Certificate{cert} cfg.RootCAs = roots @@ -122,3 +118,43 @@ func devTLSConfigFromFiles(caFile, certFile, return cfg, nil } + +// verifyServerCertMatchesURI is used on tls connections dialled to a connect +// server to ensure that the certificate it presented has the correct identity. +func verifyServerCertMatchesURI(certs []*x509.Certificate, + expected connect.CertURI) error { + expectedStr := expected.URI().String() + + if len(certs) < 1 { + return errors.New("peer certificate mismatch") + } + + // Only check the first cert assuming this is the only leaf. It's not clear if + // services might ever legitimately present multiple leaf certificates or if + // the slice is just to allow presenting the whole chain of intermediates. + cert := certs[0] + + // Our certs will only ever have a single URI for now so only check that + if len(cert.URIs) < 1 { + return errors.New("peer certificate mismatch") + } + // We may want to do better than string matching later in some special + // cases and/or encapsulate the "match" logic inside the CertURI + // implementation but for now this is all we need. + if cert.URIs[0].String() == expectedStr { + return nil + } + return errors.New("peer certificate mismatch") +} + +// serverVerifyCerts is the verifyFunc for use on Connect servers. +func serverVerifyCerts(rawCerts [][]byte, chains [][]*x509.Certificate) error { + // TODO(banks): implement me + return nil +} + +// clientVerifyCerts is the verifyFunc for use on Connect clients. +func clientVerifyCerts(rawCerts [][]byte, chains [][]*x509.Certificate) error { + // TODO(banks): implement me + return nil +} diff --git a/connect/tls_test.go b/connect/tls_test.go index 0c99df3adc..3605f22dbb 100644 --- a/connect/tls_test.go +++ b/connect/tls_test.go @@ -1,45 +1,103 @@ package connect import ( - "crypto/tls" + "crypto/x509" "testing" + "github.com/hashicorp/consul/agent/connect" "github.com/stretchr/testify/require" ) func TestReloadableTLSConfig(t *testing.T) { - base := TestTLSConfig(t, "ca1", "web") + require := require.New(t) + verify, _ := testVerifier(t, nil) + base := defaultTLSConfig(verify) c := NewReloadableTLSConfig(base) - a := &TestAuther{ - Return: nil, - } + // The dynamic config should be the one we loaded (with some different hooks) + got := c.TLSConfig() + expect := *base + // Equal and even cmp.Diff fail on tls.Config due to unexported fields in + // each. Compare a few things to prove it's returning the bits we + // specifically set. + require.Equal(expect.Certificates, got.Certificates) + require.Equal(expect.RootCAs, got.RootCAs) + require.Equal(expect.ClientCAs, got.ClientCAs) + require.Equal(expect.InsecureSkipVerify, got.InsecureSkipVerify) + require.Equal(expect.MinVersion, got.MinVersion) + require.Equal(expect.CipherSuites, got.CipherSuites) + require.NotNil(got.GetClientCertificate) + require.NotNil(got.GetConfigForClient) + require.Contains(got.NextProtos, "h2") - // The dynamic config should be the one we loaded, but with the passed auther - expect := base - expect.VerifyPeerCertificate = a.Auth - require.Equal(t, base, c.TLSConfig(a)) + ca := connect.TestCA(t, nil) - // The server config should return same too for new connections - serverCfg := c.ServerTLSConfig() - require.NotNil(t, serverCfg.GetConfigForClient) - got, err := serverCfg.GetConfigForClient(&tls.ClientHelloInfo{}) - require.Nil(t, err) - require.Equal(t, base, got) + // Now change the config as if we just loaded certs from Consul + new := TestTLSConfig(t, "web", ca) + err := c.SetTLSConfig(new) + require.Nil(err) - // Now change the config as if we just rotated to a new CA - new := TestTLSConfig(t, "ca2", "web") - err = c.SetTLSConfig(new) - require.Nil(t, err) + // Change the passed config to ensure SetTLSConfig made a copy otherwise this + // is racey. + expect = *new + new.Certificates = nil - // The dynamic config should be the one we loaded (with same auther due to nil) - require.Equal(t, new, c.TLSConfig(nil)) - - // The server config should return same too for new connections - serverCfg = c.ServerTLSConfig() - require.NotNil(t, serverCfg.GetConfigForClient) - got, err = serverCfg.GetConfigForClient(&tls.ClientHelloInfo{}) - require.Nil(t, err) - require.Equal(t, new, got) + // The dynamic config should be the one we loaded (with some different hooks) + got = c.TLSConfig() + require.Equal(expect.Certificates, got.Certificates) + require.Equal(expect.RootCAs, got.RootCAs) + require.Equal(expect.ClientCAs, got.ClientCAs) + require.Equal(expect.InsecureSkipVerify, got.InsecureSkipVerify) + require.Equal(expect.MinVersion, got.MinVersion) + require.Equal(expect.CipherSuites, got.CipherSuites) + require.NotNil(got.GetClientCertificate) + require.NotNil(got.GetConfigForClient) + require.Contains(got.NextProtos, "h2") +} + +func Test_verifyServerCertMatchesURI(t *testing.T) { + ca1 := connect.TestCA(t, nil) + + tests := []struct { + name string + certs []*x509.Certificate + expected connect.CertURI + wantErr bool + }{ + { + name: "simple match", + certs: TestPeerCertificates(t, "web", ca1), + expected: connect.TestSpiffeIDService(t, "web"), + wantErr: false, + }, + { + name: "mismatch", + certs: TestPeerCertificates(t, "web", ca1), + expected: connect.TestSpiffeIDService(t, "db"), + wantErr: true, + }, + { + name: "no certs", + certs: []*x509.Certificate{}, + expected: connect.TestSpiffeIDService(t, "db"), + wantErr: true, + }, + { + name: "nil certs", + certs: nil, + expected: connect.TestSpiffeIDService(t, "db"), + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := verifyServerCertMatchesURI(tt.certs, tt.expected) + if tt.wantErr { + require.NotNil(t, err) + } else { + require.Nil(t, err) + } + }) + } }