Skip to content

Commit

Permalink
[v12] Backport IP pinning for Kube and DB access (#23418)
Browse files Browse the repository at this point in the history
* Add secure client IP propagation throughout teleport (#21080)

* Allow node to handle old and new way of client IP propagation on same listener

With addition of signed PROXY headers, node was listening on multiplexer, but because
 of that it couldn't processing incoming connection from older proxies
 when ProxyHelloSignature was used, because
 both ends were waiting for the other side to send data first.
 Here we integrate ability to handle PROXY headers into connection itself,
 so we can start ssh server without waiting for multiplexer to detect connection

* Enabled IP pinning enforcement for Kube and DB (#22310)

* Don't allow different tcp version IP addresses in signed PROXY headers

* Send signed proxy header to the kube service

Because it was checking version, which was empty, signed headers were not sent,
 when we contacted leaf cluster's kube service

* Temporary disable ip propagation tests.
  • Loading branch information
AntonAM committed Apr 3, 2023
1 parent 437ff26 commit 6519aab
Show file tree
Hide file tree
Showing 63 changed files with 2,057 additions and 483 deletions.
8 changes: 8 additions & 0 deletions api/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,14 @@ const (
// allowed GCP service accounts.
TraitGCPServiceAccounts = "gcp_service_accounts"
)
const (
// ProxyHelloSignature is a string which Teleport proxy will send
// right after the initial SSH "handshake/version" message if it detects
// talking to a Teleport server.
//
// This is also leveraged by tsh to propagate its tracing span ID.
ProxyHelloSignature = "Teleport-Proxy"
)

const (
// TimeoutGetClusterAlerts is the timeout for grabbing cluster alerts from tctl and tsh
Expand Down
3 changes: 2 additions & 1 deletion api/observability/tracing/ssh/ssh.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
oteltrace "go.opentelemetry.io/otel/trace"
"golang.org/x/crypto/ssh"

"github.com/gravitational/teleport/api/constants"
"github.com/gravitational/teleport/api/observability/tracing"
"github.com/gravitational/teleport/api/utils/sshutils"
)
Expand Down Expand Up @@ -149,7 +150,7 @@ func NewClientConn(ctx context.Context, conn net.Conn, addr string, config *ssh.
if len(hp.TracingContext) > 0 {
payloadJSON, err := json.Marshal(hp)
if err == nil {
payload := fmt.Sprintf("%s%s\x00", sshutils.ProxyHelloSignature, payloadJSON)
payload := fmt.Sprintf("%s%s\x00", constants.ProxyHelloSignature, payloadJSON)
if _, err := conn.Write([]byte(payload)); err != nil {
log.WithError(err).Warnf("Failed to pass along tracing context to proxy %v", addr)
}
Expand Down
12 changes: 12 additions & 0 deletions api/utils/sshutils/conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ func ConnectProxyTransport(sconn ssh.Conn, req *DialReq, exclusive bool) (conn *
if exclusive {
return NewExclusiveChConn(sconn, channel), false, nil
}

return NewChConn(sconn, channel), false, nil
}

Expand All @@ -92,6 +93,17 @@ type DialReq struct {

// ConnType is the type of connection requested, either node or application.
ConnType types.TunnelType `json:"conn_type"`

// TeleportVersion shows what teleport version is the node that we're trying to dial
TeleportVersion string `json:"teleport_version,omitempty"`

// ClientSrcAddr is the original observed client address, it is used to propagate
// correct client IP through indirect connections inside teleport
ClientSrcAddr string `json:"client_src_addr,omitempty"`

// ClientDstAddr is the original client's destination address, it is used to propagate
// correct client point of contact through indirect connections inside teleport
ClientDstAddr string `json:"client_dst_addr,omitempty"`
}

// CheckAndSetDefaults verifies all the values are valid.
Expand Down
9 changes: 0 additions & 9 deletions api/utils/sshutils/ssh.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,6 @@ import (
"github.com/gravitational/teleport/api/defaults"
)

const (
// ProxyHelloSignature is a string which Teleport proxy will send
// right after the initial SSH "handshake/version" message if it detects
// talking to a Teleport server.
//
// This is also leveraged by tsh to propagate its tracing span ID.
ProxyHelloSignature = "Teleport-Proxy"
)

// HandshakePayload structure is sent as a JSON blob by the teleport
// proxy to every SSH server who identifies itself as Teleport server
//
Expand Down
89 changes: 89 additions & 0 deletions integration/db/db_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import (
"github.com/gravitational/teleport/lib/auth"
"github.com/gravitational/teleport/lib/defaults"
"github.com/gravitational/teleport/lib/events"
"github.com/gravitational/teleport/lib/modules"
"github.com/gravitational/teleport/lib/service"
"github.com/gravitational/teleport/lib/services"
"github.com/gravitational/teleport/lib/srv/db"
Expand Down Expand Up @@ -82,6 +83,8 @@ func TestDatabaseAccess(t *testing.T) {
t.Run("CassandraRootCluster", pack.testCassandraRootCluster)
t.Run("CassandraLeafCluster", pack.testCassandraLeafCluster)

t.Run("IPPinning", pack.testIPPinning)

// This test should go last because it rotates the Database CA.
t.Run("RotateTrustedCluster", pack.testRotateTrustedCluster)
}
Expand All @@ -96,6 +99,92 @@ func TestDatabaseAccessSeparateListeners(t *testing.T) {
t.Run("MongoSeparateListener", pack.testMongoSeparateListener)
}

// testIPPinning tests a scenario where a user with IP pinning
// connects to a database
func (p *DatabasePack) testIPPinning(t *testing.T) {
modules.SetTestModules(t, &modules.TestModules{
TestBuildType: modules.BuildEnterprise,
TestFeatures: modules.Features{DB: true},
})

type testCase struct {
desc string
targetCluster databaseClusterPack
pinnedIP string
wantClientErr string
}

testCases := []testCase{
{
desc: "root cluster, no pinned ip",
targetCluster: p.Root,
},
{
desc: "root cluster, correct pinned ip",
targetCluster: p.Root,
pinnedIP: "127.0.0.1",
},
{
desc: "root cluster, incorrect pinned ip",
targetCluster: p.Root,
wantClientErr: "pinned IP doesn't match observed client IP",
pinnedIP: "127.0.0.2",
},
{
desc: "leaf cluster, no pinned ip",
targetCluster: p.Leaf,
},
{
desc: "leaf cluster, correct pinned ip",
targetCluster: p.Leaf,
pinnedIP: "127.0.0.1",
},
{
desc: "leaf cluster, incorrect pinned ip",
targetCluster: p.Leaf,
wantClientErr: "pinned IP doesn't match observed client IP",
pinnedIP: "127.0.0.2",
},
}

for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
// Connect to the database service in root cluster.
testClient, err := postgres.MakeTestClient(context.Background(), common.TestClientConfig{
AuthClient: p.Root.Cluster.GetSiteAPI(p.Root.Cluster.Secrets.SiteName),
AuthServer: p.Root.Cluster.Process.GetAuthServer(),
Address: p.Root.Cluster.Web,
Cluster: tc.targetCluster.Cluster.Secrets.SiteName,
Username: p.Root.User.GetName(),
PinnedIP: tc.pinnedIP,
RouteToDatabase: tlsca.RouteToDatabase{
ServiceName: tc.targetCluster.PostgresService.Name,
Protocol: tc.targetCluster.PostgresService.Protocol,
Username: "postgres",
Database: "test",
},
})
if tc.wantClientErr != "" {
require.ErrorContains(t, err, tc.wantClientErr)
return
}
require.NoError(t, err)

wantQueryCount := tc.targetCluster.postgres.QueryCount() + 1

// Execute a query.
result, err := testClient.Exec(context.Background(), "select 1").ReadAll()
require.NoError(t, err)
require.Equal(t, []*pgconn.Result{postgres.TestQueryResponse}, result)
require.Equal(t, wantQueryCount, tc.targetCluster.postgres.QueryCount())

// Disconnect.
err = testClient.Close(context.Background())
require.NoError(t, err)
})
}
}

// testPostgresRootCluster tests a scenario where a user connects
// to a Postgres database running in a root cluster.
func (p *DatabasePack) testPostgresRootCluster(t *testing.T) {
Expand Down
10 changes: 7 additions & 3 deletions integration/helpers/discard.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ package helpers

import (
"context"
"fmt"
"net"

"github.com/gravitational/trace"
"golang.org/x/crypto/ssh"
Expand All @@ -33,13 +33,13 @@ type DiscardServer struct {
sshServer *sshutils.Server
}

func NewDiscardServer(host string, port int, hostSigner ssh.Signer) (*DiscardServer, error) {
func NewDiscardServer(hostSigner ssh.Signer, listener net.Listener) (*DiscardServer, error) {
ds := &DiscardServer{}

// create underlying ssh server
sshServer, err := sshutils.NewServer(
"integration-discard-server",
utils.NetAddr{AddrNetwork: "tcp", Addr: fmt.Sprintf("%v:%v", host, port)},
utils.NetAddr{AddrNetwork: "tcp", Addr: listener.Addr().String()},
ds,
[]ssh.Signer{hostSigner},
sshutils.AuthMethods{
Expand All @@ -50,6 +50,10 @@ func NewDiscardServer(host string, port int, hostSigner ssh.Signer) (*DiscardSer
if err != nil {
return nil, trace.Wrap(err)
}

if err := sshServer.SetListener(listener); err != nil {
return nil, trace.Wrap(err)
}
ds.sshServer = sshServer

return ds, nil
Expand Down
16 changes: 16 additions & 0 deletions integration/helpers/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import (
"github.com/gravitational/teleport/lib/client/identityfile"
"github.com/gravitational/teleport/lib/cloud"
"github.com/gravitational/teleport/lib/defaults"
"github.com/gravitational/teleport/lib/multiplexer"
"github.com/gravitational/teleport/lib/service"
"github.com/gravitational/teleport/lib/services"
"github.com/gravitational/teleport/lib/teleagent"
Expand Down Expand Up @@ -311,6 +312,21 @@ func WaitForDatabaseServers(t *testing.T, authServer *auth.Server, dbs []service
}
}

// CreatePROXYEnabledListener creates net.Listener that can handle receiving signed PROXY headers
func CreatePROXYEnabledListener(ctx context.Context, t *testing.T, address string, caGetter multiplexer.CertAuthorityGetter, clusterName string) (net.Listener, error) {
t.Helper()

listener, err := net.Listen("tcp", address)
require.NoError(t, err)

return multiplexer.NewPROXYEnabledListener(multiplexer.Config{
Listener: listener,
Context: ctx,
CertAuthorityGetter: caGetter,
LocalClusterName: clusterName,
})
}

// MakeTestServers starts an Auth and a Proxy Service.
// Besides those processes, it also returns a provision token which can be used to add other services.
func MakeTestServers(t *testing.T) (auth *service.TeleportProcess, proxy *service.TeleportProcess, provisionToken string) {
Expand Down

0 comments on commit 6519aab

Please sign in to comment.