sshforward: implement ssh socket forwarding by tonistiigi · Pull Request #608 · moby/buildkit

tonistiigi · 2018-09-06T22:55:18Z

Add ssh agent socket forwarding support through a SSH mount type in LLB. Optional ID can be assigned on a mount to support multiple sockets.

buildctl build --ssh default
buildctl build --ssh default=$SSH_AUTH_SOCK // same as above
buildctl build --ssh default=key1,key2
buildctl build --ssh myapp.ssh=key1

@AkihiroSuda @tiborvass

Follow-up: expose in Dockerfile, expose to git sources

AkihiroSuda · 2018-09-07T01:28:55Z

session/sshforward/ssh.proto

+
+service SSH {
+	rpc CheckAgent(CheckAgentRequest) returns (CheckAgentResponse);
+  rpc ForwardAgent(stream BytesMessage) returns (stream BytesMessage);


nit: indent

AkihiroSuda · 2018-09-07T05:17:49Z

session/sshforward/ssh.go

+// DefaultID is the default ssh ID
+const DefaultID = "default"
+
+const KeySSHID = "buildkitd.ssh.id"


buildkitd -> buildkit?

AkihiroSuda · 2018-09-07T05:19:47Z

session/sshforward/sshprovider/agentprovider.go

+			return nil, err
+		}
+		if conf.ID == "" {
+			conf.ID = "default"


Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>

tiborvass · 2018-09-10T17:57:35Z

client/client_test.go

+	}
+
+	s := &server{l: l}
+	go s.run(agent)


the error is ignored

An error on a single connection shouldn't fail the build. If it causes process to exit then that will become the error.

tiborvass · 2018-09-10T18:18:43Z

client/llb/exec.go

