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

gnupg: change default keyserver to non-SKS #63952

Merged
merged 2 commits into from Jun 30, 2019
Merged

gnupg: change default keyserver to non-SKS #63952

merged 2 commits into from Jun 30, 2019

Conversation

alyssais
Copy link
Member

@alyssais alyssais commented Jun 30, 2019 •
edited

I’ve asked the people behind keys.openpgp.org if they’re ready to become a distro default, and am awaiting a definitive answer from them. Until then this PR should not be merged.

This is a breaking change, but I think it should still be backported to stable because the alternative is leaving people vulnerable.

Motivation for this change

See https://gist.github.com/rjhansen/67ab921ffb4084c865b3618d6955275f.

The SKS network is vulnerable to certificate poisoning, which can destroy GnuPG installations. keys.openpgp.org is a new non-SKS keyserver that is resistant to this type of attack.

With such an attack being possible, it is unsafe to use SKS keyservers for almost anything, and so we should protect our users from a now unsafe default. keys.openpgp.org offers some (but not all) functionality of SKS, and is better than nothing.

This default is only present in gnupg22. gnupg20 and gnupg1orig are not affected.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@alyssais
gnupg: change default keyserver to non-SKS
c727083 
See https://gist.github.com/rjhansen/67ab921ffb4084c865b3618d6955275f.

The SKS network is vulnerable to certificate poisoning, which can
destroy GnuPG installations. keys.openpgp.org is a new non-SKS keyserver
that is resistant to this type of attack.

With such an attack being possible, it is unsafe to use SKS keyservers
for almost anything, and so we should protect our users from a now
unsafe default. keys.openpgp.org offers some (but not all) functionality
of SKS, and is better than nothing.

This default is only present in gnupg22. gnupg20 and gnupg1orig are not
affected.
@alyssais alyssais added 1.severity: security 9.needs: port to stable A PR needs a backport to the stable release. 2.status: wait-for-upstream Waiting for upstream fix (or their other action). labels Jun 30, 2019
@alyssais alyssais requested a review from grahamc June 30, 2019 14:14
@ofborg ofborg bot requested review from fpletz, vrthra and peti June 30, 2019 14:25
@dkg
Copy link

dkg commented Jun 30, 2019 •
edited

hi! as one of the folks whose OpenPGP certificate has been affected by this flooding attack, as the author of draft-dkg-openpgp-abuse-resistant-keystore which describes the attack, and as one of the debian developers responsible for co-maintaining both GnuPG and SKS, i'm definitely interested in this sort of change.

I'm not convinced that the nice simple change you've done is correct, though. :/ What sort of tests have you run?

In particular, i'm concerned that http_session_new() in dirmngr/http.c adjusts what certificate authority to use based on whether the selected keyserver is the default keyserver or not. if it is the default, it switches acceptable certificate authorities over to Kristian Fiskerstrand's custom authority, which is dedicated to providing certificates for the SKS pool. That authority, of course, has not signed off on the X.509 cert offered by keys.openpgp.org. So if it is willing to accept certifications from Kristian's CA to a server that is not the SKS pool, that seems like a problem. I've reported it upstream.

I note that later in the same function, the system CAs might be added as well, which might be why this appears to work for you. i note that add_system_cas will not be explicitly set to 1 when is_hkps_pool is set. But perhaps it is initialized as 1 because the caller is setting HTTP_FLAG_TRUST_SYS unilaterally? that would mean that dirmngr was being overly permissive in accepting system CAs for the HKPS SKS pool itself, when we know they should only be validated by Kristian's CA. I've reported that upstream as well.

lukateras
lukateras approved these changes Jun 30, 2019
View reviewed changes
Copy link
Member

@lukateras lukateras left a comment •
edited

Choose a reason for hiding this comment

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

Looks good to me, except that this needs to include https://dev.gnupg.org/rG1c9cc97e9d47d73763810dcb4a36b6cdf31a2254 for the reasons @dkg has stated above.

@alyssais
gnupg: apply default server CA verification patch
ba23c14 
See discussion at
NixOS#63952 (comment).

Upstream commit:

commit 1c9cc97e9d47d73763810dcb4a36b6cdf31a2254
Author: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
Date:   Sun Jun 30 11:54:35 2019 -0400

    dirmngr: Only use SKS pool CA for SKS pool

    * dirmngr/http.c (http_session_new): when checking whether the
    keyserver is the HKPS pool, check specifically against the pool name,
    as ./configure might have been used to select a different default
    keyserver.  It makes no sense to apply Kristian's certificate
    authority to anything other than the literal host
    hkps.pool.sks-keyservers.net.

    Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
    GnuPG-Bug-Id: 4593
@alyssais
Copy link
Member Author

alyssais commented Jun 30, 2019 via email

@alyssais alyssais marked this pull request as ready for review June 30, 2019 21:09
@alyssais
Copy link
Member Author

Got the go ahead from the Hagrid (keys.openpgp.org) people, so I’m going to go ahead and merge this.

@alyssais alyssais merged commit fd593be into NixOS:master Jun 30, 2019
@alyssais alyssais deleted the gnupg-sks branch June 30, 2019 21:10
alyssais added a commit to alyssais/nixpkgs that referenced this pull request Jun 30, 2019
@alyssais
gnupg: apply default server CA verification patch
9abbb2e 
See discussion at
NixOS#63952 (comment).

Upstream commit:

commit 1c9cc97e9d47d73763810dcb4a36b6cdf31a2254
Author: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
Date:   Sun Jun 30 11:54:35 2019 -0400

    dirmngr: Only use SKS pool CA for SKS pool

    * dirmngr/http.c (http_session_new): when checking whether the
    keyserver is the HKPS pool, check specifically against the pool name,
    as ./configure might have been used to select a different default
    keyserver.  It makes no sense to apply Kristian's certificate
    authority to anything other than the literal host
    hkps.pool.sks-keyservers.net.

    Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
    GnuPG-Bug-Id: 4593

(cherry picked from commit ba23c14)
@alyssais alyssais mentioned this pull request Jun 30, 2019
Closed
10 tasks
@lukateras
Copy link
Member

Thanks!

@dylex dylex mentioned this pull request Jul 3, 2019
Closed
@Valodim Valodim mentioned this pull request Feb 27, 2020
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lukateras lukateras lukateras approved these changes

@grahamc grahamc Awaiting requested review from grahamc

@fpletz fpletz Awaiting requested review from fpletz

@vrthra vrthra Awaiting requested review from vrthra

@peti peti Awaiting requested review from peti

Assignees
No one assigned
Labels
1.severity: security 2.status: wait-for-upstream Waiting for upstream fix (or their other action). 9.needs: port to stable A PR needs a backport to the stable release. 10.rebuild-darwin: 11-100 10.rebuild-linux: 501-1000 10.rebuild-linux: 501+
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@alyssais @dkg @lukateras