SSH-based auth for llb.Git operations by alexcb · Pull Request #1782 · moby/buildkit

alexcb · 2020-11-06T23:50:14Z

This PR implements ssh-based authentication for git operations and fixes #1564

It provides the ability to make use of the sshforwarding from the client, and introduces the ability to specify known_host entries.

Here's an example client to illustrate how to use the new Git source options:

alpine := llb.Image("docker.io/library/alpine:latest")

// Example of fetching a git source that is fetched over https (this is existing functionality)
openGitState := llb.Git("github.com/alexcb/acbutils", "master")
openGitMount := llb.AddMount("/git-src", openGitState, llb.Readonly)


// Example of fetching from a non-standard private repo (new functionality)

// step 1, specify the expected entry for known_hosts (can be fetched with ssh-keyscan -t rsa 192.168.0.116 )
myServerKey := "192.168.0.116 ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQC9aa+HfOctyCxO0ruREnLg4FRTMEdm8zixIDDgzEHBCvOrHnxOjo7UTd7fkOh21sYOT7B8bULEfeDCh5H2fF5yg14XewI/USa6jAx3bjMgT3FwXa7Oi267fNXC6pbTEp/gyWSQcmpQHuOup+MhjoMLhuQv9u8FwE8U6ahvwEoQ6qOLsECSUp6OHTg326bxHsGFAyzi4Cq3a6OxUTb8bhWe56HUxI0h9PgbrnY9FsQzAHjBGp3BPy7rqTW20ON36TOIQsLDoRcLNrP4lz3oBlzCcSgLhkNgpth+FOKw52mveCVs7EnmoI8GLH22hh0BTzhgL9EFG81O5RBIFOmoSFmGKSADVBtI4z4tWxL8zw/gVRY7W+peoVsGGtAx8uFQIGnHFzuBFxCO6gPwGvYUsETNF3bgaBurfceqAOvOEtk0u4N5g5M9pIgfZmpde+Ubh8BIwkYFtoJM6CchO23sZIlVm3o3aoBhJi/MShANS4ckvoEybbugQVdS3ZaRw/8P0ek="

// step 2, specify the git repo to clone, and tell buildkit to make an ssh mount available along 
// with the expected server host key
privateGitState2 := llb.Git("[email protected]:my/test/path/testrepo.git", "master",
    llb.MountSSHSock(), llb.KnownSSHHosts(myServerKey))
privateGitMount2 := llb.AddMount("/git-src", privateGitState2, llb.Readonly)

// then add the git source to a Run command:
state := alpine.
    Run(llb.Args([]string{"ls", "/git-src"}), openGitMount).
    Run(llb.Args([]string{"ls", "/git-src"}), privateGitMount).
    Root()

- fixes assumption that ssh git clones must be via the `git` user. - allows passing the SSH_AUTH_SOCK from the client to GitSource - allows passing a known_host entry for ssh Signed-off-by: Alex Couture-Beil <[email protected]>

AkihiroSuda · 2020-11-11T04:46:34Z

 	url := ""

 	for _, prefix := range []string{
-		"http://", "https://", "git://", "git@",


Without this change, when I do

llb.Git("[email protected]:earthly/earthly.git", "main")

it causes the string to get trimmed into github.com:earthly/earthly.git, which then causes buildkit to attempt to parse the string as if it were a URL (in https://github.com/moby/buildkit/pull/1782/files#diff-dc3734c7af40d455fbd22d64b372bf572aa7314209bafc3d5f1f809203186d6eL27 )

which ultimately gives me the error: failed to load cache key: parse https://github.com:earthly/earthly.git: invalid port ":earthly" after host.

I wonder if there's a way we could reproduce this error via an integration test to show that this fixes the issue? I don't know enough about buildkit's integrating testing to add one (but would be keen on learning with some help).

This is the reason why AttrFullRemoteURL was added I believe that should be used instead to get this info.

I'll back out this change, and try using AttrFullRemoteURL in a follow up PR.

Signed-off-by: Alex Couture-Beil <[email protected]>

tonistiigi · 2020-11-12T04:00:45Z


+// sshGitRegexp is used to detect if the git repo uses ssh
+// e.g. git@... or [email protected]:my/really/strange/repo.git
+var sshGitRegexp, _ = regexp.Compile("[a-z0-9_]+@[^/]+:.+")


Not sure if these changes are related to the rest of the PR. The identifier logic is copied from Docker description https://github.com/moby/moby/blob/master/pkg/urlutil/urlutil.go#L21 and need to be aware of the ambiguity concerns (eg. @ could be valid for HTTP as well) and maybe change docs. I'd rather handle that separately. @thaJeztah

That's correct that it's not related to mounting the ssh socket, i'll back this change out in this PR, then open a new one to tackle the issue of using a non-git username for ssh cloning.

tonistiigi · 2020-11-12T04:06:25Z

+
+func MountSSHSock() GitOption {
+	return gitOptionFunc(func(gi *GitInfo) {
+		gi.MountSSHSock = true


This should be the ssh id, not a bool. If identifier is SSH based it should be set automatically.

great point, changed this to a string which is now passed from the client.

If identifier is SSH based it should be set automatically.

That part does not look addressed yet.

I missed that part, i'm working on implementing it as well as a ssh keyscan for when no keys are supplied.

tonistiigi · 2020-11-12T04:07:21Z

 	url := ""

 	for _, prefix := range []string{
-		"http://", "https://", "git://", "git@",


This is the reason why AttrFullRemoteURL was added I believe that should be used instead to get this info.

- back out changes to changing the git url - fix gid - ignore global ssh config option when specifying known hosts Signed-off-by: Alex Couture-Beil <[email protected]>

Signed-off-by: Alex Couture-Beil <[email protected]>

tonistiigi

As discussed in slack, please follow-up with PRs that make the auth default on ssh URL and fix the URL parsing issue you discovered.

alexcb force-pushed the acb-other-git-user branch from 22df3dd to 72730c2 Compare November 6, 2020 23:51

alexcb marked this pull request as draft November 9, 2020 18:24

alexcb force-pushed the acb-other-git-user branch 2 times, most recently from 7378134 to e186e4a Compare November 10, 2020 23:54

Support cloning from non-standard git repos

f2c8eb1

- fixes assumption that ssh git clones must be via the `git` user. - allows passing the SSH_AUTH_SOCK from the client to GitSource - allows passing a known_host entry for ssh Signed-off-by: Alex Couture-Beil <[email protected]>

alexcb force-pushed the acb-other-git-user branch from e186e4a to f2c8eb1 Compare November 10, 2020 23:55

alexcb changed the title ~~Allow cloning git repo over ssh with non-git username~~ SSH-based auth for llb.Git operations Nov 11, 2020

alexcb marked this pull request as ready for review November 11, 2020 04:02

AkihiroSuda reviewed Nov 11, 2020

View reviewed changes

Comment thread source/git/gitsource.go Outdated

AkihiroSuda reviewed Nov 11, 2020

View reviewed changes

change ssh auth sock perms to 0700

486c5fd

Signed-off-by: Alex Couture-Beil <[email protected]>

alexcb force-pushed the acb-other-git-user branch from 2b69307 to 486c5fd Compare November 11, 2020 16:20

tonistiigi reviewed Nov 12, 2020

View reviewed changes

Comment thread source/git/gitsource.go Outdated

Comment thread source/git/gitsource.go Outdated

Comment thread source/git/gitsource.go Outdated

Fixes related to the PR

92e89a3

- back out changes to changing the git url - fix gid - ignore global ssh config option when specifying known hosts Signed-off-by: Alex Couture-Beil <[email protected]>

alexcb force-pushed the acb-other-git-user branch 4 times, most recently from 402307a to 2596b6f Compare November 13, 2020 03:22

disable host key checking when none are given

985bd6e

Signed-off-by: Alex Couture-Beil <[email protected]>

alexcb force-pushed the acb-other-git-user branch from 2596b6f to 985bd6e Compare November 13, 2020 03:26

alexcb requested review from AkihiroSuda and tonistiigi November 13, 2020 18:20

alexcb mentioned this pull request Nov 13, 2020

Bitbucket Support earthly/earthly#482

Closed

tonistiigi approved these changes Nov 16, 2020

View reviewed changes

tonistiigi merged commit 8dc3a54 into moby:master Nov 16, 2020

This was referenced Nov 16, 2020

Fix parsing ssh-based git sources #1802

Merged

use buildkit ssh mount for git clones earthly/earthly#518

Merged

alexcb mentioned this pull request Apr 20, 2021

Enable ssh forwarding when building a remote target docker/cli#3050

Merged

crazy-max mentioned this pull request Dec 4, 2022

Fails because of no rights in private submodule docker/build-push-action#737

Closed

Uh oh!

Conversation

alexcb commented Nov 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tonistiigi left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

alexcb commented Nov 6, 2020 •

edited

Loading