Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Backport: send the correct SNI from tsh to auth server #3931

Merged
merged 2 commits into from
Jun 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/auth/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,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 @@ -350,28 +349,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 @@ -86,6 +86,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
2 changes: 1 addition & 1 deletion lib/client/keyagent.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ func (a *LocalKeyAgent) GetCerts() (*x509.CertPool, error) {
return a.keyStore.GetCerts(a.proxyHost)
}

func (a *LocalKeyAgent) GetCertsPEM() ([]byte, error) {
func (a *LocalKeyAgent) GetCertsPEM() ([][]byte, error) {
return a.keyStore.GetCertsPEM(a.proxyHost)
}

Expand Down
2 changes: 1 addition & 1 deletion lib/client/keyagent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func (s *KeyAgentTestSuite) SetUpSuite(c *check.C) {
pemBytes, ok := fixtures.PEMBytes["rsa"]
c.Assert(ok, check.Equals, true)

s.tlsca, err = newSelfSignedCA(pemBytes)
s.tlsca, _, err = newSelfSignedCA(pemBytes)
c.Assert(err, check.IsNil)

// temporary key to use during tests
Expand Down
64 changes: 46 additions & 18 deletions lib/client/keystore.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,8 @@ type LocalKeyStore interface {
GetCerts(proxy string) (*x509.CertPool, error)

// GetCertsPEM gets trusted TLS certificates of certificate authorities.
GetCertsPEM(proxy string) ([]byte, error)
// Each returned byte slice contains an individual PEM block.
GetCertsPEM(proxy string) ([][]byte, error)
}

// FSLocalKeyStore implements LocalKeyStore interface using the filesystem.
Expand Down Expand Up @@ -233,8 +234,22 @@ func (fs *FSLocalKeyStore) GetKey(proxyHost string, username string) (*Key, erro
fs.log.Error(err)
return nil, trace.Wrap(err)
}
tlsCA, err := fs.GetCertsPEM(proxyHost)
if err != nil {
fs.log.Error(err)
return nil, trace.Wrap(err)
}

key := &Key{Pub: pub, Priv: priv, Cert: cert, ProxyHost: proxyHost, TLSCert: tlsCert}
key := &Key{
Pub: pub,
Priv: priv,
Cert: cert,
ProxyHost: proxyHost,
TLSCert: tlsCert,
TrustedCA: []auth.TrustedCerts{{
TLSCertificates: tlsCA,
}},
}

// Validate the key loaded from disk.
err = key.CheckCert()
Expand Down Expand Up @@ -289,36 +304,49 @@ func (fs *FSLocalKeyStore) SaveCerts(proxy string, cas []auth.TrustedCerts) erro
return nil
}

// GetCertsPEM returns trusted TLS certificates of certificate authorities PEM block
func (fs *FSLocalKeyStore) GetCertsPEM(proxy string) ([]byte, error) {
// GetCertsPEM returns trusted TLS certificates of certificate authorities PEM
// blocks.
func (fs *FSLocalKeyStore) GetCertsPEM(proxy string) ([][]byte, error) {
dir, err := fs.dirFor(proxy, false)
if err != nil {
return nil, trace.Wrap(err)
}
return ioutil.ReadFile(filepath.Join(dir, fileNameTLSCerts))
}

// GetCerts returns trusted TLS certificates of certificate authorities
func (fs *FSLocalKeyStore) GetCerts(proxy string) (*x509.CertPool, error) {
dir, err := fs.dirFor(proxy, false)
data, err := ioutil.ReadFile(filepath.Join(dir, fileNameTLSCerts))
if err != nil {
return nil, trace.Wrap(err)
}
bytes, err := ioutil.ReadFile(filepath.Join(dir, fileNameTLSCerts))
if err != nil {
return nil, trace.ConvertSystemError(err)
}
pool := x509.NewCertPool()
for len(bytes) > 0 {
var block *pem.Block
block, bytes = pem.Decode(bytes)
var blocks [][]byte
for len(data) > 0 {
block, rest := pem.Decode(data)
if block == nil {
break
}
if block.Type != "CERTIFICATE" || len(block.Headers) != 0 {
fs.log.Debugf("Skipping PEM block type=%v headers=%v.", block.Type, block.Headers)
continue
}
// rest contains the remainder of data after reading a block.
// Therefore, the block length is len(data) - len(rest).
// Use that length to slice the block from the start of data.
blocks = append(blocks, data[:len(data)-len(rest)])
data = rest
}
return blocks, nil
}

// GetCerts returns trusted TLS certificates of certificate authorities as
// x509.CertPool.
func (fs *FSLocalKeyStore) GetCerts(proxy string) (*x509.CertPool, error) {
blocks, err := fs.GetCertsPEM(proxy)
if err != nil {
return nil, trace.Wrap(err)
}
pool := x509.NewCertPool()
for _, bytes := range blocks {
block, _ := pem.Decode(bytes)
if block == nil {
continue
}

cert, err := x509.ParseCertificate(block.Bytes)
if err != nil {
Expand Down
58 changes: 38 additions & 20 deletions lib/client/keystore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"os"
"time"

"github.com/gravitational/teleport/lib/auth"
"github.com/gravitational/teleport/lib/auth/testauthority"
"github.com/gravitational/teleport/lib/defaults"
"github.com/gravitational/teleport/lib/services"
Expand All @@ -37,25 +38,33 @@ import (
)

type KeyStoreTestSuite struct {
storeDir string
store *FSLocalKeyStore
keygen *testauthority.Keygen
tlsca *tlsca.CertAuthority
storeDir string
store *FSLocalKeyStore
keygen *testauthority.Keygen
tlsCA *tlsca.CertAuthority
tlsCACert auth.TrustedCerts
}

var _ = fmt.Printf
var _ = check.Suite(&KeyStoreTestSuite{})

func newSelfSignedCA(privateKey []byte) (*tlsca.CertAuthority, error) {
func newSelfSignedCA(privateKey []byte) (*tlsca.CertAuthority, auth.TrustedCerts, error) {
rsaKey, err := ssh.ParseRawPrivateKey(privateKey)
if err != nil {
return nil, trace.Wrap(err)
return nil, auth.TrustedCerts{}, trace.Wrap(err)
}
key, cert, err := tlsca.GenerateSelfSignedCAWithPrivateKey(rsaKey.(*rsa.PrivateKey), pkix.Name{
CommonName: "localhost",
Organization: []string{"localhost"},
}, nil, defaults.CATTL)
return tlsca.New(cert, key)
if err != nil {
return nil, auth.TrustedCerts{}, trace.Wrap(err)
}
ca, err := tlsca.New(cert, key)
if err != nil {
return nil, auth.TrustedCerts{}, trace.Wrap(err)
}
return ca, auth.TrustedCerts{TLSCertificates: [][]byte{cert}}, nil
}

func (s *KeyStoreTestSuite) SetUpSuite(c *check.C) {
Expand All @@ -68,7 +77,7 @@ func (s *KeyStoreTestSuite) SetUpSuite(c *check.C) {
c.Assert(s.store, check.NotNil)
c.Assert(utils.IsDir(s.store.KeyDir), check.Equals, true)

s.tlsca, err = newSelfSignedCA(CAPriv)
s.tlsCA, s.tlsCACert, err = newSelfSignedCA(CAPriv)
c.Assert(err, check.IsNil)
}

Expand All @@ -88,13 +97,13 @@ func (s *KeyStoreTestSuite) TestListKeys(c *check.C) {
for i := 0; i < keyNum; i++ {
key := s.makeSignedKey(c, false)
host := fmt.Sprintf("host-%v", i)
s.store.AddKey(host, "bob", key)
c.Assert(s.addKey(host, "bob", key), check.IsNil)
key.ProxyHost = host
keys[i] = *key
}
// add 1 key for "sam"
samKey := s.makeSignedKey(c, false)
s.store.AddKey("sam.host", "sam", samKey)
c.Assert(s.addKey("sam.host", "sam", samKey), check.IsNil)

// read all bob keys:
for i := 0; i < keyNum; i++ {
Expand All @@ -115,7 +124,7 @@ func (s *KeyStoreTestSuite) TestKeyCRUD(c *check.C) {
key := s.makeSignedKey(c, false)

// add key:
err := s.store.AddKey("host.a", "bob", key)
err := s.addKey("host.a", "bob", key)
c.Assert(err, check.IsNil)

// load back and compare:
Expand All @@ -140,9 +149,9 @@ func (s *KeyStoreTestSuite) TestDeleteAll(c *check.C) {
key := s.makeSignedKey(c, false)

// add keys
err := s.store.AddKey("proxy.example.com", "foo", key)
err := s.addKey("proxy.example.com", "foo", key)
c.Assert(err, check.IsNil)
err = s.store.AddKey("proxy.example.com", "bar", key)
err = s.addKey("proxy.example.com", "bar", key)
c.Assert(err, check.IsNil)

// check keys exist
Expand Down Expand Up @@ -220,7 +229,7 @@ func (s *KeyStoreTestSuite) makeSignedKey(c *check.C, makeExpired bool) *Key {
}
subject, err := identity.Subject()
c.Assert(err, check.IsNil)
tlsCert, err := s.tlsca.GenerateCertificate(tlsca.CertificateRequest{
tlsCert, err := s.tlsCA.GenerateCertificate(tlsca.CertificateRequest{
Clock: clock,
PublicKey: cryptoPubKey,
Subject: subject,
Expand All @@ -239,10 +248,11 @@ func (s *KeyStoreTestSuite) makeSignedKey(c *check.C, makeExpired bool) *Key {
})
c.Assert(err, check.IsNil)
return &Key{
Priv: priv,
Pub: pub,
Cert: cert,
TLSCert: tlsCert,
Priv: priv,
Pub: pub,
Cert: cert,
TLSCert: tlsCert,
TrustedCA: []auth.TrustedCerts{s.tlsCACert},
}
}

Expand All @@ -256,7 +266,7 @@ func (s *KeyStoreTestSuite) TestCheckKey(c *check.C) {
c.Assert(err, check.IsNil)
key.Cert = ssh.MarshalAuthorizedKey(ellipticCertificate)

err = s.store.AddKey("host.a", "bob", key)
err = s.addKey("host.a", "bob", key)
c.Assert(err, check.IsNil)

_, err = s.store.GetKey("host.a", "bob")
Expand All @@ -278,13 +288,21 @@ func (s *KeyStoreTestSuite) TestCheckKeyFIPS(c *check.C) {
c.Assert(err, check.IsNil)
key.Cert = ssh.MarshalAuthorizedKey(ellipticCertificate)

err = s.store.AddKey("host.a", "bob", key)
err = s.addKey("host.a", "bob", key)
c.Assert(err, check.IsNil)

_, err = s.store.GetKey("host.a", "bob")
c.Assert(err, check.NotNil)
}

func (s *KeyStoreTestSuite) addKey(host, user string, key *Key) error {
if err := s.store.AddKey(host, user, key); err != nil {
return err
}
// Also write the trusted CA certs for the host.
return s.store.SaveCerts(host, []auth.TrustedCerts{s.tlsCACert})
}

var (
CAPriv = []byte(`-----BEGIN RSA PRIVATE KEY-----
MIIEowIBAAKCAQEAwBgwn+vkjCcKEr2fbX1mLN555B9amVYfD/fUZBNbXKpHaqYn
Expand Down
Loading