add-apt-repository downloads gpg key in an insecure fashion

[Marc Deslauriers]

add-apt-repository can add PPAs and automatically import the PPA gpg key.

Unfortunately, it uses apt-key, which in turn uses gpg to download the key from a keyserver.

gpg downloads keys from keyservers using the short key id, which is trivial to collide.

It is therefore possible to either MITM the point where gpg downloads the key from the keyserver, or to simply upload a second colliding key to the keyserver. This can result in being able to MITM packages installed from PPAs.

[Marc Deslauriers (mdeslaur)]

Upstream gpg 1.4.12 now uses the full key length when fetching remote keys:

http://git.gnupg.org/cgi-bin/gitweb.cgi?p=gnupg.git;a=commit;h=6fe25e5602fabe92c68e5ba30e4777221e8612df

[Marc Deslauriers (mdeslaur)]

gnupg 2.0.19 has the equivalent fix also:

http://git.gnupg.org/cgi-bin/gitweb.cgi?p=gnupg.git;a=commit;h=3005b0a6f43e53bed2f9b6fba7ad1205bdb29bc5

[Marc Deslauriers (mdeslaur)]

http://www.ubuntu.com/usn/usn-1570-1/

[dkg (dkg0)]

I don't think this bug is fixed. it looks to me like the keyserver operator (or anyone who can MITM the keyserver) can still inject arbitrary keys here.

/usr/share/pyshared/softwareproperties/ppa.py appears to run "apt-key adv --keyserver $whatever --recv $fingerprint"

and "apt-key adv" is just shelling out to gpg.

if your keyserver happens to return the wrong thing (whether by malice or by accident), it will still just get imported.

If you'd like to try, consider using the (absurdly low-fi) fake "keyserver" hkp://dkg.fifthhorseman.net:80/, which will always return my key, regardless of what keyid (or fingerprint) you request from it.

  add-apt-repository --keyserver hkp://dkg.fifthhorseman.net:80/ ppa:kernel-ppa/ppa

[Marc Deslauriers (mdeslaur)]

Ah yes, apt-key still needs a fix to validate the downloaded key. Reopening.

I wish we could actually validate the web of trust on PPA keys instead of solely relying on key ids though.

