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

greedy52 · 2024-12-10T02:43:41Z

GitHub proxy #48762

will stack another PR for tsh git ssh/config/clone commands on top of this

lib/cryptosuites/suites.go

espadolini

Are you sure that hooking into lib/srv/forward.Server to essentially add a completely separate mode gated by a flag (or like three different and potentially conflicting flags) is easier than writing something new that's going to be structurally guaranteed to work as intended just for the purpose of forwarding the ssh git protocol?

espadolini · 2024-12-12T15:05:47Z

lib/auth/authclient/clt.go

+	// GitServerGetter defines a service to get Git servers.
+	services.GitServerGetter


If ClientI already has a way to get a gitserver.Client which has GetGitServer and ListGitServers, why do we need to extend ClientI with methods at the top level?

We are casting ClientI to ProxyAccessPoint in a few places, which forces authclient to have same interface as Cache. Also gitserver.Client is not suitable for "AccessPoint".

one other option is to have a gitserver.ReadOnlyClient or decouple ClientI from ProxyAccessPoint somehow. any recommendation is welcome.

lib/proxy/router.go

lib/srv/forward/sshserver.go

lib/reversetunnel/remotesite.go

lib/srv/authhandlers.go

lib/srv/git/git.go

lib/srv/git/github.go

greedy52 · 2024-12-16T14:17:58Z

Are you sure that hooking into lib/srv/forward.Server to essentially add a completely separate mode gated by a flag (or like three different and potentially conflicting flags) is easier than writing something new that's going to be structurally guaranteed to work as intended just for the purpose of forwarding the ssh git protocol?

will do 👍

greedy52 · 2024-12-20T18:06:26Z

i am splitting out some parts of this PR to separate ones like

GitHub proxy: git command recorder #50505

public-teleport-github-review-bot · 2025-01-02T14:19:44Z

@greedy52 - this PR will require admin approval to merge due to its size. Consider breaking it up into a series smaller changes.

greedy52 · 2025-01-07T17:02:11Z

@espadolini PTAL

lib/cryptosuites/suites.go

lib/proxy/router.go

lib/reversetunnel/localsite.go

lib/srv/forward/sshserver.go

lib/sshutils/reply.go

lib/srv/git/forward.go

espadolini · 2025-01-07T19:13:48Z

lib/srv/git/forward.go

+	}
+
+	// Use auth.UserKeyAuth to verify user cert is signed by UserCA.
+	permissions, err := s.auth.UserKeyAuth(conn, key)


This will also do regular RBAC on the TargetServer unless you also change the check for h.c.Component == teleport.ComponentForwardingNode in authhandlers.go

there is a check to avoid that:

teleport/lib/srv/authhandlers.go

Lines 471 to 473 in 67c298b

if h.isProxy() || h.c.Component == teleport.ComponentForwardingGit {

return permissions, nil

}

but in the long run, we should do some refactoring to have dedicated auth handler for each type.

lib/srv/git/forward.go

espadolini · 2025-01-07T19:19:13Z

lib/srv/git/forward.go

+		return true, nil
+	case sshutils.ExecRequest:
+		return true, trace.Wrap(s.handleExec(ctx, ch, req, remoteSession))
+	case sshutils.EnvRequest:


Do we ever want and/or need envvars in the ssh git protocol?

openssh sends LANG and GIT_PROTOCOL (when using v2 protocol). they are optional but it would be nice to forward them.

…/48762_ssh

Tener

A few comments.

In general, I wonder about the decision to reuse ssh connection functionality for this. If we ever want to support other protocols then suddenly we need to reinvent a lot of things. The implementation ends up rather intrusive as we need to hijack a number of bits, e.g. the routing to nodes.

I assume this point was discussed before, so I'm keen to learn the rationale behind this vs e.g. reusing the infrastructure for TCP apps.

api/types/server.go

lib/proxy/router.go

lib/srv/authhandlers.go

espadolini · 2025-01-09T15:20:01Z

In general, I wonder about the decision to reuse ssh connection functionality for this. If we ever want to support other protocols then suddenly we need to reinvent a lot of things.

I can't speak for the decision to use the git ssh protocol vs the git http protocol, but if we're going to use the git ssh protocol, reusing our existing SSH transport makes sense IMO. My initial recommendation was to actually make the configuration bits real static nodes in a similar fashion to the openssh ones, but we've decided to use a dedicated configuration type for git servers.

…/48762_ssh

greedy52 · 2025-01-09T21:51:39Z

In general, I wonder about the decision to reuse ssh connection functionality for this. If we ever want to support other protocols then suddenly we need to reinvent a lot of things.

I can't speak for the decision to use the git ssh protocol vs the git http protocol, but if we're going to use the git ssh protocol, reusing our existing SSH transport makes sense IMO. My initial recommendation was to actually make the configuration bits real static nodes in a similar fashion to the openssh ones, but we've decided to use a dedicated configuration type for git servers.

The very first git proxy poc is actually http-based for AWS CodeCommit using AWS app access. But AWS CodeCommit is dead now.

Like Edoardo said, this is what we end up with after a few iterations of poc and RFD discussions. GitLab supports SSH CA as well. Even if we have to do HTTP later, I don't think it's that difficult.

…/48762_ssh

greedy52 · 2025-01-10T16:01:43Z

@espadolini @Tener could you take another look? thanks!

greedy52 mentioned this pull request Dec 10, 2024

GitHub proxy #48762

Open

17 tasks

Base automatically changed from STeve/48762_github_oauth_flow to master December 10, 2024 17:50

