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.

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

Anyone can make any PGP key unimportable #57

Open
ygrek opened this issue Jun 13, 2018 · 9 comments
Open

Anyone can make any PGP key unimportable #57

ygrek opened this issue Jun 13, 2018 · 9 comments
Labels
blocker bug Something isn't working

Comments

@ygrek
Copy link
Member

ygrek commented Jun 13, 2018

Original report by Yegor Timoshenko (Bitbucket: yegortimoshenko, GitHub: yegortimoshenko).


See: https://pgp.mit.edu/pks/lookup?op=vindex&search=0x16E0CF8D6B0B9508

$ gpg --keyserver pgp.mit.edu --recv 0x16E0CF8D6B0B9508
gpg: packet(13) too large
gpg: read_block: read error: Invalid packet
gpg: no valid OpenPGP data found.
gpg: Total number processed: 0

You'll need bash, curl, go, gpg and make to reproduce this issue.

Download someone's PGP key off a keyserver:

curl https://pgp.key-server.io/download/0x31743FFC33E746C5 | gpg --dearmor > /tmp/key.gpg

Clone https://gitlab.com/yegortimoshenko/sks-tools repo an run make:

git clone https://gitlab.com/yegortimoshenko/sks-tools
cd sks-tools
make

Run sks-forge-uid with preferred keyserver (I tested with pgp.mit.edu) and -rand flag:

./sks-forge-uid pgp.mit.edu -rand < /tmp/key.gpg

Then try to download the key off that keyserver:

gpg --keyserver pgp.mit.edu --recv 0x31743FFC33E746C5

Technical explanation: UID packets that this script generates are too big which causes GPG to error out instead of importing the key. Possible solution would be to make sure only RFC-compliant, valid (see #41) packets are ever accepted by the keyserver.

@ygrek
Copy link
Member Author

ygrek commented Jun 13, 2018

Original comment by Daniel Kahn Gillmor (Bitbucket: dkgdkg, GitHub: dkgdkg).


I agree this is a problem, but it's one of several open DoS mechanisms supported by SKS.

i'm not sure the right way to resolve it other than replacing SKS with a minimalist update network (for example, as proposed in on the gnupg-users mailing list earlier this year).

At the very least, SKS could reject User ID packets that are not valid UTF-8. that would limit Yegor's specific attack (since the random bytestreams he's passing as User IDs are very unlikely to be valid UTF-8 sequences). but it doesn't address the bigger picture.

@ygrek
Copy link
Member Author

ygrek commented Jun 13, 2018

Original comment by Micah Lee (Bitbucket: micahflee, GitHub: micahflee).


One way to solve this is to validate input. When SKS receives a public key, it can make sure that all User ID and Revocation Certificate packets contain valid Signature packets. If those packet signatures don't verify, delete them from the public key. This will stop anyone from pushing large amounts of useless/malicious data onto anyone's public key except for their own.

(It will also have the added benefit of fixing the usability bug described in #41, going forward at least.)

@ygrek
Copy link
Member Author

ygrek commented Jun 14, 2018

Original comment by Daniel Kahn Gillmor (Bitbucket: dkgdkg, GitHub: dkgdkg).


Micah, I understand that you want to see OpenPGP keyservers do cryptographic verification, but i don't know how you expect SKS keyservers that do this verification to interact with the keyservers that do not do this verification. synchronizing keyservers with disjoint sets of key material is a non-trivial task.

I'd be happy to see work on an SKS replacement that (a) does cryptographic verification of first-party certifications, (b) ignores third-party certifications entirely, and (c) ignores user IDs, user attributes, and subkey packets that are not correctly signed by the primary key. but i don't know how that would sync with the current network. maybe it'll just replace it at some point?

As a side note, I've reported GnuPG's failure to skip this packet and deal with the rest of the certificate.

@ygrek
Copy link
Member Author

ygrek commented Jun 14, 2018

Original comment by Yegor Timoshenko (Bitbucket: yegortimoshenko, GitHub: yegortimoshenko).


I'm not sure that this is the right way to go. Yes, GnuPG import can be made resilient towards invalid input, but being able to append invalid packets that one would have to process to import someone's key is dangerous in and of itself. It's likely that there are other invalid packets that SKS will gladly accept, but will choke GnuPG. Other OpenPGP implementations (Go crypto/openpgp package, OpenPGP.js) can't even handle keys with invalid UID packets as seen in #41, let alone this case. The problem here is clearly that SKS accepts invalid input.

@ygrek
Copy link
Member Author

ygrek commented Jun 14, 2018

Original comment by Yegor Timoshenko (Bitbucket: yegortimoshenko, GitHub: yegortimoshenko).


[...] i don't know how you expect SKS keyservers that do this verification to interact with the keyservers that do not do this verification. synchronizing keyservers with disjoint sets of key material is a non-trivial task.

Can't keyservers that do verification just drop any invalid packets on sync?

@ygrek
Copy link
Member Author

ygrek commented Jun 14, 2018

Original comment by Micah Lee (Bitbucket: micahflee, GitHub: micahflee).


DKG, that's a great point about syncing with peers. I had assumed that the packets on public keys were accumulative: if someone pushes sig1 to a key on SKS1, and someone else pushes sig2 to the same key on SKS2, when they sync both will end up with both sigs in an undefined order. It seems like, in this case, you could just ignore some of the packets. But I don't actually know how it's implemented, so I can definitely see this being non-trivial.

Given this, I have an idea for solution that I think can solve this issue, the bug in #41, and also hopefully fix other issues related to SKS not doing input validation. It can do output validation instead.

We need to introduce a new concept called an output filter, and SKS can include three of them:

  • Full: output includes all packets (the status quo)
  • Useful: output only includes first-party verified packets, and also third-party certificates
  • Minimal: output only includes first-party verified packets

SKS would still store all packets associated with every key, and syncing with peers would not be affected at all. It's just that when users search for and fetch keys, either from the web interface or from software like gpg, the output depends on which filter the user chooses to use.

Users can choose which filter they'd like to use by prefixing the URI. For example, you could set your keyserver to any of these:

  • hkps://hkps.pool.sks-keyservers.net
  • hkps://hkps.pool.sks-keyservers.net/full
  • hkps://hkps.pool.sks-keyservers.net/useful
  • hkps://hkps.pool.sks-keyservers.net/minimal

If you leave off the filter, then it uses whatever the default filter that that SKS instance is configured to use. (It would be most useful if SKS instances configured the default to Useful, but they could also choose Full if they want to continue serving bad data.)

So, this URL would show all of the information associated with my key, including faked UIDs and revocation certificates:

https://sks-keyservers.net/full/pks/lookup?search=0x403C2657CD994F73&op=index

This URL would only show information associated with my key that users expect to see:

https://sks-keyservers.net/useful/pks/lookup?search=0x403C2657CD994F73&op=index

And this URL would only show information that I myself added to my key, excluding third-party certifications:

https://sks-keyservers.net/minimal/pks/lookup?search=0x403C2657CD994F73&op=index

This will solve the currently-broken usability issues that SKS suffers from, and I believe it will also solve most other issues related to the lack of input validation (such as the DoS described in this issue), so long as users choose to use the Useful or Minimal filters.

@ygrek
Copy link
Member Author

ygrek commented Jun 14, 2018

Original comment by Yegor Timoshenko (Bitbucket: yegortimoshenko, GitHub: yegortimoshenko).


This is not compatible with HKP spec (https://tools.ietf.org/html/draft-shaw-openpgp-hkp-00), maybe it will be easier to add &filter=useful attribute to /pks/lookup.

@ygrek
Copy link
Member Author

ygrek commented Jun 14, 2018

Original comment by Micah Lee (Bitbucket: micahflee, GitHub: micahflee).


The reason I was suggesting a prefix is so you can use it with software that interacts with keyservers, like gpg --keyserver [blah]. Can you configure gpg, enigmail, etc. to use &filter=useful?

Although, I don't think it's incompatible with the spec. It's just that each SKS instance would be hosting four separate keyservers at different URIs with the same domain: the default one, and one for each filter.

The spec says:

  1. Requesting Data From A Keyserver

Keyserver requests are done via a HTTP GET URL that encodes the
request within it. Specifically, the abs_path (see [2], section
3.2) is built up of the base request "/pks/lookup", followed by any
variables.

So if your keyserver is hkps://sks, then in this case abs_path will be hkps://sks/pks/lookup. But if your keyserver is hkps://sks/useful, then abs_path will be hkps://sks/useful/pks/lookup.

@ygrek
Copy link
Member Author

ygrek commented Oct 9, 2018

Original comment by PABLO GARCIA DE LOS SALMONES VALENCIA (Bitbucket: DevPGSV, GitHub: DevPGSV).


Is it possible for SKS servers to request the versión of other servers using recon?

If possible, perhaps this could be the behaviour of a v1.1.7 server when tring to sync with other servers:

  • Ask version
    • If version < 1.1.7: sync with 1.1.6 login
    • If version == 1.1.17: sync with output filter

If next version has input validation when submitting keys, and input/output validation when syncing with recon with other servers, the a SKS pool of 1.1.7 servers would not have this problem, and it would (should) dissapear as servers upgrade.

The difficult part would be syncing the 1.1.7 server with a 1.1.6 server without "contaminating" the 1.1.7 with invalid keys.
Perhaps when syncing with <1.1.7 servers, invalid packets should be saved in a different location (or tagged as invalid) and then deleted just as syncing with that server is done.

A different suggestion

Go to the next MAYOR version (something like 1.2.0 or 2.0.0), including the appropiate input/validations, and not allowing syncing between servers of different mayor versions.
A pool of 2.0.0 servers would avoid this problem.

There could be specific "bridge" servers syncing <2.0.0 servers with >=2.00 servers. These servers would sync like a 1.1.6, hourly (or daily) the database could be migrated to the new version while filtering out invalid keys, and then the new database would be offered to the >=2.0.0 servers.

This would be (I think) a way to implement the SKS replacement proposed by @DKGDKG allowing syncing keys between the new and old versions without usign mail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant