diff --git a/.drone.yml b/.drone.yml index 4affa9d69826b..49967de8c3c81 100644 --- a/.drone.yml +++ b/.drone.yml @@ -4215,6 +4215,10 @@ steps: image: docker commands: - | + if [ "${DRONE_REPO}" != "gravitational/teleport" ]; then + echo "---> Not publishing ${DRONE_REPO} packages to repos" + exit 78 + fi # length will be 0 after filtering if this is a pre-release, >0 otherwise FILTERED_TAG_LENGTH=$(echo ${DRONE_TAG} | egrep -v '(alpha|beta|dev|rc)' | wc -c) if [ $$FILTERED_TAG_LENGTH -eq 0 ]; then @@ -4381,6 +4385,6 @@ volumes: name: drone-s3-debrepo-pvc --- kind: signature -hmac: 94c64c4b8eb79102ae9e2a619ffdfc76f232ce49b29eb042f907bedd6c26f74e +hmac: 2ed9788b54187803ad301968e2b84c79cdc34de7eab79fe6eed6321ddd4f439e ... diff --git a/CHANGELOG.md b/CHANGELOG.md index eb40ebe67b320..c688b838fd718 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,63 @@ # Changelog +## 7.1.1 + +This release of Teleport contains multiple bug fixes and security fixes. + +* Fixed an issue with starting Teleport with `--bootstrap` flag. [#8128](https://github.com/gravitational/teleport/pull/8128) +* Added support for non-blocking access requests via `--request-nowait` flag. [#7979](https://github.com/gravitational/teleport/pull/7979) +* Added support for a profile specific kubeconfig file. [#8048](https://github.com/gravitational/teleport/pull/8048) + +### Security fixes + +As part of a routine security audit of Teleport, several security vulnerabilities +and miscellaneous issues were discovered. Below are the issues found, their +impact, and the components of Teleport they affect. + +#### Server Access + +An attacker with privileged network position could forge SSH host certificates +that Teleport would incorrectly validate in specific code paths. The specific +paths of concern are: + +* Using `tsh` with an identity file (commonly used for service accounts). This + could lead to potentially leaking of sensitive commands the service account + runs or in the case of proxy recording mode, the attacker could also gain + control of the SSH agent being used. + +* Teleport agents could incorrectly connect to an attacker controlled cluster. + Note, this would not give the attacker access or control of resources (like + SSH, Kubernetes, Applications, or Database servers) because Teleport agents + will still reject all connections without a valid x509 or SSH user + certificate. + +#### Database Access + +When connecting to a Postgres database, an attacker could craft a database name +or a username in a way that would have allowed them control over the resulting +connection string. + +An attacker could have probed connections to other reachable database servers +and alter connection parameters such as disable TLS or connect to a database +authenticated by a password. + +#### All + +During an internal security exercise our engineers have discovered a +vulnerability in Teleport build infrastructure that could have been potentially +used to alter build artifacts. We have found no evidence of any exploitation. In +an effort to be open and transparent with our customers, we encourage all +customers to upgrade to the latest patch release. + +#### Actions + +For all users, we recommend upgrading all components of their Teleport cluster. +If upgrading all components is not possible, we recommend upgrading `tsh` and +Teleport agents (including trusted cluster proxies) that use reverse tunnels. + +Upgrades should follow the normal Teleport upgrade procedure: +https://goteleport.com/teleport/docs/admin-guide/#upgrading-teleport. + ## 7.1.0 This release of Teleport contains a feature and bug fix. diff --git a/Makefile b/Makefile index e603e5e2b4e95..68043c7c622b0 100644 --- a/Makefile +++ b/Makefile @@ -11,7 +11,7 @@ # Stable releases: "1.0.0" # Pre-releases: "1.0.0-alpha.1", "1.0.0-beta.2", "1.0.0-rc.3" # Master/dev branch: "1.0.0-dev" -VERSION=7.1.0 +VERSION=7.1.1 DOCKER_IMAGE ?= quay.io/gravitational/teleport DOCKER_IMAGE_CI ?= quay.io/gravitational/teleport-ci diff --git a/api/constants/constants.go b/api/constants/constants.go index b27e05aa7ea37..d989ede68694e 100644 --- a/api/constants/constants.go +++ b/api/constants/constants.go @@ -102,6 +102,9 @@ const ( AWSConsoleURL = "https://console.aws.amazon.com" // AWSAccountIDLabel is the key of the label containing AWS account ID. AWSAccountIDLabel = "aws_account_id" + + // RSAKeySize is the size of the RSA key. + RSAKeySize = 2048 ) // SecondFactorType is the type of 2FA authentication. diff --git a/api/utils/sshutils/callback.go b/api/utils/sshutils/callback.go new file mode 100644 index 0000000000000..44e97d17d8eac --- /dev/null +++ b/api/utils/sshutils/callback.go @@ -0,0 +1,86 @@ +/* +Copyright 2021 Gravitational, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package sshutils + +import ( + "github.com/gravitational/trace" + "github.com/sirupsen/logrus" + "golang.org/x/crypto/ssh" +) + +// CheckersGetter defines a function that returns a list of ssh public keys. +type CheckersGetter func() ([]ssh.PublicKey, error) + +// HostKeyCallbackConfig is the host key callback configuration. +type HostKeyCallbackConfig struct { + // GetHostCheckers is used to fetch host checking (public) keys. + GetHostCheckers CheckersGetter + // HostKeyFallback sets optional callback to check non-certificate keys. + HostKeyFallback ssh.HostKeyCallback + // FIPS allows to set FIPS mode which will validate algorithms. + FIPS bool + // OnCheckCert is called on SSH certificate validation. + OnCheckCert func(*ssh.Certificate) +} + +// Check validates the config. +func (c *HostKeyCallbackConfig) Check() error { + if c.GetHostCheckers == nil { + return trace.BadParameter("missing GetHostCheckers") + } + return nil +} + +// NewHostKeyCallback returns host key callback function with the specified parameters. +func NewHostKeyCallback(conf HostKeyCallbackConfig) (ssh.HostKeyCallback, error) { + if err := conf.Check(); err != nil { + return nil, trace.Wrap(err) + } + checker := CertChecker{ + CertChecker: ssh.CertChecker{ + IsHostAuthority: makeIsHostAuthorityFunc(conf.GetHostCheckers), + HostKeyFallback: conf.HostKeyFallback, + }, + FIPS: conf.FIPS, + OnCheckCert: conf.OnCheckCert, + } + return checker.CheckHostKey, nil +} + +func makeIsHostAuthorityFunc(getCheckers CheckersGetter) func(key ssh.PublicKey, host string) bool { + return func(key ssh.PublicKey, host string) bool { + checkers, err := getCheckers() + if err != nil { + logrus.WithError(err).Errorf("Failed to get checkers for %v.", host) + return false + } + for _, checker := range checkers { + switch v := key.(type) { + case *ssh.Certificate: + if KeysEqual(v.SignatureKey, checker) { + return true + } + default: + if KeysEqual(key, checker) { + return true + } + } + } + logrus.Debugf("No CA for host %v.", host) + return false + } +} diff --git a/api/utils/sshutils/checker.go b/api/utils/sshutils/checker.go new file mode 100644 index 0000000000000..6d8c43d731ae4 --- /dev/null +++ b/api/utils/sshutils/checker.go @@ -0,0 +1,130 @@ +/* +Copyright 2019-2021 Gravitational, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package sshutils + +import ( + "crypto/rsa" + "net" + + "github.com/gravitational/teleport/api/constants" + + "github.com/gravitational/trace" + "golang.org/x/crypto/ssh" +) + +// CertChecker is a drop-in replacement for ssh.CertChecker. In FIPS mode, +// checks if the certificate (or key) were generated with a supported algorithm. +type CertChecker struct { + ssh.CertChecker + + // FIPS means in addition to checking the validity of the key or + // certificate, also check that FIPS 140-2 algorithms were used. + FIPS bool + + // OnCheckCert is called when validating host certificate. + OnCheckCert func(*ssh.Certificate) +} + +// Authenticate checks the validity of a user certificate. +func (c *CertChecker) Authenticate(conn ssh.ConnMetadata, key ssh.PublicKey) (*ssh.Permissions, error) { + err := c.validateFIPS(key) + if err != nil { + return nil, trace.Wrap(err) + } + + perms, err := c.CertChecker.Authenticate(conn, key) + if err != nil { + return nil, trace.Wrap(err) + } + + return perms, nil +} + +// CheckCert checks certificate metadata and signature. +func (c *CertChecker) CheckCert(principal string, cert *ssh.Certificate) error { + err := c.validateFIPS(cert) + if err != nil { + return trace.Wrap(err) + } + + err = c.CertChecker.CheckCert(principal, cert) + if err != nil { + return trace.Wrap(err) + } + + if c.OnCheckCert != nil { + c.OnCheckCert(cert) + } + + return nil +} + +// CheckHostKey checks the validity of a host certificate. +func (c *CertChecker) CheckHostKey(addr string, remote net.Addr, key ssh.PublicKey) error { + err := c.validateFIPS(key) + if err != nil { + return trace.Wrap(err) + } + + err = c.CertChecker.CheckHostKey(addr, remote, key) + if err != nil { + return trace.Wrap(err) + } + + if cert, ok := key.(*ssh.Certificate); ok && c.OnCheckCert != nil { + c.OnCheckCert(cert) + } + + return nil +} + +func (c *CertChecker) validateFIPS(key ssh.PublicKey) error { + // When not in FIPS mode, accept all algorithms and key sizes. + if !c.FIPS { + return nil + } + + switch cert := key.(type) { + case *ssh.Certificate: + err := validateFIPSAlgorithm(cert.Key) + if err != nil { + return trace.Wrap(err) + } + err = validateFIPSAlgorithm(cert.SignatureKey) + if err != nil { + return trace.Wrap(err) + } + return nil + default: + return validateFIPSAlgorithm(key) + } +} + +func validateFIPSAlgorithm(key ssh.PublicKey) error { + cryptoKey, ok := key.(ssh.CryptoPublicKey) + if !ok { + return trace.BadParameter("unable to determine underlying public key") + } + k, ok := cryptoKey.CryptoPublicKey().(*rsa.PublicKey) + if !ok { + return trace.BadParameter("only RSA keys supported") + } + if k.N.BitLen() != constants.RSAKeySize { + return trace.BadParameter("found %v-bit key, only %v-bit supported", k.N.BitLen(), constants.RSAKeySize) + } + return nil +} diff --git a/lib/utils/checker_test.go b/api/utils/sshutils/checker_test.go similarity index 57% rename from lib/utils/checker_test.go rename to api/utils/sshutils/checker_test.go index ce9c92929e9a7..f3849907938ad 100644 --- a/lib/utils/checker_test.go +++ b/api/utils/sshutils/checker_test.go @@ -1,5 +1,5 @@ /* -Copyright 2019 Gravitational, Inc. +Copyright 2019-2021 Gravitational, Inc. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -14,90 +14,86 @@ See the License for the specific language governing permissions and limitations under the License. */ -package utils +package sshutils import ( "crypto/ecdsa" "crypto/elliptic" "crypto/rand" "crypto/rsa" + "testing" - "golang.org/x/crypto/ssh" - - "github.com/gravitational/teleport" + "github.com/gravitational/teleport/api/constants" - "gopkg.in/check.v1" + "github.com/stretchr/testify/require" + "golang.org/x/crypto/ssh" ) -type CheckerSuite struct{} - -var _ = check.Suite(&CheckerSuite{}) - -// TestValidate checks what algorithm are supported in regular (non-FIPS) mode. -func (s *CheckerSuite) TestValidate(c *check.C) { +// TestCheckerValidate checks what algorithm are supported in regular (non-FIPS) mode. +func TestCheckerValidate(t *testing.T) { checker := CertChecker{} - rsaKey, err := rsa.GenerateKey(rand.Reader, teleport.RSAKeySize) - c.Assert(err, check.IsNil) + rsaKey, err := rsa.GenerateKey(rand.Reader, constants.RSAKeySize) + require.NoError(t, err) smallRSAKey, err := rsa.GenerateKey(rand.Reader, 1024) - c.Assert(err, check.IsNil) + require.NoError(t, err) ellipticKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) - c.Assert(err, check.IsNil) + require.NoError(t, err) // 2048-bit RSA keys are valid. cryptoKey := rsaKey.Public() sshKey, err := ssh.NewPublicKey(cryptoKey) - c.Assert(err, check.IsNil) - err = checker.validate(sshKey) - c.Assert(err, check.IsNil) + require.NoError(t, err) + err = checker.validateFIPS(sshKey) + require.NoError(t, err) // 1024-bit RSA keys are valid. cryptoKey = smallRSAKey.Public() sshKey, err = ssh.NewPublicKey(cryptoKey) - c.Assert(err, check.IsNil) - err = checker.validate(sshKey) - c.Assert(err, check.IsNil) + require.NoError(t, err) + err = checker.validateFIPS(sshKey) + require.NoError(t, err) // ECDSA keys are valid. cryptoKey = ellipticKey.Public() sshKey, err = ssh.NewPublicKey(cryptoKey) - c.Assert(err, check.IsNil) - err = checker.validate(sshKey) - c.Assert(err, check.IsNil) + require.NoError(t, err) + err = checker.validateFIPS(sshKey) + require.NoError(t, err) } -// TestValidate makes sure the public key is a valid algorithm +// TestCheckerValidateFIPS makes sure the public key is a valid algorithm // that Teleport supports while in FIPS mode. -func (s *CheckerSuite) TestValidateFIPS(c *check.C) { +func TestCheckerValidateFIPS(t *testing.T) { checker := CertChecker{ FIPS: true, } - rsaKey, err := rsa.GenerateKey(rand.Reader, teleport.RSAKeySize) - c.Assert(err, check.IsNil) + rsaKey, err := rsa.GenerateKey(rand.Reader, constants.RSAKeySize) + require.NoError(t, err) smallRSAKey, err := rsa.GenerateKey(rand.Reader, 1024) - c.Assert(err, check.IsNil) + require.NoError(t, err) ellipticKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) - c.Assert(err, check.IsNil) + require.NoError(t, err) // 2048-bit RSA keys are valid. cryptoKey := rsaKey.Public() sshKey, err := ssh.NewPublicKey(cryptoKey) - c.Assert(err, check.IsNil) - err = checker.validate(sshKey) - c.Assert(err, check.IsNil) + require.NoError(t, err) + err = checker.validateFIPS(sshKey) + require.NoError(t, err) // 1024-bit RSA keys are not valid. cryptoKey = smallRSAKey.Public() sshKey, err = ssh.NewPublicKey(cryptoKey) - c.Assert(err, check.IsNil) - err = checker.validate(sshKey) - c.Assert(err, check.NotNil) + require.NoError(t, err) + err = checker.validateFIPS(sshKey) + require.Error(t, err) // ECDSA keys are not valid. cryptoKey = ellipticKey.Public() sshKey, err = ssh.NewPublicKey(cryptoKey) - c.Assert(err, check.IsNil) - err = checker.validate(sshKey) - c.Assert(err, check.NotNil) + require.NoError(t, err) + err = checker.validateFIPS(sshKey) + require.Error(t, err) } diff --git a/api/utils/sshutils/ssh.go b/api/utils/sshutils/ssh.go index 669fb4226f988..1daf0c63fd06d 100644 --- a/api/utils/sshutils/ssh.go +++ b/api/utils/sshutils/ssh.go @@ -48,6 +48,37 @@ func ParseCertificate(buf []byte) (*ssh.Certificate, error) { return cert, nil } +// ParseKnownHosts parses provided known_hosts entries into ssh.PublicKey list. +func ParseKnownHosts(knownHosts [][]byte) ([]ssh.PublicKey, error) { + var keys []ssh.PublicKey + for _, line := range knownHosts { + for { + _, _, publicKey, _, bytes, err := ssh.ParseKnownHosts(line) + if err == io.EOF { + break + } else if err != nil { + return nil, trace.Wrap(err, "failed parsing known hosts: %v; raw line: %q", err, line) + } + keys = append(keys, publicKey) + line = bytes + } + } + return keys, nil +} + +// ParseAuthorizedKeys parses provided authorized_keys entries into ssh.PublicKey list. +func ParseAuthorizedKeys(authorizedKeys [][]byte) ([]ssh.PublicKey, error) { + var keys []ssh.PublicKey + for _, line := range authorizedKeys { + publicKey, _, _, _, err := ssh.ParseAuthorizedKey(line) + if err != nil { + return nil, trace.Wrap(err, "failed parsing authorized keys: %v; raw line: %q", err, line) + } + keys = append(keys, publicKey) + } + return keys, nil +} + // ProxyClientSSHConfig returns an ssh.ClientConfig with SSH credentials from this // Key and HostKeyCallback matching SSH CAs in the Key. // @@ -64,7 +95,7 @@ func ProxyClientSSHConfig(sshCert, privKey []byte, caCerts [][]byte) (*ssh.Clien return nil, trace.Wrap(err, "failed to convert key pair to auth method") } - hostKeyCallback, err := HostKeyCallback(caCerts) + hostKeyCallback, err := HostKeyCallback(caCerts, false) if err != nil { return nil, trace.Wrap(err, "failed to convert certificate authorities to HostKeyCallback") } @@ -172,19 +203,10 @@ func AsAgentKeys(sshCert *ssh.Certificate, privKey []byte) ([]agent.AddedKey, er // If not CAs are present in the Key, the returned ssh.HostKeyCallback is nil. // This causes golang.org/x/crypto/ssh to prompt the user to verify host key // fingerprint (same as OpenSSH does for an unknown host). -func HostKeyCallback(caCerts [][]byte) (ssh.HostKeyCallback, error) { - var trustedKeys []ssh.PublicKey - for _, caCert := range caCerts { - for { - _, _, publicKey, _, bytes, err := ssh.ParseKnownHosts(caCert) - if err == io.EOF { - break - } else if err != nil { - return nil, trace.Wrap(err, "failed parsing CA cert: %v; raw CA cert line: %q", err, caCert) - } - trustedKeys = append(trustedKeys, publicKey) - caCert = bytes - } +func HostKeyCallback(caCerts [][]byte, withHostKeyFallback bool) (ssh.HostKeyCallback, error) { + trustedKeys, err := ParseKnownHosts(caCerts) + if err != nil { + return nil, trace.Wrap(err) } // No CAs are provided, return a nil callback which will prompt the user @@ -193,18 +215,33 @@ func HostKeyCallback(caCerts [][]byte) (ssh.HostKeyCallback, error) { return nil, nil } - return func(host string, a net.Addr, hostKey ssh.PublicKey) error { - clusterCert, ok := hostKey.(*ssh.Certificate) - if ok { - hostKey = clusterCert.SignatureKey - } - for _, trustedKey := range trustedKeys { - if KeysEqual(trustedKey, hostKey) { + callbackConfig := HostKeyCallbackConfig{ + GetHostCheckers: func() ([]ssh.PublicKey, error) { + return trustedKeys, nil + }, + } + + if withHostKeyFallback { + callbackConfig.HostKeyFallback = hostKeyFallbackFunc(trustedKeys) + } + + callback, err := NewHostKeyCallback(callbackConfig) + if err != nil { + return nil, trace.Wrap(err) + } + + return callback, nil +} + +func hostKeyFallbackFunc(knownHosts []ssh.PublicKey) func(hostname string, remote net.Addr, key ssh.PublicKey) error { + return func(hostname string, remote net.Addr, key ssh.PublicKey) error { + for _, knownHost := range knownHosts { + if KeysEqual(key, knownHost) { return nil } } - return trace.AccessDenied("host %v is untrusted or Teleport CA has been rotated; try getting new credentials by logging in again ('tsh login') or re-exporting the identity file ('tctl auth sign' or 'tsh login -o'), depending on how you got them initially", host) - }, nil + return trace.AccessDenied("host %v presented a public key instead of a host certificate which isn't among known hosts", hostname) + } } // KeysEqual is constant time compare of the keys to avoid timing attacks diff --git a/api/utils/sshutils/ssh_test.go b/api/utils/sshutils/ssh_test.go new file mode 100644 index 0000000000000..c4f84013a7741 --- /dev/null +++ b/api/utils/sshutils/ssh_test.go @@ -0,0 +1,55 @@ +/* +Copyright 2021 Gravitational, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package sshutils + +import ( + "encoding/base64" + "fmt" + "testing" + + "github.com/stretchr/testify/require" + "golang.org/x/crypto/ssh" +) + +// TestHostKeyCallback verifies that host key callback properly validates +// host certificates. +func TestHostKeyCallback(t *testing.T) { + ca, err := MakeTestSSHCA() + require.NoError(t, err) + + realCert, err := MakeRealHostCert(ca) + require.NoError(t, err) + + spoofedCert, err := MakeSpoofedHostCert(ca) + require.NoError(t, err) + + hostKeyCallback, err := HostKeyCallback([][]byte{ + []byte(makeKnownHostsLine("127.0.0.1", ca.PublicKey())), + }, false) + require.NoError(t, err) + + err = hostKeyCallback("127.0.0.1:3022", nil, realCert.PublicKey()) + require.NoError(t, err, "host key callback rejected valid host certificate") + + err = hostKeyCallback("127.0.0.1:3022", nil, spoofedCert.PublicKey()) + require.Error(t, err, "host key callback accepted spoofed host certificate") +} + +func makeKnownHostsLine(host string, key ssh.PublicKey) string { + return fmt.Sprintf("%v %v %v", host, key.Type(), + base64.StdEncoding.EncodeToString(key.Marshal())) +} diff --git a/api/utils/sshutils/test.go b/api/utils/sshutils/test.go new file mode 100644 index 0000000000000..50e89c9d56e82 --- /dev/null +++ b/api/utils/sshutils/test.go @@ -0,0 +1,110 @@ +/* +Copyright 2021 Gravitational, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package sshutils + +import ( + "crypto/rand" + "crypto/rsa" + "time" + + "github.com/gravitational/teleport/api/constants" + + "github.com/gravitational/trace" + "golang.org/x/crypto/ssh" +) + +// MakeTestSSHCA generates a new SSH certificate authority for tests. +func MakeTestSSHCA() (ssh.Signer, error) { + privateKey, err := rsa.GenerateKey(rand.Reader, constants.RSAKeySize) + if err != nil { + return nil, trace.Wrap(err) + } + ca, err := ssh.NewSignerFromKey(privateKey) + if err != nil { + return nil, trace.Wrap(err) + } + return ca, nil +} + +// MakeSpoofedHostCert makes an SSH host certificate that claims to be signed +// by the provided CA but in fact is signed by a different CA. +func MakeSpoofedHostCert(realCA ssh.Signer) (ssh.Signer, error) { + fakeCA, err := MakeTestSSHCA() + if err != nil { + return nil, trace.Wrap(err) + } + return makeHostCert(realCA.PublicKey(), fakeCA) +} + +// MakeRealHostCert makes an SSH host certificate that is signed by the +// provided CA. +func MakeRealHostCert(realCA ssh.Signer) (ssh.Signer, error) { + return makeHostCert(realCA.PublicKey(), realCA) +} + +func makeHostCert(signKey ssh.PublicKey, signer ssh.Signer) (ssh.Signer, error) { + priv, err := rsa.GenerateKey(rand.Reader, constants.RSAKeySize) + if err != nil { + return nil, trace.Wrap(err) + } + + privSigner, err := ssh.NewSignerFromKey(priv) + if err != nil { + return nil, trace.Wrap(err) + } + + pub, err := ssh.NewPublicKey(priv.Public()) + if err != nil { + return nil, trace.Wrap(err) + } + + nonce := make([]byte, 32) + if _, err = rand.Read(nonce); err != nil { + return nil, trace.Wrap(err) + } + + cert := &ssh.Certificate{ + Nonce: nonce, + Key: pub, + CertType: ssh.HostCert, + SignatureKey: signKey, + ValidPrincipals: []string{"127.0.0.1"}, + ValidBefore: uint64(time.Now().Add(time.Hour).Unix()), + } + + // We cannot use ssh.Certificate SignCert method since we're intentionally + // setting invalid signature key to make a spoofed cert in some tests. + // + // When marshaling cert for signing, last 4 bytes containing trailing + // signature length are dropped: + // + // https://cs.opensource.google/go/x/crypto/+/32db7946:ssh/certs.go;l=456-462 + bytesForSigning := cert.Marshal() + bytesForSigning = bytesForSigning[:len(bytesForSigning)-4] + + cert.Signature, err = signer.Sign(rand.Reader, bytesForSigning) + if err != nil { + return nil, trace.Wrap(err) + } + + certSigner, err := ssh.NewCertSigner(cert, privSigner) + if err != nil { + return nil, trace.Wrap(err) + } + + return certSigner, nil +} diff --git a/api/version.go b/api/version.go index 9a2cbac68068e..c542ac57bdd5e 100644 --- a/api/version.go +++ b/api/version.go @@ -3,7 +3,7 @@ package api const ( - Version = "7.1.0" + Version = "7.1.1" ) // Gitref variable is automatically set to the output of git-describe diff --git a/constants.go b/constants.go index 3548cbfe57e86..c6e9e0a90b665 100644 --- a/constants.go +++ b/constants.go @@ -680,9 +680,6 @@ const ( ChanSession = "session" ) -// RSAKeySize is the size of the RSA key. -const RSAKeySize = 2048 - // A principal name for use in SSH certificates. type Principal string diff --git a/examples/chart/teleport-cluster/Chart.yaml b/examples/chart/teleport-cluster/Chart.yaml index 306724bca23ca..fdc7b1c7e11d4 100644 --- a/examples/chart/teleport-cluster/Chart.yaml +++ b/examples/chart/teleport-cluster/Chart.yaml @@ -1,7 +1,7 @@ name: teleport-cluster apiVersion: v2 -version: "7.1.0" -appVersion: "7.1.0" +version: "7.1.1" +appVersion: "7.1.1" description: Teleport is a unified access plane for your infrastructure icon: https://goteleport.com/images/logos/logo-teleport-square.svg keywords: diff --git a/examples/chart/teleport-kube-agent/Chart.yaml b/examples/chart/teleport-kube-agent/Chart.yaml index 6fffd1577da3e..b9a49fe785aab 100644 --- a/examples/chart/teleport-kube-agent/Chart.yaml +++ b/examples/chart/teleport-kube-agent/Chart.yaml @@ -1,7 +1,7 @@ name: teleport-kube-agent apiVersion: v2 -version: "7.1.0" -appVersion: "7.1.0" +version: "7.1.1" +appVersion: "7.1.1" description: Teleport provides a secure SSH and Kubernetes remote access solution that doesn't get in the way. icon: https://goteleport.com/images/logos/logo-teleport-square.svg keywords: diff --git a/lib/auth/auth_test.go b/lib/auth/auth_test.go index d0841cf6cc235..4dd88a484ad55 100644 --- a/lib/auth/auth_test.go +++ b/lib/auth/auth_test.go @@ -1007,7 +1007,7 @@ func (s *AuthSuite) TestSAMLConnectorCRUDEventsEmitted(c *C) { ca, err := tlsca.FromKeys([]byte(fixtures.TLSCACertPEM), []byte(fixtures.TLSCAKeyPEM)) c.Assert(err, IsNil) - privateKey, err := rsa.GenerateKey(rand.Reader, teleport.RSAKeySize) + privateKey, err := rsa.GenerateKey(rand.Reader, constants.RSAKeySize) c.Assert(err, IsNil) testClock := clockwork.NewFakeClock() diff --git a/lib/auth/init.go b/lib/auth/init.go index b121f4dd03c62..1c13758b31056 100644 --- a/lib/auth/init.go +++ b/lib/auth/init.go @@ -23,7 +23,6 @@ import ( "crypto/x509" "crypto/x509/pkix" "fmt" - "net" "strings" "time" @@ -999,39 +998,31 @@ func (i *Identity) TLSConfig(cipherSuites []uint16) (*tls.Config, error) { return tlsConfig, nil } -// SSHClientConfig returns a ssh.ClientConfig used by nodes to connect to -// the reverse tunnel server. -func (i *Identity) SSHClientConfig() *ssh.ClientConfig { - return &ssh.ClientConfig{ - User: i.ID.HostUUID, - Auth: []ssh.AuthMethod{ - ssh.PublicKeys(i.KeySigner), - }, - HostKeyCallback: i.hostKeyCallback, - Timeout: apidefaults.DefaultDialTimeout, +func (i *Identity) getSSHCheckers() ([]ssh.PublicKey, error) { + checkers, err := apisshutils.ParseAuthorizedKeys(i.SSHCACertBytes) + if err != nil { + return nil, trace.Wrap(err) } + return checkers, nil } -// hostKeyCallback checks if the host certificate was signed by any of the -// known CAs. -func (i *Identity) hostKeyCallback(hostname string, remote net.Addr, key ssh.PublicKey) error { - cert, ok := key.(*ssh.Certificate) - if !ok { - return trace.BadParameter("only host certificates supported") - } - - // Loop over all CAs and see if any of them signed the certificate. - for _, k := range i.SSHCACertBytes { - pubkey, _, _, _, err := ssh.ParseAuthorizedKey(k) - if err != nil { - return trace.Wrap(err) - } - if apisshutils.KeysEqual(cert.SignatureKey, pubkey) { - return nil - } +// SSHClientConfig returns a ssh.ClientConfig used by nodes to connect to +// the reverse tunnel server. +func (i *Identity) SSHClientConfig(fips bool) (*ssh.ClientConfig, error) { + callback, err := apisshutils.NewHostKeyCallback( + apisshutils.HostKeyCallbackConfig{ + GetHostCheckers: i.getSSHCheckers, + FIPS: fips, + }) + if err != nil { + return nil, trace.Wrap(err) } - - return trace.BadParameter("no matching keys found") + return &ssh.ClientConfig{ + User: i.ID.HostUUID, + Auth: []ssh.AuthMethod{ssh.PublicKeys(i.KeySigner)}, + HostKeyCallback: callback, + Timeout: apidefaults.DefaultDialTimeout, + }, nil } // IdentityID is a combination of role, host UUID, and node name. diff --git a/lib/auth/init_test.go b/lib/auth/init_test.go index 826d0097cf5fb..33e8b8da55e50 100644 --- a/lib/auth/init_test.go +++ b/lib/auth/init_test.go @@ -28,8 +28,6 @@ import ( "testing" "time" - "github.com/google/go-cmp/cmp" - "github.com/google/go-cmp/cmp/cmpopts" "github.com/gravitational/teleport" "github.com/gravitational/teleport/api/constants" "github.com/gravitational/teleport/api/types" @@ -45,8 +43,13 @@ import ( "github.com/gravitational/teleport/lib/services/suite" "github.com/gravitational/teleport/lib/sshutils" "github.com/gravitational/teleport/lib/utils" + "github.com/gravitational/teleport/lib/utils/proxy" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/gravitational/trace" "github.com/jonboulle/clockwork" + "github.com/pborman/uuid" "github.com/stretchr/testify/require" "golang.org/x/crypto/ssh" kyaml "k8s.io/apimachinery/pkg/util/yaml" @@ -1162,3 +1165,99 @@ func resourceDiff(res1, res2 types.Resource) string { cmpopts.IgnoreFields(types.Metadata{}, "ID", "Namespace"), cmpopts.EquateEmpty()) } + +// TestIdentityChecker verifies auth identity properly validates host +// certificates when connecting to an SSH server. +func TestIdentityChecker(t *testing.T) { + ctx := context.Background() + + conf := setupConfig(t) + authServer, err := Init(conf) + require.NoError(t, err) + t.Cleanup(func() { authServer.Close() }) + + lockWatcher, err := services.NewLockWatcher(ctx, services.LockWatcherConfig{ + ResourceWatcherConfig: services.ResourceWatcherConfig{ + Component: teleport.ComponentAuth, + Client: authServer, + }, + }) + require.NoError(t, err) + authServer.SetLockWatcher(lockWatcher) + + clusterName, err := authServer.GetDomainName() + require.NoError(t, err) + + ca, err := authServer.GetCertAuthority(types.CertAuthID{ + Type: types.HostCA, + DomainName: clusterName, + }, true) + require.NoError(t, err) + + signers, err := sshutils.GetSigners(ca) + require.NoError(t, err) + require.Len(t, signers, 1) + + realCert, err := apisshutils.MakeRealHostCert(signers[0]) + require.NoError(t, err) + + spoofedCert, err := apisshutils.MakeSpoofedHostCert(signers[0]) + require.NoError(t, err) + + tests := []struct { + desc string + cert ssh.Signer + err bool + }{ + { + desc: "should be able to connect with real cert", + cert: realCert, + err: false, + }, + { + desc: "should not be able to connect with spoofed cert", + cert: spoofedCert, + err: true, + }, + } + + for _, test := range tests { + t.Run(test.desc, func(t *testing.T) { + handler := sshutils.NewChanHandlerFunc(func(_ context.Context, ccx *sshutils.ConnectionContext, nch ssh.NewChannel) { + ch, _, err := nch.Accept() + require.NoError(t, err) + require.NoError(t, ch.Close()) + }) + sshServer, err := sshutils.NewServer( + "test", + utils.NetAddr{AddrNetwork: "tcp", Addr: "localhost:0"}, + handler, + []ssh.Signer{test.cert}, + sshutils.AuthMethods{NoClient: true}, + sshutils.SetInsecureSkipHostValidation(), + ) + require.NoError(t, err) + t.Cleanup(func() { sshServer.Close() }) + require.NoError(t, sshServer.Start()) + + identity, err := GenerateIdentity(authServer, IdentityID{ + Role: types.RoleNode, + HostUUID: uuid.New(), + NodeName: "node-1", + }, nil, nil) + require.NoError(t, err) + + sshClientConfig, err := identity.SSHClientConfig(false) + require.NoError(t, err) + + dialer := proxy.DialerFromEnvironment(sshServer.Addr()) + sconn, err := dialer.Dial("tcp", sshServer.Addr(), sshClientConfig) + if test.err { + require.Error(t, err) + } else { + require.NoError(t, err) + require.NoError(t, sconn.Close()) + } + }) + } +} diff --git a/lib/auth/native/native.go b/lib/auth/native/native.go index cfdafe616a2d6..cccd79453c624 100644 --- a/lib/auth/native/native.go +++ b/lib/auth/native/native.go @@ -150,7 +150,7 @@ func (k *Keygen) precomputeKeys() { // GenerateKeyPair returns fresh priv/pub keypair, takes about 300ms to // execute. func GenerateKeyPair(passphrase string) ([]byte, []byte, error) { - priv, err := rsa.GenerateKey(rand.Reader, teleport.RSAKeySize) + priv, err := rsa.GenerateKey(rand.Reader, constants.RSAKeySize) if err != nil { return nil, nil, err } diff --git a/lib/client/api.go b/lib/client/api.go index 238a1b210163b..f6c491c397a56 100644 --- a/lib/client/api.go +++ b/lib/client/api.go @@ -1087,7 +1087,13 @@ func NewClient(c *Config) (tc *TeleportClient, err error) { return nil, trace.Wrap(err) } - tc.localAgent, err = NewLocalAgent(keystore, webProxyHost, c.Username, c.AddKeysToAgent) + tc.localAgent, err = NewLocalAgent(LocalAgentConfig{ + Keystore: keystore, + ProxyHost: webProxyHost, + Username: c.Username, + KeysOption: c.AddKeysToAgent, + Insecure: c.InsecureSkipVerify, + }) if err != nil { return nil, trace.Wrap(err) } @@ -2028,7 +2034,7 @@ func (tc *TeleportClient) connectToProxy(ctx context.Context) (*ProxyClient, err signers, err := tc.localAgent.certsForCluster("") // errNoLocalKeyStore is returned when running in the proxy. The proxy // should be passing auth methods via tc.Config.AuthMethods. - if err != nil && !errors.Is(err, errNoLocalKeyStore) { + if err != nil && !errors.Is(err, errNoLocalKeyStore) && !trace.IsNotFound(err) { return nil, trace.Wrap(err) } if len(signers) > 0 { diff --git a/lib/client/interfaces.go b/lib/client/interfaces.go index 3d32dc5a2cf3a..4adca8ec4fbc1 100644 --- a/lib/client/interfaces.go +++ b/lib/client/interfaces.go @@ -404,7 +404,7 @@ func (k *Key) CheckCert() error { // A valid principal is always passed in because the principals are not being // checked here, but rather the validity period, signature, and algorithms. - certChecker := utils.CertChecker{ + certChecker := sshutils.CertChecker{ FIPS: isFIPS(), } err = certChecker.CheckCert(cert.ValidPrincipals[0], cert) @@ -421,8 +421,8 @@ func (k *Key) CheckCert() error { // If not CAs are present in the Key, the returned ssh.HostKeyCallback is nil. // This causes golang.org/x/crypto/ssh to prompt the user to verify host key // fingerprint (same as OpenSSH does for an unknown host). -func (k *Key) HostKeyCallback() (ssh.HostKeyCallback, error) { - return sshutils.HostKeyCallback(k.SSHCAs()) +func (k *Key) HostKeyCallback(withHostKeyFallback bool) (ssh.HostKeyCallback, error) { + return sshutils.HostKeyCallback(k.SSHCAs(), withHostKeyFallback) } // RootClusterName extracts the root cluster name from the issuer diff --git a/lib/client/keyagent.go b/lib/client/keyagent.go index 220ed7cc645c1..37d7bfbc351fa 100644 --- a/lib/client/keyagent.go +++ b/lib/client/keyagent.go @@ -33,7 +33,6 @@ import ( "github.com/gravitational/teleport" "github.com/gravitational/teleport/api/utils/sshutils" "github.com/gravitational/teleport/lib/auth" - "github.com/gravitational/teleport/lib/utils" "github.com/gravitational/teleport/lib/utils/prompt" "github.com/sirupsen/logrus" @@ -66,6 +65,9 @@ type LocalKeyAgent struct { // proxyHost is the proxy for the cluster that his key agent holds keys for. proxyHost string + + // insecure allows to accept public host keys. + insecure bool } // NewKeyStoreCertChecker returns a new certificate checker @@ -74,7 +76,7 @@ func NewKeyStoreCertChecker(keyStore LocalKeyStore) ssh.HostKeyCallback { // CheckHostSignature checks if the given host key was signed by a Teleport // certificate authority (CA) or a host certificate the user has seen before. return func(addr string, remote net.Addr, key ssh.PublicKey) error { - certChecker := utils.CertChecker{ + certChecker := sshutils.CertChecker{ CertChecker: ssh.CertChecker{ IsHostAuthority: func(key ssh.PublicKey, addr string) bool { keys, err := keyStore.GetKnownHostKeys("") @@ -118,21 +120,31 @@ func shouldAddKeysToAgent(addKeysToAgent string) bool { return (addKeysToAgent == AddKeysToAgentAuto && agentSupportsSSHCertificates()) || addKeysToAgent == AddKeysToAgentOnly || addKeysToAgent == AddKeysToAgentYes } +// LocalAgentConfig contains parameters for creating the local keys agent. +type LocalAgentConfig struct { + Keystore LocalKeyStore + ProxyHost string + Username string + KeysOption string + Insecure bool +} + // NewLocalAgent reads all available credentials from the provided LocalKeyStore // and loads them into the local and system agent -func NewLocalAgent(keystore LocalKeyStore, proxyHost, username string, keysOption string) (a *LocalKeyAgent, err error) { +func NewLocalAgent(conf LocalAgentConfig) (a *LocalKeyAgent, err error) { a = &LocalKeyAgent{ log: logrus.WithFields(logrus.Fields{ trace.Component: teleport.ComponentKeyAgent, }), Agent: agent.NewKeyring(), - keyStore: keystore, + keyStore: conf.Keystore, noHosts: make(map[string]bool), - username: username, - proxyHost: proxyHost, + username: conf.Username, + proxyHost: conf.ProxyHost, + insecure: conf.Insecure, } - if shouldAddKeysToAgent(keysOption) { + if shouldAddKeysToAgent(conf.KeysOption) { a.sshAgent = connectToSSHAgent() } else { log.Debug("Skipping connection to the local ssh-agent.") @@ -315,13 +327,14 @@ func (a *LocalKeyAgent) UserRefusedHosts() bool { // CheckHostSignature checks if the given host key was signed by a Teleport // certificate authority (CA) or a host certificate the user has seen before. func (a *LocalKeyAgent) CheckHostSignature(addr string, remote net.Addr, key ssh.PublicKey) error { - certChecker := utils.CertChecker{ + certChecker := sshutils.CertChecker{ CertChecker: ssh.CertChecker{ IsHostAuthority: a.checkHostCertificate, HostKeyFallback: a.checkHostKey, }, FIPS: isFIPS(), } + a.log.Debugf("Checking key: %s.", ssh.MarshalAuthorizedKey(key)) err := certChecker.CheckHostKey(addr, remote, key) if err != nil { a.log.Debugf("Host validation failed: %v.", err) @@ -360,6 +373,15 @@ func (a *LocalKeyAgent) checkHostCertificate(key ssh.PublicKey, addr string) boo func (a *LocalKeyAgent) checkHostKey(addr string, remote net.Addr, key ssh.PublicKey) error { var err error + // Unless --insecure flag was given, prohibit public keys or host certs + // not signed by Teleport. + if !a.insecure { + a.log.Debugf("Host %s presented a public key not signed by Teleport. Rejecting due to insecure mode being OFF.", addr) + return trace.BadParameter("host %s presented a public key not signed by Teleport", addr) + } + + a.log.Warnf("Host %s presented a public key not signed by Teleport. Proceeding due to insecure mode being ON.", addr) + // Check if this exact host is in the local cache. keys, _ := a.keyStore.GetKnownHostKeys(addr) if len(keys) > 0 && sshutils.KeysEqual(key, keys[0]) { @@ -528,7 +550,7 @@ func (a *LocalKeyAgent) certsForCluster(clusterName string) ([]ssh.Signer, error certs = append(certs, s) } if len(certs) == 0 { - return nil, trace.BadParameter("no auth method available") + return nil, trace.NotFound("no auth method available") } return certs, nil } diff --git a/lib/client/keyagent_test.go b/lib/client/keyagent_test.go index 70a894e31164d..acf0b4b0ff088 100644 --- a/lib/client/keyagent_test.go +++ b/lib/client/keyagent_test.go @@ -105,7 +105,13 @@ func (s *KeyAgentTestSuite) TestAddKey(c *check.C) { // make a new local agent keystore, err := NewFSLocalKeyStore(s.keyDir) c.Assert(err, check.IsNil) - lka, err := NewLocalAgent(keystore, s.hostname, s.username, AddKeysToAgentAuto) + lka, err := NewLocalAgent( + LocalAgentConfig{ + Keystore: keystore, + ProxyHost: s.hostname, + Username: s.username, + KeysOption: AddKeysToAgentAuto, + }) c.Assert(err, check.IsNil) // add the key to the local agent, this should write the key @@ -172,7 +178,12 @@ func (s *KeyAgentTestSuite) TestLoadKey(c *check.C) { // make a new local agent keystore, err := NewFSLocalKeyStore(s.keyDir) c.Assert(err, check.IsNil) - lka, err := NewLocalAgent(keystore, s.hostname, s.username, AddKeysToAgentAuto) + lka, err := NewLocalAgent(LocalAgentConfig{ + Keystore: keystore, + ProxyHost: s.hostname, + Username: s.username, + KeysOption: AddKeysToAgentAuto, + }) c.Assert(err, check.IsNil) // unload any keys that might be in the agent for this user @@ -232,7 +243,12 @@ func (s *KeyAgentTestSuite) TestHostCertVerification(c *check.C) { // Make a new local agent. keystore, err := NewFSLocalKeyStore(s.keyDir) c.Assert(err, check.IsNil) - lka, err := NewLocalAgent(keystore, s.hostname, s.username, AddKeysToAgentAuto) + lka, err := NewLocalAgent(LocalAgentConfig{ + Keystore: keystore, + ProxyHost: s.hostname, + Username: s.username, + KeysOption: AddKeysToAgentAuto, + }) c.Assert(err, check.IsNil) // By default user has not refused any hosts. @@ -317,7 +333,13 @@ func (s *KeyAgentTestSuite) TestHostKeyVerification(c *check.C) { // make a new local agent keystore, err := NewFSLocalKeyStore(s.keyDir) c.Assert(err, check.IsNil) - lka, err := NewLocalAgent(keystore, s.hostname, s.username, AddKeysToAgentAuto) + lka, err := NewLocalAgent(LocalAgentConfig{ + Keystore: keystore, + ProxyHost: s.hostname, + Username: s.username, + KeysOption: AddKeysToAgentAuto, + Insecure: true, + }) c.Assert(err, check.IsNil) // by default user has not refused any hosts: @@ -373,7 +395,12 @@ func (s *KeyAgentTestSuite) TestDefaultHostPromptFunc(c *check.C) { keystore, err := NewFSLocalKeyStore(s.keyDir) c.Assert(err, check.IsNil) - a, err := NewLocalAgent(keystore, s.hostname, s.username, AddKeysToAgentAuto) + a, err := NewLocalAgent(LocalAgentConfig{ + Keystore: keystore, + ProxyHost: s.hostname, + Username: s.username, + KeysOption: AddKeysToAgentAuto, + }) c.Assert(err, check.IsNil) _, keyBytes, err := keygen.GenerateKeyPair("") diff --git a/lib/client/keystore_test.go b/lib/client/keystore_test.go index 9f7995e2c26ae..e04cafd07ff25 100644 --- a/lib/client/keystore_test.go +++ b/lib/client/keystore_test.go @@ -273,7 +273,7 @@ func TestProxySSHConfig(t *testing.T) { []ssh.Signer{hostSigner}, sshutils.AuthMethods{ PublicKey: func(conn ssh.ConnMetadata, key ssh.PublicKey) (*ssh.Permissions, error) { - certChecker := utils.CertChecker{ + certChecker := apisshutils.CertChecker{ CertChecker: ssh.CertChecker{ IsUserAuthority: func(cert ssh.PublicKey) bool { // Makes sure that user presented key signed by or with trusted authority. diff --git a/lib/jwt/jwt.go b/lib/jwt/jwt.go index bc5c73cd706df..3225dcab2d841 100644 --- a/lib/jwt/jwt.go +++ b/lib/jwt/jwt.go @@ -23,7 +23,7 @@ import ( "crypto/rsa" "time" - "github.com/gravitational/teleport" + "github.com/gravitational/teleport/api/constants" "github.com/gravitational/teleport/lib/utils" "github.com/gravitational/trace" @@ -239,7 +239,7 @@ type Claims struct { // GenerateKeyPair generates and return a PEM encoded private and public // key in the format used by this package. func GenerateKeyPair() ([]byte, []byte, error) { - privateKey, err := rsa.GenerateKey(rand.Reader, teleport.RSAKeySize) + privateKey, err := rsa.GenerateKey(rand.Reader, constants.RSAKeySize) if err != nil { return nil, nil, trace.Wrap(err) } diff --git a/lib/multiplexer/multiplexer_test.go b/lib/multiplexer/multiplexer_test.go index 9cddf36e7015e..5c7496ee1b9ec 100644 --- a/lib/multiplexer/multiplexer_test.go +++ b/lib/multiplexer/multiplexer_test.go @@ -36,7 +36,7 @@ import ( "golang.org/x/crypto/ssh" - "github.com/gravitational/teleport" + "github.com/gravitational/teleport/api/constants" "github.com/gravitational/teleport/lib/fixtures" "github.com/gravitational/teleport/lib/httplib" "github.com/gravitational/teleport/lib/multiplexer/test" @@ -575,7 +575,7 @@ func TestMux(t *testing.T) { certPool.AppendCertsFromPEM(caCert) // Sign server certificate. - serverRSAKey, err := rsa.GenerateKey(rand.Reader, teleport.RSAKeySize) + serverRSAKey, err := rsa.GenerateKey(rand.Reader, constants.RSAKeySize) require.NoError(t, err) serverPEM, err := ca.GenerateCertificate(tlsca.CertificateRequest{ Subject: pkix.Name{CommonName: "localhost"}, @@ -588,7 +588,7 @@ func TestMux(t *testing.T) { require.NoError(t, err) // Sign client certificate with database access identity. - clientRSAKey, err := rsa.GenerateKey(rand.Reader, teleport.RSAKeySize) + clientRSAKey, err := rsa.GenerateKey(rand.Reader, constants.RSAKeySize) require.NoError(t, err) subject, err := (&tlsca.Identity{ Username: "alice", diff --git a/lib/reversetunnel/agent.go b/lib/reversetunnel/agent.go index 5c98c8accbf9b..b8de1c281d109 100644 --- a/lib/reversetunnel/agent.go +++ b/lib/reversetunnel/agent.go @@ -23,7 +23,6 @@ package reversetunnel import ( "context" "fmt" - "net" "sync" "time" @@ -98,6 +97,8 @@ type AgentConfig struct { Lease track.Lease // Log optionally specifies the logger Log log.FieldLogger + // FIPS indicates if Teleport was started in FIPS mode. + FIPS bool } // CheckAndSetDefaults checks parameters and sets default values @@ -226,29 +227,20 @@ func (a *Agent) getPrincipalsList() []string { return out } -func (a *Agent) checkHostSignature(hostport string, remote net.Addr, key ssh.PublicKey) error { - cert, ok := key.(*ssh.Certificate) - if !ok { - return trace.BadParameter("expected certificate") - } +func (a *Agent) getHostCheckers() ([]ssh.PublicKey, error) { cas, err := a.AccessPoint.GetCertAuthorities(types.HostCA, false) if err != nil { - return trace.Wrap(err, "failed to fetch remote certs") + return nil, trace.Wrap(err) } + var keys []ssh.PublicKey for _, ca := range cas { checkers, err := sshutils.GetCheckers(ca) if err != nil { - return trace.BadParameter("error parsing key: %v", err) - } - for _, checker := range checkers { - if apisshutils.KeysEqual(checker, cert.SignatureKey) { - a.setPrincipals(cert.ValidPrincipals) - return nil - } + return nil, trace.Wrap(err) } + keys = append(keys, checkers...) } - return trace.NotFound( - "no matching keys found when checking server's host signature") + return keys, nil } func (a *Agent) connect() (conn *ssh.Client, err error) { @@ -261,12 +253,25 @@ func (a *Agent) connect() (conn *ssh.Client, err error) { continue } + callback, err := apisshutils.NewHostKeyCallback( + apisshutils.HostKeyCallbackConfig{ + GetHostCheckers: a.getHostCheckers, + OnCheckCert: func(cert *ssh.Certificate) { + a.setPrincipals(cert.ValidPrincipals) + }, + FIPS: a.FIPS, + }) + if err != nil { + a.log.Debugf("Failed to create host key callback for %v: %v.", a.Addr.Addr, err) + continue + } + // Build a new client connection. This is done to get access to incoming // global requests which dialer.Dial would not provide. conn, chans, reqs, err := ssh.NewClientConn(pconn, a.Addr.Addr, &ssh.ClientConfig{ User: a.Username, Auth: []ssh.AuthMethod{authMethod}, - HostKeyCallback: a.checkHostSignature, + HostKeyCallback: callback, Timeout: apidefaults.DefaultDialTimeout, }) if err != nil { diff --git a/lib/reversetunnel/agent_test.go b/lib/reversetunnel/agent_test.go new file mode 100644 index 0000000000000..c817aa386585f --- /dev/null +++ b/lib/reversetunnel/agent_test.go @@ -0,0 +1,115 @@ +/* +Copyright 2021 Gravitational, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package reversetunnel + +import ( + "context" + "crypto/rand" + "crypto/rsa" + "testing" + + "github.com/gravitational/teleport/api/types" + apisshutils "github.com/gravitational/teleport/api/utils/sshutils" + "github.com/gravitational/teleport/lib/auth" + "github.com/gravitational/teleport/lib/services" + "github.com/gravitational/teleport/lib/sshutils" + "github.com/gravitational/teleport/lib/utils" + "github.com/gravitational/trace" + + "github.com/stretchr/testify/require" + "golang.org/x/crypto/ssh" +) + +// TestAgentCertChecker validates that reverse tunnel agents properly validate +// SSH host certificates. +func TestAgentCertChecker(t *testing.T) { + handler := sshutils.NewChanHandlerFunc(func(_ context.Context, ccx *sshutils.ConnectionContext, nch ssh.NewChannel) { + ch, _, err := nch.Accept() + require.NoError(t, err) + require.NoError(t, ch.Close()) + }) + + ca, err := apisshutils.MakeTestSSHCA() + require.NoError(t, err) + + spoofedCert, err := apisshutils.MakeSpoofedHostCert(ca) + require.NoError(t, err) + + sshServer, err := sshutils.NewServer( + "test", + utils.NetAddr{AddrNetwork: "tcp", Addr: "localhost:0"}, + handler, + []ssh.Signer{spoofedCert}, + sshutils.AuthMethods{NoClient: true}, + sshutils.SetInsecureSkipHostValidation(), + ) + require.NoError(t, err) + t.Cleanup(func() { sshServer.Close() }) + require.NoError(t, sshServer.Start()) + + priv, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + signer, err := ssh.NewSignerFromKey(priv) + require.NoError(t, err) + + events := make(chan string) + + agent, err := NewAgent(AgentConfig{ + Addr: *utils.MustParseAddr(sshServer.Addr()), + EventsC: events, + Context: context.Background(), + Client: &fakeClient{caKey: ca.PublicKey()}, + AccessPoint: &fakeClient{caKey: ca.PublicKey()}, + Signer: signer, + Username: "foo", + }) + require.NoError(t, err) + + _, err = agent.connect() + require.Error(t, err, "agent should reject invalid host certificate") +} + +type fakeClient struct { + auth.Client + caKey ssh.PublicKey +} + +func (fc *fakeClient) GetCertAuthorities(caType types.CertAuthType, loadKeys bool, opts ...services.MarshalOption) ([]types.CertAuthority, error) { + ca, err := types.NewCertAuthority(types.CertAuthoritySpecV2{ + Type: types.HostCA, + ClusterName: "example.com", + CheckingKeys: [][]byte{ssh.MarshalAuthorizedKey(fc.caKey)}, + }) + if err != nil { + return nil, trace.Wrap(err) + } + return []types.CertAuthority{ca}, nil +} + +func (fc *fakeClient) GetClusterNetworkingConfig(ctx context.Context, opts ...services.MarshalOption) (types.ClusterNetworkingConfig, error) { + return types.NewClusterNetworkingConfigFromConfigFile(types.ClusterNetworkingConfigSpecV2{}) +} + +func (fc *fakeClient) GetClusterConfig(opts ...services.MarshalOption) (types.ClusterConfig, error) { + return types.NewClusterConfig(types.ClusterConfigSpecV3{ + LegacySessionRecordingConfigSpec: &types.LegacySessionRecordingConfigSpec{ + Mode: "off", + ProxyChecksHostKeys: "yes", + }, + }) +} diff --git a/lib/reversetunnel/agentpool.go b/lib/reversetunnel/agentpool.go index 12289465e45f3..80bc3d3faeb04 100644 --- a/lib/reversetunnel/agentpool.go +++ b/lib/reversetunnel/agentpool.go @@ -90,6 +90,8 @@ type AgentPoolConfig struct { ProxyAddr string // Cluster is a cluster name of the proxy. Cluster string + // FIPS indicates if Teleport was started in FIPS mode. + FIPS bool } // CheckAndSetDefaults checks and sets defaults @@ -272,6 +274,7 @@ func (m *AgentPool) addAgent(lease track.Lease) error { Component: m.cfg.Component, Tracker: m.proxyTracker, Lease: lease, + FIPS: m.cfg.FIPS, }) if err != nil { // ensure that lease has been released; OK to call multiple times. diff --git a/lib/reversetunnel/rc_manager.go b/lib/reversetunnel/rc_manager.go index 0a64086eaef97..dab996a592468 100644 --- a/lib/reversetunnel/rc_manager.go +++ b/lib/reversetunnel/rc_manager.go @@ -73,6 +73,8 @@ type RemoteClusterTunnelManagerConfig struct { Clock clockwork.Clock // KubeDialAddr is an optional address of a local kubernetes proxy. KubeDialAddr utils.NetAddr + // FIPS indicates if Teleport was started in FIPS mode. + FIPS bool } func (c *RemoteClusterTunnelManagerConfig) CheckAndSetDefaults() error { @@ -211,6 +213,7 @@ func (w *RemoteClusterTunnelManager) realNewAgentPool(ctx context.Context, clust Clock: w.cfg.Clock, KubeDialAddr: w.cfg.KubeDialAddr, ReverseTunnelServer: w.cfg.ReverseTunnelServer, + FIPS: w.cfg.FIPS, // RemoteClusterManager only runs on proxies. Component: teleport.ComponentProxy, diff --git a/lib/reversetunnel/srv.go b/lib/reversetunnel/srv.go index 89086234e4070..6187ee20fa3bf 100644 --- a/lib/reversetunnel/srv.go +++ b/lib/reversetunnel/srv.go @@ -822,7 +822,7 @@ func (s *server) checkClientCert(logger *log.Entry, user string, clusterName str return trace.NotFound("cluster %v has no matching CA keys", clusterName) } - checker := utils.CertChecker{ + checker := apisshutils.CertChecker{ FIPS: s.FIPS, } if err := checker.CheckCert(user, cert); err != nil { diff --git a/lib/service/connect.go b/lib/service/connect.go index f8acf1ffd3e6f..290f180a510d4 100644 --- a/lib/service/connect.go +++ b/lib/service/connect.go @@ -831,7 +831,11 @@ func (process *TeleportProcess) newClient(authServers []utils.NetAddr, identity logger = process.log.WithField("proxy-addr", proxyAddr) logger.Debug("Attempting to connect to Auth Server through tunnel.") - tunnelClient, err := process.newClientThroughTunnel(proxyAddr, tlsConfig, identity.SSHClientConfig()) + sshClientConfig, err := identity.SSHClientConfig(process.Config.FIPS) + if err != nil { + return nil, trace.Wrap(err) + } + tunnelClient, err := process.newClientThroughTunnel(proxyAddr, tlsConfig, sshClientConfig) if err != nil { directErrLogger.Debug("Failed to connect to Auth Server directly.") logger.WithError(err).Debug("Failed to connect to Auth Server through tunnel.") diff --git a/lib/service/db.go b/lib/service/db.go index aa2bae4fa5fe6..024cb5b4101ac 100644 --- a/lib/service/db.go +++ b/lib/service/db.go @@ -201,6 +201,7 @@ func (process *TeleportProcess) initDatabaseService() (retErr error) { AccessPoint: conn.Client, HostSigner: conn.ServerIdentity.KeySigner, Cluster: clusterName, + FIPS: process.Config.FIPS, }) if err != nil { return trace.Wrap(err) diff --git a/lib/service/kubernetes.go b/lib/service/kubernetes.go index 977a760dc58d8..e9b5230fae653 100644 --- a/lib/service/kubernetes.go +++ b/lib/service/kubernetes.go @@ -143,6 +143,7 @@ func (process *TeleportProcess) initKubernetesService(log *logrus.Entry, conn *C HostSigner: conn.ServerIdentity.KeySigner, Cluster: conn.ServerIdentity.Cert.Extensions[utils.CertExtensionAuthority], Server: shtl, + FIPS: process.Config.FIPS, }) if err != nil { return trace.Wrap(err) diff --git a/lib/service/service.go b/lib/service/service.go index ddc73d14d738f..a0065511c8e9c 100644 --- a/lib/service/service.go +++ b/lib/service/service.go @@ -1859,6 +1859,7 @@ func (process *TeleportProcess) initSSH() error { HostSigner: conn.ServerIdentity.KeySigner, Cluster: conn.ServerIdentity.Cert.Extensions[utils.CertExtensionAuthority], Server: s, + FIPS: process.Config.FIPS, }) if err != nil { return trace.Wrap(err) @@ -2153,6 +2154,10 @@ func (process *TeleportProcess) getAdditionalPrincipals(role types.SystemRole) ( switch role { case types.RoleProxy: addrs = append(process.Config.Proxy.PublicAddrs, + process.Config.Proxy.WebAddr, + process.Config.Proxy.SSHAddr, + process.Config.Proxy.ReverseTunnelListenAddr, + process.Config.Proxy.MySQLAddr, utils.NetAddr{Addr: string(teleport.PrincipalLocalhost)}, utils.NetAddr{Addr: string(teleport.PrincipalLoopbackV4)}, utils.NetAddr{Addr: string(teleport.PrincipalLoopbackV6)}, @@ -2818,6 +2823,7 @@ func (process *TeleportProcess) initProxyEndpoint(conn *Connector) error { LocalCluster: conn.ServerIdentity.Cert.Extensions[utils.CertExtensionAuthority], KubeDialAddr: utils.DialAddrFromListenAddr(cfg.Proxy.Kube.ListenAddr), ReverseTunnelServer: tsrv, + FIPS: process.Config.FIPS, }) if err != nil { return trace.Wrap(err) @@ -3243,6 +3249,7 @@ func (process *TeleportProcess) initApps() { AccessPoint: accessPoint, HostSigner: conn.ServerIdentity.KeySigner, Cluster: clusterName, + FIPS: process.Config.FIPS, }) if err != nil { return trace.Wrap(err) diff --git a/lib/srv/authhandlers.go b/lib/srv/authhandlers.go index 452c938ec0819..b28cf954435f6 100644 --- a/lib/srv/authhandlers.go +++ b/lib/srv/authhandlers.go @@ -259,7 +259,7 @@ func (h *AuthHandlers) UserKeyAuth(conn ssh.ConnMetadata, key ssh.PublicKey) (*s if h.c.Clock != nil { clock = h.c.Clock.Now } - certChecker := utils.CertChecker{ + certChecker := apisshutils.CertChecker{ CertChecker: ssh.CertChecker{ IsUserAuthority: h.IsUserAuthority, Clock: clock, @@ -314,7 +314,7 @@ func (h *AuthHandlers) UserKeyAuth(conn ssh.ConnMetadata, key ssh.PublicKey) (*s func (h *AuthHandlers) HostKeyAuth(addr string, remote net.Addr, key ssh.PublicKey) error { // Check if the given host key was signed by a Teleport certificate // authority (CA) or fallback to host key checking if it's allowed. - certChecker := utils.CertChecker{ + certChecker := apisshutils.CertChecker{ CertChecker: ssh.CertChecker{ IsHostAuthority: h.IsHostAuthority, HostKeyFallback: h.hostKeyCallback, diff --git a/lib/srv/db/access_test.go b/lib/srv/db/access_test.go index 788efd43c5b9d..57f2b1e34d678 100644 --- a/lib/srv/db/access_test.go +++ b/lib/srv/db/access_test.go @@ -19,6 +19,7 @@ package db import ( "context" "crypto/tls" + "fmt" "net" "os" "sort" @@ -393,6 +394,63 @@ func TestAccessDisabled(t *testing.T) { require.Contains(t, err.Error(), "this Teleport cluster is not licensed for database access") } +// TestPostgresInjectionDatabase makes sure Postgres connection is not +// susceptible to malicious database name injections. +func TestPostgresInjectionDatabase(t *testing.T) { + ctx := context.Background() + testCtx := setupTestContext(ctx, t, withSelfHostedPostgres("postgres")) + go testCtx.startHandlingConnections() + + postgresServer := testCtx.postgres["postgres"].db + + // Make sure the role allows wildcard database users and names. + testCtx.createUserAndRole(ctx, t, "alice", "admin", []string{types.Wildcard}, []string{types.Wildcard}) + + // Connect and make sure connection parameters are as expected. + psql, err := testCtx.postgresClient(ctx, "alice", "postgres", "alice", "test&user=bob") + require.NoError(t, err) + + select { + case p := <-postgresServer.ParametersCh(): + require.Equal(t, map[string]string{"user": "alice", "database": "test&user=bob"}, p) + case <-time.After(time.Second): + t.Fatal("didn't receive startup message parameters after 1s") + } + + err = psql.Close(ctx) + require.NoError(t, err) +} + +// TestPostgresInjectionUser makes sure Postgres connection is not +// susceptible to malicious user name injections. +func TestPostgresInjectionUser(t *testing.T) { + ctx := context.Background() + testCtx := setupTestContext(ctx, t, withSelfHostedPostgres("postgres")) + go testCtx.startHandlingConnections() + + postgresServer := testCtx.postgres["postgres"].db + + // Make sure the role allows wildcard database users and names. + testCtx.createUserAndRole(ctx, t, "alice", "admin", []string{types.Wildcard}, []string{types.Wildcard}) + + // Construct malicious username that simulates the connection string. + user := fmt.Sprintf("alice@localhost:%v?database=prod&foo=", postgresServer.Port()) + + // Connect and make sure startup parameters are as expected. + psql, err := testCtx.postgresClient(ctx, "alice", "postgres", user, "test") + require.NoError(t, err) + + select { + case p := <-postgresServer.ParametersCh(): + require.Equal(t, map[string]string{"user": user, "database": "test"}, p) + case <-time.After(time.Second): + t.Fatal("didn't receive startup message parameters after 1s") + } + + err = psql.Close(ctx) + require.NoError(t, err) +} + type testContext struct { hostID string clusterName string diff --git a/lib/srv/db/postgres/engine.go b/lib/srv/db/postgres/engine.go index 68987e2f98249..58fb95214d52f 100644 --- a/lib/srv/db/postgres/engine.go +++ b/lib/srv/db/postgres/engine.go @@ -368,11 +368,12 @@ func (e *Engine) getConnectConfig(ctx context.Context, sessionCtx *common.Sessio // The driver requires the config to be built by parsing the connection // string so parse the basic template and then fill in the rest of // parameters such as TLS configuration. - config, err := pgconn.ParseConfig(fmt.Sprintf("postgres://%s@%s/?database=%s", - sessionCtx.DatabaseUser, sessionCtx.Server.GetURI(), sessionCtx.DatabaseName)) + config, err := pgconn.ParseConfig(fmt.Sprintf("postgres://%s", sessionCtx.Server.GetURI())) if err != nil { return nil, trace.Wrap(err) } + config.User = sessionCtx.DatabaseUser + config.Database = sessionCtx.DatabaseName // Pgconn adds fallbacks to retry connection without TLS if the TLS // attempt fails. Reset the fallbacks to avoid retries, otherwise // it's impossible to debug TLS connection errors. diff --git a/lib/srv/db/postgres/test.go b/lib/srv/db/postgres/test.go index 9b5c732b0d225..d9ed78bbb1d46 100644 --- a/lib/srv/db/postgres/test.go +++ b/lib/srv/db/postgres/test.go @@ -39,11 +39,12 @@ import ( // parameters. func MakeTestClient(ctx context.Context, config common.TestClientConfig) (*pgconn.PgConn, error) { // Client will be connecting directly to the multiplexer address. - pgconnConfig, err := pgconn.ParseConfig(fmt.Sprintf("postgres://%v@%v/?database=%v", - config.RouteToDatabase.Username, config.Address, config.RouteToDatabase.Database)) + pgconnConfig, err := pgconn.ParseConfig(fmt.Sprintf("postgres://%v", config.Address)) if err != nil { return nil, trace.Wrap(err) } + pgconnConfig.User = config.RouteToDatabase.Username + pgconnConfig.Database = config.RouteToDatabase.Database pgconnConfig.TLSConfig, err = common.MakeTestClientTLSConfig(config) if err != nil { return nil, trace.Wrap(err) @@ -70,6 +71,8 @@ type TestServer struct { log logrus.FieldLogger // queryCount keeps track of the number of queries the server has received. queryCount uint32 + // parametersCh receives startup message connection parameters. + parametersCh chan map[string]string } // NewTestServer returns a new instance of a test Postgres server. @@ -99,6 +102,7 @@ func NewTestServer(config common.TestServerConfig) (*TestServer, error) { trace.Component: defaults.ProtocolPostgres, "name": config.Name, }), + parametersCh: make(chan map[string]string, 100), }, nil } @@ -192,14 +196,17 @@ func (s *TestServer) startTLS(conn net.Conn) (*pgproto3.Backend, error) { } func (s *TestServer) handleStartup(client *pgproto3.Backend) error { - startupMessage, err := client.ReceiveStartupMessage() + startupMessageI, err := client.ReceiveStartupMessage() if err != nil { return trace.Wrap(err) } - if _, ok := startupMessage.(*pgproto3.StartupMessage); !ok { + startupMessage, ok := startupMessageI.(*pgproto3.StartupMessage) + if !ok { return trace.BadParameter("expected *pgproto3.StartupMessage, got: %#v", startupMessage) } s.log.Debugf("Received %#v.", startupMessage) + // Push connect parameters into the channel so tests can consume them. + s.parametersCh <- startupMessage.Parameters // If auth token is specified, used it for password authentication, this // simulates cloud provider IAM auth. if s.cfg.AuthToken != "" { @@ -272,6 +279,11 @@ func (s *TestServer) QueryCount() uint32 { return atomic.LoadUint32(&s.queryCount) } +// ParametersCh returns channel that receives startup message parameters. +func (s *TestServer) ParametersCh() chan map[string]string { + return s.parametersCh +} + // Close closes the server listener. func (s *TestServer) Close() error { return s.listener.Close() diff --git a/lib/sshutils/authority.go b/lib/sshutils/authority.go index c145cb9178e5c..3d40d15d2ce2f 100644 --- a/lib/sshutils/authority.go +++ b/lib/sshutils/authority.go @@ -37,6 +37,22 @@ func GetCheckers(ca types.CertAuthority) ([]ssh.PublicKey, error) { return out, nil } +// GetSigners returns SSH signers for the provided authority. +func GetSigners(ca types.CertAuthority) ([]ssh.Signer, error) { + var signers []ssh.Signer + for _, kp := range ca.GetActiveKeys().SSH { + if len(kp.PrivateKey) == 0 { + continue + } + signer, err := ssh.ParsePrivateKey(kp.PrivateKey) + if err != nil { + return nil, trace.Wrap(err) + } + signers = append(signers, signer) + } + return signers, nil +} + // ValidateSigners returns a list of signers that could be used to sign keys. func ValidateSigners(ca types.CertAuthority) error { keys := ca.GetActiveKeys().SSH diff --git a/lib/sshutils/server.go b/lib/sshutils/server.go index a813a4cc16e17..c8823bf8534d7 100644 --- a/lib/sshutils/server.go +++ b/lib/sshutils/server.go @@ -31,6 +31,7 @@ import ( "golang.org/x/crypto/ssh" "github.com/gravitational/teleport" + "github.com/gravitational/teleport/api/utils/sshutils" "github.com/gravitational/teleport/lib/defaults" "github.com/gravitational/teleport/lib/limiter" "github.com/gravitational/teleport/lib/utils" @@ -602,7 +603,7 @@ func validateHostSigner(fips bool, signer ssh.Signer) error { return trace.BadParameter("at least one valid principal is required in host certificate") } - certChecker := utils.CertChecker{ + certChecker := sshutils.CertChecker{ FIPS: fips, } err := certChecker.CheckCert(cert.ValidPrincipals[0], cert) diff --git a/lib/tlsca/ca_test.go b/lib/tlsca/ca_test.go index 8316fa1f273f1..c6321c4138eb6 100644 --- a/lib/tlsca/ca_test.go +++ b/lib/tlsca/ca_test.go @@ -24,6 +24,7 @@ import ( "time" "github.com/gravitational/teleport" + "github.com/gravitational/teleport/api/constants" "github.com/gravitational/teleport/lib/fixtures" "github.com/google/go-cmp/cmp" @@ -38,7 +39,7 @@ func TestPrincipals(t *testing.T) { ca, err := FromKeys([]byte(fixtures.TLSCACertPEM), []byte(fixtures.TLSCAKeyPEM)) require.NoError(t, err) - privateKey, err := rsa.GenerateKey(rand.Reader, teleport.RSAKeySize) + privateKey, err := rsa.GenerateKey(rand.Reader, constants.RSAKeySize) require.NoError(t, err) hostnames := []string{"localhost", "example.com"} @@ -71,7 +72,7 @@ func TestKubeExtensions(t *testing.T) { ca, err := FromKeys([]byte(fixtures.TLSCACertPEM), []byte(fixtures.TLSCAKeyPEM)) require.NoError(t, err) - privateKey, err := rsa.GenerateKey(rand.Reader, teleport.RSAKeySize) + privateKey, err := rsa.GenerateKey(rand.Reader, constants.RSAKeySize) require.NoError(t, err) expires := clock.Now().Add(time.Hour) diff --git a/lib/tlsca/parsegen.go b/lib/tlsca/parsegen.go index a2f9ccd3e838e..2429bc0803396 100644 --- a/lib/tlsca/parsegen.go +++ b/lib/tlsca/parsegen.go @@ -28,7 +28,8 @@ import ( "math/big" "time" - "github.com/gravitational/teleport" + "github.com/gravitational/teleport/api/constants" + "github.com/gravitational/trace" "github.com/jonboulle/clockwork" "golang.org/x/crypto/ssh" @@ -110,7 +111,7 @@ func GenerateSelfSignedCAWithConfig(config GenerateCAConfig) (certPEM []byte, er // GenerateSelfSignedCA generates self-signed certificate authority used for internal inter-node communications func GenerateSelfSignedCA(entity pkix.Name, dnsNames []string, ttl time.Duration) ([]byte, []byte, error) { - priv, err := rsa.GenerateKey(rand.Reader, teleport.RSAKeySize) + priv, err := rsa.GenerateKey(rand.Reader, constants.RSAKeySize) keyPEM := pem.EncodeToMemory(&pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(priv)}) if err != nil { return nil, nil, trace.Wrap(err) diff --git a/lib/utils/certs.go b/lib/utils/certs.go index 7ad23e3326245..da795ea62def8 100644 --- a/lib/utils/certs.go +++ b/lib/utils/certs.go @@ -26,7 +26,7 @@ import ( "math/big" "time" - "github.com/gravitational/teleport" + "github.com/gravitational/teleport/api/constants" "github.com/gravitational/teleport/api/utils/tlsutils" "github.com/gravitational/trace" @@ -67,7 +67,7 @@ func (ks *KeyStore) GetKeyPair() (*rsa.PrivateKey, []byte, error) { // GenerateSelfSignedSigningCert generates self-signed certificate used for digital signatures func GenerateSelfSignedSigningCert(entity pkix.Name, dnsNames []string, ttl time.Duration) ([]byte, []byte, error) { - priv, err := rsa.GenerateKey(rand.Reader, teleport.RSAKeySize) + priv, err := rsa.GenerateKey(rand.Reader, constants.RSAKeySize) if err != nil { return nil, nil, trace.Wrap(err) } diff --git a/lib/utils/chconn_test.go b/lib/utils/chconn_test.go index 481e764f2d164..180fdd52299b4 100644 --- a/lib/utils/chconn_test.go +++ b/lib/utils/chconn_test.go @@ -25,7 +25,7 @@ import ( "testing" "time" - "github.com/gravitational/teleport" + "github.com/gravitational/teleport/api/constants" "github.com/gravitational/teleport/api/utils/sshutils" "github.com/stretchr/testify/require" @@ -87,7 +87,7 @@ func startSSHServer(t *testing.T, listener net.Listener, sshConnCh chan<- sshCon require.NoError(t, err) t.Cleanup(func() { nConn.Close() }) - privateKey, err := rsa.GenerateKey(rand.Reader, teleport.RSAKeySize) + privateKey, err := rsa.GenerateKey(rand.Reader, constants.RSAKeySize) require.NoError(t, err) _, private, err := MarshalPrivateKey(privateKey) diff --git a/lib/utils/checker.go b/lib/utils/checker.go index f213151a47d21..0acd51b798bd8 100644 --- a/lib/utils/checker.go +++ b/lib/utils/checker.go @@ -22,107 +22,22 @@ import ( "crypto/elliptic" "crypto/rand" "crypto/rsa" - "net" "time" - "golang.org/x/crypto/ssh" - - "github.com/gravitational/teleport" + "github.com/gravitational/teleport/api/constants" "github.com/gravitational/trace" + "golang.org/x/crypto/ssh" ) -// CertChecker is a drop-in replacement for ssh.CertChecker. In FIPS mode, -// checks if the certificate (or key) were generated with a supported algorithm. -type CertChecker struct { - ssh.CertChecker - - // FIPS means in addition to checking the validity of the key or - // certificate, also check that FIPS 140-2 algorithms were used. - FIPS bool -} - -// Authenticate checks the validity of a user certificate. -func (c *CertChecker) Authenticate(conn ssh.ConnMetadata, key ssh.PublicKey) (*ssh.Permissions, error) { - err := c.validate(key) - if err != nil { - return nil, trace.Wrap(err) - } - - perms, err := c.CertChecker.Authenticate(conn, key) - if err != nil { - return nil, trace.Wrap(err) - } - - return perms, nil -} - -// CheckCert checks certificate metadata and signature. -func (c *CertChecker) CheckCert(principal string, cert *ssh.Certificate) error { - err := c.validate(cert) - if err != nil { - return trace.Wrap(err) - } - - return c.CertChecker.CheckCert(principal, cert) -} - -// CheckHostKey checks the validity of a host certificate. -func (c *CertChecker) CheckHostKey(addr string, remote net.Addr, key ssh.PublicKey) error { - err := c.validate(key) - if err != nil { - return trace.Wrap(err) - } - - return c.CertChecker.CheckHostKey(addr, remote, key) -} - -func (c *CertChecker) validate(key ssh.PublicKey) error { - // When not in FIPS mode, accept all algorithms and key sizes. - if !c.FIPS { - return nil - } - - switch cert := key.(type) { - case *ssh.Certificate: - err := validateAlgorithm(cert.Key) - if err != nil { - return trace.Wrap(err) - } - err = validateAlgorithm(cert.SignatureKey) - if err != nil { - return trace.Wrap(err) - } - return nil - default: - return validateAlgorithm(key) - } -} - -func validateAlgorithm(key ssh.PublicKey) error { - cryptoKey, ok := key.(ssh.CryptoPublicKey) - if !ok { - return trace.BadParameter("unable to determine underlying public key") - } - k, ok := cryptoKey.CryptoPublicKey().(*rsa.PublicKey) - if !ok { - return trace.BadParameter("only RSA keys supported") - } - if k.N.BitLen() != teleport.RSAKeySize { - return trace.BadParameter("found %v-bit key, only %v-bit supported", k.N.BitLen(), teleport.RSAKeySize) - } - - return nil -} - // CreateCertificate creates a valid 2048-bit RSA certificate. func CreateCertificate(principal string, certType uint32) (*ssh.Certificate, ssh.Signer, error) { // Create RSA key for CA and certificate to be signed by CA. - caKey, err := rsa.GenerateKey(rand.Reader, teleport.RSAKeySize) + caKey, err := rsa.GenerateKey(rand.Reader, constants.RSAKeySize) if err != nil { return nil, nil, trace.Wrap(err) } - key, err := rsa.GenerateKey(rand.Reader, teleport.RSAKeySize) + key, err := rsa.GenerateKey(rand.Reader, constants.RSAKeySize) if err != nil { return nil, nil, trace.Wrap(err) } diff --git a/lib/utils/tls.go b/lib/utils/tls.go index bb8612b29d53b..dd2a5b99c009a 100644 --- a/lib/utils/tls.go +++ b/lib/utils/tls.go @@ -28,8 +28,7 @@ import ( "os" "time" - "github.com/gravitational/teleport" - + "github.com/gravitational/teleport/api/constants" "github.com/gravitational/trace" log "github.com/sirupsen/logrus" @@ -91,7 +90,7 @@ type TLSCredentials struct { // GenerateSelfSignedCert generates a self-signed certificate that // is valid for given domain names and ips, returns PEM-encoded bytes with key and cert func GenerateSelfSignedCert(hostNames []string) (*TLSCredentials, error) { - priv, err := rsa.GenerateKey(rand.Reader, teleport.RSAKeySize) + priv, err := rsa.GenerateKey(rand.Reader, constants.RSAKeySize) if err != nil { return nil, trace.Wrap(err) } diff --git a/lib/web/apiserver.go b/lib/web/apiserver.go index 6337a8ad53e39..90e305ec749ff 100644 --- a/lib/web/apiserver.go +++ b/lib/web/apiserver.go @@ -42,6 +42,7 @@ import ( apidefaults "github.com/gravitational/teleport/api/defaults" "github.com/gravitational/teleport/api/types" apievents "github.com/gravitational/teleport/api/types/events" + apisshutils "github.com/gravitational/teleport/api/utils/sshutils" "github.com/gravitational/teleport/lib/auth" "github.com/gravitational/teleport/lib/auth/u2f" "github.com/gravitational/teleport/lib/client" @@ -2418,6 +2419,19 @@ func (h *Handler) ProxyWithRoles(ctx *SessionContext) (reversetunnel.Tunnel, err // ProxyHostPort returns the address of the proxy server using --proxy // notation, i.e. "localhost:8030,8023" func (h *Handler) ProxyHostPort() string { + // Proxy web address can set in the config like 0.0.0.0:3080. + // + // In this case, the dial will succeed (dialing 0.0.0.0 is same a dialing + // localhost in Go) but the SSH host certificate will not validate since + // 0.0.0.0 is never a valid principal (auth server explicitly removes it + // when issuing host certs). + // + // As such, replace 0.0.0.0 with localhost in this case: proxy listens on + // all interfaces and localhost is always included in the valid principal + // set. + if net.ParseIP(h.cfg.ProxyWebAddr.Host()).IsUnspecified() { + return fmt.Sprintf("localhost:%v,%s", h.cfg.ProxyWebAddr.Port(defaults.HTTPListenPort), h.sshPort) + } return fmt.Sprintf("%s,%s", h.cfg.ProxyWebAddr.String(), h.sshPort) } @@ -2456,6 +2470,14 @@ func makeTeleportClientConfig(ctx *SessionContext) (*client.Config, error) { return nil, trace.BadParameter("failed to get client TLS config: %v", err) } + callback, err := apisshutils.NewHostKeyCallback( + apisshutils.HostKeyCallbackConfig{ + GetHostCheckers: ctx.getCheckers, + }) + if err != nil { + return nil, trace.Wrap(err) + } + config := &client.Config{ Username: ctx.user, Agent: agent, @@ -2463,7 +2485,7 @@ func makeTeleportClientConfig(ctx *SessionContext) (*client.Config, error) { TLS: tlsConfig, AuthMethods: []ssh.AuthMethod{ssh.PublicKeys(signers...)}, DefaultPrincipal: cert.ValidPrincipals[0], - HostKeyCallback: func(string, net.Addr, ssh.PublicKey) error { return nil }, + HostKeyCallback: callback, } return config, nil diff --git a/lib/web/sessions.go b/lib/web/sessions.go index a5c7359fcf0ef..47ede308aa651 100644 --- a/lib/web/sessions.go +++ b/lib/web/sessions.go @@ -33,13 +33,14 @@ import ( apiclient "github.com/gravitational/teleport/api/client" "github.com/gravitational/teleport/api/client/proto" "github.com/gravitational/teleport/api/types" - "github.com/gravitational/teleport/api/utils/sshutils" + apisshutils "github.com/gravitational/teleport/api/utils/sshutils" "github.com/gravitational/teleport/lib/auth" "github.com/gravitational/teleport/lib/auth/u2f" "github.com/gravitational/teleport/lib/client" "github.com/gravitational/teleport/lib/reversetunnel" "github.com/gravitational/teleport/lib/services" "github.com/gravitational/teleport/lib/services/local" + "github.com/gravitational/teleport/lib/sshutils" "github.com/gravitational/teleport/lib/tlsca" "github.com/gravitational/teleport/lib/utils" @@ -304,9 +305,25 @@ func (c *SessionContext) GetAgent() (agent.Agent, *ssh.Certificate, error) { return keyring, cert, nil } +func (c *SessionContext) getCheckers() ([]ssh.PublicKey, error) { + cas, err := c.unsafeAccessPoint.GetCertAuthorities(types.HostCA, false) + if err != nil { + return nil, trace.Wrap(err) + } + var keys []ssh.PublicKey + for _, ca := range cas { + checkers, err := sshutils.GetCheckers(ca) + if err != nil { + return nil, trace.Wrap(err) + } + keys = append(keys, checkers...) + } + return keys, nil +} + // GetSSHCertificate returns the *ssh.Certificate associated with this session. func (c *SessionContext) GetSSHCertificate() (*ssh.Certificate, error) { - return sshutils.ParseCertificate(c.session.GetPub()) + return apisshutils.ParseCertificate(c.session.GetPub()) } // GetX509Certificate returns the *x509.Certificate associated with this session. diff --git a/tool/tsh/tsh.go b/tool/tsh/tsh.go index 876eb00ef24e5..9c050d1f3518b 100644 --- a/tool/tsh/tsh.go +++ b/tool/tsh/tsh.go @@ -988,7 +988,10 @@ func setupNoninteractiveClient(tc *client.TeleportClient, key *client.Key) error }, } err := checker.CheckHostKey(hostname, remote, hostKey) - if err != nil && oldHostKeyCallback != nil { + if err != nil { + if oldHostKeyCallback == nil { + return trace.Wrap(err) + } errOld := oldHostKeyCallback(hostname, remote, hostKey) if errOld != nil { return trace.NewAggregate(err, errOld) @@ -1690,7 +1693,7 @@ func makeClient(cf *CLIConf, useProfileLogin bool) (*client.TeleportClient, erro if err != nil { return nil, trace.Wrap(err) } - hostAuthFunc, err := key.HostKeyCallback() + hostAuthFunc, err := key.HostKeyCallback(cf.InsecureSkipVerify) if err != nil { return nil, trace.Wrap(err) } diff --git a/tool/tsh/tsh_test.go b/tool/tsh/tsh_test.go index 11f1f14210a09..58f2535a1503e 100644 --- a/tool/tsh/tsh_test.go +++ b/tool/tsh/tsh_test.go @@ -213,6 +213,47 @@ func TestOIDCLogin(t *testing.T) { // request to be generated. } +// TestLoginIdentityOut makes sure that "tsh login --out " command +// writes identity credentials to the specified path. +func TestLoginIdentityOut(t *testing.T) { + os.RemoveAll(profile.FullProfilePath("")) + t.Cleanup(func() { + os.RemoveAll(profile.FullProfilePath("")) + }) + + connector := mockConnector(t) + + alice, err := types.NewUser("alice@example.com") + require.NoError(t, err) + alice.SetRoles([]string{"admin"}) + + authProcess, proxyProcess := makeTestServers(t, connector, alice) + + authServer := authProcess.GetAuthServer() + require.NotNil(t, authServer) + + proxyAddr, err := proxyProcess.ProxyWebAddr() + require.NoError(t, err) + + identPath := filepath.Join(t.TempDir(), "ident") + + err = Run([]string{ + "login", + "--insecure", + "--debug", + "--auth", connector.GetName(), + "--proxy", proxyAddr.String(), + "--out", identPath, + }, cliOption(func(cf *CLIConf) error { + cf.mockSSOLogin = mockSSOLogin(t, authServer, alice) + return nil + })) + require.NoError(t, err) + + _, err = client.KeyFromIdentityFile(identPath) + require.NoError(t, err) +} + func TestRelogin(t *testing.T) { os.RemoveAll(profile.FullProfilePath("")) t.Cleanup(func() { @@ -394,7 +435,7 @@ func TestIdentityRead(t *testing.T) { require.NoError(t, err) require.NotNil(t, k) - cb, err := k.HostKeyCallback() + cb, err := k.HostKeyCallback(false) require.NoError(t, err) require.Nil(t, cb) @@ -412,7 +453,7 @@ func TestIdentityRead(t *testing.T) { require.NoError(t, err) require.NotNil(t, k) - cb, err := k.HostKeyCallback() + cb, err := k.HostKeyCallback(true) require.NoError(t, err) require.NotNil(t, cb) diff --git a/version.go b/version.go index 4fda8392dca00..544afb3107a23 100644 --- a/version.go +++ b/version.go @@ -3,7 +3,7 @@ package teleport const ( - Version = "7.1.0" + Version = "7.1.1" ) // Gitref variable is automatically set to the output of git-describe