Skip to content

Commit

Permalink
GitHub proxy part 6: proxing Git using SSH transport (#49980)
Browse files Browse the repository at this point in the history
* GitHub proxy part 6: proxing Git using SSH transport

* better command parsing and update suite

* refactor

* revert unnecearrty files

* address review comments

* ut fix

* revert localsite_test.go

* change special suffix to teleport-github-org for routing

* fix routing ut

* minor typo edit

* fix ut after sshca change

* add UT to sshutils

* minor review comments

* fix api ut because of special suffix change

* GitServerReadOnlyClient

* downgrade error to warning

* run go mod tidy. not sure why it's needed

* rename mock.go to mock_test.go
  • Loading branch information
greedy52 authored and mvbrock committed Jan 18, 2025
1 parent dfcd83d commit a718918
Show file tree
Hide file tree
Showing 41 changed files with 2,176 additions and 74 deletions.
7 changes: 6 additions & 1 deletion api/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -4942,11 +4942,16 @@ func (c *Client) UserTasksServiceClient() *usertaskapi.Client {
return usertaskapi.NewClient(usertaskv1.NewUserTaskServiceClient(c.conn))
}

// GitServerClient returns a client for managing git servers
// GitServerClient returns a client for managing Git servers
func (c *Client) GitServerClient() *gitserverclient.Client {
return gitserverclient.NewClient(gitserverpb.NewGitServerServiceClient(c.conn))
}

// GitServerReadOnlyClient returns the read-only client for Git servers.
func (c *Client) GitServerReadOnlyClient() gitserverclient.ReadOnlyClient {
return c.GitServerClient()
}

// GetCertAuthority retrieves a CA by type and domain.
func (c *Client) GetCertAuthority(ctx context.Context, id types.CertAuthID, loadKeys bool) (types.CertAuthority, error) {
ca, err := c.TrustClient().GetCertAuthority(ctx, &trustpb.GetCertAuthorityRequest{
Expand Down
8 changes: 8 additions & 0 deletions api/client/gitserver/gitserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,14 @@ import (
"github.com/gravitational/teleport/api/types"
)

// ReadOnlyClient defines getter functions for Git servers.
type ReadOnlyClient interface {
// ListGitServers returns a paginated list of Git servers.
ListGitServers(ctx context.Context, pageSize int, pageToken string) ([]types.Server, string, error)
// GetGitServer returns a Git server by name.
GetGitServer(ctx context.Context, name string) (types.Server, error)
}

// Client is an Git servers client.
type Client struct {
grpcClient gitserverv1.GitServerServiceClient
Expand Down
2 changes: 1 addition & 1 deletion api/types/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -1496,5 +1496,5 @@ const (
const (
// GitHubOrgServerDomain is the sub domain used in the hostname of a
// types.Server to indicate the GitHub organization of a Git server.
GitHubOrgServerDomain = "github-org"
GitHubOrgServerDomain = "teleport-github-org"
)
3 changes: 3 additions & 0 deletions api/types/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -626,6 +626,9 @@ func (s *ServerV2) githubCheckAndSetDefaults() error {
return trace.Wrap(err, "invalid GitHub organization name")
}

// Set SSH host port for connection and "fake" hostname for routing. These
// values are hard-coded and cannot be customized.
s.Spec.Addr = "github.com:22"
s.Spec.Hostname = MakeGitHubOrgServerDomain(s.Spec.GitHub.Organization)
if s.Metadata.Labels == nil {
s.Metadata.Labels = make(map[string]string)
Expand Down
5 changes: 3 additions & 2 deletions api/types/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,8 @@ func TestServerCheckAndSetDefaults(t *testing.T) {
},
},
Spec: ServerSpecV2{
Hostname: "my-org.github-org",
Addr: "github.com:22",
Hostname: "my-org.teleport-github-org",
GitHub: &GitHubServerMetadata{
Integration: "my-org",
Organization: "my-org",
Expand Down Expand Up @@ -807,7 +808,7 @@ func TestGetCloudMetadataAWS(t *testing.T) {

func TestGitServerOrgDomain(t *testing.T) {
domain := MakeGitHubOrgServerDomain("my-org")
require.Equal(t, "my-org.github-org", domain)
require.Equal(t, "my-org.teleport-github-org", domain)

githubNodeAddr := domain + ":22"
org, ok := GetGitHubOrgFromNodeAddr(githubNodeAddr)
Expand Down
3 changes: 3 additions & 0 deletions constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,9 @@ const (
// ComponentRolloutController represents the autoupdate_agent_rollout controller.
ComponentRolloutController = "rollout-controller"

// ComponentForwardingGit represents the SSH proxy that forwards Git commands.
ComponentForwardingGit = "git:forward"

// VerboseLogsEnvVar forces all logs to be verbose (down to DEBUG level)
VerboseLogsEnvVar = "TELEPORT_DEBUG"

Expand Down
2 changes: 2 additions & 0 deletions integrations/terraform/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1596,6 +1596,8 @@ github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D
github.com/mattn/go-runewidth v0.0.4/go.mod h1:LwmH8dsx7+W8Uxz3IHJYH5QSwggIsqBzpuz5H//U1FU=
github.com/mattn/go-runewidth v0.0.16 h1:E5ScNMtiwvlvB5paMFdw9p4kSQzbXFikJ5SQO6TULQc=
github.com/mattn/go-runewidth v0.0.16/go.mod h1:Jdepj2loyihRzMpdS35Xk/zdY8IAYHsh153qUoGf23w=
github.com/mattn/go-shellwords v1.0.12 h1:M2zGm7EW6UQJvDeQxo4T51eKPurbeFbe8WtebGE2xrk=
github.com/mattn/go-shellwords v1.0.12/go.mod h1:EZzvwXDESEeg03EKmM+RmDnNOPKG4lLtQsUlTZDWQ8Y=
github.com/mattn/go-sqlite3 v1.14.14/go.mod h1:NyWgC/yNuGj7Q9rpYnZvas74GogHl5/Z4A/KQRfk6bU=
github.com/mattn/go-sqlite3 v1.14.22/go.mod h1:Uh1q+B4BYcTPb+yiD3kU8Ct7aC0hY9fxUwlHK0RXw+Y=
github.com/mattn/go-sqlite3 v1.14.24 h1:tpSp2G2KyMnnQu99ngJ47EIkWVmliIizyZBfPrBWDRM=
Expand Down
7 changes: 7 additions & 0 deletions lib/auth/authclient/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/gravitational/trace"
"google.golang.org/grpc"

"github.com/gravitational/teleport/api/client/gitserver"
"github.com/gravitational/teleport/api/client/proto"
accessmonitoringrules "github.com/gravitational/teleport/api/gen/proto/go/teleport/accessmonitoringrules/v1"
"github.com/gravitational/teleport/api/gen/proto/go/teleport/autoupdate/v1"
Expand Down Expand Up @@ -320,6 +321,9 @@ type ReadProxyAccessPoint interface {

// GetAutoUpdateAgentRollout gets the AutoUpdateAgentRollout from the backend.
GetAutoUpdateAgentRollout(ctx context.Context) (*autoupdate.AutoUpdateAgentRollout, error)

// GitServerReadOnlyClient returns the read-only client for Git servers.
GitServerReadOnlyClient() gitserver.ReadOnlyClient
}

// SnowflakeSessionWatcher is watcher interface used by Snowflake web session watcher.
Expand Down Expand Up @@ -1264,6 +1268,9 @@ type Cache interface {

// GetPluginStaticCredentialsByLabels will get a list of plugin static credentials resource by matching labels.
GetPluginStaticCredentialsByLabels(ctx context.Context, labels map[string]string) ([]types.PluginStaticCredentials, error)

// GitServerGetter defines methods for fetching Git servers.
services.GitServerGetter
}

type NodeWrapper struct {
Expand Down
3 changes: 3 additions & 0 deletions lib/auth/authclient/clt.go
Original file line number Diff line number Diff line change
Expand Up @@ -1900,4 +1900,7 @@ type ClientI interface {

// GitServerClient returns git server client.
GitServerClient() *gitserver.Client

// GitServerReadOnlyClient returns the read-only client for Git servers.
GitServerReadOnlyClient() gitserver.ReadOnlyClient
}
1 change: 1 addition & 0 deletions lib/cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ func ForRemoteProxy(cfg Config) Config {
{Kind: types.KindDatabaseServer},
{Kind: types.KindDatabaseService},
{Kind: types.KindKubeServer},
{Kind: types.KindGitServer},
}
cfg.QueueSize = defaults.ProxyQueueSize
return cfg
Expand Down
10 changes: 10 additions & 0 deletions lib/cache/git_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,21 @@ import (

"github.com/gravitational/trace"

"github.com/gravitational/teleport/api/client/gitserver"
apidefaults "github.com/gravitational/teleport/api/defaults"
"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/lib/services"
)

// GitServerReadOnlyClient returns the read-only client for Git servers.
//
// Note that Cache implements GitServerReadOnlyClient to satisfy
// auth.ProxyAccessPoint but also has the getter functions at top level to
// satisfy auth.Cache.
func (c *Cache) GitServerReadOnlyClient() gitserver.ReadOnlyClient {
return c
}

func (c *Cache) GetGitServer(ctx context.Context, name string) (types.Server, error) {
ctx, span := c.Tracer.Start(ctx, "cache/GetGitServer")
defer span.End()
Expand Down
11 changes: 9 additions & 2 deletions lib/cryptosuites/suites.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,10 @@ const (
// GitHubProxyCASSH represents the SSH key for GitHub proxy CAs.
GitHubProxyCASSH

// GitClient represents a key used to forward Git commands to Git services
// like GitHub.
GitClient

// keyPurposeMax is 1 greater than the last valid key purpose, used to test that all values less than this
// are valid for each suite.
keyPurposeMax
Expand Down Expand Up @@ -187,8 +191,8 @@ var (
ProxyKubeClient: RSA2048,
// EC2InstanceConnect has always used Ed25519 by default.
EC2InstanceConnect: Ed25519,
// GitHubProxyCASSH uses same algorithms as UserCASSH.
GitHubProxyCASSH: RSA2048,
GitHubProxyCASSH: Ed25519,
GitClient: Ed25519,
}

// balancedV1 strikes a balance between security, compatibility, and
Expand Down Expand Up @@ -220,6 +224,7 @@ var (
ProxyKubeClient: ECDSAP256,
EC2InstanceConnect: Ed25519,
GitHubProxyCASSH: Ed25519,
GitClient: Ed25519,
}

// fipsv1 is an algorithm suite tailored for FIPS compliance. It is based on
Expand Down Expand Up @@ -251,6 +256,7 @@ var (
ProxyKubeClient: ECDSAP256,
EC2InstanceConnect: ECDSAP256,
GitHubProxyCASSH: ECDSAP256,
GitClient: ECDSAP256,
}

// hsmv1 in an algorithm suite tailored for clusters using an HSM or KMS
Expand Down Expand Up @@ -284,6 +290,7 @@ var (
ProxyKubeClient: ECDSAP256,
EC2InstanceConnect: Ed25519,
GitHubProxyCASSH: ECDSAP256,
GitClient: Ed25519,
}

allSuites = map[types.SignatureAlgorithmSuite]suite{
Expand Down
39 changes: 38 additions & 1 deletion lib/proxy/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"context"
"errors"
"fmt"
"math/rand/v2"
"net"
"os"
"sync"
Expand Down Expand Up @@ -277,7 +278,6 @@ func (r *Router) DialHost(ctx context.Context, clientSrcAddr, clientDstAddr net.
}
}
}

} else {
return nil, trace.ConnectionProblem(errors.New("connection problem"), "direct dialing to nodes not found in inventory is not supported")
}
Expand Down Expand Up @@ -377,6 +377,7 @@ func (r *Router) getRemoteCluster(ctx context.Context, clusterName string, check
type site interface {
GetNodes(ctx context.Context, fn func(n readonly.Server) bool) ([]types.Server, error)
GetClusterNetworkingConfig(ctx context.Context) (types.ClusterNetworkingConfig, error)
GetGitServers(context.Context, func(readonly.Server) bool) ([]types.Server, error)
}

// remoteSite is a site implementation that wraps
Expand All @@ -392,6 +393,17 @@ func (r remoteSite) GetNodes(ctx context.Context, fn func(n readonly.Server) boo
return nil, trace.Wrap(err)
}

servers, err := watcher.CurrentResourcesWithFilter(ctx, fn)
return servers, trace.Wrap(err)
}

// GetGitServers uses the wrapped sites GitServerWatcher to filter git servers.
func (r remoteSite) GetGitServers(ctx context.Context, fn func(n readonly.Server) bool) ([]types.Server, error) {
watcher, err := r.site.GitServerWatcher()
if err != nil {
return nil, trace.Wrap(err)
}

return watcher.CurrentResourcesWithFilter(ctx, fn)
}

Expand All @@ -409,6 +421,9 @@ func (r remoteSite) GetClusterNetworkingConfig(ctx context.Context) (types.Clust
// getServer attempts to locate a node matching the provided host and port in
// the provided site.
func getServer(ctx context.Context, host, port string, site site) (types.Server, error) {
if org, ok := types.GetGitHubOrgFromNodeAddr(host); ok {
return getGitHubServer(ctx, org, site)
}
return getServerWithResolver(ctx, host, port, site, nil /* use default resolver */)
}

Expand Down Expand Up @@ -562,3 +577,25 @@ func (r *Router) GetSiteClient(ctx context.Context, clusterName string) (authcli
}
return site.GetClient()
}

func getGitHubServer(ctx context.Context, gitHubOrg string, site site) (types.Server, error) {
servers, err := site.GetGitServers(ctx, func(s readonly.Server) bool {
github := s.GetGitHub()
return github != nil && github.Organization == gitHubOrg
})
if err != nil {
return nil, trace.Wrap(err)
}

switch len(servers) {
case 0:
return nil, trace.NotFound("unable to locate Git server for GitHub organization %s", gitHubOrg)
case 1:
return servers[0], nil
default:
// It's unusual but possible to have multiple servers per organization
// (e.g. possibly a second Git server for a manual CA rotation). Pick a
// random one.
return servers[rand.N(len(servers))], nil
}
}
53 changes: 51 additions & 2 deletions lib/proxy/router_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (

"github.com/google/uuid"
"github.com/gravitational/trace"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"golang.org/x/crypto/ssh"

Expand All @@ -43,8 +44,9 @@ import (
)

type testSite struct {
cfg types.ClusterNetworkingConfig
nodes []types.Server
cfg types.ClusterNetworkingConfig
nodes []types.Server
gitServers []types.Server
}

func (t testSite) GetClusterNetworkingConfig(ctx context.Context) (types.ClusterNetworkingConfig, error) {
Expand All @@ -61,6 +63,16 @@ func (t testSite) GetNodes(ctx context.Context, fn func(n readonly.Server) bool)

return out, nil
}
func (t testSite) GetGitServers(ctx context.Context, fn func(n readonly.Server) bool) ([]types.Server, error) {
var out []types.Server
for _, s := range t.gitServers {
if fn(s) {
out = append(out, s)
}
}

return out, nil
}

type server struct {
name string
Expand Down Expand Up @@ -351,6 +363,11 @@ func TestGetServers(t *testing.T) {
},
)

gitServers := []types.Server{
makeGitHubServer(t, "org1"),
makeGitHubServer(t, "org2"),
}

// ensure tests don't have order-dependence
rand.Shuffle(len(servers), func(i, j int) {
servers[i], servers[j] = servers[j], servers[i]
Expand Down Expand Up @@ -489,6 +506,28 @@ func TestGetServers(t *testing.T) {
require.True(t, srv.IsOpenSSHNode())
},
},
{
name: "git server",
site: testSite{cfg: &unambiguousCfg, gitServers: gitServers},
host: "org2.teleport-github-org",
errAssertion: require.NoError,
serverAssertion: func(t *testing.T, srv types.Server) {
require.NotNil(t, srv)
require.NotNil(t, srv.GetGitHub())
assert.Equal(t, "org2", srv.GetGitHub().Organization)
},
},
{
name: "git server not found",
site: testSite{cfg: &unambiguousCfg, gitServers: gitServers},
host: "org-not-found.teleport-github-org",
errAssertion: func(t require.TestingT, err error, i ...interface{}) {
require.True(t, trace.IsNotFound(err), i...)
},
serverAssertion: func(t *testing.T, srv types.Server) {
require.Nil(t, srv)
},
},
}

ctx := context.Background()
Expand Down Expand Up @@ -891,3 +930,13 @@ func TestRouter_DialSite(t *testing.T) {
})
}
}

func makeGitHubServer(t *testing.T, org string) types.Server {
t.Helper()
server, err := types.NewGitHubServer(types.GitHubServerMetadata{
Integration: org,
Organization: org,
})
require.NoError(t, err)
return server
}
Loading

0 comments on commit a718918

Please sign in to comment.