Skip to content

gnupg: Support declarative GPG keysfiles#810

Merged
rycee merged 1 commit intonix-community:masterfrom
MilesBreslin:gpg_keys
Nov 26, 2021
Merged

gnupg: Support declarative GPG keysfiles#810
rycee merged 1 commit intonix-community:masterfrom
MilesBreslin:gpg_keys

Conversation

@MilesBreslin
Copy link
Copy Markdown
Contributor

Adds a new option programs.gpg.keyfiles to be an array of paths to gpg key exports.

Is implemented as an activation script that runs gpg --import $key for each key.

Copy link
Copy Markdown
Member

@rycee rycee left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! I've added a few comments.

Comment thread modules/programs/gpg.nix Outdated
Comment thread modules/programs/gpg.nix Outdated
Comment thread modules/programs/gpg.nix Outdated
Comment thread modules/programs/gpg.nix Outdated
@MilesBreslin MilesBreslin force-pushed the gpg_keys branch 7 times, most recently from 642e750 to 1413ee2 Compare August 20, 2019 19:18
Copy link
Copy Markdown
Contributor

@veehaitch veehaitch left a comment

Choose a reason for hiding this comment

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

I'd like to revive this PR :) happy to lend a hand.

Comment thread modules/programs/gpg.nix Outdated
importGgpKeys = dag.entryAfter ["linkGeneration"] (
let
importKey = keyfile: ''
${pkgs.gnupg}/bin/gpg --import ${lib.escapeShellArg (builtins.toString (pkgs.copyPathToStore keyfile))}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Instead of adding the given keys to any preexisting state of GnuPG, we could also create the file pubring.kbx in its entirety and symlink it similar to .gnupg/gpg.conf.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I really like this idea, but I'm unsure how to implement it

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.

How does this work though, does this then prevent me from ever importing gpg keys without adding them to my h-m configuration first? I'd prefer to just have them imported on top, like I think is happening now.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@colemickens That's correct. I think, however, that such an approach is well aligned with the goal of achieving a declarative, reproducible configuration. Moreover, home-manager works like this in other places as well, for example programs.git.

@MilesBreslin We could create a derivation which sets export GNUPGHOME=$TMPDIR, imports all configured keys and then produces pubring.kbx as an output.

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.

Hm. To me, Git's configuration is completely static, whereas I regularly have to import gpg keys on the fly to work with sops or just do general things; so I view them differently (however, I am of course a fan of having everything managed, so I get this desire).

If GPG had a way to read from multiple pubrings, I'd be a lot more excited about pre-forming and symlinking the primary one.

Can we support both, maybe?

Comment thread modules/programs/gpg.nix Outdated
'';
};

keyfiles = mkOption {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It would be great to have an option to also give the ownertrust of a key. This information is stored in the file .gnupg/trustdb.gpg. Unfortuantely, it is regularly written to by GnuPG even on operations which one would regard as read-only. Therefore, symlinking into the Nix store (as I'd suggest for the public keys, see below) would likely cause unexpected issues. It could be a middle ground to copy the trustdb.gpg file instead of symlinking it.

Copy link
Copy Markdown
Contributor Author

@MilesBreslin MilesBreslin left a comment

Choose a reason for hiding this comment

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

I've added mutable and immutable support for both key files and trust. Since the trustdb needs to be writable, it gets copied on each activation.

Comment thread modules/programs/gpg.nix Outdated

# Copy immutable trust
(optionalString (trust != null && !cfg.mutableTrust) ''
install -m 0700 ${keyringFiles}/trustdb.gpg "$HOME/.gnupg/trustdb.gpg"
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Is there a better way to do this?

@stale
Copy link
Copy Markdown

stale bot commented May 27, 2021

Thank you for your contribution! I marked this pull request as stale due to inactivity. If this remains inactive for another 7 days, I will close this PR. Please read the relevant sections below before commenting.

If you are the original author of the PR

  • GitHub sometimes doesn't notify people who commented / reviewed a PR previously, when you (force) push commits. If you have addressed the reviews you can officially ask for a review from those who commented to you or anyone else.
  • If it is unfinished but you plan to finish it, please mark it as a draft.
  • If you don't expect to work on it any time soon, please consider closing it with a short comment encouraging someone else to pick up your work.
  • To get things rolling again, rebase the PR against the target branch and address valid comments.

If you are not the original author of the issue

  • If you want to pick up the work on this PR, please create a new PR and indicate that it supercedes and closes this PR.

@stale stale bot added the status: stale label May 27, 2021
@stale stale bot closed this Jun 3, 2021
@colemickens
Copy link
Copy Markdown
Member

:( I'd still like to see this.

@MilesBreslin
Copy link
Copy Markdown
Contributor Author

Is there something I can do to push this along? I've been running with my own version of this for a while now, but I'd still like to get it upstream.

@rycee rycee added pinned Prevent marking as stale and removed status: stale labels Jun 4, 2021
@rycee rycee reopened this Jun 4, 2021
@rycee
Copy link
Copy Markdown
Member

rycee commented Oct 31, 2021

I finally had a bit of time to look at this more carefully. Rebased and made a few cleanups and such. The most noticeable one is renaming programs.gpg.keyfiles to programs.gpg.publicKeys, which seems to me a more descriptive name.

Please have a look at the last few commits and see if I made any mistakes.

@MilesBreslin
Copy link
Copy Markdown
Contributor Author

These changes look good at a glance. It's been a while since I've looked at this, but it looks like everything is there and should be functional. I'll give it a go this weekend.

Thanks for helping organize this. This looks better!

@MilesBreslin
Copy link
Copy Markdown
Contributor Author

Tested on my configuration. It seems to be working with no issues now. I use mutableTrust = false and mutableKeys = false. There was an issue with the trust not getting set, but I've added commits to both fix that and test for that.

@MilesBreslin
Copy link
Copy Markdown
Contributor Author

Existing file '/home/user/.gnupg/pubring.kbx.old' would be clobbered by backing up '/home/user/.gnupg/pubring.kbx'
Please move the above files and try again or use -b <ext> to move automatically.

Looks like this needs to be more forceful.

@MilesBreslin
Copy link
Copy Markdown
Contributor Author

Looking into this further, this is just a migration issue on my end. This is working as intended.

@rycee
Copy link
Copy Markdown
Member

rycee commented Nov 25, 2021

@MilesBreslin Nice, so this is OK to merge? If so, then I can squash and merge as soon as possible.

@MilesBreslin
Copy link
Copy Markdown
Contributor Author

@rycee Yes, this should be good to merge.

@rycee rycee merged commit ea1794a into nix-community:master Nov 26, 2021
@rycee
Copy link
Copy Markdown
Member

rycee commented Nov 26, 2021

Great, merged to master now. Good job!

@ebbertd
Copy link
Copy Markdown

ebbertd commented Nov 28, 2021

Thank you for making this possible. This works great.

peterhoeg pushed a commit to peterhoeg/home-manager that referenced this pull request Dec 6, 2021
kalbasit pushed a commit to kalbasit/home-manager that referenced this pull request Dec 18, 2021
kalbasit pushed a commit to kalbasit/home-manager that referenced this pull request Dec 18, 2021
rycee pushed a commit that referenced this pull request Dec 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pinned Prevent marking as stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants