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.

Sign up for GitHub

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

Jump to bottom

nixos/ssh: add ssh-agent socket support in programs.ssh.startAgent #169155

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

nixos/ssh: add ssh-agent socket support in programs.ssh.startAgent #169155

wants to merge 2 commits into from

Conversation

anthr76
Copy link
Contributor

@anthr76 anthr76 commented Apr 18, 2022 •
edited

Also add SSH_AUTH_SOCK SSH_AGENT_PID to the service. SSH_AUTH_SOCK is referenced for the socket location on ExecStart and ExecStartPre.

I'm a week-old nix user so a good set of eyes is appreciated.

Description of changes
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.05 Release Notes (or backporting 21.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@anthr76
nixos/ssh: add socket to agent service
70f5ad3 
Also add `SSH_AUTH_SOCK` `SSH_AGENT_PID` to the service. This allows
systemd to track the ssh-agent socket natively.

Signed-off-by: Anthony Rabbito <hello@anthonyrabbito.com>
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/831

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Oct 30, 2022
@RaitoBezarius
Copy link
Member

Will give it a try.

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Dec 22, 2022
@anthr76
Copy link
Contributor Author

anthr76 commented Jul 11, 2023

Is there any way to bring priority to this?

@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 19, 2024
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label May 22, 2024
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/3978

@anthr76
Copy link
Contributor Author

anthr76 commented May 22, 2024 •
edited

(tagging last few folks on git history) @acid-bong @pbsds @SuperSandro2000 @sigprof

@pbsds
Copy link
Contributor

pbsds commented May 23, 2024

Please rewrite the git history to remove the merge commit, and fix the merge conflic

@sigprof
Copy link
Contributor

sigprof commented May 24, 2024

Does this actually work?

Apparently the upstream OpenSSH does not have any systemd support at all, and features like systemd service state notifications and socket activation support are added in distro-specific patches (e.g., see https://salsa.debian.org/ssh-team/openssh/-/tree/master/debian/patches, in particular systemd-readiness.patch and systemd-socket-activation.patch). However, I don't see any patches like that in the Nixpkgs package for OpenSSH.

Also the systemd-socket-activation.patch mentioned above handles socket activation only for sshd and does not make any changes to ssh-agent. There was an attempt to add socket activation to ssh-agent, but the patch was rejected by the OpenSSH upstream due to the libsystemd licensing. Some other patches can be found in https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1068416, but they had not been accepted in Debian yet.

So it looks like adding the systemd socket activation support to the NixOS module for OpenSSH won't work until the corresponding code is added to the actual openssh package.

@Aleksanaa Aleksanaa changed the title fix: add ssh-agent socket support in programs.ssh.startAgent nixos/ssh: add ssh-agent socket support in programs.ssh.startAgent May 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
2.status: merge conflict 6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 12. first-time contribution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@anthr76 @nixos-discourse @RaitoBezarius @pbsds @sigprof @wegank @Janik-Haag