From eefe26530cd837ee5221ed724a6fd7d4a9ac49d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Wed, 23 Nov 2022 11:45:39 +0100 Subject: [PATCH 01/11] Remove t.Parallel() from lib/teleterm/api/uri tests Those tests are too fast and simple to run them in parallel. --- lib/teleterm/api/uri/uri_test.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/lib/teleterm/api/uri/uri_test.go b/lib/teleterm/api/uri/uri_test.go index 54a6756d39e40..28c7ee944c5f9 100644 --- a/lib/teleterm/api/uri/uri_test.go +++ b/lib/teleterm/api/uri/uri_test.go @@ -26,7 +26,6 @@ import ( ) func TestString(t *testing.T) { - t.Parallel() tests := []struct { in uri.ResourceURI out string @@ -46,10 +45,7 @@ func TestString(t *testing.T) { } for _, tt := range tests { - tt := tt t.Run(fmt.Sprintf("%v", tt.in), func(t *testing.T) { - t.Parallel() - out := tt.in.String() require.Equal(t, tt.out, out) }) @@ -57,7 +53,6 @@ func TestString(t *testing.T) { } func TestParseClusterURI(t *testing.T) { - t.Parallel() tests := []struct { in string out uri.ResourceURI @@ -81,10 +76,7 @@ func TestParseClusterURI(t *testing.T) { } for _, tt := range tests { - tt := tt t.Run(tt.in, func(t *testing.T) { - t.Parallel() - out, err := uri.ParseClusterURI(tt.in) require.NoError(t, err) require.Equal(t, tt.out, out) From 99ebc407682e854f4e470d5135763aad5f7a175c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Wed, 23 Nov 2022 11:45:55 +0100 Subject: [PATCH 02/11] Add uri.ResourceURI.GetDbName --- lib/teleterm/api/uri/uri.go | 16 +++++++++++++ lib/teleterm/api/uri/uri_test.go | 39 ++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/lib/teleterm/api/uri/uri.go b/lib/teleterm/api/uri/uri.go index 47c2f6fc76e76..b774978c83498 100644 --- a/lib/teleterm/api/uri/uri.go +++ b/lib/teleterm/api/uri/uri.go @@ -25,6 +25,8 @@ import ( var pathClusters = urlpath.New("/clusters/:cluster/*") var pathLeafClusters = urlpath.New("/clusters/:cluster/leaves/:leaf/*") +var pathDbs = urlpath.New("/clusters/:cluster/dbs/:dbName") +var pathLeafDbs = urlpath.New("/clusters/:cluster/leaves/:leaf/dbs/:dbName") // New creates an instance of ResourceURI func New(path string) ResourceURI { @@ -92,6 +94,20 @@ func (r ResourceURI) GetLeafClusterName() string { return result.Params["leaf"] } +func (r ResourceURI) GetDbName() string { + result, ok := pathDbs.Match(r.path) + if ok { + return result.Params["dbName"] + } + + result, ok = pathLeafDbs.Match(r.path) + if ok { + return result.Params["dbName"] + } + + return "" +} + // AppendServer appends server segment to the URI func (r ResourceURI) AppendServer(id string) ResourceURI { r.path = fmt.Sprintf("%v/servers/%v", r.path, id) diff --git a/lib/teleterm/api/uri/uri_test.go b/lib/teleterm/api/uri/uri_test.go index 28c7ee944c5f9..8765db9d29739 100644 --- a/lib/teleterm/api/uri/uri_test.go +++ b/lib/teleterm/api/uri/uri_test.go @@ -83,3 +83,42 @@ func TestParseClusterURI(t *testing.T) { }) } } + +func TestGetDbName(t *testing.T) { + tests := []struct { + in uri.ResourceURI + out string + }{ + { + uri.NewClusterURI("foo").AppendDB("postgres"), + "postgres", + }, + { + uri.NewClusterURI("foo").AppendLeafCluster("bar").AppendDB("postgres"), + "postgres", + }, + { + uri.NewClusterURI("foo"), + "", + }, + { + uri.NewClusterURI("foo").AppendLeafCluster("bar"), + "", + }, + { + uri.NewClusterURI("foo").AppendKube("k8s"), + "", + }, + { + uri.NewClusterURI("foo").AppendLeafCluster("bar").AppendKube("k8s"), + "", + }, + } + + for _, tt := range tests { + t.Run(tt.in.String(), func(t *testing.T) { + out := tt.in.GetDbName() + require.Equal(t, tt.out, out) + }) + } +} From d0bb77eb2734a6c5bf4ec164a54d7860ac86a132 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Wed, 23 Nov 2022 11:54:26 +0100 Subject: [PATCH 03/11] Extract some alpnproxy test helpers into separate package Those helpers will be useful in lib/teleterm tests where we need to create a gateway which spawns an alpnproxy.LocalProxy underneath. --- lib/srv/alpnproxy/conn_test.go | 6 +- lib/srv/alpnproxy/helpers_test.go | 72 +------------------- lib/srv/alpnproxy/local_proxy_test.go | 7 +- lib/srv/alpnproxy/proxy_test.go | 19 +++--- lib/srv/alpnproxytest/helpers.go | 94 +++++++++++++++++++++++++++ 5 files changed, 116 insertions(+), 82 deletions(-) create mode 100644 lib/srv/alpnproxytest/helpers.go diff --git a/lib/srv/alpnproxy/conn_test.go b/lib/srv/alpnproxy/conn_test.go index c8371956e1da9..15984371760d6 100644 --- a/lib/srv/alpnproxy/conn_test.go +++ b/lib/srv/alpnproxy/conn_test.go @@ -26,6 +26,8 @@ import ( "time" "github.com/stretchr/testify/require" + + "github.com/gravitational/teleport/lib/srv/alpnproxytest" ) func TestPingConnection(t *testing.T) { @@ -337,7 +339,9 @@ func makeTLSConn(t *testing.T, server, client net.Conn) (*tls.Conn, *tls.Conn) { // Server go func() { tlsConn := tls.Server(server, &tls.Config{ - Certificates: []tls.Certificate{mustGenCertSignedWithCA(t, mustGenSelfSignedCert(t))}, + Certificates: []tls.Certificate{ + alpnproxytest.MustGenCertSignedWithCA(t, alpnproxytest.MustGenSelfSignedCert(t)), + }, }) tlsConnChan <- struct { *tls.Conn diff --git a/lib/srv/alpnproxy/helpers_test.go b/lib/srv/alpnproxy/helpers_test.go index d38d4e004c643..971ded8322aa3 100644 --- a/lib/srv/alpnproxy/helpers_test.go +++ b/lib/srv/alpnproxy/helpers_test.go @@ -18,12 +18,8 @@ package alpnproxy import ( "context" - "crypto/rand" - "crypto/rsa" "crypto/tls" "crypto/x509" - "crypto/x509/pkix" - "encoding/pem" "fmt" "io" "net" @@ -38,8 +34,8 @@ import ( "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/lib/auth" - "github.com/gravitational/teleport/lib/defaults" "github.com/gravitational/teleport/lib/srv/alpnproxy/common" + "github.com/gravitational/teleport/lib/srv/alpnproxytest" "github.com/gravitational/teleport/lib/tlsca" ) @@ -53,7 +49,7 @@ type Suite struct { } func NewSuite(t *testing.T) *Suite { - ca := mustGenSelfSignedCert(t) + ca := alpnproxytest.MustGenSelfSignedCert(t) pool := x509.NewCertPool() pool.AddCert(ca.Cert) l, err := net.Listen("tcp", "127.0.0.1:0") @@ -108,7 +104,7 @@ func (s *Suite) GetCertPool() *x509.CertPool { } func (s *Suite) CreateProxyServer(t *testing.T) *Proxy { - serverCert := mustGenCertSignedWithCA(t, s.ca) + serverCert := alpnproxytest.MustGenCertSignedWithCA(t, s.ca) tlsConfig := &tls.Config{ NextProtos: common.ProtocolsToString(common.SupportedProtocols), ClientAuth: tls.VerifyClientCertIfGiven, @@ -149,68 +145,6 @@ func (s *Suite) Start(t *testing.T) { }) } -func mustGenSelfSignedCert(t *testing.T) *tlsca.CertAuthority { - caKey, caCert, err := tlsca.GenerateSelfSignedCA(pkix.Name{ - CommonName: "localhost", - }, []string{"localhost"}, defaults.CATTL) - require.NoError(t, err) - - ca, err := tlsca.FromKeys(caCert, caKey) - require.NoError(t, err) - return ca -} - -type signOptions struct { - identity tlsca.Identity - clock clockwork.Clock -} - -func withIdentity(identity tlsca.Identity) signOptionsFunc { - return func(o *signOptions) { - o.identity = identity - } -} - -func withClock(clock clockwork.Clock) signOptionsFunc { - return func(o *signOptions) { - o.clock = clock - } -} - -type signOptionsFunc func(o *signOptions) - -func mustGenCertSignedWithCA(t *testing.T, ca *tlsca.CertAuthority, opts ...signOptionsFunc) tls.Certificate { - options := signOptions{ - identity: tlsca.Identity{Username: "test-user"}, - clock: clockwork.NewRealClock(), - } - - for _, opt := range opts { - opt(&options) - } - - subj, err := options.identity.Subject() - require.NoError(t, err) - - privateKey, err := rsa.GenerateKey(rand.Reader, 2048) - require.NoError(t, err) - - tlsCert, err := ca.GenerateCertificate(tlsca.CertificateRequest{ - Clock: options.clock, - PublicKey: privateKey.Public(), - Subject: subj, - NotAfter: options.clock.Now().UTC().Add(time.Minute), - DNSNames: []string{"localhost", "*.localhost"}, - }) - require.NoError(t, err) - - keyRaw := x509.MarshalPKCS1PrivateKey(privateKey) - keyPEM := pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: keyRaw}) - cert, err := tls.X509KeyPair(tlsCert, keyPEM) - require.NoError(t, err) - return cert -} - func mustReadFromConnection(t *testing.T, conn net.Conn, want string) { buff, err := io.ReadAll(conn) require.NoError(t, err) diff --git a/lib/srv/alpnproxy/local_proxy_test.go b/lib/srv/alpnproxy/local_proxy_test.go index 07ed04e76ebc9..e34c2758522f1 100644 --- a/lib/srv/alpnproxy/local_proxy_test.go +++ b/lib/srv/alpnproxy/local_proxy_test.go @@ -42,6 +42,7 @@ import ( "github.com/gravitational/teleport/lib/defaults" "github.com/gravitational/teleport/lib/srv/alpnproxy/common" + "github.com/gravitational/teleport/lib/srv/alpnproxytest" "github.com/gravitational/teleport/lib/tlsca" ) @@ -375,13 +376,13 @@ func TestCheckDBCerts(t *testing.T) { for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - tlsCert := mustGenCertSignedWithCA(t, suite.ca, - withIdentity(tlsca.Identity{ + tlsCert := alpnproxytest.MustGenCertSignedWithCA(t, suite.ca, + alpnproxytest.WithIdentity(tlsca.Identity{ Username: "test-user", Groups: []string{"test-group"}, RouteToDatabase: dbRouteInCert, }), - withClock(tt.clock), + alpnproxytest.WithClock(tt.clock), ) lp.SetCerts([]tls.Certificate{tlsCert}) tt.errAssertFn(t, lp.CheckDBCerts(tt.dbRoute)) diff --git a/lib/srv/alpnproxy/proxy_test.go b/lib/srv/alpnproxy/proxy_test.go index 59410b48feec8..4a248847c7372 100644 --- a/lib/srv/alpnproxy/proxy_test.go +++ b/lib/srv/alpnproxy/proxy_test.go @@ -33,6 +33,7 @@ import ( "github.com/gravitational/teleport/api/constants" "github.com/gravitational/teleport/lib/srv/alpnproxy/common" + "github.com/gravitational/teleport/lib/srv/alpnproxytest" "github.com/gravitational/teleport/lib/srv/db/dbutils" "github.com/gravitational/teleport/lib/tlsca" ) @@ -79,7 +80,7 @@ func TestProxyKubeHandler(t *testing.T) { ) suite := NewSuite(t) - kubeCert := mustGenCertSignedWithCA(t, suite.ca) + kubeCert := alpnproxytest.MustGenCertSignedWithCA(t, suite.ca) suite.router.AddKubeHandler(func(ctx context.Context, conn net.Conn) error { defer conn.Close() tlsConn := tls.Server(conn, &tls.Config{ @@ -118,8 +119,8 @@ func TestProxyTLSDatabaseHandler(t *testing.T) { ) suite := NewSuite(t) - clientCert := mustGenCertSignedWithCA(t, suite.ca, - withIdentity(tlsca.Identity{ + clientCert := alpnproxytest.MustGenCertSignedWithCA(t, suite.ca, + alpnproxytest.WithIdentity(tlsca.Identity{ Username: "test-user", Groups: []string{"test-group"}, RouteToDatabase: tlsca.RouteToDatabase{ @@ -215,8 +216,8 @@ func TestProxyRouteToDatabase(t *testing.T) { ) suite := NewSuite(t) - clientCert := mustGenCertSignedWithCA(t, suite.ca, - withIdentity(tlsca.Identity{ + clientCert := alpnproxytest.MustGenCertSignedWithCA(t, suite.ca, + alpnproxytest.WithIdentity(tlsca.Identity{ Username: "test-user", Groups: []string{"test-group"}, RouteToDatabase: tlsca.RouteToDatabase{ @@ -358,13 +359,13 @@ func TestProxyMakeConnectionHandler(t *testing.T) { }) svr := suite.CreateProxyServer(t) - customCA := mustGenSelfSignedCert(t) + customCA := alpnproxytest.MustGenSelfSignedCert(t) // Create a ConnectionHandler from the proxy server. alpnConnHandler := svr.MakeConnectionHandler(&tls.Config{ NextProtos: []string{string(common.ProtocolHTTP)}, Certificates: []tls.Certificate{ - mustGenCertSignedWithCA(t, customCA), + alpnproxytest.MustGenCertSignedWithCA(t, customCA), }, }) @@ -609,8 +610,8 @@ func TestProxyPingConnections(t *testing.T) { dataWritten := "message ping connection" suite := NewSuite(t) - clientCert := mustGenCertSignedWithCA(t, suite.ca, - withIdentity(tlsca.Identity{ + clientCert := alpnproxytest.MustGenCertSignedWithCA(t, suite.ca, + alpnproxytest.WithIdentity(tlsca.Identity{ Username: "test-user", Groups: []string{"test-group"}, RouteToDatabase: tlsca.RouteToDatabase{ diff --git a/lib/srv/alpnproxytest/helpers.go b/lib/srv/alpnproxytest/helpers.go new file mode 100644 index 0000000000000..30bbc6b317ee8 --- /dev/null +++ b/lib/srv/alpnproxytest/helpers.go @@ -0,0 +1,94 @@ +// Copyright 2022 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 alpnproxytest + +import ( + "crypto/rand" + "crypto/rsa" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "testing" + "time" + + "github.com/jonboulle/clockwork" + "github.com/stretchr/testify/require" + + "github.com/gravitational/teleport/lib/defaults" + "github.com/gravitational/teleport/lib/tlsca" +) + +func MustGenSelfSignedCert(t *testing.T) *tlsca.CertAuthority { + caKey, caCert, err := tlsca.GenerateSelfSignedCA(pkix.Name{ + CommonName: "localhost", + }, []string{"localhost"}, defaults.CATTL) + require.NoError(t, err) + + ca, err := tlsca.FromKeys(caCert, caKey) + require.NoError(t, err) + return ca +} + +type signOptions struct { + identity tlsca.Identity + clock clockwork.Clock +} + +func WithIdentity(identity tlsca.Identity) SignOptionsFunc { + return func(o *signOptions) { + o.identity = identity + } +} + +func WithClock(clock clockwork.Clock) SignOptionsFunc { + return func(o *signOptions) { + o.clock = clock + } +} + +type SignOptionsFunc func(o *signOptions) + +func MustGenCertSignedWithCA(t *testing.T, ca *tlsca.CertAuthority, opts ...SignOptionsFunc) tls.Certificate { + options := signOptions{ + identity: tlsca.Identity{Username: "test-user"}, + clock: clockwork.NewRealClock(), + } + + for _, opt := range opts { + opt(&options) + } + + subj, err := options.identity.Subject() + require.NoError(t, err) + + privateKey, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + tlsCert, err := ca.GenerateCertificate(tlsca.CertificateRequest{ + Clock: options.clock, + PublicKey: privateKey.Public(), + Subject: subj, + NotAfter: options.clock.Now().UTC().Add(time.Minute), + DNSNames: []string{"localhost", "*.localhost"}, + }) + require.NoError(t, err) + + keyRaw := x509.MarshalPKCS1PrivateKey(privateKey) + keyPEM := pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: keyRaw}) + cert, err := tls.X509KeyPair(tlsCert, keyPEM) + require.NoError(t, err) + return cert +} From f3af0273092cda00fa61e440f4381c680d7fafee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Wed, 23 Nov 2022 11:55:32 +0100 Subject: [PATCH 04/11] Extract CheckCertSubject from alpnproxy.LocalProxy.CheckDbCerts We're going to need a standalone version of this function in the next commit. --- lib/srv/alpnproxy/local_proxy.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/srv/alpnproxy/local_proxy.go b/lib/srv/alpnproxy/local_proxy.go index 69c4e9171f680..2ef08371aa32e 100644 --- a/lib/srv/alpnproxy/local_proxy.go +++ b/lib/srv/alpnproxy/local_proxy.go @@ -292,7 +292,13 @@ func (l *LocalProxy) CheckDBCerts(dbRoute tlsca.RouteToDatabase) error { return trace.Wrap(err) } - // Check the subject matches. + err = CheckCertSubject(cert, dbRoute) + return trace.Wrap(err) +} + +// CheckCertSubject checks if the route to the database from the cert matches the provided route in +// terms of username and database (if present). +func CheckCertSubject(cert *x509.Certificate, dbRoute tlsca.RouteToDatabase) error { identity, err := tlsca.FromSubject(cert.Subject, cert.NotAfter) if err != nil { return trace.Wrap(err) @@ -305,6 +311,7 @@ func (l *LocalProxy) CheckDBCerts(dbRoute tlsca.RouteToDatabase) error { return trace.Errorf("certificate subject is for database name %s, but need %s", identity.RouteToDatabase.Database, dbRoute.Database) } + return nil } From 80170e2db7d23e01e7c15b0fea65b3f6a682fd55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Wed, 23 Nov 2022 11:57:52 +0100 Subject: [PATCH 05/11] Add gatewaytest.MustGenAndSaveCert This function builds on top of alpnproxytest.MustGenCertSignedWithCA. In alpnproxy, LocalProxy operates on certs solely in memory through NewLocalProxy and LocalProxy.SetCerts. Connect on the other hand assumes that the key pair can be loaded from disk, so we have to provide a function which generates the certs and then saves them to file. In the next commit, we're going to check the subject of the cert, so Connect tests need a way of generating valid certs. --- lib/teleterm/clusters/cluster_databases.go | 2 +- lib/teleterm/gatewaytest/helpers.go | 53 ++++++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/lib/teleterm/clusters/cluster_databases.go b/lib/teleterm/clusters/cluster_databases.go index b90ba48c62785..b946c91c48def 100644 --- a/lib/teleterm/clusters/cluster_databases.go +++ b/lib/teleterm/clusters/cluster_databases.go @@ -154,7 +154,7 @@ func (c *Cluster) GetDatabases(ctx context.Context, r *api.GetDatabasesRequest) return response, nil } -// ReissueDBCerts issues new certificates for specific DB access +// ReissueDBCerts issues new certificates for specific DB access and saves them to disk. func (c *Cluster) ReissueDBCerts(ctx context.Context, routeToDatabase tlsca.RouteToDatabase) error { // When generating certificate for MongoDB access, database username must // be encoded into it. This is required to be able to tell which database diff --git a/lib/teleterm/gatewaytest/helpers.go b/lib/teleterm/gatewaytest/helpers.go index 4f2620123383e..abb3b4cd19d71 100644 --- a/lib/teleterm/gatewaytest/helpers.go +++ b/lib/teleterm/gatewaytest/helpers.go @@ -15,14 +15,20 @@ package gatewaytest import ( + "crypto/rsa" + "crypto/x509" + "encoding/pem" "fmt" "net" + "os" "testing" "time" "github.com/gravitational/trace" "github.com/stretchr/testify/require" "golang.org/x/exp/slices" + + "github.com/gravitational/teleport/lib/srv/alpnproxytest" ) const timeout = time.Second * 5 @@ -128,3 +134,50 @@ func (m *MockListener) Addr() net.Addr { func (m *MockListener) RealAddr() net.Addr { return m.realListener.Addr() } + +type KeyPairPaths struct { + CertPath string + KeyPath string +} + +func MustGenAndSaveCert(t *testing.T, opts ...alpnproxytest.SignOptionsFunc) KeyPairPaths { + t.Helper() + + dir := t.TempDir() + + ca := alpnproxytest.MustGenSelfSignedCert(t) + + tlsCert := alpnproxytest.MustGenCertSignedWithCA(t, ca, opts...) + + privateKey, ok := tlsCert.PrivateKey.(*rsa.PrivateKey) + if !ok { + t.Fatal("Failed to cast tlsCert.PrivateKey") + } + + // Save the cert. + + certFile, err := os.CreateTemp(dir, "cert") + require.NoError(t, err) + + pemCert := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: tlsCert.Certificate[0]}) + + _, err = certFile.Write(pemCert) + require.NoError(t, err) + require.NoError(t, certFile.Close()) + + // Save the private key. + + keyFile, err := os.CreateTemp(dir, "key") + require.NoError(t, err) + + pemPrivateKey := pem.EncodeToMemory(&pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(privateKey)}) + + _, err = keyFile.Write(pemPrivateKey) + require.NoError(t, err) + require.NoError(t, keyFile.Close()) + + return KeyPairPaths{ + CertPath: certFile.Name(), + KeyPath: keyFile.Name(), + } +} From 00dc26803654221fb2cd8bd4080498b22b2adcaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Wed, 23 Nov 2022 12:11:31 +0100 Subject: [PATCH 06/11] Verify cert subject before using cert in gateway --- .../dbcmd_cli_command_provider_test.go | 34 +++++++++++++++--- lib/teleterm/daemon/daemon_test.go | 18 ++++++++-- lib/teleterm/gateway/gateway.go | 36 +++++++++++++++++++ lib/teleterm/gateway/gateway_test.go | 30 +++++++++++++--- 4 files changed, 108 insertions(+), 10 deletions(-) diff --git a/lib/teleterm/clusters/dbcmd_cli_command_provider_test.go b/lib/teleterm/clusters/dbcmd_cli_command_provider_test.go index 22a696a172748..274c43b0c5eeb 100644 --- a/lib/teleterm/clusters/dbcmd_cli_command_provider_test.go +++ b/lib/teleterm/clusters/dbcmd_cli_command_provider_test.go @@ -25,8 +25,11 @@ import ( "github.com/gravitational/teleport/lib/client" "github.com/gravitational/teleport/lib/defaults" + "github.com/gravitational/teleport/lib/srv/alpnproxytest" "github.com/gravitational/teleport/lib/teleterm/api/uri" "github.com/gravitational/teleport/lib/teleterm/gateway" + "github.com/gravitational/teleport/lib/teleterm/gatewaytest" + "github.com/gravitational/teleport/lib/tlsca" ) type fakeExec struct{} @@ -89,17 +92,29 @@ func TestDbcmdCLICommandProviderGetCommand(t *testing.T) { clusters: []*Cluster{&cluster}, } dbcmdCLICommandProvider := NewDbcmdCLICommandProvider(fakeStorage, fakeExec{}) + + keyPairPaths := gatewaytest.MustGenAndSaveCert(t, alpnproxytest.WithIdentity(tlsca.Identity{ + Username: "alice", + Groups: []string{"test-group"}, + RouteToDatabase: tlsca.RouteToDatabase{ + ServiceName: "foo", + Protocol: defaults.ProtocolPostgres, + Username: "alice", + }, + })) + gateway, err := gateway.New( gateway.Config{ TargetURI: cluster.URI.AppendDB("foo").String(), TargetName: "foo", + TargetUser: "alice", TargetSubresourceName: tc.targetSubresourceName, Protocol: defaults.ProtocolPostgres, LocalAddress: "localhost", WebProxyAddr: "localhost:1337", Insecure: true, - CertPath: "../../../fixtures/certs/proxy1.pem", - KeyPath: "../../../fixtures/certs/proxy1-key.pem", + CertPath: keyPairPaths.CertPath, + KeyPath: keyPairPaths.KeyPath, CLICommandProvider: dbcmdCLICommandProvider, TCPPortAllocator: gateway.NetTCPPortAllocator{}, }, @@ -122,6 +137,17 @@ func TestDbcmdCLICommandProviderGetCommand_ReturnsErrorIfClusterIsNotFound(t *te clusters: []*Cluster{}, } dbcmdCLICommandProvider := NewDbcmdCLICommandProvider(fakeStorage, fakeExec{}) + + keyPairPaths := gatewaytest.MustGenAndSaveCert(t, alpnproxytest.WithIdentity(tlsca.Identity{ + Username: "alice", + Groups: []string{"test-group"}, + RouteToDatabase: tlsca.RouteToDatabase{ + ServiceName: "foo", + Protocol: defaults.ProtocolPostgres, + Username: "alice", + }, + })) + gateway, err := gateway.New( gateway.Config{ TargetURI: uri.NewClusterURI("quux").AppendDB("foo").String(), @@ -132,8 +158,8 @@ func TestDbcmdCLICommandProviderGetCommand_ReturnsErrorIfClusterIsNotFound(t *te LocalAddress: "localhost", WebProxyAddr: "localhost:1337", Insecure: true, - CertPath: "../../../fixtures/certs/proxy1.pem", - KeyPath: "../../../fixtures/certs/proxy1-key.pem", + CertPath: keyPairPaths.CertPath, + KeyPath: keyPairPaths.KeyPath, CLICommandProvider: dbcmdCLICommandProvider, TCPPortAllocator: gateway.NetTCPPortAllocator{}, }, diff --git a/lib/teleterm/daemon/daemon_test.go b/lib/teleterm/daemon/daemon_test.go index ea9d60bb8bf29..3d28fa3a05426 100644 --- a/lib/teleterm/daemon/daemon_test.go +++ b/lib/teleterm/daemon/daemon_test.go @@ -27,10 +27,12 @@ import ( "google.golang.org/grpc/credentials/insecure" "github.com/gravitational/teleport/lib/defaults" + "github.com/gravitational/teleport/lib/srv/alpnproxytest" "github.com/gravitational/teleport/lib/teleterm/api/uri" "github.com/gravitational/teleport/lib/teleterm/clusters" "github.com/gravitational/teleport/lib/teleterm/gateway" "github.com/gravitational/teleport/lib/teleterm/gatewaytest" + "github.com/gravitational/teleport/lib/tlsca" ) type mockGatewayCreator struct { @@ -46,6 +48,18 @@ func (m *mockGatewayCreator) CreateGateway(ctx context.Context, params clusters. hs.Close() }) + resourceURI := uri.New(params.TargetURI) + + keyPairPaths := gatewaytest.MustGenAndSaveCert(m.t, alpnproxytest.WithIdentity(tlsca.Identity{ + Username: params.TargetUser, + Groups: []string{"test-group"}, + RouteToDatabase: tlsca.RouteToDatabase{ + ServiceName: resourceURI.GetDbName(), + Protocol: defaults.ProtocolPostgres, + Username: params.TargetUser, + }, + })) + gateway, err := gateway.New(gateway.Config{ LocalPort: params.LocalPort, TargetURI: params.TargetURI, @@ -53,8 +67,8 @@ func (m *mockGatewayCreator) CreateGateway(ctx context.Context, params clusters. TargetName: params.TargetURI, TargetSubresourceName: params.TargetSubresourceName, Protocol: defaults.ProtocolPostgres, - CertPath: "../../../fixtures/certs/proxy1.pem", - KeyPath: "../../../fixtures/certs/proxy1-key.pem", + CertPath: keyPairPaths.CertPath, + KeyPath: keyPairPaths.KeyPath, Insecure: true, WebProxyAddr: hs.Listener.Addr().String(), CLICommandProvider: params.CLICommandProvider, diff --git a/lib/teleterm/gateway/gateway.go b/lib/teleterm/gateway/gateway.go index 6b9832c9935c8..81dc2d1229a0f 100644 --- a/lib/teleterm/gateway/gateway.go +++ b/lib/teleterm/gateway/gateway.go @@ -30,6 +30,7 @@ import ( alpn "github.com/gravitational/teleport/lib/srv/alpnproxy" alpncommon "github.com/gravitational/teleport/lib/srv/alpnproxy/common" "github.com/gravitational/teleport/lib/teleterm/api/uri" + "github.com/gravitational/teleport/lib/tlsca" "github.com/gravitational/teleport/lib/utils" ) @@ -82,6 +83,11 @@ func New(cfg Config) (*Gateway, error) { return nil, trace.Wrap(err) } + if err := checkCertSubject(tlsCert, cfg.RouteToDatabase()); err != nil { + return nil, trace.Wrap(err, + "database certificate check failed, try restarting the database connection") + } + localProxyConfig := alpn.LocalProxyConfig{ InsecureSkipVerify: cfg.Insecure, RemoteProxyAddr: cfg.WebProxyAddr, @@ -226,6 +232,14 @@ func (g *Gateway) CLICommand() (string, error) { return cliCommand, nil } +// RouteToDatabase returns tlsca.RouteToDatabase based on the config of the gateway. +// +// The tlsca.RouteToDatabase.Database field is skipped, as it's an optional field and gateways can +// change their Config.TargetSubresourceName at any moment. +func (g *Gateway) RouteToDatabase() tlsca.RouteToDatabase { + return g.cfg.RouteToDatabase() +} + // ReloadCert loads the key pair from cfg.CertPath & cfg.KeyPath and updates the cert of the running // local proxy. This is typically done after the cert is reissued and saved to disk. // @@ -239,11 +253,33 @@ func (g *Gateway) ReloadCert() error { return trace.Wrap(err) } + if err := checkCertSubject(tlsCert, g.RouteToDatabase()); err != nil { + return trace.Wrap(err, + "database certificate check failed, try restarting the database connection") + } + g.localProxy.SetCerts([]tls.Certificate{tlsCert}) return nil } +// checkCertSubject checks if the cert subject matches the expected db route. +// +// Database certs are scoped per database server but not per database user or database name. +// It might happen that after we save the cert but before we load it, another process obtains a +// cert for another db user. +// +// Before using the cert for the proxy, we have to perform this check. +func checkCertSubject(tlsCert tls.Certificate, dbRoute tlsca.RouteToDatabase) error { + cert, err := utils.TLSCertToX509(tlsCert) + if err != nil { + return trace.Wrap(err) + } + + err = alpn.CheckCertSubject(cert, dbRoute) + return trace.Wrap(err) +} + // Gateway describes local proxy that creates a gateway to the remote Teleport resource. // // Gateway is not safe for concurrent use in itself. However, all access to gateways is gated by diff --git a/lib/teleterm/gateway/gateway_test.go b/lib/teleterm/gateway/gateway_test.go index e077ca5e890b6..5d73c126d98c8 100644 --- a/lib/teleterm/gateway/gateway_test.go +++ b/lib/teleterm/gateway/gateway_test.go @@ -25,8 +25,10 @@ import ( "github.com/stretchr/testify/require" "github.com/gravitational/teleport/lib/defaults" + "github.com/gravitational/teleport/lib/srv/alpnproxytest" "github.com/gravitational/teleport/lib/teleterm/api/uri" "github.com/gravitational/teleport/lib/teleterm/gatewaytest" + "github.com/gravitational/teleport/lib/tlsca" ) func TestCLICommandUsesCLICommandProvider(t *testing.T) { @@ -52,14 +54,24 @@ func TestGatewayStart(t *testing.T) { hs.Close() }) + keyPairPaths := gatewaytest.MustGenAndSaveCert(t, alpnproxytest.WithIdentity(tlsca.Identity{ + Username: "alice", + Groups: []string{"test-group"}, + RouteToDatabase: tlsca.RouteToDatabase{ + ServiceName: "foo", + Protocol: defaults.ProtocolPostgres, + Username: "alice", + }, + })) + gateway, err := New( Config{ TargetName: "foo", TargetURI: uri.NewClusterURI("bar").AppendDB("foo").String(), TargetUser: "alice", Protocol: defaults.ProtocolPostgres, - CertPath: "../../../fixtures/certs/proxy1.pem", - KeyPath: "../../../fixtures/certs/proxy1-key.pem", + CertPath: keyPairPaths.CertPath, + KeyPath: keyPairPaths.KeyPath, Insecure: true, WebProxyAddr: hs.Listener.Addr().String(), CLICommandProvider: mockCLICommandProvider{}, @@ -148,14 +160,24 @@ func createGateway(t *testing.T, tcpPortAllocator TCPPortAllocator) *Gateway { hs.Close() }) + keyPairPaths := gatewaytest.MustGenAndSaveCert(t, alpnproxytest.WithIdentity(tlsca.Identity{ + Username: "alice", + Groups: []string{"test-group"}, + RouteToDatabase: tlsca.RouteToDatabase{ + ServiceName: "foo", + Protocol: defaults.ProtocolPostgres, + Username: "alice", + }, + })) + gateway, err := New( Config{ TargetName: "foo", TargetURI: uri.NewClusterURI("bar").AppendDB("foo").String(), TargetUser: "alice", Protocol: defaults.ProtocolPostgres, - CertPath: "../../../fixtures/certs/proxy1.pem", - KeyPath: "../../../fixtures/certs/proxy1-key.pem", + CertPath: keyPairPaths.CertPath, + KeyPath: keyPairPaths.KeyPath, Insecure: true, WebProxyAddr: hs.Listener.Addr().String(), CLICommandProvider: mockCLICommandProvider{}, From 11658eb806854675fc8361fcb06fc09bd36e7d00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Mon, 28 Nov 2022 12:01:12 +0100 Subject: [PATCH 07/11] Remove alpnproxytest, copy relevant code to gatewaytest This reverts commit d0bb77eb2. --- lib/srv/alpnproxy/conn_test.go | 6 +- lib/srv/alpnproxy/helpers_test.go | 72 +++++++++++++- lib/srv/alpnproxy/local_proxy_test.go | 7 +- lib/srv/alpnproxy/proxy_test.go | 19 ++-- lib/srv/alpnproxytest/helpers.go | 94 ------------------- .../dbcmd_cli_command_provider_test.go | 9 +- lib/teleterm/daemon/daemon_test.go | 5 +- lib/teleterm/gateway/gateway_test.go | 9 +- lib/teleterm/gatewaytest/helpers.go | 48 +++++++++- 9 files changed, 136 insertions(+), 133 deletions(-) delete mode 100644 lib/srv/alpnproxytest/helpers.go diff --git a/lib/srv/alpnproxy/conn_test.go b/lib/srv/alpnproxy/conn_test.go index 15984371760d6..c8371956e1da9 100644 --- a/lib/srv/alpnproxy/conn_test.go +++ b/lib/srv/alpnproxy/conn_test.go @@ -26,8 +26,6 @@ import ( "time" "github.com/stretchr/testify/require" - - "github.com/gravitational/teleport/lib/srv/alpnproxytest" ) func TestPingConnection(t *testing.T) { @@ -339,9 +337,7 @@ func makeTLSConn(t *testing.T, server, client net.Conn) (*tls.Conn, *tls.Conn) { // Server go func() { tlsConn := tls.Server(server, &tls.Config{ - Certificates: []tls.Certificate{ - alpnproxytest.MustGenCertSignedWithCA(t, alpnproxytest.MustGenSelfSignedCert(t)), - }, + Certificates: []tls.Certificate{mustGenCertSignedWithCA(t, mustGenSelfSignedCert(t))}, }) tlsConnChan <- struct { *tls.Conn diff --git a/lib/srv/alpnproxy/helpers_test.go b/lib/srv/alpnproxy/helpers_test.go index 971ded8322aa3..d38d4e004c643 100644 --- a/lib/srv/alpnproxy/helpers_test.go +++ b/lib/srv/alpnproxy/helpers_test.go @@ -18,8 +18,12 @@ package alpnproxy import ( "context" + "crypto/rand" + "crypto/rsa" "crypto/tls" "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" "fmt" "io" "net" @@ -34,8 +38,8 @@ import ( "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/lib/auth" + "github.com/gravitational/teleport/lib/defaults" "github.com/gravitational/teleport/lib/srv/alpnproxy/common" - "github.com/gravitational/teleport/lib/srv/alpnproxytest" "github.com/gravitational/teleport/lib/tlsca" ) @@ -49,7 +53,7 @@ type Suite struct { } func NewSuite(t *testing.T) *Suite { - ca := alpnproxytest.MustGenSelfSignedCert(t) + ca := mustGenSelfSignedCert(t) pool := x509.NewCertPool() pool.AddCert(ca.Cert) l, err := net.Listen("tcp", "127.0.0.1:0") @@ -104,7 +108,7 @@ func (s *Suite) GetCertPool() *x509.CertPool { } func (s *Suite) CreateProxyServer(t *testing.T) *Proxy { - serverCert := alpnproxytest.MustGenCertSignedWithCA(t, s.ca) + serverCert := mustGenCertSignedWithCA(t, s.ca) tlsConfig := &tls.Config{ NextProtos: common.ProtocolsToString(common.SupportedProtocols), ClientAuth: tls.VerifyClientCertIfGiven, @@ -145,6 +149,68 @@ func (s *Suite) Start(t *testing.T) { }) } +func mustGenSelfSignedCert(t *testing.T) *tlsca.CertAuthority { + caKey, caCert, err := tlsca.GenerateSelfSignedCA(pkix.Name{ + CommonName: "localhost", + }, []string{"localhost"}, defaults.CATTL) + require.NoError(t, err) + + ca, err := tlsca.FromKeys(caCert, caKey) + require.NoError(t, err) + return ca +} + +type signOptions struct { + identity tlsca.Identity + clock clockwork.Clock +} + +func withIdentity(identity tlsca.Identity) signOptionsFunc { + return func(o *signOptions) { + o.identity = identity + } +} + +func withClock(clock clockwork.Clock) signOptionsFunc { + return func(o *signOptions) { + o.clock = clock + } +} + +type signOptionsFunc func(o *signOptions) + +func mustGenCertSignedWithCA(t *testing.T, ca *tlsca.CertAuthority, opts ...signOptionsFunc) tls.Certificate { + options := signOptions{ + identity: tlsca.Identity{Username: "test-user"}, + clock: clockwork.NewRealClock(), + } + + for _, opt := range opts { + opt(&options) + } + + subj, err := options.identity.Subject() + require.NoError(t, err) + + privateKey, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + tlsCert, err := ca.GenerateCertificate(tlsca.CertificateRequest{ + Clock: options.clock, + PublicKey: privateKey.Public(), + Subject: subj, + NotAfter: options.clock.Now().UTC().Add(time.Minute), + DNSNames: []string{"localhost", "*.localhost"}, + }) + require.NoError(t, err) + + keyRaw := x509.MarshalPKCS1PrivateKey(privateKey) + keyPEM := pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: keyRaw}) + cert, err := tls.X509KeyPair(tlsCert, keyPEM) + require.NoError(t, err) + return cert +} + func mustReadFromConnection(t *testing.T, conn net.Conn, want string) { buff, err := io.ReadAll(conn) require.NoError(t, err) diff --git a/lib/srv/alpnproxy/local_proxy_test.go b/lib/srv/alpnproxy/local_proxy_test.go index e34c2758522f1..07ed04e76ebc9 100644 --- a/lib/srv/alpnproxy/local_proxy_test.go +++ b/lib/srv/alpnproxy/local_proxy_test.go @@ -42,7 +42,6 @@ import ( "github.com/gravitational/teleport/lib/defaults" "github.com/gravitational/teleport/lib/srv/alpnproxy/common" - "github.com/gravitational/teleport/lib/srv/alpnproxytest" "github.com/gravitational/teleport/lib/tlsca" ) @@ -376,13 +375,13 @@ func TestCheckDBCerts(t *testing.T) { for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - tlsCert := alpnproxytest.MustGenCertSignedWithCA(t, suite.ca, - alpnproxytest.WithIdentity(tlsca.Identity{ + tlsCert := mustGenCertSignedWithCA(t, suite.ca, + withIdentity(tlsca.Identity{ Username: "test-user", Groups: []string{"test-group"}, RouteToDatabase: dbRouteInCert, }), - alpnproxytest.WithClock(tt.clock), + withClock(tt.clock), ) lp.SetCerts([]tls.Certificate{tlsCert}) tt.errAssertFn(t, lp.CheckDBCerts(tt.dbRoute)) diff --git a/lib/srv/alpnproxy/proxy_test.go b/lib/srv/alpnproxy/proxy_test.go index 4a248847c7372..59410b48feec8 100644 --- a/lib/srv/alpnproxy/proxy_test.go +++ b/lib/srv/alpnproxy/proxy_test.go @@ -33,7 +33,6 @@ import ( "github.com/gravitational/teleport/api/constants" "github.com/gravitational/teleport/lib/srv/alpnproxy/common" - "github.com/gravitational/teleport/lib/srv/alpnproxytest" "github.com/gravitational/teleport/lib/srv/db/dbutils" "github.com/gravitational/teleport/lib/tlsca" ) @@ -80,7 +79,7 @@ func TestProxyKubeHandler(t *testing.T) { ) suite := NewSuite(t) - kubeCert := alpnproxytest.MustGenCertSignedWithCA(t, suite.ca) + kubeCert := mustGenCertSignedWithCA(t, suite.ca) suite.router.AddKubeHandler(func(ctx context.Context, conn net.Conn) error { defer conn.Close() tlsConn := tls.Server(conn, &tls.Config{ @@ -119,8 +118,8 @@ func TestProxyTLSDatabaseHandler(t *testing.T) { ) suite := NewSuite(t) - clientCert := alpnproxytest.MustGenCertSignedWithCA(t, suite.ca, - alpnproxytest.WithIdentity(tlsca.Identity{ + clientCert := mustGenCertSignedWithCA(t, suite.ca, + withIdentity(tlsca.Identity{ Username: "test-user", Groups: []string{"test-group"}, RouteToDatabase: tlsca.RouteToDatabase{ @@ -216,8 +215,8 @@ func TestProxyRouteToDatabase(t *testing.T) { ) suite := NewSuite(t) - clientCert := alpnproxytest.MustGenCertSignedWithCA(t, suite.ca, - alpnproxytest.WithIdentity(tlsca.Identity{ + clientCert := mustGenCertSignedWithCA(t, suite.ca, + withIdentity(tlsca.Identity{ Username: "test-user", Groups: []string{"test-group"}, RouteToDatabase: tlsca.RouteToDatabase{ @@ -359,13 +358,13 @@ func TestProxyMakeConnectionHandler(t *testing.T) { }) svr := suite.CreateProxyServer(t) - customCA := alpnproxytest.MustGenSelfSignedCert(t) + customCA := mustGenSelfSignedCert(t) // Create a ConnectionHandler from the proxy server. alpnConnHandler := svr.MakeConnectionHandler(&tls.Config{ NextProtos: []string{string(common.ProtocolHTTP)}, Certificates: []tls.Certificate{ - alpnproxytest.MustGenCertSignedWithCA(t, customCA), + mustGenCertSignedWithCA(t, customCA), }, }) @@ -610,8 +609,8 @@ func TestProxyPingConnections(t *testing.T) { dataWritten := "message ping connection" suite := NewSuite(t) - clientCert := alpnproxytest.MustGenCertSignedWithCA(t, suite.ca, - alpnproxytest.WithIdentity(tlsca.Identity{ + clientCert := mustGenCertSignedWithCA(t, suite.ca, + withIdentity(tlsca.Identity{ Username: "test-user", Groups: []string{"test-group"}, RouteToDatabase: tlsca.RouteToDatabase{ diff --git a/lib/srv/alpnproxytest/helpers.go b/lib/srv/alpnproxytest/helpers.go deleted file mode 100644 index 30bbc6b317ee8..0000000000000 --- a/lib/srv/alpnproxytest/helpers.go +++ /dev/null @@ -1,94 +0,0 @@ -// Copyright 2022 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 alpnproxytest - -import ( - "crypto/rand" - "crypto/rsa" - "crypto/tls" - "crypto/x509" - "crypto/x509/pkix" - "encoding/pem" - "testing" - "time" - - "github.com/jonboulle/clockwork" - "github.com/stretchr/testify/require" - - "github.com/gravitational/teleport/lib/defaults" - "github.com/gravitational/teleport/lib/tlsca" -) - -func MustGenSelfSignedCert(t *testing.T) *tlsca.CertAuthority { - caKey, caCert, err := tlsca.GenerateSelfSignedCA(pkix.Name{ - CommonName: "localhost", - }, []string{"localhost"}, defaults.CATTL) - require.NoError(t, err) - - ca, err := tlsca.FromKeys(caCert, caKey) - require.NoError(t, err) - return ca -} - -type signOptions struct { - identity tlsca.Identity - clock clockwork.Clock -} - -func WithIdentity(identity tlsca.Identity) SignOptionsFunc { - return func(o *signOptions) { - o.identity = identity - } -} - -func WithClock(clock clockwork.Clock) SignOptionsFunc { - return func(o *signOptions) { - o.clock = clock - } -} - -type SignOptionsFunc func(o *signOptions) - -func MustGenCertSignedWithCA(t *testing.T, ca *tlsca.CertAuthority, opts ...SignOptionsFunc) tls.Certificate { - options := signOptions{ - identity: tlsca.Identity{Username: "test-user"}, - clock: clockwork.NewRealClock(), - } - - for _, opt := range opts { - opt(&options) - } - - subj, err := options.identity.Subject() - require.NoError(t, err) - - privateKey, err := rsa.GenerateKey(rand.Reader, 2048) - require.NoError(t, err) - - tlsCert, err := ca.GenerateCertificate(tlsca.CertificateRequest{ - Clock: options.clock, - PublicKey: privateKey.Public(), - Subject: subj, - NotAfter: options.clock.Now().UTC().Add(time.Minute), - DNSNames: []string{"localhost", "*.localhost"}, - }) - require.NoError(t, err) - - keyRaw := x509.MarshalPKCS1PrivateKey(privateKey) - keyPEM := pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: keyRaw}) - cert, err := tls.X509KeyPair(tlsCert, keyPEM) - require.NoError(t, err) - return cert -} diff --git a/lib/teleterm/clusters/dbcmd_cli_command_provider_test.go b/lib/teleterm/clusters/dbcmd_cli_command_provider_test.go index 274c43b0c5eeb..81f9f6dce7fca 100644 --- a/lib/teleterm/clusters/dbcmd_cli_command_provider_test.go +++ b/lib/teleterm/clusters/dbcmd_cli_command_provider_test.go @@ -25,7 +25,6 @@ import ( "github.com/gravitational/teleport/lib/client" "github.com/gravitational/teleport/lib/defaults" - "github.com/gravitational/teleport/lib/srv/alpnproxytest" "github.com/gravitational/teleport/lib/teleterm/api/uri" "github.com/gravitational/teleport/lib/teleterm/gateway" "github.com/gravitational/teleport/lib/teleterm/gatewaytest" @@ -93,7 +92,7 @@ func TestDbcmdCLICommandProviderGetCommand(t *testing.T) { } dbcmdCLICommandProvider := NewDbcmdCLICommandProvider(fakeStorage, fakeExec{}) - keyPairPaths := gatewaytest.MustGenAndSaveCert(t, alpnproxytest.WithIdentity(tlsca.Identity{ + keyPairPaths := gatewaytest.MustGenAndSaveCert(t, tlsca.Identity{ Username: "alice", Groups: []string{"test-group"}, RouteToDatabase: tlsca.RouteToDatabase{ @@ -101,7 +100,7 @@ func TestDbcmdCLICommandProviderGetCommand(t *testing.T) { Protocol: defaults.ProtocolPostgres, Username: "alice", }, - })) + }) gateway, err := gateway.New( gateway.Config{ @@ -138,7 +137,7 @@ func TestDbcmdCLICommandProviderGetCommand_ReturnsErrorIfClusterIsNotFound(t *te } dbcmdCLICommandProvider := NewDbcmdCLICommandProvider(fakeStorage, fakeExec{}) - keyPairPaths := gatewaytest.MustGenAndSaveCert(t, alpnproxytest.WithIdentity(tlsca.Identity{ + keyPairPaths := gatewaytest.MustGenAndSaveCert(t, tlsca.Identity{ Username: "alice", Groups: []string{"test-group"}, RouteToDatabase: tlsca.RouteToDatabase{ @@ -146,7 +145,7 @@ func TestDbcmdCLICommandProviderGetCommand_ReturnsErrorIfClusterIsNotFound(t *te Protocol: defaults.ProtocolPostgres, Username: "alice", }, - })) + }) gateway, err := gateway.New( gateway.Config{ diff --git a/lib/teleterm/daemon/daemon_test.go b/lib/teleterm/daemon/daemon_test.go index 3d28fa3a05426..111c21d9a58ab 100644 --- a/lib/teleterm/daemon/daemon_test.go +++ b/lib/teleterm/daemon/daemon_test.go @@ -27,7 +27,6 @@ import ( "google.golang.org/grpc/credentials/insecure" "github.com/gravitational/teleport/lib/defaults" - "github.com/gravitational/teleport/lib/srv/alpnproxytest" "github.com/gravitational/teleport/lib/teleterm/api/uri" "github.com/gravitational/teleport/lib/teleterm/clusters" "github.com/gravitational/teleport/lib/teleterm/gateway" @@ -50,7 +49,7 @@ func (m *mockGatewayCreator) CreateGateway(ctx context.Context, params clusters. resourceURI := uri.New(params.TargetURI) - keyPairPaths := gatewaytest.MustGenAndSaveCert(m.t, alpnproxytest.WithIdentity(tlsca.Identity{ + keyPairPaths := gatewaytest.MustGenAndSaveCert(m.t, tlsca.Identity{ Username: params.TargetUser, Groups: []string{"test-group"}, RouteToDatabase: tlsca.RouteToDatabase{ @@ -58,7 +57,7 @@ func (m *mockGatewayCreator) CreateGateway(ctx context.Context, params clusters. Protocol: defaults.ProtocolPostgres, Username: params.TargetUser, }, - })) + }) gateway, err := gateway.New(gateway.Config{ LocalPort: params.LocalPort, diff --git a/lib/teleterm/gateway/gateway_test.go b/lib/teleterm/gateway/gateway_test.go index 5d73c126d98c8..b412570067997 100644 --- a/lib/teleterm/gateway/gateway_test.go +++ b/lib/teleterm/gateway/gateway_test.go @@ -25,7 +25,6 @@ import ( "github.com/stretchr/testify/require" "github.com/gravitational/teleport/lib/defaults" - "github.com/gravitational/teleport/lib/srv/alpnproxytest" "github.com/gravitational/teleport/lib/teleterm/api/uri" "github.com/gravitational/teleport/lib/teleterm/gatewaytest" "github.com/gravitational/teleport/lib/tlsca" @@ -54,7 +53,7 @@ func TestGatewayStart(t *testing.T) { hs.Close() }) - keyPairPaths := gatewaytest.MustGenAndSaveCert(t, alpnproxytest.WithIdentity(tlsca.Identity{ + keyPairPaths := gatewaytest.MustGenAndSaveCert(t, tlsca.Identity{ Username: "alice", Groups: []string{"test-group"}, RouteToDatabase: tlsca.RouteToDatabase{ @@ -62,7 +61,7 @@ func TestGatewayStart(t *testing.T) { Protocol: defaults.ProtocolPostgres, Username: "alice", }, - })) + }) gateway, err := New( Config{ @@ -160,7 +159,7 @@ func createGateway(t *testing.T, tcpPortAllocator TCPPortAllocator) *Gateway { hs.Close() }) - keyPairPaths := gatewaytest.MustGenAndSaveCert(t, alpnproxytest.WithIdentity(tlsca.Identity{ + keyPairPaths := gatewaytest.MustGenAndSaveCert(t, tlsca.Identity{ Username: "alice", Groups: []string{"test-group"}, RouteToDatabase: tlsca.RouteToDatabase{ @@ -168,7 +167,7 @@ func createGateway(t *testing.T, tcpPortAllocator TCPPortAllocator) *Gateway { Protocol: defaults.ProtocolPostgres, Username: "alice", }, - })) + }) gateway, err := New( Config{ diff --git a/lib/teleterm/gatewaytest/helpers.go b/lib/teleterm/gatewaytest/helpers.go index abb3b4cd19d71..57cc8cda2c4b6 100644 --- a/lib/teleterm/gatewaytest/helpers.go +++ b/lib/teleterm/gatewaytest/helpers.go @@ -15,8 +15,11 @@ package gatewaytest import ( + "crypto/rand" "crypto/rsa" + "crypto/tls" "crypto/x509" + "crypto/x509/pkix" "encoding/pem" "fmt" "net" @@ -25,10 +28,12 @@ import ( "time" "github.com/gravitational/trace" + "github.com/jonboulle/clockwork" "github.com/stretchr/testify/require" "golang.org/x/exp/slices" - "github.com/gravitational/teleport/lib/srv/alpnproxytest" + "github.com/gravitational/teleport/lib/defaults" + "github.com/gravitational/teleport/lib/tlsca" ) const timeout = time.Second * 5 @@ -140,14 +145,14 @@ type KeyPairPaths struct { KeyPath string } -func MustGenAndSaveCert(t *testing.T, opts ...alpnproxytest.SignOptionsFunc) KeyPairPaths { +func MustGenAndSaveCert(t *testing.T, identity tlsca.Identity) KeyPairPaths { t.Helper() dir := t.TempDir() - ca := alpnproxytest.MustGenSelfSignedCert(t) + ca := mustGenCACert(t) - tlsCert := alpnproxytest.MustGenCertSignedWithCA(t, ca, opts...) + tlsCert := mustGenCertSignedWithCA(t, ca, identity) privateKey, ok := tlsCert.PrivateKey.(*rsa.PrivateKey) if !ok { @@ -181,3 +186,38 @@ func MustGenAndSaveCert(t *testing.T, opts ...alpnproxytest.SignOptionsFunc) Key KeyPath: keyFile.Name(), } } + +func mustGenCACert(t *testing.T) *tlsca.CertAuthority { + caKey, caCert, err := tlsca.GenerateSelfSignedCA(pkix.Name{ + CommonName: "localhost", + }, []string{"localhost"}, defaults.CATTL) + require.NoError(t, err) + + ca, err := tlsca.FromKeys(caCert, caKey) + require.NoError(t, err) + return ca +} + +func mustGenCertSignedWithCA(t *testing.T, ca *tlsca.CertAuthority, identity tlsca.Identity) tls.Certificate { + clock := clockwork.NewRealClock() + subj, err := identity.Subject() + require.NoError(t, err) + + privateKey, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + tlsCert, err := ca.GenerateCertificate(tlsca.CertificateRequest{ + Clock: clock, + PublicKey: privateKey.Public(), + Subject: subj, + NotAfter: clock.Now().UTC().Add(time.Minute), + DNSNames: []string{"localhost", "*.localhost"}, + }) + require.NoError(t, err) + + keyRaw := x509.MarshalPKCS1PrivateKey(privateKey) + keyPEM := pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: keyRaw}) + cert, err := tls.X509KeyPair(tlsCert, keyPEM) + require.NoError(t, err) + return cert +} From cc0ba70f0e92d52d7054e71e72eee34c615732f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Mon, 28 Nov 2022 12:08:10 +0100 Subject: [PATCH 08/11] uri: Add test name, add godoc for GetDbName --- lib/teleterm/api/uri/uri.go | 1 + lib/teleterm/api/uri/uri_test.go | 37 +++++++++++++++++++------------- 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/lib/teleterm/api/uri/uri.go b/lib/teleterm/api/uri/uri.go index b774978c83498..c13d811e87c20 100644 --- a/lib/teleterm/api/uri/uri.go +++ b/lib/teleterm/api/uri/uri.go @@ -94,6 +94,7 @@ func (r ResourceURI) GetLeafClusterName() string { return result.Params["leaf"] } +// GetDbName extracts the database name from r. Returns an empty string if r is not a database URI. func (r ResourceURI) GetDbName() string { result, ok := pathDbs.Match(r.path) if ok { diff --git a/lib/teleterm/api/uri/uri_test.go b/lib/teleterm/api/uri/uri_test.go index 8765db9d29739..04273a1c68c9f 100644 --- a/lib/teleterm/api/uri/uri_test.go +++ b/lib/teleterm/api/uri/uri_test.go @@ -86,37 +86,44 @@ func TestParseClusterURI(t *testing.T) { func TestGetDbName(t *testing.T) { tests := []struct { - in uri.ResourceURI - out string + name string + in uri.ResourceURI + out string }{ { - uri.NewClusterURI("foo").AppendDB("postgres"), - "postgres", + name: "returns root cluster db name", + in: uri.NewClusterURI("foo").AppendDB("postgres"), + out: "postgres", }, { - uri.NewClusterURI("foo").AppendLeafCluster("bar").AppendDB("postgres"), - "postgres", + name: "returns leaf cluster db name", + in: uri.NewClusterURI("foo").AppendLeafCluster("bar").AppendDB("postgres"), + out: "postgres", }, { - uri.NewClusterURI("foo"), - "", + name: "returns empty string when given root cluster URI", + in: uri.NewClusterURI("foo"), + out: "", }, { - uri.NewClusterURI("foo").AppendLeafCluster("bar"), - "", + name: "returns empty string when given leaf cluster URI", + in: uri.NewClusterURI("foo").AppendLeafCluster("bar"), + out: "", }, { - uri.NewClusterURI("foo").AppendKube("k8s"), - "", + name: "returns empty string when given root cluster non-db resource URI", + in: uri.NewClusterURI("foo").AppendKube("k8s"), + out: "", }, { - uri.NewClusterURI("foo").AppendLeafCluster("bar").AppendKube("k8s"), - "", + name: "returns empty string when given leaf cluster non-db resource URI", + in: uri.NewClusterURI("foo").AppendLeafCluster("bar").AppendKube("k8s"), + out: "", }, } for _, tt := range tests { - t.Run(tt.in.String(), func(t *testing.T) { + t.Run(tt.name, func(t *testing.T) { out := tt.in.GetDbName() require.Equal(t, tt.out, out) }) From 137835da5debeaaf253a447ec8d7dc662a8246e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Mon, 28 Nov 2022 12:12:44 +0100 Subject: [PATCH 09/11] Use require.True for private key typecast check --- lib/teleterm/gatewaytest/helpers.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/teleterm/gatewaytest/helpers.go b/lib/teleterm/gatewaytest/helpers.go index 57cc8cda2c4b6..e8e7be6d0a17e 100644 --- a/lib/teleterm/gatewaytest/helpers.go +++ b/lib/teleterm/gatewaytest/helpers.go @@ -155,9 +155,7 @@ func MustGenAndSaveCert(t *testing.T, identity tlsca.Identity) KeyPairPaths { tlsCert := mustGenCertSignedWithCA(t, ca, identity) privateKey, ok := tlsCert.PrivateKey.(*rsa.PrivateKey) - if !ok { - t.Fatal("Failed to cast tlsCert.PrivateKey") - } + require.True(t, ok, "Failed to cast tlsCert.PrivateKey") // Save the cert. From 543d331bc14a2cae6fab622293cc2ad945417677 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Mon, 28 Nov 2022 17:14:11 +0100 Subject: [PATCH 10/11] Update comment for GetDbName (r -> path) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Marek Smoliński --- lib/teleterm/api/uri/uri.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/teleterm/api/uri/uri.go b/lib/teleterm/api/uri/uri.go index c13d811e87c20..a0107606f1e51 100644 --- a/lib/teleterm/api/uri/uri.go +++ b/lib/teleterm/api/uri/uri.go @@ -94,7 +94,7 @@ func (r ResourceURI) GetLeafClusterName() string { return result.Params["leaf"] } -// GetDbName extracts the database name from r. Returns an empty string if r is not a database URI. +// GetDbName extracts the database name from r. Returns an empty string if path is not a database URI. func (r ResourceURI) GetDbName() string { result, ok := pathDbs.Match(r.path) if ok { From 1f8cbaf37bbcadd0bcdebf5a59dad8ab1a0f898f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Tue, 29 Nov 2022 10:52:35 +0100 Subject: [PATCH 11/11] Remove unnecessary assignment to err --- lib/srv/alpnproxy/local_proxy.go | 3 +-- lib/teleterm/gateway/gateway.go | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/srv/alpnproxy/local_proxy.go b/lib/srv/alpnproxy/local_proxy.go index 2ef08371aa32e..4f95c84e6fe08 100644 --- a/lib/srv/alpnproxy/local_proxy.go +++ b/lib/srv/alpnproxy/local_proxy.go @@ -292,8 +292,7 @@ func (l *LocalProxy) CheckDBCerts(dbRoute tlsca.RouteToDatabase) error { return trace.Wrap(err) } - err = CheckCertSubject(cert, dbRoute) - return trace.Wrap(err) + return trace.Wrap(CheckCertSubject(cert, dbRoute)) } // CheckCertSubject checks if the route to the database from the cert matches the provided route in diff --git a/lib/teleterm/gateway/gateway.go b/lib/teleterm/gateway/gateway.go index 81dc2d1229a0f..3ebe263ccefa0 100644 --- a/lib/teleterm/gateway/gateway.go +++ b/lib/teleterm/gateway/gateway.go @@ -276,8 +276,7 @@ func checkCertSubject(tlsCert tls.Certificate, dbRoute tlsca.RouteToDatabase) er return trace.Wrap(err) } - err = alpn.CheckCertSubject(cert, dbRoute) - return trace.Wrap(err) + return trace.Wrap(alpn.CheckCertSubject(cert, dbRoute)) } // Gateway describes local proxy that creates a gateway to the remote Teleport resource.