Skip to content

nixos/gnome-keyring: add SSH support by exporting SSH_AUTH_SOCK#310978

Open
no-mood wants to merge 7 commits intoNixOS:masterfrom
no-mood:enable-SSH-Support
Open

nixos/gnome-keyring: add SSH support by exporting SSH_AUTH_SOCK#310978
no-mood wants to merge 7 commits intoNixOS:masterfrom
no-mood:enable-SSH-Support

Conversation

@no-mood
Copy link
Copy Markdown

@no-mood no-mood commented May 12, 2024

Description of changes

Added SSH support inside the gnome-keyring module, similar to ssh.nix and gnupg.nix by exporting SSH_AUTH_SOCK.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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/)
  • 24.05 Release Notes (or backporting 23.05 and 23.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
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: GNOME GNOME desktop environment and its underlying platform 8.has: module (update) This PR changes an existing module in `nixos/` labels May 12, 2024
@NixOSInfra NixOSInfra added the 12.first-time contribution This PR is the author's first one; please be gentle! label May 12, 2024
@no-mood no-mood changed the title Added ssh support by exporting SSH_AUTH_SOCK Gnome-Keyring: Added ssh support by exporting SSH_AUTH_SOCK May 12, 2024
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels May 12, 2024
source = "${pkgs.gnome.gnome-keyring}/bin/gnome-keyring-daemon";
};

environment.extraInit = lib.mkIf cfg.enableSSHSupport ''
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm. Is there no easy way to express this with environment.sessionVariables or environment.variables?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

At first I was trying to use something like this:

environment.sessionVariables = {
  SSH_AUTH_SOCK = "${builtins.getEnv "XDG_RUNTIME_DIR"}/keyring/ssh";
};

Then I relied on what is done with SSH and GnuPG, I thought it was a better practice.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This ${builtins.getEnv "XDG_RUNTIME_DIR"} is $XDG_RUNTIME_DIR in eval, not in activation.

The current way is not that bad though:

environment.extraInit = ''
if [ -z "$SSH_AUTH_SOCK" -a -n "$XDG_RUNTIME_DIR" ]; then
export SSH_AUTH_SOCK="$XDG_RUNTIME_DIR/yubikey-agent/yubikey-agent.sock"
fi
'';

Comment thread nixos/modules/services/desktops/gnome/gnome-keyring.nix Outdated
Copy link
Copy Markdown
Member

@pluiedev pluiedev left a comment

Choose a reason for hiding this comment

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

Also, please make sure the title of both the PR and the commit fit existing conventions. They should look something more like nixos/gnome-keyring: add SSH support by exporting SSH_AUTH_SOCK

@no-mood no-mood changed the title Gnome-Keyring: Added ssh support by exporting SSH_AUTH_SOCK nixos/gnome-keyring: add SSH support by exporting SSH_AUTH_SOCK May 12, 2024
@no-mood
Copy link
Copy Markdown
Author

no-mood commented May 12, 2024

I hadn't realized that in my configuration, the gnome-keyring-daemon is manually launched by my window manager (Hyprland):

exec-once = [ "${gnome.gnome-keyring}/bin/gnome-keyring-daemon --start --components=secrets,ssh" ];

This means that exporting the SSH_AUTH_SOCK variable alone isn't sufficient to handle ssh keys inside gnome-keyring. Unless specified with --components, only the secrets component is run.
Optionals components should be then passed to the wrapper?

There's a home-manager gnome-keyring module that does this in a similar way, but we're still missing a nixos module

Update: These issues may be related; unfortunately, I just discovered them:
#284173
#166887

Copy link
Copy Markdown

@maanu1234 maanu1234 left a comment

Choose a reason for hiding this comment

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

👍

@addy419
Copy link
Copy Markdown

addy419 commented May 19, 2024

The gnome keyring component for setting ssh is moved to gcr https://wiki.archlinux.org/title/GNOME/Keyring#Setup_gcr

Seems like the systemd component is added to gcr

PKG_CONFIG_SYSTEMD_SYSTEMDUSERUNITDIR = "${placeholder "out"}/lib/systemd/user";

So starting the service and setting SSH_AUTH_SOCK should do the trick? In theory

@Aleksanaa
Copy link
Copy Markdown
Member

There is another problem. SSH_AUTH_SOCK should be mutually exclusive in various modules to avoid undefined behavior. In the current implementation, they will not conflict with each other during eval.

@no-mood
Copy link
Copy Markdown
Author

no-mood commented May 19, 2024

There is another problem. SSH_AUTH_SOCK should be mutually exclusive in various modules to avoid undefined behavior. In the current implementation, they will not conflict with each other during eval.

Something like this was added here:

assertions = [
{ assertion = cfg.agent.enableSSHSupport -> !config.programs.ssh.startAgent;
message = "You can't use ssh-agent and GnuPG agent with SSH support enabled at the same time!";
}
];

I feel like we could use add another layer and make an option. Something like services.keyring = gnome-keyring

@pluiedev
Copy link
Copy Markdown
Member

I feel like we could use add another layer and make an option. Something like services.keyring = gnome-keyring

An option for setting SSH_AUTH_SOCK would definitely be nice — I use 1Password as my SSH signer at the moment and that also requires setting SSH_AUTH_SOCK. Having a dedicated option for that would allow both GNOME Keyring and 1Pass to use the same mechanism which should make things much simpler

@no-mood
Copy link
Copy Markdown
Author

no-mood commented May 19, 2024

An option for setting SSH_AUTH_SOCK would definitely be nice — I use 1Password as my SSH signer at the moment and that also requires setting SSH_AUTH_SOCK. Having a dedicated option for that would allow both GNOME Keyring and 1Pass to use the same mechanism which should make things much simpler

I agree. Is this the right place for this or should I open a new issue?
Could I propose a solution with a commit?

@pluiedev
Copy link
Copy Markdown
Member

An option for setting SSH_AUTH_SOCK would definitely be nice — I use 1Password as my SSH signer at the moment and that also requires setting SSH_AUTH_SOCK. Having a dedicated option for that would allow both GNOME Keyring and 1Pass to use the same mechanism which should make things much simpler

I agree. Is this the right place for this or should I open a new issue? Could I propose a solution with a commit?

I think it's reasonable to add it in this PR and then change the title (therefore repurpose) the PR to reflect the change

@no-mood
Copy link
Copy Markdown
Author

no-mood commented May 24, 2024

Before making a commit, the idea would be to add a level of abstraction and create an additional option that sets $SSH_AUTH_SOCK. Each module will then offer an option to be set as SSH Agent using that option.

Example: The new option will be users.users.<name>.SSHAgent. Each module, like Gnome-Keyring, instead of setting SSH_AUTH_SOCK directly, will access the SSHAgent parameter like this:
users.users.<name>.SSHAgent ="$XDG_RUNTIME_DIR/ssh-agent".

By creating this option, checking for other active SSH agents won't be needed.

@no-mood no-mood force-pushed the enable-SSH-Support branch from 9da36e9 to f61231c Compare May 27, 2024 19:16
@no-mood no-mood force-pushed the enable-SSH-Support branch from f61231c to cdb7ef7 Compare May 27, 2024 19:28
@no-mood no-mood force-pushed the enable-SSH-Support branch from cdb7ef7 to f27533c Compare May 27, 2024 19:32
@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jul 4, 2024
@gabyx
Copy link
Copy Markdown
Contributor

gabyx commented Sep 30, 2024

any progress on this?

@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2.status: merge conflict This PR has merge conflicts with the target branch 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 12.first-time contribution This PR is the author's first one; please be gentle!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants