SSH-based auth for llb.Git operations #1782

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("alex@192.168.0.116: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 <alex@earthly.dev>

source/git/gitsource.go

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

client/llb/source.go

@@ -201,7 +201,7 @@ func Git(remote, ref string, opts ...GitOption) State {
 	url := ""

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


Without this change, when I do

llb.Git("git@github.com: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 <alex@earthly.dev>

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

source/gitidentifier.go

 	"strings"

 	"github.com/pkg/errors"
 )

+// sshGitRegexp is used to detect if the git repo uses ssh
+// e.g. git@... or otheruser@nonstandardgithost.com: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.

source/git/gitsource.go

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

client/llb/source.go

+
+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

client/llb/source.go

@@ -201,7 +201,7 @@ func Git(remote, ref string, opts ...GitOption) State {
 	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.

source/git/gitsource.go

- 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 <alex@earthly.dev>

Signed-off-by: Alex Couture-Beil <alex@earthly.dev>

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 <alex@earthly.dev>

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

source/git/gitsource.go Outdated Show resolved Hide resolved

AkihiroSuda reviewed Nov 11, 2020

View reviewed changes

change ssh auth sock perms to 0700

486c5fd

Signed-off-by: Alex Couture-Beil <alex@earthly.dev>

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

source/git/gitsource.go Outdated Show resolved Hide resolved

source/git/gitsource.go Outdated Show resolved Hide resolved

source/git/gitsource.go Outdated Show resolved Hide resolved

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 <alex@earthly.dev>

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 <alex@earthly.dev>

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

alexcb requested review from tonistiigi and AkihiroSuda 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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SSH-based auth for llb.Git operations #1782

SSH-based auth for llb.Git operations #1782

alexcb commented Nov 6, 2020 •

edited

Loading

AkihiroSuda Nov 11, 2020

alexcb Nov 11, 2020

tonistiigi Nov 12, 2020

alexcb Nov 12, 2020

tonistiigi Nov 12, 2020

alexcb Nov 12, 2020

tonistiigi Nov 12, 2020

alexcb Nov 13, 2020

tonistiigi Nov 13, 2020

alexcb Nov 13, 2020

tonistiigi Nov 12, 2020

tonistiigi left a comment

SSH-based auth for llb.Git operations #1782

SSH-based auth for llb.Git operations #1782

Conversation

alexcb commented Nov 6, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tonistiigi left a comment

Choose a reason for hiding this comment

alexcb commented Nov 6, 2020 •

edited

Loading