Skip to content

Conversation

SunMar
Copy link

@SunMar SunMar commented Feb 5, 2025

This is an attempt to revive #35 (which is merge of jstarks#6), as it seems to be dead waiting for some comments to be resolved.

This PR addresses those comments, I did not make any other changes to the implementation code itself. I tested this in my own WSL environment and was able to communicate with the GPG agent through Assuan file sockets. Commands like gpg --card-status and signing Git commits worked using the native gpg binaries and without running a separate agent inside WSL.

Let me know if any other changes are needed.

NZSmartie and others added 2 commits February 5, 2025 15:07
This commit squishes the changeset presented on jstarks#6. That commit is then rebased
onto albertony/npiperelay:fork

Co-authored-by: Lex Robinson <lex@lex.me.uk>
@albertony
Copy link
Owner

Thank you. Had a quick look, and the first impression is positive. 👍

I will not have the time/opportunity to test the functionality myself, so I will review the code a bit more thoroughly later (maybe in a day or two), and any testing you are able to do is welcome (I noticed you mentioning you have already done some testing, so maybe you consider it covered?). I saw one comment in one of the old threads;

I've noticed that npiperelay.exe likes to hang around after the pipe has finished. not sure if the socket isn't getting closed.

You didn't notice anything similar in your tests, I assume?

@albertony albertony added the enhancement New feature or request label Feb 5, 2025
@SunMar
Copy link
Author

SunMar commented Feb 5, 2025

@albertony Thanks for the quick response!

For testing I tried gpg --card-status (I am using a YubiKey) and using Git to sign my commits, both I did multiple times. It worked every time, the pinentry progam also popped up as expected and if I did not have my YubiKey connected it would ask me to insert it. As signing Git commits from WSL is my only use case that's the only thing I tested. If you have any suggestions on other things I can test from the CLI then happy to try them.

Tonight I additionally tried if it would work for S.gpg-agent.ssh as well, but for me it didn't. A simple ssh-add -L works, but I couldn't get it to authenticate an SSH session. After debugging for a while it looks like the TCP connection is getting closed after the first message. Maybe in overlappedfile.go something happens that makes sense when using windows.CreateFile(), but is wrong when using windows.ConnectEx()? It's possible gpg works because it sends one-off messages that don't expect a connection to stay open. Unfortunately I don't have enough knowledge about Windows or Golang to know how to dig into this.
The OpenSSH Agent protocol being a binary protocol also makes it tricky to debug. I can't just connect manually, try step-by-step a list of commands and see exactly when it fails. I may take another crack at this in the future, but for now I hope this limitation is ok. The SSH agent can also be connected to using the named pipe //./pipe/openssh-ssh-agent which works great and has no issue (no Assuan support needed), so that would be the preferred way anyways and not via S.gpg-agent.ssh.

As for hanging around after the pipe is finished, in that PR I do see after that comment he pushed terminate when pipe or stdin closes in example. It enables the -ei flag, I assume that solved the problem. In my tests I was already using both -ei and -ep and did not experience any hanging pipes, but I'll keep an eye out for it.

@SunMar
Copy link
Author

SunMar commented Feb 6, 2025

This morning I did a few more tests on the SSH agent, and those showed that NZSmartie/npiperelay (which is used in most guides out there for the GPG agent) exhibits the same behavior, for me it only works for the GPG agent and not for the Assuan socket of the SSH agent. I additionally tested both npiperelay versions using socat (I was using systemd socket services) and that didn't work either. So it doesn't look like these changes break when porting them into your fork of npiperelay, using them for the SSH agent was already not working (at least with all the versions and setups I have tried both in my computer and in a VM). So for the SSH agent people will need to stick to using the named pipe //./pipe/openssh-ssh-agent.

@albertony
Copy link
Owner

Great. Thank you for your efforts, including the explanations here. Appreciated. If you are happy with the current state, we can merge it? And if/when we want to dig further into the mentioned issues, we can iterate on those later.

@SunMar
Copy link
Author

SunMar commented Feb 6, 2025

@albertony Yes, happy with the current state and would be great if it can be merged, thank you for actively maintaining this fork!

@albertony albertony merged commit 2f1a607 into albertony:fork Feb 6, 2025
@SunMar
Copy link
Author

SunMar commented Feb 6, 2025

@albertony thank you! 🎉

@demonbane
Copy link

I've been following this closely (and quietly I guess) since relying on a long-unmaintained fork was just a terrible idea, so I'm very happy to see this merged into a maintained fork now. Thanks to everyone involved (@NZSmartie, @Lexicality, @ndimiduk, and @SunMar) for finally getting this done, and thanks to @albertony for merging it. I've also updated wsl-gpg-systemd to use this for all future installs. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants