From 8ed46d7701bc455831cf15901e2d9da2e0c329c4 Mon Sep 17 00:00:00 2001 From: Paul Banks Date: Mon, 11 Jun 2018 17:53:14 +0100 Subject: [PATCH] Add accessor and helpers to SDK for fetching self-name and client service ID --- connect/service.go | 13 ++++++++--- connect/service_test.go | 8 +++++++ connect/tls.go | 48 ++++++++++++++++++++++++++++++++++------- 3 files changed, 58 insertions(+), 11 deletions(-) diff --git a/connect/service.go b/connect/service.go index 5c6dd2ced..462f1fb47 100644 --- a/connect/service.go +++ b/connect/service.go @@ -59,9 +59,9 @@ type Service struct { // // Caller must provide client which is already configured to speak to the local // Consul agent, and with an ACL token that has `service:write` privileges for -// the serviceID specified. -func NewService(serviceID string, client *api.Client) (*Service, error) { - return NewServiceWithLogger(serviceID, client, +// the service specified. +func NewService(serviceName string, client *api.Client) (*Service, error) { + return NewServiceWithLogger(serviceName, client, log.New(os.Stderr, "", log.LstdFlags)) } @@ -125,6 +125,13 @@ func NewDevServiceWithTLSConfig(serviceName string, logger *log.Logger, return s, nil } +// Name returns the name of the service this object represents. Note it is the +// service _name_ as used during discovery, not the ID used to uniquely identify +// an instance of the service with an agent. +func (s *Service) Name() string { + return s.service +} + // 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 diff --git a/connect/service_test.go b/connect/service_test.go index c284b9c6e..b18bf4bb4 100644 --- a/connect/service_test.go +++ b/connect/service_test.go @@ -13,6 +13,8 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" + "github.com/hashicorp/consul/agent" "github.com/hashicorp/consul/agent/connect" "github.com/hashicorp/consul/api" @@ -23,6 +25,12 @@ import ( // Assert io.Closer implementation var _ io.Closer = new(Service) +func TestService_Name(t *testing.T) { + ca := connect.TestCA(t, nil) + s := TestService(t, "web", ca) + assert.Equal(t, "web", s.Name()) +} + func TestService_Dial(t *testing.T) { ca := connect.TestCA(t, nil) diff --git a/connect/tls.go b/connect/tls.go index 4ca448371..7d679e383 100644 --- a/connect/tls.go +++ b/connect/tls.go @@ -7,6 +7,8 @@ import ( "fmt" "io/ioutil" "log" + "net" + "net/url" "sync" "github.com/hashicorp/consul/agent/connect" @@ -81,14 +83,29 @@ 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() +// CertURIFromConn is a helper to extract the service identifier URI from a +// net.Conn. If the net.Conn is not a *tls.Conn then an error is always +// returned. If the *tls.Conn didn't present a valid connect certificate, or is +// not yet past the handshake, an error is returned. +func CertURIFromConn(conn net.Conn) (connect.CertURI, error) { + tc, ok := conn.(*tls.Conn) + if !ok { + return nil, fmt.Errorf("invalid non-TLS connect client") + } + gotURI, err := extractCertURI(tc.ConnectionState().PeerCertificates) + if err != nil { + return nil, err + } + return connect.ParseCertURI(gotURI) +} +// extractCertURI returns the first URI SAN from the leaf certificate presented +// in the slice. The slice is expected to be the passed from +// tls.Conn.ConnectionState().PeerCertificates and requires that the leaf has at +// least one URI and the first URI is the correct one to use. +func extractCertURI(certs []*x509.Certificate) (*url.URL, error) { if len(certs) < 1 { - return errors.New("peer certificate mismatch") + return nil, errors.New("no peer certificate presented") } // Only check the first cert assuming this is the only leaf. It's not clear if @@ -98,16 +115,31 @@ func verifyServerCertMatchesURI(certs []*x509.Certificate, // Our certs will only ever have a single URI for now so only check that if len(cert.URIs) < 1 { + return nil, errors.New("peer certificate invalid") + } + + return cert.URIs[0], 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() + + gotURI, err := extractCertURI(certs) + if err != nil { 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 { + if gotURI.String() == expectedStr { return nil } return fmt.Errorf("peer certificate mismatch got %s, want %s", - cert.URIs[0].String(), expectedStr) + gotURI.String(), expectedStr) } // newServerSideVerifier returns a verifierFunc that wraps the provided