diff --git a/command/agent/agent.go b/command/agent/agent.go index ac83692ab..84f506b5d 100644 --- a/command/agent/agent.go +++ b/command/agent/agent.go @@ -288,10 +288,12 @@ func (a *Agent) consulConfig() *consul.Config { // Copy the TLS configuration base.VerifyIncoming = a.config.VerifyIncoming base.VerifyOutgoing = a.config.VerifyOutgoing + base.VerifyServerHostname = a.config.VerifyServerHostname base.CAFile = a.config.CAFile base.CertFile = a.config.CertFile base.KeyFile = a.config.KeyFile base.ServerName = a.config.ServerName + base.Domain = a.config.Domain // Setup the ServerUp callback base.ServerUp = a.state.ConsulServerUp diff --git a/command/agent/config.go b/command/agent/config.go index 98b5f3544..ce6511107 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -188,6 +188,14 @@ type Config struct { // certificate authority. This is used to verify authenticity of server nodes. VerifyOutgoing bool `mapstructure:"verify_outgoing"` + // VerifyServerHostname is used to enable hostname verification of servers. This + // ensures that the certificate presented is valid for server... + // This prevents a compromised client from being restarted as a server, and then + // intercepting request traffic as well as being added as a raft peer. This should be + // enabled by default with VerifyOutgoing, but for legacy reasons we cannot break + // existing clients. + VerifyServerHostname bool `mapstructure:"verify_server_hostname"` + // CAFile is a path to a certificate authority file. This is used with VerifyIncoming // or VerifyOutgoing to verify the TLS connection. CAFile string `mapstructure:"ca_file"` @@ -838,6 +846,9 @@ func MergeConfig(a, b *Config) *Config { if b.VerifyOutgoing { result.VerifyOutgoing = true } + if b.VerifyServerHostname { + result.VerifyServerHostname = true + } if b.CAFile != "" { result.CAFile = b.CAFile } diff --git a/command/agent/config_test.go b/command/agent/config_test.go index c8ec77796..ac22af300 100644 --- a/command/agent/config_test.go +++ b/command/agent/config_test.go @@ -245,7 +245,7 @@ func TestDecodeConfig(t *testing.T) { } // TLS - input = `{"verify_incoming": true, "verify_outgoing": true}` + input = `{"verify_incoming": true, "verify_outgoing": true, "verify_server_hostname": true}` config, err = DecodeConfig(bytes.NewReader([]byte(input))) if err != nil { t.Fatalf("err: %s", err) @@ -259,6 +259,10 @@ func TestDecodeConfig(t *testing.T) { t.Fatalf("bad: %#v", config) } + if config.VerifyServerHostname != true { + t.Fatalf("bad: %#v", config) + } + // TLS keys input = `{"ca_file": "my/ca/file", "cert_file": "my.cert", "key_file": "key.pem", "server_name": "example.com"}` config, err = DecodeConfig(bytes.NewReader([]byte(input))) diff --git a/consul/client.go b/consul/client.go index c37ace6d7..1a61841a7 100644 --- a/consul/client.go +++ b/consul/client.go @@ -1,7 +1,6 @@ package consul import ( - "crypto/tls" "fmt" "log" "math/rand" @@ -91,10 +90,9 @@ func NewClient(config *Config) (*Client, error) { config.LogOutput = os.Stderr } - // Create the tlsConfig - var tlsConfig *tls.Config - var err error - if tlsConfig, err = config.tlsConfig().OutgoingTLSConfig(); err != nil { + // Create the tls Wrapper + tlsWrap, err := config.tlsConfig().OutgoingTLSWrapper() + if err != nil { return nil, err } @@ -104,7 +102,7 @@ func NewClient(config *Config) (*Client, error) { // Create server c := &Client{ config: config, - connPool: NewPool(config.LogOutput, clientRPCCache, clientMaxStreams, tlsConfig), + connPool: NewPool(config.LogOutput, clientRPCCache, clientMaxStreams, tlsWrap), eventCh: make(chan serf.Event, 256), logger: logger, shutdownCh: make(chan struct{}), @@ -357,7 +355,7 @@ func (c *Client) RPC(method string, args interface{}, reply interface{}) error { // Forward to remote Consul TRY_RPC: - if err := c.connPool.RPC(server.Addr, server.Version, method, args, reply); err != nil { + if err := c.connPool.RPC(c.config.Datacenter, server.Addr, server.Version, method, args, reply); err != nil { c.lastServer = nil c.lastRPCTime = time.Time{} return err diff --git a/consul/config.go b/consul/config.go index 0b45c97c6..541c2bce0 100644 --- a/consul/config.go +++ b/consul/config.go @@ -56,6 +56,9 @@ type Config struct { // Node name is the name we use to advertise. Defaults to hostname. NodeName string + // Domain is the DNS domain for the records. Defaults to "consul." + Domain string + // RaftConfig is the configuration used for Raft in the local DC RaftConfig *raft.Config @@ -100,6 +103,14 @@ type Config struct { // server nodes. VerifyOutgoing bool + // VerifyServerHostname is used to enable hostname verification of servers. This + // ensures that the certificate presented is valid for server... + // This prevents a compromised client from being restarted as a server, and then + // intercepting request traffic as well as being added as a raft peer. This should be + // enabled by default with VerifyOutgoing, but for legacy reasons we cannot break + // existing clients. + VerifyServerHostname bool + // CAFile is a path to a certificate authority file. This is used with VerifyIncoming // or VerifyOutgoing to verify the TLS connection. CAFile string @@ -267,13 +278,15 @@ func DefaultConfig() *Config { func (c *Config) tlsConfig() *tlsutil.Config { tlsConf := &tlsutil.Config{ - VerifyIncoming: c.VerifyIncoming, - VerifyOutgoing: c.VerifyOutgoing, - CAFile: c.CAFile, - CertFile: c.CertFile, - KeyFile: c.KeyFile, - NodeName: c.NodeName, - ServerName: c.ServerName} - + VerifyIncoming: c.VerifyIncoming, + VerifyOutgoing: c.VerifyOutgoing, + VerifyServerHostname: c.VerifyServerHostname, + CAFile: c.CAFile, + CertFile: c.CertFile, + KeyFile: c.KeyFile, + NodeName: c.NodeName, + ServerName: c.ServerName, + Domain: c.Domain, + } return tlsConf } diff --git a/consul/pool.go b/consul/pool.go index 89d0654f8..3512fa621 100644 --- a/consul/pool.go +++ b/consul/pool.go @@ -2,7 +2,6 @@ package consul import ( "container/list" - "crypto/tls" "fmt" "io" "net" @@ -135,8 +134,8 @@ type ConnPool struct { // Pool maps an address to a open connection pool map[string]*Conn - // TLS settings - tlsConfig *tls.Config + // TLS wrapper + tlsWrap tlsutil.DCWrapper // Used to indicate the pool is shutdown shutdown bool @@ -148,13 +147,13 @@ type ConnPool struct { // Set maxTime to 0 to disable reaping. maxStreams is used to control // the number of idle streams allowed. // If TLS settings are provided outgoing connections use TLS. -func NewPool(logOutput io.Writer, maxTime time.Duration, maxStreams int, tlsConfig *tls.Config) *ConnPool { +func NewPool(logOutput io.Writer, maxTime time.Duration, maxStreams int, tlsWrap tlsutil.DCWrapper) *ConnPool { pool := &ConnPool{ logOutput: logOutput, maxTime: maxTime, maxStreams: maxStreams, pool: make(map[string]*Conn), - tlsConfig: tlsConfig, + tlsWrap: tlsWrap, shutdownCh: make(chan struct{}), } if maxTime > 0 { @@ -183,14 +182,14 @@ func (p *ConnPool) Shutdown() error { // Acquire is used to get a connection that is // pooled or to return a new connection -func (p *ConnPool) acquire(addr net.Addr, version int) (*Conn, error) { +func (p *ConnPool) acquire(dc string, addr net.Addr, version int) (*Conn, error) { // Check for a pooled ocnn if conn := p.getPooled(addr, version); conn != nil { return conn, nil } // Create a new connection - return p.getNewConn(addr, version) + return p.getNewConn(dc, addr, version) } // getPooled is used to return a pooled connection @@ -206,7 +205,7 @@ func (p *ConnPool) getPooled(addr net.Addr, version int) *Conn { } // getNewConn is used to return a new connection -func (p *ConnPool) getNewConn(addr net.Addr, version int) (*Conn, error) { +func (p *ConnPool) getNewConn(dc string, addr net.Addr, version int) (*Conn, error) { // Try to dial the conn conn, err := net.DialTimeout("tcp", addr.String(), 10*time.Second) if err != nil { @@ -220,7 +219,7 @@ func (p *ConnPool) getNewConn(addr net.Addr, version int) (*Conn, error) { } // Check if TLS is enabled - if p.tlsConfig != nil { + if p.tlsWrap != nil { // Switch the connection into TLS mode if _, err := conn.Write([]byte{byte(rpcTLS)}); err != nil { conn.Close() @@ -228,7 +227,7 @@ func (p *ConnPool) getNewConn(addr net.Addr, version int) (*Conn, error) { } // Wrap the connection in a TLS client - tlsConn, err := tlsutil.WrapTLSClient(conn, p.tlsConfig) + tlsConn, err := p.tlsWrap(dc, conn) if err != nil { conn.Close() return nil, err @@ -314,11 +313,11 @@ func (p *ConnPool) releaseConn(conn *Conn) { } // getClient is used to get a usable client for an address and protocol version -func (p *ConnPool) getClient(addr net.Addr, version int) (*Conn, *StreamClient, error) { +func (p *ConnPool) getClient(dc string, addr net.Addr, version int) (*Conn, *StreamClient, error) { retries := 0 START: // Try to get a conn first - conn, err := p.acquire(addr, version) + conn, err := p.acquire(dc, addr, version) if err != nil { return nil, nil, fmt.Errorf("failed to get conn: %v", err) } @@ -340,9 +339,9 @@ START: } // RPC is used to make an RPC call to a remote host -func (p *ConnPool) RPC(addr net.Addr, version int, method string, args interface{}, reply interface{}) error { +func (p *ConnPool) RPC(dc string, addr net.Addr, version int, method string, args interface{}, reply interface{}) error { // Get a usable client - conn, sc, err := p.getClient(addr, version) + conn, sc, err := p.getClient(dc, addr, version) if err != nil { return fmt.Errorf("rpc error: %v", err) } diff --git a/consul/raft_rpc.go b/consul/raft_rpc.go index e0ee4c68e..545895e19 100644 --- a/consul/raft_rpc.go +++ b/consul/raft_rpc.go @@ -1,12 +1,12 @@ package consul import ( - "crypto/tls" "fmt" - "github.com/hashicorp/consul/tlsutil" "net" "sync" "time" + + "github.com/hashicorp/consul/tlsutil" ) // RaftLayer implements the raft.StreamLayer interface, @@ -18,8 +18,8 @@ type RaftLayer struct { // connCh is used to accept connections connCh chan net.Conn - // TLS configuration - tlsConfig *tls.Config + // TLS wrapper + tlsWrap tlsutil.Wrapper // Tracks if we are closed closed bool @@ -30,12 +30,12 @@ type RaftLayer struct { // NewRaftLayer is used to initialize a new RaftLayer which can // be used as a StreamLayer for Raft. If a tlsConfig is provided, // then the connection will use TLS. -func NewRaftLayer(addr net.Addr, tlsConfig *tls.Config) *RaftLayer { +func NewRaftLayer(addr net.Addr, tlsWrap tlsutil.Wrapper) *RaftLayer { layer := &RaftLayer{ - addr: addr, - connCh: make(chan net.Conn), - tlsConfig: tlsConfig, - closeCh: make(chan struct{}), + addr: addr, + connCh: make(chan net.Conn), + tlsWrap: tlsWrap, + closeCh: make(chan struct{}), } return layer } @@ -87,7 +87,7 @@ func (l *RaftLayer) Dial(address string, timeout time.Duration) (net.Conn, error } // Check for tls mode - if l.tlsConfig != nil { + if l.tlsWrap != nil { // Switch the connection into TLS mode if _, err := conn.Write([]byte{byte(rpcTLS)}); err != nil { conn.Close() @@ -95,7 +95,7 @@ func (l *RaftLayer) Dial(address string, timeout time.Duration) (net.Conn, error } // Wrap the connection in a TLS client - conn, err = tlsutil.WrapTLSClient(conn, l.tlsConfig) + conn, err = l.tlsWrap(conn) if err != nil { return nil, err } diff --git a/consul/rpc.go b/consul/rpc.go index 412c627de..6359d1cf6 100644 --- a/consul/rpc.go +++ b/consul/rpc.go @@ -208,7 +208,7 @@ func (s *Server) forwardLeader(method string, args interface{}, reply interface{ if server == nil { return structs.ErrNoLeader } - return s.connPool.RPC(server.Addr, server.Version, method, args, reply) + return s.connPool.RPC(s.config.Datacenter, server.Addr, server.Version, method, args, reply) } // forwardDC is used to forward an RPC call to a remote DC, or fail if no servers @@ -229,7 +229,7 @@ func (s *Server) forwardDC(method, dc string, args interface{}, reply interface{ // Forward to remote Consul metrics.IncrCounter([]string{"consul", "rpc", "cross-dc", dc}, 1) - return s.connPool.RPC(server.Addr, server.Version, method, args, reply) + return s.connPool.RPC(dc, server.Addr, server.Version, method, args, reply) } // globalRPC is used to forward an RPC request to one server in each datacenter. diff --git a/consul/server.go b/consul/server.go index 0fe981113..d8d5dc1df 100644 --- a/consul/server.go +++ b/consul/server.go @@ -15,6 +15,7 @@ import ( "time" "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/tlsutil" "github.com/hashicorp/golang-lru" "github.com/hashicorp/raft" "github.com/hashicorp/raft-boltdb" @@ -182,9 +183,9 @@ func NewServer(config *Config) (*Server, error) { config.LogOutput = os.Stderr } - // Create the tlsConfig for outgoing connections + // Create the tls wrapper for outgoing connections tlsConf := config.tlsConfig() - tlsConfig, err := tlsConf.OutgoingTLSConfig() + tlsWrap, err := tlsConf.OutgoingTLSWrapper() if err != nil { return nil, err } @@ -207,7 +208,7 @@ func NewServer(config *Config) (*Server, error) { // Create server s := &Server{ config: config, - connPool: NewPool(config.LogOutput, serverRPCCache, serverMaxStreams, tlsConfig), + connPool: NewPool(config.LogOutput, serverRPCCache, serverMaxStreams, tlsWrap), eventChLAN: make(chan serf.Event, 256), eventChWAN: make(chan serf.Event, 256), localConsuls: make(map[string]*serverParts), @@ -242,7 +243,7 @@ func NewServer(config *Config) (*Server, error) { } // Initialize the RPC layer - if err := s.setupRPC(tlsConfig); err != nil { + if err := s.setupRPC(tlsWrap); err != nil { s.Shutdown() return nil, fmt.Errorf("Failed to start RPC layer: %v", err) } @@ -410,7 +411,7 @@ func (s *Server) setupRaft() error { } // setupRPC is used to setup the RPC listener -func (s *Server) setupRPC(tlsConfig *tls.Config) error { +func (s *Server) setupRPC(tlsWrap tlsutil.DCWrapper) error { // Create endpoints s.endpoints.Status = &Status{s} s.endpoints.Catalog = &Catalog{s} @@ -453,7 +454,10 @@ func (s *Server) setupRPC(tlsConfig *tls.Config) error { return fmt.Errorf("RPC advertise address is not advertisable: %v", addr) } - s.raftLayer = NewRaftLayer(advertise, tlsConfig) + // Provide a DC specific wrapper. Raft replication is only + // ever done in the same datacenter, so we can provide it as a constant. + wrapper := tlsutil.SpecificDC(s.config.Datacenter, tlsWrap) + s.raftLayer = NewRaftLayer(advertise, wrapper) return nil } diff --git a/test/hostname/Alice.crt b/test/hostname/Alice.crt new file mode 100644 index 000000000..b56d79179 --- /dev/null +++ b/test/hostname/Alice.crt @@ -0,0 +1,25 @@ +-----BEGIN CERTIFICATE----- +MIIEOzCCAiWgAwIBAgIRAOpEyvnjEG/Z15f0PrOT7iowCwYJKoZIhvcNAQELMBMx +ETAPBgNVBAMTCENlcnRBdXRoMB4XDTE1MDUxMTIyNTMxN1oXDTE3MDUxMTIyNTMx +OFowEDEOMAwGA1UEAxMFQWxpY2UwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEK +AoIBAQCyxmMV9V0Cdp2mAXxL6h64cWLQlKsumsQfhZNImea8jLYT7+yyLpeHIF4G +7JiushloTnERyTi1wbq9BlU3BVYdX6tqvPXFFwFUXyOkDaSGS3vMCZUYd9PZg0TI +pyQK0/6+jSU7x7jDGVUMhJyvmXB9CgKxG0S8WiR6uGB9oWrTeDnXAzN1T4wNE4M+ +a3P1ToT2k2IDklZ1t5gg6u9EiOAzK7QfpKXrO2MsGyGHhm+tQqNP6LuZv0u2nGW3 +up+i3beQOvLQV0aeiy7zfR3KkIUCvDnmiPnkm35o6wmqFOXTNIU6VoT/l4WtU85F +Ikdtk1gkDLO1iyKiMRbj/hlRqKGxAgMBAAGjgZAwgY0wDgYDVR0PAQH/BAQDAgC4 +MB0GA1UdJQQWMBQGCCsGAQUFBwMBBggrBgEFBQcDAjAdBgNVHQ4EFgQUgt2Os881 +V/je/BOaLavjeorhbi4wHwYDVR0jBBgwFoAUB2s4Gdz7ornOiti84HF+W+nwAj8w +HAYDVR0RBBUwE4IRc2VydmVyLmRjMS5jb25zdWwwCwYJKoZIhvcNAQELA4ICAQCX +Thsbgo1Z5maIyvBJOKX5vQifaSF8kRtX9fZvipvzHCjYxvOHfaTvgtWyxHXCc3tK +DyBswsc2MeHiZ5g0KG113lwLrhcSwEsg5yo0eB7tOTQp1rmCiF6DQYs1XyOqD7P8 +S6clMgJWgpM8Ltw5mYALqDpShv1ND3AOJqENj/0tvdP7Y7cilG+s76HFXRcwKTRw +4rVP+Wr+t4WdXeS8cGxboQqGc40L3HNd5cxsbIM1kucfdrPBljWmyM9aiO1Nipm2 +8dyyir8AFnvoGQ6DPi58jVCCbqosL/GXtVk+IgJ+8eE5T8jvhxBovzxArSSVYIaj +ZxYi85ixfLr1DC5mg5CWWB8ZzmjaUwfyQAcL/F3Q11CkqHw1VDoDzvTBWbguBu6X +xXexlgOQx4/lr8X1pjbbAZktNTOYDt4dTuhrKPU35zW35wTnSBoPrQ3cpGlRcszE +IksZSHi41IQd0zUOGCNZYpPFq8mTwu5ECGHfNvWDH7zEuSkO54tS5Dukxqd8VIQl +h9GB2Uyel8tFm4s/Dx9+glKyvsXDJQz3JmFaB2wPyAPZ1KL4GFI5R0LjUVSFJapP +TO3Ia24naOu3qYXWQK6jGwaCbTT6tdhgNy8EI0aDmv2AgqOXycutMJXF5UqkDmwY +ZqpVdf/TrmBy42pk/C0vpqiy6E4N7WllxhiY2AekkA== +-----END CERTIFICATE----- diff --git a/test/hostname/Alice.key b/test/hostname/Alice.key new file mode 100644 index 000000000..fc37e7258 --- /dev/null +++ b/test/hostname/Alice.key @@ -0,0 +1,27 @@ +-----BEGIN RSA PRIVATE KEY----- +MIIEogIBAAKCAQEAssZjFfVdAnadpgF8S+oeuHFi0JSrLprEH4WTSJnmvIy2E+/s +si6XhyBeBuyYrrIZaE5xEck4tcG6vQZVNwVWHV+rarz1xRcBVF8jpA2khkt7zAmV +GHfT2YNEyKckCtP+vo0lO8e4wxlVDIScr5lwfQoCsRtEvFokerhgfaFq03g51wMz +dU+MDRODPmtz9U6E9pNiA5JWdbeYIOrvRIjgMyu0H6Sl6ztjLBshh4ZvrUKjT+i7 +mb9Ltpxlt7qfot23kDry0FdGnosu830dypCFArw55oj55Jt+aOsJqhTl0zSFOlaE +/5eFrVPORSJHbZNYJAyztYsiojEW4/4ZUaihsQIDAQABAoIBAF3C9szZdwKHu38J +YGtgSuRpc235yx4SRbJSmECHlyBknEowl2+MSCSysR3okNtuxSyTl3HAm2GYTZw9 +6guFXPji6EB/AldwDV5213Z/QT698Bu/GtdOYWm/EyA5qQmUzhKabGDCCwEoFBcQ +piziyMCLs4W3y4ENtfw3H0REmIZ3s0XQRzuDdFCEMbr0Ij6EhP3hSD6es4PWTeHY +LSwoXm0WAxyZudJLhWZaBRxvl+TDY7nVV1jRPQ+ojMJjXfyPo+c2hbbS6luj++qH +6qO7fEpr8EXhO8/0/bPUi0ozE1LVy1kXtEwfszesU9r5XeBq7yTCIa7TTJ35Niwf +T7Ar9NECgYEA0L77+B/3dtedhsMdyiHpcxV6A77OIHsezh8uJzE+nQEgqvJ88N1W +BbF7YByYmaP1/dBrPI7ON52AnDOyo2lM7fVOwr7Ch12tpoa5HFb2WndKt6KokXi/ +Tk8+zoCCZICCv1mtfIaepRTmxAeyqaFthchAv1nc7ojS1BWeXMLa9usCgYEA2z6N +YD8wV44d/qIMaSDVJlusyp8pi9l0ddB591KOYHhJ5RRF1qEd0pshj9sW6pcGGJqf +XFHAkEr/ZIACJK65Y+AFcbgzhyqX8Vy9LLYzWtpFP4SpjH19pYDTHaXvWsIjBlNG +poxtGYCQ8Uedm8IhtrbUorElQVjPlmRGU14B2tMCgYAPQCTAd/VoZVBI7DBc+CVK +FyOW6nW8wcH6ZSTGED720YJFevnNzx3dxJ2y4+PyNZxfMr7i6bv/LC6dOtmuPp80 +M1vRtoYXxaxOIkGb5G6TJWv8BpIyLpQrcHayN4lPNmRW/oJCOsOUY/aIE9fltLl/ +sKWqVTJi6vQcMogjVskQiQKBgHcH7f+sLtXKTdSaLDzDW5X4vcZARXEs/YKdTiqN +wsjzZcMej5AoZyWZnc4Zd8ajeebPw+d+Zxqv7RqmOQOrbPGhhbMo+6jN4jJjVD27 +KgSQbno+z0J8O0QovfXhyiKvNg7QFZKEuRLYb1jftd0DuAQYHTe7D2v8CLAw/tFy +P3WLAoGABcEQEDUWqxfFFCed4mYSoOHvD44YMIzeMMOHXRnGGWug0WkULxzUV1L4 +fTFPCqo6xsn/F3i7xRFpIWXlOzjZKHvw16ZpeZBNcdPjyk7XifhafJYLuknRe1fZ +lzLjhmvizTpd9GQIUS+39aGwGE9JI3H0NAdNA4pvEdKlPhJnG5U= +-----END RSA PRIVATE KEY----- diff --git a/test/hostname/CertAuth.crt b/test/hostname/CertAuth.crt new file mode 100644 index 000000000..6c9d01ed0 --- /dev/null +++ b/test/hostname/CertAuth.crt @@ -0,0 +1,29 @@ +-----BEGIN CERTIFICATE----- +MIIFADCCAuqgAwIBAgIBATALBgkqhkiG9w0BAQswEzERMA8GA1UEAxMIQ2VydEF1 +dGgwHhcNMTUwNTExMjI0NjQzWhcNMjUwNTExMjI0NjU0WjATMREwDwYDVQQDEwhD +ZXJ0QXV0aDCCAiIwDQYJKoZIhvcNAQEBBQADggIPADCCAgoCggIBALcMByyynHsA ++K4PJwo5+XHygaEZAhPGvHiKQK2Cbc9NDm0ZTzx0rA/dRTZlvouhDyzcJHm+6R1F +j6zQv7iaSC3qQtJiPnPsfZ+/0XhFZ3fQWMnfDiGbZpF1kJF01ofB6vnsuocFC0zG +aGC+SZiLAzs+QMP3Bebw1elCBIeoN+8NWnRYmLsYIaYGJGBSbNo/lCpLTuinofUn +L3ehWEGv1INwpHnSVeN0Ml2GFe23d7PUlj/wNIHgUdpUR+KEJxIP3klwtsI3QpSH +c4VjWdf4aIcka6K3IFuw+K0PUh3xAAPnMpAQOtCZk0AhF5rlvUbevC6jADxpKxLp +OONmvCTer4LtyNURAoBH52vbK0r/DNcTpPEFV0IP66nXUFgkk0mRKsu8HTb4IOkC +X3K4mp18EiWUUtrHZAnNct0iIniDBqKK0yhSNhztG6VakVt/1WdQY9Ey3mNtxN1O +thqWFKdpKUzPKYC3P6PfVpiE7+VbWTLLXba+8BPe8BxWPsVkjJqGSGnCte4COusz +M8/7bbTgifwJfsepwFtZG53tvwjWlO46Exl30VoDNTaIGvs1fO0GqJlh2A7FN5F2 +S1rS5VYHtPK8QdmUSvyq+7JDBc1HNT5I2zsIQbNcLwDTZ5EsbU6QR7NHDJKxjv/w +bs3eTXJSSNcFD74wRU10pXjgE5wOFu9TAgMBAAGjYzBhMA4GA1UdDwEB/wQEAwIA +BjAPBgNVHRMBAf8EBTADAQH/MB0GA1UdDgQWBBQHazgZ3Puiuc6K2LzgcX5b6fAC +PzAfBgNVHSMEGDAWgBQHazgZ3Puiuc6K2LzgcX5b6fACPzALBgkqhkiG9w0BAQsD +ggIBAEmeNrSUhpHg1I8dtfqu9hCU/6IZThjtcFA+QcPkkMa+Z1k0SOtsgW8MdlcA +gCf5g5yQZ0DdpWM9nDB6xDIhQdccm91idHgf8wmpEHUj0an4uyn2ESCt8eqrAWf7 +AClYORCASTYfguJCxcfvwtI1uqaOeCxSOdmFay79UVitVsWeonbCRGsVgBDifJxw +G2oCQqoYAmXPM4J6syk5GHhB1O9MMq+g1+hOx9s+XHyTui9FL4V+IUO1ygVqEQB5 +PSiRBvcIsajSGVao+vK0gf2XfcXzqr3y3NhBky9rFMp1g+ykb2yWekV4WiROJlCj +TsWwWZDRyjiGahDbho/XW8JciouHZhJdjhmO31rqW3HdFviCTdXMiGk3GQIzz/Jg +P+enOaHXoY9lcxzDvY9z1BysWBgNvNrMnVge/fLP9o+a0a0PRIIVl8T0Ef3zeg1O +CLCSy/1Vae5Tx63ZTFvGFdOSusYkG9rlAUHXZE364JRCKzM9Bz0bM+t+LaO0MaEb +YoxcXEPU+gB2IvmARpInN3oHexR6ekuYHVTRGdWrdmuHFzc7eFwygRqTFdoCCU+G +QZEkd+lOEyv0zvQqYg+Jp0AEGz2B2zB53uBVECtn0EqrSdPtRzUBSByXVs6QhSXn +eVmy+z3U3MecP63X6oSPXekqSyZFuegXpNNuHkjNoL4ep2ix +-----END CERTIFICATE----- diff --git a/tlsutil/config.go b/tlsutil/config.go index ab781de13..105934d3a 100644 --- a/tlsutil/config.go +++ b/tlsutil/config.go @@ -6,9 +6,19 @@ import ( "fmt" "io/ioutil" "net" + "strings" "time" ) +// DCWrapper is a function that is used to wrap a non-TLS connection +// and returns an appropriate TLS connection or error. This takes +// a datacenter as an argument. +type DCWrapper func(dc string, conn net.Conn) (net.Conn, error) + +// Wrapper is a variant of DCWrapper, where the DC is provided as +// a constant value. This is usually done by currying DCWrapper. +type Wrapper func(conn net.Conn) (net.Conn, error) + // Config used to create tls.Config type Config struct { // VerifyIncoming is used to verify the authenticity of incoming connections. @@ -22,6 +32,14 @@ type Config struct { // server nodes. VerifyOutgoing bool + // VerifyServerHostname is used to enable hostname verification of servers. This + // ensures that the certificate presented is valid for server... + // This prevents a compromised client from being restarted as a server, and then + // intercepting request traffic as well as being added as a raft peer. This should be + // enabled by default with VerifyOutgoing, but for legacy reasons we cannot break + // existing clients. + VerifyServerHostname bool + // CAFile is a path to a certificate authority file. This is used with VerifyIncoming // or VerifyOutgoing to verify the TLS connection. CAFile string @@ -40,6 +58,9 @@ type Config struct { // ServerName is used with the TLS certificate to ensure the name we // provide matches the certificate ServerName string + + // Domain is the Consul TLD being used. Defaults to "consul." + Domain string } // AppendCA opens and parses the CA file and adds the certificates to @@ -78,6 +99,10 @@ func (c *Config) KeyPair() (*tls.Certificate, error) { // requests. It will return a nil config if this configuration should // not use TLS for outgoing connections. func (c *Config) OutgoingTLSConfig() (*tls.Config, error) { + // If VerifyServerHostname is true, that implies VerifyOutgoing + if c.VerifyServerHostname { + c.VerifyOutgoing = true + } if !c.VerifyOutgoing { return nil, nil } @@ -90,6 +115,11 @@ func (c *Config) OutgoingTLSConfig() (*tls.Config, error) { tlsConfig.ServerName = c.ServerName tlsConfig.InsecureSkipVerify = false } + if c.VerifyServerHostname { + // ServerName is filled in dynamically based on the target DC + tlsConfig.ServerName = "VerifyServerHostname" + tlsConfig.InsecureSkipVerify = false + } // Ensure we have a CA if VerifyOutgoing is set if c.VerifyOutgoing && c.CAFile == "" { @@ -113,6 +143,51 @@ func (c *Config) OutgoingTLSConfig() (*tls.Config, error) { return tlsConfig, nil } +// OutgoingTLSWrapper returns a a DCWrapper based on the OutgoingTLS +// configuration. If hostname verification is on, the wrapper +// will properly generate the dynamic server name for verification. +func (c *Config) OutgoingTLSWrapper() (DCWrapper, error) { + // Get the TLS config + tlsConfig, err := c.OutgoingTLSConfig() + if err != nil { + return nil, err + } + + // Check if TLS is not enabled + if tlsConfig == nil { + return nil, nil + } + + // Strip the trailing '.' from the domain if any + domain := strings.TrimSuffix(c.Domain, ".") + + // Generate the wrapper based on hostname verification + if c.VerifyServerHostname { + wrapper := func(dc string, conn net.Conn) (net.Conn, error) { + conf := *tlsConfig + conf.ServerName = "server." + dc + "." + domain + return WrapTLSClient(conn, &conf) + } + return wrapper, nil + } else { + wrapper := func(dc string, c net.Conn) (net.Conn, error) { + return WrapTLSClient(c, tlsConfig) + } + return wrapper, nil + } +} + +// SpecificDC is used to invoke a static datacenter +// and turns a DCWrapper into a Wrapper type. +func SpecificDC(dc string, tlsWrap DCWrapper) Wrapper { + if tlsWrap == nil { + return nil + } + return func(conn net.Conn) (net.Conn, error) { + return tlsWrap(dc, conn) + } +} + // Wrap a net.Conn into a client tls connection, performing any // additional verification as needed. // diff --git a/tlsutil/config_test.go b/tlsutil/config_test.go index 150fddccd..cbd127ac8 100644 --- a/tlsutil/config_test.go +++ b/tlsutil/config_test.go @@ -7,6 +7,8 @@ import ( "io/ioutil" "net" "testing" + + "github.com/hashicorp/yamux" ) func TestConfig_AppendCA_None(t *testing.T) { @@ -133,6 +135,29 @@ func TestConfig_OutgoingTLS_ServerName(t *testing.T) { } } +func TestConfig_OutgoingTLS_VerifyHostname(t *testing.T) { + conf := &Config{ + VerifyServerHostname: true, + CAFile: "../test/ca/root.cer", + } + tls, err := conf.OutgoingTLSConfig() + if err != nil { + t.Fatalf("err: %v", err) + } + if tls == nil { + t.Fatalf("expected config") + } + if len(tls.RootCAs.Subjects()) != 1 { + t.Fatalf("expect root cert") + } + if tls.ServerName != "VerifyServerHostname" { + t.Fatalf("expect server name") + } + if tls.InsecureSkipVerify { + t.Fatalf("should not skip built-in verification") + } +} + func TestConfig_OutgoingTLS_WithKeyPair(t *testing.T) { conf := &Config{ VerifyOutgoing: true, @@ -168,8 +193,16 @@ func startTLSServer(config *Config) (net.Conn, chan error) { } client, server := net.Pipe() + + // Use yamux to buffer the reads, otherwise it's easy to deadlock + muxConf := yamux.DefaultConfig() + serverSession, _ := yamux.Server(server, muxConf) + clientSession, _ := yamux.Client(client, muxConf) + clientConn, _ := clientSession.Open() + serverConn, _ := serverSession.Accept() + go func() { - tlsServer := tls.Server(server, tlsConfigServer) + tlsServer := tls.Server(serverConn, tlsConfigServer) if err := tlsServer.Handshake(); err != nil { errc <- err } @@ -183,7 +216,107 @@ func startTLSServer(config *Config) (net.Conn, chan error) { io.Copy(ioutil.Discard, tlsServer) tlsServer.Close() }() - return client, errc + return clientConn, errc +} + +func TestConfig_outgoingWrapper_OK(t *testing.T) { + config := &Config{ + CAFile: "../test/hostname/CertAuth.crt", + CertFile: "../test/hostname/Alice.crt", + KeyFile: "../test/hostname/Alice.key", + VerifyServerHostname: true, + Domain: "consul", + } + + client, errc := startTLSServer(config) + if client == nil { + t.Fatalf("startTLSServer err: %v", <-errc) + } + + wrap, err := config.OutgoingTLSWrapper() + if err != nil { + t.Fatalf("OutgoingTLSWrapper err: %v", err) + } + + tlsClient, err := wrap("dc1", client) + if err != nil { + t.Fatalf("wrapTLS err: %v", err) + } + defer tlsClient.Close() + if err := tlsClient.(*tls.Conn).Handshake(); err != nil { + t.Fatalf("write err: %v", err) + } + + err = <-errc + if err != nil { + t.Fatalf("server: %v", err) + } +} + +func TestConfig_outgoingWrapper_BadDC(t *testing.T) { + config := &Config{ + CAFile: "../test/hostname/CertAuth.crt", + CertFile: "../test/hostname/Alice.crt", + KeyFile: "../test/hostname/Alice.key", + VerifyServerHostname: true, + Domain: "consul", + } + + client, errc := startTLSServer(config) + if client == nil { + t.Fatalf("startTLSServer err: %v", <-errc) + } + + wrap, err := config.OutgoingTLSWrapper() + if err != nil { + t.Fatalf("OutgoingTLSWrapper err: %v", err) + } + + tlsClient, err := wrap("dc2", client) + if err != nil { + t.Fatalf("wrapTLS err: %v", err) + } + defer tlsClient.Close() + err = tlsClient.(*tls.Conn).Handshake() + + if _, ok := err.(x509.HostnameError); !ok { + t.Fatalf("should get hostname err: %v", err) + } + + <-errc +} + +func TestConfig_outgoingWrapper_BadCert(t *testing.T) { + config := &Config{ + CAFile: "../test/ca/root.cer", + CertFile: "../test/key/ourdomain.cer", + KeyFile: "../test/key/ourdomain.key", + VerifyServerHostname: true, + Domain: "consul", + } + + client, errc := startTLSServer(config) + if client == nil { + t.Fatalf("startTLSServer err: %v", <-errc) + } + + wrap, err := config.OutgoingTLSWrapper() + if err != nil { + t.Fatalf("OutgoingTLSWrapper err: %v", err) + } + + tlsClient, err := wrap("dc1", client) + if err != nil { + t.Fatalf("wrapTLS err: %v", err) + } + defer tlsClient.Close() + err = tlsClient.(*tls.Conn).Handshake() + + if _, ok := err.(x509.HostnameError); !ok { + t.Fatalf("should get hostname err: %v", err) + } + + <-errc } func TestConfig_wrapTLS_OK(t *testing.T) { diff --git a/website/source/docs/agent/encryption.html.markdown b/website/source/docs/agent/encryption.html.markdown index 76624e0be..36db8ad0a 100644 --- a/website/source/docs/agent/encryption.html.markdown +++ b/website/source/docs/agent/encryption.html.markdown @@ -64,7 +64,8 @@ using OpenSSL. Note: client certificates must have for client and server authentication. TLS can be used to verify the authenticity of the servers or verify the authenticity of clients. -These modes are controlled by the [`verify_outgoing`](/docs/agent/options.html#verify_outgoing) +These modes are controlled by the [`verify_outgoing`](/docs/agent/options.html#verify_outgoing), +[`verify_server_hostname`](/docs/agent/options.html#verify_server_hostname), and [`verify_incoming`](/docs/agent/options.html#verify_incoming) options, respectively. If [`verify_outgoing`](/docs/agent/options.html#verify_outgoing) is set, agents verify the @@ -74,6 +75,14 @@ by the certificate authority present on all agents, set via the agent's appropriate key pair set using [`cert_file`](/docs/agent/options.html#cert_file) and [`key_file`](/docs/agent/options.html#key_file). +If [`verify_server_hostname`](/docs/agent/options.html#verify_server_hostname) is set, then +outgoing connections perform hostname verification. All servers must have a certificate +valid for "server.\.\" or the client will reject the handshake. This is +a new configuration as of 0.5.1, and it is used to prevent a compromised client from being +able to restart in server mode and perform a MITM attack. New deployments should set this +to true, and generate the proper certificates, but this is defaulted to false to avoid breaking +existing deployments. + If [`verify_incoming`](/docs/agent/options.html#verify_incoming) is set, the servers verify the authenticity of all incoming connections. All clients must have a valid key pair set using [`cert_file`](/docs/agent/options.html#cert_file) and diff --git a/website/source/docs/agent/options.html.markdown b/website/source/docs/agent/options.html.markdown index 0487b6f72..7e8cdda1f 100644 --- a/website/source/docs/agent/options.html.markdown +++ b/website/source/docs/agent/options.html.markdown @@ -584,6 +584,14 @@ definitions support being updated during a reload. will not make use of TLS for outgoing connections. This applies to clients and servers as both will make outgoing connections. +* `verify_server_hostname` - If set to + true, Consul verifies for all outgoing connections that the TLS certificate presented by the servers + matches "server.." hostname. This implies `verify_outgoing`. + By default, this is false, and Consul does not verify the hostname of the certificate, only + that it is signed by a trusted CA. This setting is important to prevent a compromised + client from being restarted as a server, and thus being able to perform a MITM attack + or to be added as a Raft peer. This is new in 0.5.1. + * `watches` - Watches is a list of watch specifications which allow an external process to be automatically invoked when a particular data view is updated. See the