+	})
+}
+
+var SSHOptional = sshOptionFunc(func(si *SSHInfo) {


why make this writable?

tiborvass · 2018-09-10T18:28:31Z

session/sshforward/sshprovider/agentprovider.go

+func NewSSHAgentProvider(confs []AgentConfig) (session.Attachable, error) {
+	m := map[string]source{}
+	for _, conf := range confs {
+		if len(conf.Paths) == 0 || len(conf.Paths) == 1 && conf.Paths[0] == "" {


~~why have the OR condition if you error out for the same condition further below?~~ nevermind I understand now.

tiborvass · 2018-09-10T18:37:16Z

session/sshforward/sshprovider/agentprovider.go

+		}
+		fi, err := os.Stat(p)
+		if err != nil {
+			return source{}, errors.Wrapf(err, "failed to stat %s", p)


unnecessary wrapping, PathError already has similar message such as stat /my/path: No such file or directory

tiborvass · 2018-09-10T18:44:00Z

session/sshforward/sshprovider/agentprovider.go

+	return errors.Errorf("removing keys not allowed by buildkit")
+}
+
+func (a *readOnlyAgent) Lock(_ []byte) error {


Might as well protect Unlock too, no?

It is hard to unlock unexpectedly to the user, what the other cases are protecting from.

tiborvass · 2018-09-10T18:51:40Z

session/sshforward/ssh.go

+		id = DefaultID
+	}
+
+	go s.run(ctx, l, id)


error is ignored

tiborvass · 2018-09-10T18:53:04Z

session/sshforward/ssh.go

+				return err
+			}
+
+			go Copy(ctx, conn, stream)


error is ignored

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>

tiborvass

LGTM

AkihiroSuda · 2018-09-11T10:31:57Z

Can we merge?

AkihiroSuda · 2018-10-01T05:48:08Z

@tonistiigi

Any ETA of Dockerfile frontend for this? ^^

tonistiigi · 2018-10-01T05:55:26Z

@AkihiroSuda I'll work on the release/test stages for the experimental dockerfiles in next days so we can run them with travis and can probably do it after that. If you have cycles feel free to submit it yourself. The UI should be the same as the secrets with extra mount type and should automatically set up the environment variable if it is unset.

AkihiroSuda · 2018-10-02T05:38:19Z

@tonistiigi #655

…D=$SSH_AUTH_SOCK`) Unlike `docker build --secret`, `docker build --ssh` allows the build container to use SSH keys with passphrases. $ eval $(ssh-agent) $ ssh-add ~/.ssh/id_rsa (Input your passphrase here) $ docker build --ssh default=$SSH_AUTH_SOCK ... This feature requires the daemon with `CapExecMountSSH` build capability (moby/moby#37973) . Currently, the official Dockerfile frontend does not provide the syntax for using the SSH forwarder. However, the experimental `RUN --mount=type=ssh` syntax can be enabled by using the Dockerfile frontend image built with the `BUILDTAGS="dfrunmount dfssh"`, via the `# syntax =` "shebang". The Dockerfile for the Dockerfile frontend is available at github.com/moby/buildkit/frontend/dockerfile/cmd/dockerfile-frontend) The pre-built image is also available as `tonistiigi/dockerfile:ssh20181002` . An example Dockerfile with `RUN --mount=type=ssh`: # syntax = tonistiigi/dockerfile:ssh20181002 FROM alpine RUN apk add --no-cache openssh-client RUN mkdir -p -m 0700 ~/.ssh && ssh-keyscan gitlab.com >> ~/.ssh/known_hosts RUN --mount=type=ssh ssh git@gitlab.com | tee /hello # "Welcome to GitLab, @GITLAB_USERNAME_ASSOCIATED_WITH_SSHKEY" should be printed here More info available at moby/buildkit#608, moby/buildkit#655 Signed-off-by: Akihiro Suda <suda.akihiro@lab.ntt.co.jp>

…D=$SSH_AUTH_SOCK`) Unlike `docker build --secret`, `docker build --ssh` allows the build container to use SSH keys with passphrases. $ eval $(ssh-agent) $ ssh-add ~/.ssh/id_rsa (Input your passphrase here) $ docker build --ssh default=$SSH_AUTH_SOCK ... This feature requires the daemon with `CapExecMountSSH` build capability (moby/moby#37973) . Currently, the official Dockerfile frontend does not provide the syntax for using the SSH forwarder. However, the experimental `RUN --mount=type=ssh` syntax can be enabled by using the Dockerfile frontend image built with the `BUILDTAGS="dfrunmount dfssh"`, via the `# syntax =` "shebang". The Dockerfile for the Dockerfile frontend is available at github.com/moby/buildkit/frontend/dockerfile/cmd/dockerfile-frontend) The pre-built image is also available as `tonistiigi/dockerfile:ssh20181002` . An example Dockerfile with `RUN --mount=type=ssh`: # syntax = tonistiigi/dockerfile:ssh20181002 FROM alpine RUN apk add --no-cache openssh-client RUN mkdir -p -m 0700 ~/.ssh && ssh-keyscan gitlab.com >> ~/.ssh/known_hosts RUN --mount=type=ssh ssh git@gitlab.com | tee /hello # "Welcome to GitLab, @GITLAB_USERNAME_ASSOCIATED_WITH_SSHKEY" should be printed here More info available at moby/buildkit#608, moby/buildkit#655 Signed-off-by: Akihiro Suda <suda.akihiro@lab.ntt.co.jp> Upstream-commit: db7399a016bed833205a17129ed80fad4d15e48d Component: cli

…D=$SSH_AUTH_SOCK`) Unlike `docker build --secret`, `docker build --ssh` allows the build container to use SSH keys with passphrases. $ eval $(ssh-agent) $ ssh-add ~/.ssh/id_rsa (Input your passphrase here) $ docker build --ssh default=$SSH_AUTH_SOCK ... This feature requires the daemon with `CapExecMountSSH` build capability (moby/moby#37973) . Currently, the official Dockerfile frontend does not provide the syntax for using the SSH forwarder. However, the experimental `RUN --mount=type=ssh` syntax can be enabled by using the Dockerfile frontend image built with the `BUILDTAGS="dfrunmount dfssh"`, via the `# syntax =` "shebang". The Dockerfile for the Dockerfile frontend is available at github.com/moby/buildkit/frontend/dockerfile/cmd/dockerfile-frontend) The pre-built image is also available as `tonistiigi/dockerfile:ssh20181002` . An example Dockerfile with `RUN --mount=type=ssh`: # syntax = tonistiigi/dockerfile:ssh20181002 FROM alpine RUN apk add --no-cache openssh-client RUN mkdir -p -m 0700 ~/.ssh && ssh-keyscan gitlab.com >> ~/.ssh/known_hosts RUN --mount=type=ssh ssh git@gitlab.com | tee /hello # "Welcome to GitLab, @GITLAB_USERNAME_ASSOCIATED_WITH_SSHKEY" should be printed here More info available at moby/buildkit#608, moby/buildkit#655 Signed-off-by: Akihiro Suda <suda.akihiro@lab.ntt.co.jp> (cherry picked from commit db7399a) Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

…D=$SSH_AUTH_SOCK`) Unlike `docker build --secret`, `docker build --ssh` allows the build container to use SSH keys with passphrases. $ eval $(ssh-agent) $ ssh-add ~/.ssh/id_rsa (Input your passphrase here) $ docker build --ssh default=$SSH_AUTH_SOCK ... This feature requires the daemon with `CapExecMountSSH` build capability (moby/moby#37973) . Currently, the official Dockerfile frontend does not provide the syntax for using the SSH forwarder. However, the experimental `RUN --mount=type=ssh` syntax can be enabled by using the Dockerfile frontend image built with the `BUILDTAGS="dfrunmount dfssh"`, via the `# syntax =` "shebang". The Dockerfile for the Dockerfile frontend is available at github.com/moby/buildkit/frontend/dockerfile/cmd/dockerfile-frontend) The pre-built image is also available as `tonistiigi/dockerfile:ssh20181002` . An example Dockerfile with `RUN --mount=type=ssh`: # syntax = tonistiigi/dockerfile:ssh20181002 FROM alpine RUN apk add --no-cache openssh-client RUN mkdir -p -m 0700 ~/.ssh && ssh-keyscan gitlab.com >> ~/.ssh/known_hosts RUN --mount=type=ssh ssh git@gitlab.com | tee /hello # "Welcome to GitLab, @GITLAB_USERNAME_ASSOCIATED_WITH_SSHKEY" should be printed here More info available at moby/buildkit#608, moby/buildkit#655 Signed-off-by: Akihiro Suda <suda.akihiro@lab.ntt.co.jp>

…D=$SSH_AUTH_SOCK`) Unlike `docker build --secret`, `docker build --ssh` allows the build container to use SSH keys with passphrases. $ eval $(ssh-agent) $ ssh-add ~/.ssh/id_rsa (Input your passphrase here) $ docker build --ssh default=$SSH_AUTH_SOCK ... This feature requires the daemon with `CapExecMountSSH` build capability (moby/moby#37973) . Currently, the official Dockerfile frontend does not provide the syntax for using the SSH forwarder. However, the experimental `RUN --mount=type=ssh` syntax can be enabled by using the Dockerfile frontend image built with the `BUILDTAGS="dfrunmount dfssh"`, via the `# syntax =` "shebang". The Dockerfile for the Dockerfile frontend is available at github.com/moby/buildkit/frontend/dockerfile/cmd/dockerfile-frontend) The pre-built image is also available as `tonistiigi/dockerfile:ssh20181002` . An example Dockerfile with `RUN --mount=type=ssh`: # syntax = tonistiigi/dockerfile:ssh20181002 FROM alpine RUN apk add --no-cache openssh-client RUN mkdir -p -m 0700 ~/.ssh && ssh-keyscan gitlab.com >> ~/.ssh/known_hosts RUN --mount=type=ssh ssh git@gitlab.com | tee /hello # "Welcome to GitLab, @GITLAB_USERNAME_ASSOCIATED_WITH_SSHKEY" should be printed here More info available at moby/buildkit#608, moby/buildkit#655 Signed-off-by: Akihiro Suda <suda.akihiro@lab.ntt.co.jp> (cherry picked from commit db7399a016bed833205a17129ed80fad4d15e48d) Signed-off-by: Sebastiaan van Stijn <github@gone.nl> Upstream-commit: e942084530002e5e02466b3f5941f0dc0136675e Component: cli

tonistiigi force-pushed the ssh-forwarding branch from 2618dcd to 56ef9fc Compare September 6, 2018 22:56

AkihiroSuda reviewed Sep 7, 2018

View reviewed changes

tonistiigi force-pushed the ssh-forwarding branch 2 times, most recently from 5a557bd to 1b7f365 Compare September 7, 2018 01:52

AkihiroSuda reviewed Sep 7, 2018

View reviewed changes

session/sshforward/sshprovider/agentprovider.go Outdated

return nil, err

}

if conf.ID == "" {

conf.ID = "default"

Copy link

Member

AkihiroSuda Sep 7, 2018

Choose a reason for hiding this comment

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

const?

AkihiroSuda approved these changes Sep 7, 2018

View reviewed changes

tonistiigi added 2 commits September 7, 2018 08:57

sshforward: implement ssh socket forwarding

6888956

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>

client: add ssh socket test

9337126

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>

tonistiigi force-pushed the ssh-forwarding branch from 1b7f365 to 68502db Compare September 7, 2018 23:16

tiborvass reviewed Sep 10, 2018

View reviewed changes

session/sshforward/ssh.go Outdated

id = DefaultID

}

go s.run(ctx, l, id)

Copy link

Collaborator

tiborvass Sep 10, 2018

Choose a reason for hiding this comment

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

error is ignored

tiborvass reviewed Sep 10, 2018

View reviewed changes

session/sshforward/ssh.go

return err

}

go Copy(ctx, conn, stream)

Copy link

Collaborator

tiborvass Sep 10, 2018

Choose a reason for hiding this comment

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

error is ignored

tonistiigi added 2 commits September 10, 2018 14:12

sshprovider: allow keys from local files

347420e

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>

vendor: add x/crypto

1604b1b

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>

tonistiigi force-pushed the ssh-forwarding branch from 68502db to 1604b1b Compare September 10, 2018 21:13

tiborvass approved these changes Sep 10, 2018

View reviewed changes

tonistiigi merged commit 1508ae0 into moby:master Sep 11, 2018

AkihiroSuda mentioned this pull request Oct 5, 2018

build: add SSH agent socket forwarder (docker build --ssh $SSHMOUNTID=$SSH_AUTH_SOCK) docker/cli#1419

Merged

Conversation

tonistiigi commented Sep 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Choose a reason for hiding this comment

Uh oh!

tiborvass Sep 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tiborvass Sep 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

tiborvass left a comment

Choose a reason for hiding this comment

Uh oh!

AkihiroSuda commented Sep 11, 2018

Uh oh!

AkihiroSuda commented Oct 1, 2018

Uh oh!

tonistiigi commented Oct 1, 2018

Uh oh!

AkihiroSuda commented Oct 2, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tonistiigi commented Sep 6, 2018 •

edited

Loading

tiborvass Sep 10, 2018 •

edited

Loading

tiborvass Sep 10, 2018 •

edited

Loading