Handle Cygwin / Git Bash sockets forwarding on Windows#82
Merged
chrmarti merged 1 commit intodevcontainers:mainfrom Nov 14, 2022
Merged
Handle Cygwin / Git Bash sockets forwarding on Windows#82chrmarti merged 1 commit intodevcontainers:mainfrom
chrmarti merged 1 commit intodevcontainers:mainfrom
Conversation
Contributor
|
@amurzeau Thanks for your PR! Code looks good. I will need to do some more testing myself (expect some delay as I will have some time off). My (older) GPG4Win install still worked. |
3353604 to
9245a1c
Compare
6 tasks
7 tasks
16 tasks
Member
|
@cla-bot check |
Contributor
Author
|
I think I will need to sign the CLA, but I don't see any link to click on, maybe not ready yet ? |
Member
|
@amurzeau sorry for any confusion, yes we're still investigating. Thanks for your patience! |
Contributor
Author
|
@microsoft-github-policy-service agree |
chrmarti
requested changes
Nov 9, 2022
Contributor
chrmarti
left a comment
There was a problem hiding this comment.
Looks great, left a few comments. Thanks!
1cfd99b to
9e65800
Compare
chrmarti
requested changes
Nov 10, 2022
Contributor
chrmarti
left a comment
There was a problem hiding this comment.
Looks good, left one more question regarding the updated error handling.
This is used to forward ssh-agent connection to Git Bash' ssh-agent.
Here is the explanation of what is required to connect to a cygwin / git
bash unix domain socket on Windows:
- Port parsing:
- Git Bash' unix sockets requires connecting to the port whose number
is in the socket file along with a cookie.
- The socket file contains something like `!<socket >63488 s 44693F4F-E2572CA5-537862AB-248DFDEF`
- The port here is `63488` and the cookie is `44693F4F-E2572CA5-537862AB-248DFDEF`.
- So I retrieve the port and cookie using a regex and convert it to a
number.
- If the file content does not match the regex, I assume this is a
GPG socket and use the existing code to parse it.
- When I have the port and the cookie, I connect to `127.0.0.1:<port>`,
then I do the following handshake.
- Cygwin / Git Bash socket Handshake:
- The handshake consists in:
- the client must send the cookie as 16 raw bytes
- The cookie is formatted in the socket file as 4 32 bits hex
integers. They must be send to the ssh-agent server in little
endian as 16 raw bytes (this means according to the above
example: `0x4F 0x3F 0x69 0x44 0xA5 0x72 ...`).
- the server send back the same 16 bytes if the cookie is valid,
else closes the connection (so the client must skip these 16
bytes, as done in `skipHeader`)
- the client must send pid and user id and user effective id
information in a 12 bytes packet
- I set the pid to a real value from process.pid, but ssh-agent
ignores it
- user id and user effective id are both set to 0
- the server send back the same information, but about the server, I
just ignore these 12 bytes too in `skipHeader`; this is a function
that just skip the handshake data).
As the server send back data in the handshake phase (16 + 12 bytes), I
need to skip them through the use of `skipHeader`.
Then actual data transfer can take place.
See also:
https://stackoverflow.com/questions/23086038/what-mechanism-is-used-by-msys-cygwin-to-emulate-unix-domain-sockets
https://github.com/abourget/secrets-bridge/blob/094959a1553943e0727f6524289e12e8aab697bf/pkg/agentfwd/agentconn_windows.go#L15
Fix: devcontainers#62
9e65800 to
bd684d6
Compare
chrmarti
approved these changes
Nov 14, 2022
Contributor
chrmarti
left a comment
There was a problem hiding this comment.
LGTM, thanks! (Will ask for a second review and then merge.)
joshspicer
approved these changes
Nov 14, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is used to forward ssh-agent connection to Git Bash' ssh-agent.
I'm not sure this is the cleanest way to implement this, but I've not found existing library functions that could reduce the amount of code.
I've checked that the forwarding work using a "test" and a pipe server to be able to test this case:
ssh-add => \\.\pipe\mypipe => test => call connectLocal to forward to Git Bash's ssh-agent socketA way to test with VS Code would be better than using a manual test, but I don't how I could integrate it in VS Code.
I've also tried to test the connection to gpg-agent (via the socket file named
S.gpg-agent.ssh), but I couldn't make it works, despite expected packets are sent to gpg-agent according to wireshark. It seems that something broke on gpg-agent side according to:Here is the explanation of what is required to connect to a cygwin / git bash unix domain socket on Windows:
!<socket >63488 s 44693F4F-E2572CA5-537862AB-248DFDEF63488and the cookie is44693F4F-E2572CA5-537862AB-248DFDEF.127.0.0.1:<port>, then I do the following handshake.- The cookie is formatted in the socket file as 4 32 bits hex integers. They must be send to the ssh-agent server in little endian as 16 raw bytes (this means according to the above example:
0x4F 0x3F 0x69 0x44 0xA5 0x72 ...).skipHeader)skipHeader; this is a function that just skip the handshake data).As the server send back data in the handshake phase (16 + 12 bytes), I need to skip them through the use of
skipHeader.Then actual data transfer can take place.
See also:
https://stackoverflow.com/questions/23086038/what-mechanism-is-used-by-msys-cygwin-to-emulate-unix-domain-sockets
https://github.com/abourget/secrets-bridge/blob/094959a1553943e0727f6524289e12e8aab697bf/pkg/agentfwd/agentconn_windows.go#L15
Fix: #62