Skip to content

Commit

Permalink
Send the correct SNI from tsh to auth server
Browse files Browse the repository at this point in the history
SNI is used to indicate which cluster's CA to use for client cert
validation. If SNI is not sent, or set as "teleport.cluster.local"
(which is default in the client config), auth server will attempt to
validate against all known CAs.

The list of CA subjects is sent to the client during handshake, before
client sends its own client cert. If this list is too long, handshake
will fail. The limit is 65535 bytes, because TLS wire encoding uses 2
bytes for a length prefix. In teleport, this fits ~520-540 trusted
cluster CAs.

To avoid handshake failures on such large setups, all clients must send
the correct SNI. In some future version, we should enforce this to catch
such issues early. For now, added a debug log to report clients using
the default ServerName. Also added a check for large number of CAs, to
print a helpful error.

Updates #3870
  • Loading branch information
Andrew Lytvynov authored and awly committed Jun 29, 2020
1 parent b128000 commit d2c03e9
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 19 deletions.
1 change: 1 addition & 0 deletions lib/auth/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,7 @@ func (a *TestAuthServer) NewRemoteClient(identity TestIdentity, addr net.Addr, p
}
tlsConfig.Certificates = []tls.Certificate{*cert}
tlsConfig.RootCAs = pool
tlsConfig.ServerName = EncodeClusterName(a.ClusterName)
addrs := []utils.NetAddr{{
AddrNetwork: addr.Network(),
Addr: addr.String()}}
Expand Down
32 changes: 31 additions & 1 deletion lib/auth/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"crypto/tls"
"crypto/x509"
"math"
"net"
"net/http"

Expand Down Expand Up @@ -135,7 +136,16 @@ func (t *TLSServer) Serve(listener net.Listener) error {
func (t *TLSServer) GetConfigForClient(info *tls.ClientHelloInfo) (*tls.Config, error) {
var clusterName string
var err error
if info.ServerName != "" {
switch info.ServerName {
case "":
// Client does not use SNI, will validate against all known CAs.
case teleport.APIDomain:
// REMOVE IN 4.4: all 4.3+ clients must specify the correct cluster name.
//
// Instead, this case should either default to current cluster CAs or
// return an error.
t.Debugf("Client %q sent %q in SNI, which causes this auth server to send all known CAs in TLS handshake. If this client is version 4.2 or older, this is expected; if this client is version 4.3 or above, please let us know at https://github.com/gravitational/teleport/issues/new", info.Conn.RemoteAddr(), info.ServerName)
default:
clusterName, err = DecodeClusterName(info.ServerName)
if err != nil {
if !trace.IsNotFound(err) {
Expand All @@ -159,6 +169,26 @@ func (t *TLSServer) GetConfigForClient(info *tls.ClientHelloInfo) (*tls.Config,
// this falls back to the default config
return nil, nil
}

// Per https://tools.ietf.org/html/rfc5246#section-7.4.4 the total size of
// the known CA subjects sent to the client can't exceed 2^16-1 (due to
// 2-byte length encoding). The crypto/tls stack will panic if this
// happens. To make the error less cryptic, catch this condition and return
// a better error.
//
// This may happen with a very large (>500) number of trusted clusters, if
// the client doesn't send the correct ServerName in its ClientHelloInfo
// (see the switch at the top of this func).
var totalSubjectsLen int64
for _, s := range pool.Subjects() {
// Each subject in the list gets a separate 2-byte length prefix.
totalSubjectsLen += 2
totalSubjectsLen += int64(len(s))
}
if totalSubjectsLen >= int64(math.MaxUint16) {
return nil, trace.BadParameter("number of CAs in client cert pool is too large (%d) and cannot be encoded in a TLS handshake; this is due to a large number of trusted clusters; try updating tsh to the latest version; if that doesn't help, remove some trusted clusters", len(pool.Subjects()))
}

tlsCopy := t.TLS.Clone()
tlsCopy.ClientCAs = pool
for _, cert := range tlsCopy.Certificates {
Expand Down
21 changes: 3 additions & 18 deletions lib/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package client
import (
"bytes"
"context"
"crypto/tls"
"encoding/json"
"io"
"io/ioutil"
Expand Down Expand Up @@ -352,28 +351,14 @@ func (proxy *ProxyClient) ConnectToCluster(ctx context.Context, clusterName stri
})
}

// Because Teleport clients can't be configured (yet), they take the default
// list of cipher suites from Go.
tlsConfig := utils.TLSConfig(nil)
localAgent := proxy.teleportClient.LocalAgent()
pool, err := localAgent.GetCerts()
if err != nil {
return nil, trace.Wrap(err)
}
tlsConfig.RootCAs = pool
key, err := localAgent.GetKey()
if err != nil {
return nil, trace.Wrap(err, "failed to fetch TLS key for %v", proxy.teleportClient.Username)
}
if len(key.TLSCert) != 0 {
tlsCert, err := tls.X509KeyPair(key.TLSCert, key.Priv)
if err != nil {
return nil, trace.Wrap(err, "failed to parse TLS cert and key")
}
tlsConfig.Certificates = append(tlsConfig.Certificates, tlsCert)
}
if len(tlsConfig.Certificates) == 0 {
return nil, trace.BadParameter("no TLS keys found for user %v, please relogin to get new credentials", proxy.teleportClient.Username)
tlsConfig, err := key.ClientTLSConfig()
if err != nil {
return nil, trace.Wrap(err, "failed to generate client TLS config")
}
clt, err := auth.NewTLSClient(auth.ClientConfig{
Dialer: dialer,
Expand Down
7 changes: 7 additions & 0 deletions lib/client/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,13 @@ func (k *Key) ClientTLSConfig() (*tls.Config, error) {
return nil, trace.Wrap(err, "failed to parse TLS cert and key")
}
tlsConfig.Certificates = append(tlsConfig.Certificates, tlsCert)
// Use Issuer CN from the certificate to populate the correct SNI in
// requests.
leaf, err := x509.ParseCertificate(tlsCert.Certificate[0])
if err != nil {
return nil, trace.Wrap(err, "failed to parse TLS cert")
}
tlsConfig.ServerName = auth.EncodeClusterName(leaf.Issuer.CommonName)
return tlsConfig, nil
}

Expand Down

0 comments on commit d2c03e9

Please sign in to comment.