GitHub proxy: download GitHub server keys

constants.go

@@ -288,6 +288,9 @@ const (
 	// ComponentRolloutController represents the autoupdate_agent_rollout controller.
 	ComponentRolloutController = "rollout-controller"
 
+	// ComponentGit represents git proxy related services.
+	ComponentGit = "git"
+
 	// ComponentForwardingGit represents the SSH proxy that forwards Git commands.
 	ComponentForwardingGit = "git:forward"
 

lib/reversetunnel/localsite.go

@@ -389,6 +389,7 @@ func (s *localSite) dialAndForwardGit(params reversetunnelclient.DialParams) (_
 		HostUUID:        s.srv.ID,
 		TargetServer:    params.TargetServer,
 		Clock:           s.clock,
+		KeyManager:      s.srv.GitKeyManager,
 	}
 	remoteServer, err := git.NewForwardServer(serverConfig)
 	if err != nil {

lib/reversetunnel/srv.go

@@ -50,6 +50,7 @@ import (
 	"github.com/gravitational/teleport/lib/reversetunnelclient"
 	"github.com/gravitational/teleport/lib/services"
 	"github.com/gravitational/teleport/lib/services/readonly"
+	"github.com/gravitational/teleport/lib/srv/git"
 	"github.com/gravitational/teleport/lib/srv/ingress"
 	"github.com/gravitational/teleport/lib/sshca"
 	"github.com/gravitational/teleport/lib/sshutils"
@@ -223,6 +224,9 @@ type Config struct {
 
 	// PROXYSigner is used to sign PROXY headers to securely propagate client IP information.
 	PROXYSigner multiplexer.PROXYHeaderSigner
+
+	// GitKeyManager manages keys for git proxies.
+	GitKeyManager *git.KeyManager
 }
 
 // CheckAndSetDefaults checks parameters and sets default values
@@ -282,6 +286,17 @@ func (cfg *Config) CheckAndSetDefaults() error {
 	if cfg.CertAuthorityWatcher == nil {
 		return trace.BadParameter("missing parameter CertAuthorityWatcher")
 	}
+	if cfg.GitKeyManager == nil {
+		var err error
+		cfg.GitKeyManager, err = git.NewKeyManager(&git.KeyManagerConfig{
+			ParentContext: cfg.Context,
+			AuthClient:    cfg.LocalAuthClient,
+			AccessPoint:   cfg.LocalAccessPoint,
+		})
+		if err != nil {
+			return trace.Wrap(err)
+		}
+	}
 	return nil
 }
 

lib/services/watcher.go

@@ -1710,6 +1710,10 @@ func (*oktaAssignmentCollector) notifyStale() {}
 type GitServerWatcherConfig struct {
 	GitServerGetter
 	ResourceWatcherConfig
+
+	// EnableUpdateBroadcast turns on emitting updates on changes. Broadcast is
+	// opt-in for Git Server watcher.
+	EnableUpdateBroadcast bool
 }
 
 // NewGitServerWatcher returns a new instance of Git server watcher.
@@ -1737,7 +1741,7 @@ func NewGitServerWatcher(ctx context.Context, cfg GitServerWatcherConfig) (*Gene
 			return all, nil
 		},
 		ResourceKey:            types.Server.GetName,
-		DisableUpdateBroadcast: true,
+		DisableUpdateBroadcast: !cfg.EnableUpdateBroadcast,
 		CloneFunc:              types.Server.DeepCopy,
 	})
 	return w, trace.Wrap(err)

lib/srv/git/forward.go

@@ -63,6 +63,8 @@ type ForwardServerConfig struct {
 	Emitter events.StreamEmitter
 	// LockWatcher is a lock watcher.
 	LockWatcher *services.LockWatcher
+	// KeyManager manages keys for git proxies.
+	KeyManager *KeyManager
 	// HostCertificate is the SSH host certificate this in-memory server presents
 	// to the client.
 	HostCertificate ssh.Signer
@@ -108,6 +110,9 @@ func (c *ForwardServerConfig) CheckAndSetDefaults() error {
 	if c.Emitter == nil {
 		return trace.BadParameter("missing parameter Emitter")
 	}
+	if c.KeyManager == nil {
+		return trace.BadParameter("missing parameter KeyManager")
+	}
 	if c.HostCertificate == nil {
 		return trace.BadParameter("missing parameter HostCertificate")
 	}
@@ -147,7 +152,7 @@ type ForwardServer struct {
 	remoteClient *tracessh.Client
 
 	// verifyRemoteHost is a callback to verify remote host like "github.com".
-	// Can be overridden for tests. Defaults to verifyRemoteHost.
+	// Can be overridden for tests. Defaults to cfg.KeyManager.HostKeyCallback.
 	verifyRemoteHost ssh.HostKeyCallback
 	// makeRemoteSigner generates the client certificate for connecting to the
 	// remote server. Can be overridden for tests. Defaults to makeRemoteSigner.
@@ -183,7 +188,7 @@ func NewForwardServer(cfg *ForwardServerConfig) (*ForwardServer, error) {
 		logger:           logger,
 		reply:            sshutils.NewReply(logger),
 		id:               uuid.NewString(),
-		verifyRemoteHost: verifyRemoteHost(cfg.TargetServer),
+		verifyRemoteHost: cfg.KeyManager.HostKeyCallback(cfg.TargetServer),
 		makeRemoteSigner: makeRemoteSigner,
 	}
 	// TODO(greedy52) extract common parts from srv.NewAuthHandlers like
@@ -587,17 +592,6 @@ func makeRemoteSigner(ctx context.Context, cfg *ForwardServerConfig, identityCtx
 	}
 }
 
-func verifyRemoteHost(targetServer types.Server) ssh.HostKeyCallback {
-	return func(hostname string, remote net.Addr, key ssh.PublicKey) error {
-		switch targetServer.GetSubKind() {
-		case types.SubKindGitHub:
-			return VerifyGitHubHostKey(hostname, remote, key)
-		default:
-			return trace.BadParameter("unsupported subkind %q", targetServer.GetSubKind())
-		}
-	}
-}
-
 // Below functions implement srv.Server so git.ForwardServer can be used for
 // srv.NewServerContext and srv.NewAuthHandlers.
 // TODO(greedy52) decouple from srv.Server.

lib/srv/git/forward_test.go

@@ -32,6 +32,7 @@ import (
 	"github.com/stretchr/testify/require"
 	"golang.org/x/crypto/ssh"
 
+	"github.com/gravitational/teleport/api/client/gitserver"
 	"github.com/gravitational/teleport/api/constants"
 	tracessh "github.com/gravitational/teleport/api/observability/tracing/ssh"
 	"github.com/gravitational/teleport/api/types"
@@ -223,6 +224,8 @@ func TestForwardServer(t *testing.T) {
 				LockWatcher:     makeLockWatcher(t),
 				SrcAddr:         utils.MustParseAddr("127.0.0.1:12345"),
 				DstAddr:         utils.MustParseAddr("127.0.0.1:2222"),
+				// Not used in test, yet.
+				KeyManager: new(KeyManager),
 			})
 			require.NoError(t, err)
 
@@ -324,14 +327,16 @@ type mockGitHostingService struct {
 	*sshutils.Reply
 	receivedExec sshutils.ExecReq
 	exitCode     int
+	hostKey      ssh.PublicKey
 }
 
 func newMockGitHostingService(t *testing.T, caSigner ssh.Signer) *mockGitHostingService {
 	t.Helper()
 	hostCert, err := apisshutils.MakeRealHostCert(caSigner)
 	require.NoError(t, err)
 	m := &mockGitHostingService{
-		Reply: &sshutils.Reply{},
+		Reply:   &sshutils.Reply{},
+		hostKey: hostCert.PublicKey(),
 	}
 	server, err := sshutils.NewServer(
 		"git.test",
@@ -387,12 +392,21 @@ func (m *mockGitHostingService) HandleNewChan(ctx context.Context, ccx *sshutils
 
 type mockAuthClient struct {
 	authclient.ClientI
+	events types.Events
+}
+
+func (m mockAuthClient) NewWatcher(ctx context.Context, watch types.Watch) (types.Watcher, error) {
+	if m.events == nil {
+		return nil, trace.AccessDenied("unauthorized")
+	}
+	return m.events.NewWatcher(ctx, watch)
 }
 
 type mockAccessPoint struct {
 	srv.AccessPoint
 	ca               ssh.Signer
 	allowedGitHubOrg string
+	services.GitServers
 }
 
 func (m mockAccessPoint) GetClusterName(...services.MarshalOption) (types.ClusterName, error) {
@@ -437,3 +451,6 @@ func (m mockAccessPoint) GetCertAuthorities(_ context.Context, caType types.Cert
 	}
 	return []types.CertAuthority{ca}, nil
 }
+func (m mockAccessPoint) GitServerReadOnlyClient() gitserver.ReadOnlyClient {
+	return m.GitServers
+}

lib/srv/git/github.go

@@ -20,8 +20,10 @@ package git
 
 import (
 	"context"
-	"net"
-	"slices"
+	"encoding/json"
+	"log/slog"
+	"net/http"
+	"sync/atomic"
 	"time"
 
 	"github.com/gravitational/trace"
@@ -30,33 +32,118 @@ import (
 	"google.golang.org/grpc"
 	"google.golang.org/protobuf/types/known/durationpb"
 
+	"github.com/gravitational/teleport"
 	integrationv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/integration/v1"
 	"github.com/gravitational/teleport/api/types"
 	"github.com/gravitational/teleport/lib/cryptosuites"
+	"github.com/gravitational/teleport/lib/defaults"
 	"github.com/gravitational/teleport/lib/sshutils"
 )
 
-// knownGithubDotComFingerprints contains a list of known GitHub fingerprints.
-//
-// https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/githubs-ssh-key-fingerprints
-//
-// TODO(greedy52) these fingerprints can change (e.g. GitHub changed its RSA
-// key in 2023 because of an incident). Instead of hard-coding the values, we
-// should try to periodically (e.g. once per day) poll them from the API.
-var knownGithubDotComFingerprints = []string{
-	"SHA256:uNiVztksCsDhcc0u9e8BujQXVUpKZIDTMczCvj3tD2s",
-	"SHA256:p2QAMXNIC1TJYWeIOttrVc98/R1BUFWu3/LiyKgUfQM",
-	"SHA256:+DiY3wvvV6TuJJhbpZisF/zLDA0zPMSvHdkr4UvCOqU",
+// githubKeyDownloader downloads SSH keys from the GitHub meta API. The keys
+// are used to verify GitHub server when forwarding Git commands to it.
+type githubKeyDownloader struct {
+	keys atomic.Value
+	etag string
+
+	logger      *slog.Logger
+	apiEndpoint string
+	clock       clockwork.Clock
+}
+
+// newGitHubKeyDownloader creates a new githubKeyDownloader.
+func newGitHubKeyDownloader() *githubKeyDownloader {
+	return &githubKeyDownloader{
+		apiEndpoint: "https://api.github.com/meta",
+		logger:      slog.With(teleport.ComponentKey, teleport.ComponentGit),
+		clock:       clockwork.NewRealClock(),
+	}
 }
 
-// VerifyGitHubHostKey is an ssh.HostKeyCallback that verifies the host key
-// belongs to "github.com".
-func VerifyGitHubHostKey(_ string, _ net.Addr, key ssh.PublicKey) error {
-	actualFingerprint := ssh.FingerprintSHA256(key)
-	if slices.Contains(knownGithubDotComFingerprints, actualFingerprint) {
+// Start starts a task that periodically downloads SSH keys from the GitHub meta
+// API. The task is automatically stopped when provided context is closed.
+func (d *githubKeyDownloader) Start(ctx context.Context) {
+	d.logger.InfoContext(ctx, "Starting GitHub server key manager")
+	defer d.logger.InfoContext(ctx, "GitHub server key manager stopped")
+
+	// Fire a refresh immediately.
+	timer := d.clock.NewTimer(0)
+	defer timer.Stop()
+	for {
+		select {
+		case <-timer.Chan():
+			// Schedule a refresh in 24 hours upon success and in 5 minutes upon
+			// failure.
+			if err := d.refresh(ctx); err != nil {
+				d.logger.WarnContext(ctx, "Failed to download GitHub server keys", "error", err)
+				timer.Reset(time.Minute * 5)
+			} else {
+				timer.Reset(time.Hour * 24)
+			}
+		case <-ctx.Done():
+			return
+		}
+	}
+}
+
+// GetKnownKeys returns known server keys.
+func (d *githubKeyDownloader) GetKnownKeys() ([]ssh.PublicKey, error) {
+	keys := d.keys.Load()
+	if keys == nil {
+		return nil, trace.NotFound("server keys not found for github.com")
+	}
+	return keys.([]ssh.PublicKey), nil
+}
+
+func (d *githubKeyDownloader) refresh(ctx context.Context) error {
+	d.logger.DebugContext(ctx, "Calling GitHub meta API", "endpoint", d.apiEndpoint)
+	// Meta API reference:
+	// https://docs.github.com/en/rest/meta/meta#get-github-meta-information
+	req, err := http.NewRequest("GET", d.apiEndpoint, nil)
+	if err != nil {
+		return trace.Wrap(err)
+	}
+
+	// Add ETag check.
+	if d.etag != "" {
+		req.Header.Set("If-None-Match", d.etag)
+	}
+
+	client := &http.Client{
+		Timeout: defaults.HTTPRequestTimeout,
+	}
+	resp, err := client.Do(req)
+	if err != nil {
+		return trace.Wrap(err)
+	}
+	defer resp.Body.Close()
+
+	// Nothing changed. Just update the last check time.
+	if resp.StatusCode == http.StatusNotModified {
+		d.logger.DebugContext(ctx, "GitHub metadata is up-to-date")
 		return nil
 	}
-	return trace.BadParameter("cannot verify github.com: unknown fingerprint %v algo %v", actualFingerprint, key.Type())
+
+	meta := struct {
+		SSHKeys []string `json:"ssh_keys"`
+	}{}
+	if err := json.NewDecoder(resp.Body).Decode(&meta); err != nil {
+		return trace.Wrap(err)
+	}
+
+	var keys []ssh.PublicKey
+	for _, key := range meta.SSHKeys {
+		publicKey, _, _, _, err := ssh.ParseAuthorizedKey([]byte(key))
+		if err != nil {
+			return trace.Wrap(err, "parsing SSH public key")
+		}
+		keys = append(keys, publicKey)
+	}
+
+	d.etag = resp.Header.Get("ETag")
+	d.keys.Store(keys)
+	d.logger.DebugContext(ctx, "Fetched GitHub metadata", "ssh_keys", meta.SSHKeys, "etag", d.etag)
+	return nil
 }
 
 // AuthPreferenceGetter is an interface for retrieving the current configured
@@ -152,7 +239,6 @@ func MakeGitHubSigner(ctx context.Context, config GitHubSignerConfig) (ssh.Signe
 		return nil, trace.Wrap(err)
 	}
 
-	// TODO(greedy52) cache it for TTL.
 	signer, err := sshutils.NewSigner(sshKey.PrivateKeyPEM(), resp.AuthorizedKey)
 	return signer, trace.Wrap(err)
 }