Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

GitHub proxy part 6: proxing Git using SSH transport #49980

Merged
merged 23 commits into from
Jan 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
214f1ef
GitHub proxy part 6: proxing Git using SSH transport
greedy52 Dec 10, 2024
624a8ec
better command parsing and update suite
greedy52 Dec 13, 2024
2345ca3
refactor
greedy52 Dec 16, 2024
7048508
revert unnecearrty files
greedy52 Jan 2, 2025
ab3541f
Merge branch 'master' of github.com:gravitational/teleport into STeve…
greedy52 Jan 8, 2025
67c298b
address review comments
greedy52 Jan 8, 2025
e4dc0c5
ut fix
greedy52 Jan 8, 2025
330729a
revert localsite_test.go
greedy52 Jan 8, 2025
6ac577d
Merge branch 'master' of github.com:gravitational/teleport into STeve…
greedy52 Jan 9, 2025
bbd0f4c
change special suffix to teleport-github-org for routing
greedy52 Jan 9, 2025
83686c6
Merge branch 'master' of github.com:gravitational/teleport into STeve…
greedy52 Jan 9, 2025
58ca650
fix routing ut
greedy52 Jan 9, 2025
b35b0af
minor typo edit
greedy52 Jan 10, 2025
ec56f21
Merge branch 'master' of github.com:gravitational/teleport into STeve…
greedy52 Jan 10, 2025
6ce6408
fix ut after sshca change
greedy52 Jan 10, 2025
ebbb435
add UT to sshutils
greedy52 Jan 13, 2025
b0f2b0e
minor review comments
greedy52 Jan 13, 2025
0ce45f7
fix api ut because of special suffix change
greedy52 Jan 13, 2025
f37804c
GitServerReadOnlyClient
greedy52 Jan 14, 2025
91b250e
downgrade error to warning
greedy52 Jan 14, 2025
ae50b6f
Merge branch 'master' of github.com:gravitational/teleport into STeve…
greedy52 Jan 14, 2025
6a72c23
run go mod tidy. not sure why it's needed
greedy52 Jan 14, 2025
5aa2576
rename mock.go to mock_test.go
greedy52 Jan 14, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this have to happen in CASD?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could move it to backend create but I would prefer it here as it should not have other values.

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)
greedy52 marked this conversation as resolved.
Show resolved Hide resolved
}

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 {
greedy52 marked this conversation as resolved.
Show resolved Hide resolved
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
Loading