Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ssh agent only forwarded from keys on Windows #914

Closed
lajoie opened this issue Mar 31, 2019 · 21 comments · Fixed by #1695
Closed

ssh agent only forwarded from keys on Windows #914

lajoie opened this issue Mar 31, 2019 · 21 comments · Fixed by #1695

Comments

@lajoie
Copy link

lajoie commented Mar 31, 2019

Docker Engine: 18.09.2 w/ buildkit enabled
Buildkit Frontend: 1.0.2-experimental
Host OS: Windows 10.0.14393
Container OS: Debian 9.8 Linux
Git-Bash: 2.17.1.windows.2

We're attempting to share our SSH agent socket with a building image in order to support git-ssh.

Doing an ssh-add -l we can see the key is loaded.

Our dockerfile has this directive

RUN ---mount=type=ssh \
    git clone --single-branch --branch "${GIT_BRANCH}" "${GIT_URL}" "${BUILD_HOME}"

The docker build command has this argument

--ssh default

When running the docker build, we get this error:

could not parse ssh: [default]: failed to parse C:/Users/username/AppData/Local/Temp/ssh-l0E48ifEnCIz/agent.8496: ssh: no key found

If we run the git clone locally within git-bash it works. We know that git isn't finding our private key and using it directly from the file system. The key is in a non-standard location and if ssh-agent isn't running or the key is not loaded, the clone fails.

@CCP-Aporia
Copy link

CCP-Aporia commented Apr 1, 2019

Same problem here with a slightly newer Windows and Git Bash version.

Please note that it works when passing the full path to the actual ssh key, e.g.:

$ docker build --ssh default=c:/users/myuser/.ssh/id_rsa .

However, that only works when you know the actual key location on disk, so is not necessarily suitable in all scenarios. Especially TeamCity build agents won't work out of the box as they place any ssh key in a temporary folder.

It seems the problem here is that this check fails to identify the file as a Unix domain socket, and thus treats it as an actual private key file, which it fails to parse.

@tonistiigi tonistiigi changed the title ssh mount not working on Windows ssh agent only forwarded from keys on Windows Apr 2, 2019
@tonistiigi
Copy link
Member

I tried to look into it. After fixing the

