mirror of https://github.com/status-im/consul.git
Restore the 0.2 TLS verification behavior.
Namely, don't check the DNS names in TLS certificates when connecting to other servers. As of golang 1.3, crypto/tls no longer natively supports doing partial verification (verifying the cert issuer but not the hostname), so we have to disable verification entirely and then do the issuer verification ourselves. Fortunately, crypto/x509 makes this relatively straightforward. If the "server_name" configuration option is passed, we preserve the existing behavior of checking that server name everywhere. No option is provided to retain the current behavior of checking the remote certificate against the local node name, since that behavior seems clearly buggy and unintentional, and I have difficulty imagining it is actually being used anywhere. It would be relatively straightforward to restore if desired, however.
This commit is contained in:
parent
f5fa8c1d2c
commit
d174cbe7f4
|
@ -88,10 +88,8 @@ func NewClient(config *Config) (*Client, error) {
|
|||
// Create the tlsConfig
|
||||
var tlsConfig *tls.Config
|
||||
var err error
|
||||
if config.VerifyOutgoing {
|
||||
if tlsConfig, err = config.OutgoingTLSConfig(); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if tlsConfig, err = config.OutgoingTLSConfig(); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// Create a logger
|
||||
|
|
|
@ -172,16 +172,21 @@ func (c *Config) KeyPair() (*tls.Certificate, error) {
|
|||
return &cert, err
|
||||
}
|
||||
|
||||
// OutgoingTLSConfig generates a TLS configuration for outgoing requests
|
||||
// OutgoingTLSConfig generates a TLS configuration for outgoing
|
||||
// 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 !c.VerifyOutgoing {
|
||||
return nil, nil
|
||||
}
|
||||
// Create the tlsConfig
|
||||
tlsConfig := &tls.Config{
|
||||
ServerName: c.ServerName,
|
||||
RootCAs: x509.NewCertPool(),
|
||||
InsecureSkipVerify: !c.VerifyOutgoing,
|
||||
InsecureSkipVerify: true,
|
||||
}
|
||||
if tlsConfig.ServerName == "" {
|
||||
tlsConfig.ServerName = c.NodeName
|
||||
if c.ServerName != "" {
|
||||
tlsConfig.ServerName = c.ServerName
|
||||
tlsConfig.InsecureSkipVerify = false
|
||||
}
|
||||
|
||||
// Ensure we have a CA if VerifyOutgoing is set
|
||||
|
@ -206,6 +211,59 @@ func (c *Config) OutgoingTLSConfig() (*tls.Config, error) {
|
|||
return tlsConfig, nil
|
||||
}
|
||||
|
||||
// Wrap a net.Conn into a tls connection, performing any additional
|
||||
// verification as needed.
|
||||
//
|
||||
// As of go 1.3, crypto/tls only supports either doing no certificate
|
||||
// verification, or doing full verification including of the peer's
|
||||
// DNS name. For consul, we want to validate that the certificate is
|
||||
// signed by a known CA, but because consul doesn't use DNS names for
|
||||
// node names, we don't verify the certificate DNS names. Since go 1.3
|
||||
// no longer supports this mode of operation, we have to do it
|
||||
// manually.
|
||||
func wrapTLSClient(conn net.Conn, tlsConfig *tls.Config) (net.Conn, error) {
|
||||
var err error
|
||||
var tlsConn *tls.Conn
|
||||
|
||||
tlsConn = tls.Client(conn, tlsConfig)
|
||||
|
||||
// If crypto/tls is doing verification, there's no need to do
|
||||
// our own.
|
||||
if tlsConfig.InsecureSkipVerify == false {
|
||||
return tlsConn, nil
|
||||
}
|
||||
|
||||
if err = tlsConn.Handshake(); err != nil {
|
||||
tlsConn.Close()
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// The following is lightly-modified from the doFullHandshake
|
||||
// method in crypto/tls's handshake_client.go.
|
||||
opts := x509.VerifyOptions{
|
||||
Roots: tlsConfig.RootCAs,
|
||||
CurrentTime: time.Now(),
|
||||
DNSName: "",
|
||||
Intermediates: x509.NewCertPool(),
|
||||
}
|
||||
|
||||
certs := tlsConn.ConnectionState().PeerCertificates
|
||||
for i, cert := range certs {
|
||||
if i == 0 {
|
||||
continue
|
||||
}
|
||||
opts.Intermediates.AddCert(cert)
|
||||
}
|
||||
|
||||
_, err = certs[0].Verify(opts)
|
||||
if err != nil {
|
||||
tlsConn.Close()
|
||||
return nil, err
|
||||
}
|
||||
|
||||
return tlsConn, err
|
||||
}
|
||||
|
||||
// IncomingTLSConfig generates a TLS configuration for incoming requests
|
||||
func (c *Config) IncomingTLSConfig() (*tls.Config, error) {
|
||||
// Create the tlsConfig
|
||||
|
|
|
@ -78,14 +78,8 @@ func TestConfig_OutgoingTLS_OnlyCA(t *testing.T) {
|
|||
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.InsecureSkipVerify {
|
||||
t.Fatalf("expect to skip verification")
|
||||
if tls != nil {
|
||||
t.Fatalf("expected no config")
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -104,8 +98,35 @@ func TestConfig_OutgoingTLS_VerifyOutgoing(t *testing.T) {
|
|||
if len(tls.RootCAs.Subjects()) != 1 {
|
||||
t.Fatalf("expect root cert")
|
||||
}
|
||||
if tls.ServerName != "" {
|
||||
t.Fatalf("expect no server name verification")
|
||||
}
|
||||
if !tls.InsecureSkipVerify {
|
||||
t.Fatalf("should skip built-in verification")
|
||||
}
|
||||
}
|
||||
|
||||
func TestConfig_OutgoingTLS_ServerName(t *testing.T) {
|
||||
conf := &Config{
|
||||
VerifyOutgoing: true,
|
||||
CAFile: "../test/ca/root.cer",
|
||||
ServerName: "consul.example.com",
|
||||
}
|
||||
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 != "consul.example.com" {
|
||||
t.Fatalf("expect server name")
|
||||
}
|
||||
if tls.InsecureSkipVerify {
|
||||
t.Fatalf("should not skip verification")
|
||||
t.Fatalf("should not skip built-in verification")
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -126,8 +147,8 @@ func TestConfig_OutgoingTLS_WithKeyPair(t *testing.T) {
|
|||
if len(tls.RootCAs.Subjects()) != 1 {
|
||||
t.Fatalf("expect root cert")
|
||||
}
|
||||
if tls.InsecureSkipVerify {
|
||||
t.Fatalf("should not skip verification")
|
||||
if !tls.InsecureSkipVerify {
|
||||
t.Fatalf("should skip verification")
|
||||
}
|
||||
if len(tls.Certificates) != 1 {
|
||||
t.Fatalf("expected client cert")
|
||||
|
|
|
@ -221,7 +221,11 @@ func (p *ConnPool) getNewConn(addr net.Addr, version int) (*Conn, error) {
|
|||
}
|
||||
|
||||
// Wrap the connection in a TLS client
|
||||
conn = tls.Client(conn, p.tlsConfig)
|
||||
conn, err = wrapTLSClient(conn, p.tlsConfig)
|
||||
if err != nil {
|
||||
conn.Close()
|
||||
return nil, err
|
||||
}
|
||||
}
|
||||
|
||||
// Switch the multiplexing based on version
|
||||
|
|
|
@ -94,7 +94,10 @@ func (l *RaftLayer) Dial(address string, timeout time.Duration) (net.Conn, error
|
|||
}
|
||||
|
||||
// Wrap the connection in a TLS client
|
||||
conn = tls.Client(conn, l.tlsConfig)
|
||||
conn, err = wrapTLSClient(conn, l.tlsConfig)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
}
|
||||
|
||||
// Write the Raft byte to set the mode
|
||||
|
|
|
@ -145,12 +145,9 @@ func NewServer(config *Config) (*Server, error) {
|
|||
}
|
||||
|
||||
// Create the tlsConfig for outgoing connections
|
||||
var tlsConfig *tls.Config
|
||||
var err error
|
||||
if config.VerifyOutgoing {
|
||||
if tlsConfig, err = config.OutgoingTLSConfig(); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
tlsConfig, err := config.OutgoingTLSConfig()
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// Get the incoming tls config
|
||||
|
|
Loading…
Reference in New Issue