greedy52 changed the base branch from master to STeve/48762_audit_log December 11, 2024 02:53

greedy52 changed the base branch from STeve/48762_audit_log to master December 11, 2024 02:53

greedy52 force-pushed the STeve/48762_ssh branch from 73db1b8 to 1cd0ec1 Compare December 11, 2024 03:00

greedy52 mentioned this pull request Dec 11, 2024

GitHub proxy part 6.5: tsh git ssh/clone/config #50044

Merged

greedy52 force-pushed the STeve/48762_ssh branch 2 times, most recently from a5cb9a6 to 97bbddf Compare December 12, 2024 01:33

greedy52 requested review from espadolini and r0mant December 12, 2024 01:59

greedy52 marked this pull request as ready for review December 12, 2024 01:59

github-actions bot requested review from flyinghermit and rosstimothy December 12, 2024 01:59

github-actions bot added the size/lg label Dec 12, 2024

espadolini reviewed Dec 12, 2024

View reviewed changes

lib/cryptosuites/suites.go Outdated Show resolved Hide resolved

espadolini reviewed Dec 12, 2024

View reviewed changes

greedy52 added 2 commits December 17, 2024 12:18

GitHub proxy part 6: proxing Git using SSH transport

214f1ef

better command parsing and update suite

624a8ec

greedy52 mentioned this pull request Dec 20, 2024

GitHub proxy: git command recorder #50505

Open

refactor

2345ca3

greedy52 force-pushed the STeve/48762_ssh branch from 97bbddf to 2345ca3 Compare January 2, 2025 14:19

greedy52 requested a review from espadolini January 2, 2025 14:19

revert unnecearrty files

7048508

greedy52 requested review from Tener and GavinFrazar and removed request for flyinghermit January 2, 2025 15:09

espadolini reviewed Jan 7, 2025

View reviewed changes

greedy52 added 2 commits January 8, 2025 08:49

Merge branch 'master' of github.com:gravitational/teleport into STeve…

ab3541f

…/48762_ssh

address review comments

67c298b

greedy52 added the no-changelog Indicates that a PR does not require a changelog entry label Jan 8, 2025

ut fix

e4dc0c5

greedy52 mentioned this pull request Jan 8, 2025

GitHub proxy: download GitHub server keys #50891

Draft

revert localsite_test.go

330729a

greedy52 requested a review from espadolini January 9, 2025 03:15

Tener reviewed Jan 9, 2025

View reviewed changes

api/types/server.go Outdated Show resolved Hide resolved

lib/proxy/router.go Show resolved Hide resolved

lib/srv/authhandlers.go Show resolved Hide resolved

greedy52 added 2 commits January 9, 2025 12:25

Merge branch 'master' of github.com:gravitational/teleport into STeve…

6ac577d

…/48762_ssh

change special suffix to teleport-github-org for routing

bbd0f4c

greedy52 requested a review from Tener January 9, 2025 21:52

greedy52 added 2 commits January 9, 2025 16:55

Merge branch 'master' of github.com:gravitational/teleport into STeve…

83686c6

…/48762_ssh

fix routing ut

58ca650

greedy52 force-pushed the STeve/48762_ssh branch from b05b384 to 58ca650 Compare January 10, 2025 14:26

greedy52 added 3 commits January 10, 2025 10:42

minor typo edit

b35b0af

Merge branch 'master' of github.com:gravitational/teleport into STeve…

ec56f21

…/48762_ssh

fix ut after sshca change

6ce6408

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

greedy52 commented Dec 10, 2024

espadolini left a comment

espadolini Dec 12, 2024

greedy52 Jan 2, 2025

greedy52 commented Dec 16, 2024 •

edited

Loading

greedy52 commented Dec 20, 2024

public-teleport-github-review-bot bot commented Jan 2, 2025

greedy52 commented Jan 7, 2025

espadolini Jan 7, 2025

greedy52 Jan 8, 2025 •

edited

Loading

espadolini Jan 7, 2025

greedy52 Jan 8, 2025 •

edited

Loading

Tener left a comment

espadolini commented Jan 9, 2025

greedy52 commented Jan 9, 2025

greedy52 commented Jan 10, 2025

		// GitServerGetter defines a service to get Git servers.
		services.GitServerGetter

	if h.isProxy() \|\| h.c.Component == teleport.ComponentForwardingGit {
	return permissions, nil
	}

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

Are you sure you want to change the base?

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

Conversation

greedy52 commented Dec 10, 2024

espadolini left a comment

Choose a reason for hiding this comment

espadolini Dec 12, 2024

Choose a reason for hiding this comment

greedy52 Jan 2, 2025

Choose a reason for hiding this comment

greedy52 commented Dec 16, 2024 • edited Loading

greedy52 commented Dec 20, 2024

public-teleport-github-review-bot bot commented Jan 2, 2025

greedy52 commented Jan 7, 2025

espadolini Jan 7, 2025

Choose a reason for hiding this comment

greedy52 Jan 8, 2025 • edited Loading

Choose a reason for hiding this comment

espadolini Jan 7, 2025

Choose a reason for hiding this comment

greedy52 Jan 8, 2025 • edited Loading

Choose a reason for hiding this comment

Tener left a comment

Choose a reason for hiding this comment

espadolini commented Jan 9, 2025

greedy52 commented Jan 9, 2025

greedy52 commented Jan 10, 2025

greedy52 commented Dec 16, 2024 •

edited

Loading

greedy52 Jan 8, 2025 •

edited

Loading

greedy52 Jan 8, 2025 •

edited

Loading