[Zooko Wilcox-O'Hearn (zooko)]

By the way, this patch replaced a regexp with json.loads(): http://bazaar.launchpad.net/~ubuntu-branches/ubuntu/quantal/software-properties/quantal/revision/77

That was good, because a regexp could have been vulnerable to some sort of xss/injection sort of problem. But, the patch mistakenly left the following no-longer-true comment in place:

    # we ask for a JSON structure from lp_page, we could use
    # simplejson, but the format is simple enough for the regexp

That comment should be removed.

[dkg (dkg0)]

zooko, i'm pretty sure you want your comment 6 (above) to follow up on https://launchpad.net/bugs/815480 , not on this bug report.

[Zooko Wilcox-O'Hearn (zooko)]

dkg0: you're right, thanks.

[William Grant (wgrant)]

We can't really use the traditional web of trust. I trust some Launchpad PPAs, but not others, so I can't just trust any key that Launchpad has signed. The only thing add-apt-repository could do is use a local key to automatically sign when someone requests that a new repository be added, which isn't much less messy than what we have today.

[Michael Vogt (mvo)]

Looking into this a bit I think we have various options:
- switch to hpks by default in apt-key for the keyserver requests in apt-key and refuse to do hpk
- change gnupg to reject if a downloaded key is of a different keyid than the requested key [1]
- add code to apt-key to check/fixup the commandline in adv and download the keys to a tempkeyring and check that before further importing
- fix softwareproperties/ppa.py only and download there using python-hpk or a custom implementation

Feedback welcome.

[1] This would be my preferred fix, it would involve adding a new "expected_keys" parameter to import_keys_stream() in g10/keyserver.c or a new "validate_expected_keys()" call or something.

[Daniel Leidert (dleidert-deactivatedaccount)]

Using the gnupg maintainer hat: Please discuss your proposed changes to gnupg(2) with its upstream.

[Michael Vogt (mvo)]

For the workaround in apt-key I did a proof of concept here: lp:~mvo/apt/apt-key-recv-lp1016643 that shows what I have in mind. It has the added benefit that it will no longer support short keyids for --recv.

The basic idea is that if adv with --recv{,-keys} is given it wil intercept and download to a tmp keyring first, then verify that (probably unneeded though) and then export the keyid from the tmpkeyring and then importing it into the real keyring. The export via keyid step should ensure that the right keyid is used.

[Michael Vogt (mvo)]

@Daniel Leidert: Thanks for this suggestion, I added issue #1444 (https://bugs.g10code.com/gnupg/msg4421) upstream asking for a mode for --recv-key that verifies that the downloaded key matches the expected key.

[Michael Vogt (mvo)]

I added a branch for software-propoerties and its ppa.py helper at lp:~mvo/software-properties/recv-key-lp1016643

[Michael Vogt (mvo)]

I looked into g10/keyserver.c a bit and had hoped that import_keys_stream() there could be used to test if the fingerprints match the expected fingerprints, but it seems that the fpr/fpr_len arguments will only set to the fingerprint of the last key imported. So I need to check something else, maybe reading the spawn->fromchild stream into a tmp IOBUF, verify that and pass it on afterwards.

[dkg (dkg0)]

I'm glad to see you rejecting the short keyid.

If you're doing this work to make the apt-key fetching possibilities cryptographically sound, please rely only on full OpenPGPv4 fingerprints, not on the long keyid. And ensure that the received key is an OpenPGP v4 key, since v3 fingerprints are themselves spoofable.

Thanks!

[Michael Vogt (mvo)]

I pushed branches based on the lp:~mvo/software-properties/recv-key-lp1016643-precise for lucid,natty,oneiric now too and updated the branch in quantal to have better tests.

[Launchpad Janitor (janitor)]

This bug was fixed in the package software-properties - 0.82.7.3

---------------
software-properties (0.82.7.3) precise-security; urgency=low

  * SECURITY UPDATE: improve gpg key validation to prevent MITM attack
    (LP: #1016643)
    - softwareproperties/ppa.py: download gpg key to temporary keyring, and
      validate using v4 fingerprint before importing to apt keyring.
 -- Marc Deslauriers <email address hidden> Fri, 28 Sep 2012 09:17:57 -0400

[Launchpad Janitor (janitor)]

This bug was fixed in the package software-properties - 0.75.10.3

---------------
software-properties (0.75.10.3) lucid-security; urgency=low

  * SECURITY UPDATE: improve gpg key validation to prevent MITM attack
    (LP: #1016643)
    - softwareproperties/ppa.py: download gpg key to temporary keyring, and
      validate using v4 fingerprint before importing to apt keyring.
 -- Marc Deslauriers <email address hidden> Fri, 28 Sep 2012 09:26:15 -0400

[Launchpad Janitor (janitor)]

This bug was fixed in the package software-properties - 0.80.9.2

---------------
software-properties (0.80.9.2) natty-security; urgency=low

  * SECURITY UPDATE: improve gpg key validation to prevent MITM attack
    (LP: #1016643)
    - softwareproperties/ppa.py: download gpg key to temporary keyring, and
      validate using v4 fingerprint before importing to apt keyring.
 -- Marc Deslauriers <email address hidden> Fri, 28 Sep 2012 09:25:17 -0400

[Launchpad Janitor (janitor)]

This bug was fixed in the package software-properties - 0.81.13.5

---------------
software-properties (0.81.13.5) oneiric-security; urgency=low

  * SECURITY UPDATE: improve gpg key validation to prevent MITM attack
    (LP: #1016643)
    - softwareproperties/ppa.py: download gpg key to temporary keyring, and
      validate using v4 fingerprint before importing to apt keyring.
 -- Marc Deslauriers <email address hidden> Fri, 28 Sep 2012 09:24:18 -0400

[Launchpad Janitor (janitor)]

This bug was fixed in the package software-properties - 0.92.8

---------------
software-properties (0.92.8) quantal; urgency=low

  * lp:~mvo/software-properties/recv-key-lp1016643:
    - ensure fingerprint check after recv-key (LP: #1016643)
 -- Michael Vogt <email address hidden> Mon, 01 Oct 2012 19:33:35 +0200

[Steve Langasek (vorlon)]

Michael, given that software-properties has been SRUed, is there actually anything further that needs changed in apt?

[Jamie Strandboge (jdstrand)]

Thank you for reporting this bug to Ubuntu. natty has reached EOL
(End of Life) and is no longer supported. As a result, this bug
against natty is being marked "Won't Fix". Please see
https://wiki.ubuntu.com/Releases for currently supported Ubuntu
releases.

Please feel free to report any other bugs you may find.

[Jamie Strandboge (jdstrand)]

Thank you for reporting this bug to Ubuntu. hardy has reached EOL
(End of Life) and is no longer supported. As a result, this bug
against hardy is being marked "Won't Fix". Please see
https://wiki.ubuntu.com/Releases for currently supported Ubuntu
releases.

Please feel free to report any other bugs you may find.

[Rolf Leggewie (r0lf)]

lucid has seen the end of its life and is no longer receiving any updates. Marking the lucid task for this ticket as "Won't Fix".

[Mathew Hodson (mathew-hodson)]

The code for add-apt-repository is in ppa.py from the software-properties package not the apt package.