if fi.Mode()&os.ModeSocket > 0 {
I'm hit with dial unix C:\Users\Administrator\AppData\Local\Temp\ssh-dD21whKeiVjh\agent.4464: connect: An attempt was made to access a socket in a way forbidden by its access permissions.

ssh-add can connect to it though so I'm not sure what is wrong. This is about the extent of my windows knowledge. If anyone has suggestions, let me know.

As @CCP-Aporia said, workaround is to point to a key.

@lajoie
Copy link
Author

lajoie commented Apr 3, 2019

@tonistiigi what exactly does providing the full key path do? I assume it bypasses the socket and is the equivalent of doing secret mount of the key. Is that correct?

@tonistiigi
Copy link
Member

@lalyos No, it exposes an ssh-agent built into the cli with the specified key loaded. Your private key still never leaves the client. Even on exposing the socket we will not forward the full ssh-agent but the internal one that has limited capabilities and is backed by the local one.

@thaJeztah
Copy link
Member

/cc @olljanat @simonferquel

@CCP-Aporia
Copy link

@tonistiigi - from an outside perspective I don't see a difference between mapping a SSH key via the --secret or via --ssh. The only reason why I started looking at the --ssh option was because I've the issue that in my build system I don't know the location of the SSH key file (but I do have an SSH agent).

Also, this issue gets even more interesting with Windows 10's built-in OpenSSH agent, which uses a named pipe and does not expose SSH_AUTH_SOCK.

@tonistiigi
Copy link
Member

I'd still recommend ssh with a key over a secret in this case. With ssh we never send your key to the daemon. Although we don't persist your secrets either it is safer if they don't ever leave your client at all.

@thaJeztah
Copy link
Member

Could we read from a named pipe? Is there some discovery mechanism on Windows to find that pipe?

@olljanat
Copy link

olljanat commented Apr 6, 2019

To be honest I'm not familiar with this stuff but will try as I was added to cc.

Could we read from a named pipe?

Based on keepassxreboot/keepassxc#1994 I found that there should be named pipe \\.\pipe\openssh-ssh-agent

Is there some discovery mechanism on Windows to find that pipe?

Sure just run PowerShell command:
Get-ChildItem \\.\pipe\ | Format-Table FullName

@lajoie
Copy link
Author

lajoie commented Apr 9, 2019

sorry for the slow reply, I've been travelling...

From my side, one of the reasons I'd like to have the ssh-agent support is to allow the use of hardware tokens (e.g., yubikey) for our SSH keys. The agent supports that, but the specify-a-key-file models (whether sharing a secret or using the buildkit agent) does not.

@nidkil
Copy link

nidkil commented May 17, 2019

Any progress on this issue?

@MiniXC
Copy link

MiniXC commented Jun 1, 2019

Same issue on linux, sudo DOCKER_BUILDKIT=1 docker build --ssh $SSH_AUTH_SOCK . gives me

could not parse ssh: [/tmp/ssh-kFew4Mw5SPtN/agent.2407]: invalid empty ssh agent socket, make sure SSH_AUTH_SOCK is set

while sudo DOCKER_BUILDKIT=1 docker build --ssh default . results in

could not parse ssh: [default]: invalid empty ssh agent socket, make sure SSH_AUTH_SOCK is set

@lajoie
Copy link
Author

lajoie commented Jun 1, 2019

This has always worked fine for me on MacOS and Linux. Might want to double-check that you have your key loaded into the SSH agent (ssh-agent -l).

@maximveksler
Copy link

maximveksler commented Sep 28, 2019

Same problem here with a slightly newer Windows and Git Bash version.

Please note that it works when passing the full path to the actual ssh key, e.g.:

$ docker build --ssh default=c:/users/myuser/.ssh/id_rsa .

However, that only works when you know the actual key location on disk, so is not necessarily suitable in all scenarios. Especially TeamCity build agents won't work out of the box as they place any ssh key in a temporary folder.

It seems the problem here is that this check fails to identify the file as a Unix domain socket, and thus treats it as an actual private key file, which it fails to parse.

A bit more portable work around that still works would be:

docker build --ssh default=~/.ssh/id_rsa .

Tested on Git Bash for Windows.

@firecow
Copy link

firecow commented Jan 24, 2020

This bug is still present even after latest 2.2.0.0 (42247) update.
docker build --ssh default=~/.ssh/id_rsa . still "works".
Not being able to use the ssh-agent is kind of a pain, and forces big projects to have developers put a non-password protected key in ~/.ssh/id_rsa, or use other hacks.

@dolsem
Copy link

dolsem commented Mar 31, 2020

I'm having the same issue on Mac OS. Specifying the path to the key works, but ssh-add identities don't.

@mdgilene
Copy link

Any update on this? This is a real issue for my team causing us to resort to dirty workarounds.

@r4j4h
Copy link

r4j4h commented Jun 19, 2020

Running into the same issue on Linux. Pathing to non-passworded keys works but ssh-add is ignored :(

@tonistiigi
Copy link
Member

This is a windows-only issue. If you run into trouble in other OS it is something else. Debug with ssh-add -L if identities are visible and open new issue or ask in #buildkit if you can't figure it out. If you do receive the same permission denied error on accessing the ssh-agent socket then your user that is invoking docker/buildx/buildctl does not have access to the ssh-agent you are trying to forward.

For windows users: this issue has less to do with buildkit and more with how running ssh-agent socket is accessible under windows permission model. We are currently lacking maintainers who use Windows themselves and/or are familiar with these internals. If you are a windows dev please see if you can help out. Basically you just need to show how a go program can communicate with ssh-agent without getting the permission error.

@r4j4h
Copy link

r4j4h commented Jun 19, 2020

FWIW I was able to proceed in my case adding --ssh default=$SSH_AUTH_SOCK in the docker build command. Maybe that will help the aforementioned MacOS users issue.

@nicks
Copy link
Contributor

nicks commented Sep 22, 2020

I played around with this a bit on my Windows box. The behavior I was seeing was that the ModeSocket bit simply isn't on the ssh agent socket. Might be related to golang/go#33357, which sounds similar

will post a PR that worked for me

nicks added a commit to tilt-dev/buildkit that referenced this issue Sep 22, 2020
vladaionescu added a commit to earthly/buildkit-old-fork that referenced this issue Sep 30, 2020
* Fix platform typo

Signed-off-by: Jon Zeolla <[email protected]>

* build-arg add support BUILDKIT_SYNTAX

Signed-off-by: Wang Yumu <[email protected]>

* update containerd to v1.4.0, runc to v1.0.0-rc92

Signed-off-by: Akihiro Suda <[email protected]>

* cni: remove duplicate error check

Signed-off-by: Miguel Ángel Jimeno <[email protected]>

* Enable to use remote snapshots for refs

Signed-off-by: ktock <[email protected]>

* vendor: github.com/gofrs/flock v0.7.3

full diff: gofrs/flock@v0.7.0...v0.7.3

v0.7.3
-------------------------

- Fix issues in the license file, update year.

v0.7.2
-------------------------

- Ensure we release file handle if we failed to take an exclusive lock

v0.7.1
-------------------------

- Fix linting issues and add goreportcard badge

Signed-off-by: Sebastiaan van Stijn <[email protected]>

* vendor: github.com/pkg/profile v1.5.0

full diff: pkg/profile@v1.2.1...v1.5.0

v1.5.0
-------------------------

- Add MemProfileType to allow overriding type of memory profile
- Make Go 1.13 the minimum supported Go version.

v1.4.0
-------------------------

- Added goroutine profiling

v1.3.0
-------------------------

- Add ThreadcreationProfile
- Bump Go versions

Signed-off-by: Sebastiaan van Stijn <[email protected]>

* Add integration test for containerd and stargz snapshotter

Signed-off-by: ktock <[email protected]>

* vendor: update containerd to efa0e809

Signed-off-by: Tonis Tiigi <[email protected]>

* ensure containerd is running for worker tests

Signed-off-by: Cory Bennett <[email protected]>

* Enable to use stargz snapshotter without spawning plugin process

Signed-off-by: ktock <[email protected]>

* Enable to run integration tests with stargz snapshotter

Signed-off-by: ktock <[email protected]>

* vendor: update containerd to d4e7820

Signed-off-by: Ilya Dmitrichenko <[email protected]>

* Limit size of additional label for avoiding preparation failure

In containerd, there is a size limit for label size (4096 chars).
If an image has many layers (> (4096-43)/72 > 55),
`containerd.io/snapshot/remote/stargz.layers` will hit the limit of
label size and the remote snapshot preparation will fail.
This commit fixes this by limiting the size of the label.

Signed-off-by: ktock <[email protected]>

* fix tests after busybox update

Signed-off-by: Tonis Tiigi <[email protected]>

* client: avoid checking token cap on default case

Signed-off-by: Tonis Tiigi <[email protected]>

* client: allow build callback to return nil result

Signed-off-by: Tonis Tiigi <[email protected]>

* Allow stargz target in Dockerfile to use golang build cache

Signed-off-by: ktock <[email protected]>

* dockerfile: allow multiple values for ARG

Signed-off-by: Tonis Tiigi <[email protected]>

* sshprovider: on Windows, ModeSocket might not be set on the ssh socket

Fixes moby#914

Signed-off-by: Nick Santos <[email protected]>

* go.mod: github.com/containerd/console v1.0.1

full diff: containerd/console@v1.0.0...v1.0.1

Fixes compatibility with current versions of golang.org/x/sys

Signed-off-by: Sebastiaan van Stijn <[email protected]>

Co-authored-by: Jon Zeolla <[email protected]>
Co-authored-by: Tõnis Tiigi <[email protected]>
Co-authored-by: Wang Yumu <[email protected]>
Co-authored-by: Akihiro Suda <[email protected]>
Co-authored-by: Miguel Ángel Jimeno <[email protected]>
Co-authored-by: Akihiro Suda <[email protected]>
Co-authored-by: ktock <[email protected]>
Co-authored-by: Sebastiaan van Stijn <[email protected]>
Co-authored-by: Erik Sipsma <[email protected]>
Co-authored-by: Cory Bennett <[email protected]>
Co-authored-by: Ilya Dmitrichenko <[email protected]>
Co-authored-by: Edgar Lee <[email protected]>
Co-authored-by: Nick Santos <[email protected]>
Co-authored-by: Tibor Vass <[email protected]>
Co-authored-by: Vlad A. Ionescu <[email protected]>
fahedouch pushed a commit to fahedouch/buildkit that referenced this issue Nov 8, 2020
fahedouch pushed a commit to fahedouch/buildkit that referenced this issue Nov 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.