apt-add-repository does not perform ssl verification where it *needs* to

Bug #915210 reported by David
262
This bug affects 1 person
Affects Status Importance Assigned to Milestone
software-properties (Ubuntu)
Fix Released
High
Marc Deslauriers
Lucid
Fix Released
High
Marc Deslauriers
Maverick
Fix Released
High
Marc Deslauriers
Natty
Fix Released
High
Marc Deslauriers
Oneiric
Fix Released
High
Marc Deslauriers
Precise
Fix Released
High
Marc Deslauriers

Bug Description

The python code in apt-add-repository makes use of the softwareproperties module, in particular the ppa.py file.
In the ppa.py file there is the following comment:

" The signing key fingerprint is obtained from the Launchpad PPA page,
        via a secure channel, so it can be trusted.
"
However, the code in ppa.py simply uses the urllib2 module which as per the warning in the documentation ("HTTPS requests do not do any verification of the server’s certificate") does not do any verification of the server’s certificate.
As the data returned through the urllib2 call is trusted and used to configure and add new PPA's (software repository) to a system it maybe possible for an attacker (who can perform a man in the middle attack) to compromise a remote system through this means.

I tested and confirmed the bug on my local system with following relevant packages installed:
ii python-software-properties 0.81.13.1 manage the repositories that you install software from
ii software-center 5.0.3.1 Utility for browsing, installing, and removing software
ii software-properties-common 0.81.13.1 manage the repositories that you install software from (common)
ii software-properties-gtk 0.81.13.1 manage the repositories that you install software from (gtk)

Revision history for this message
Marc Deslauriers (mdeslaur) wrote :

Hi Michael, could you please take a quick look? Thanks!

Changed in software-properties (Ubuntu):
status: New → Confirmed
importance: Undecided → High
assignee: nobody → Marc Deslauriers (mdeslaur)
Revision history for this message
Marc Deslauriers (mdeslaur) wrote :

This is CVE-2011-4407

Changed in software-properties (Ubuntu Lucid):
status: New → Confirmed
Changed in software-properties (Ubuntu Maverick):
status: New → Confirmed
Changed in software-properties (Ubuntu Natty):
status: New → Confirmed
Changed in software-properties (Ubuntu Oneiric):
status: New → Confirmed
Changed in software-properties (Ubuntu Lucid):
importance: Undecided → High
Changed in software-properties (Ubuntu Maverick):
importance: Undecided → High
Changed in software-properties (Ubuntu Natty):
importance: Undecided → High
Changed in software-properties (Ubuntu Oneiric):
importance: Undecided → High
assignee: nobody → Marc Deslauriers (mdeslaur)
Changed in software-properties (Ubuntu Natty):
assignee: nobody → Marc Deslauriers (mdeslaur)
Changed in software-properties (Ubuntu Maverick):
assignee: nobody → Marc Deslauriers (mdeslaur)
Changed in software-properties (Ubuntu Lucid):
assignee: nobody → Marc Deslauriers (mdeslaur)
Revision history for this message
Marc Deslauriers (mdeslaur) wrote :

Attached is a proposed patch that switches to using pycurl instead of urllib2.

Michael, could you please review it?

Revision history for this message
David (d--) wrote :

Is pycurl a dependency of apt-add-repository?

Revision history for this message
David (d--) wrote :

On a side note perhaps it would be wise to use a custom CA_PATH which only has the _current_ CA('s) for launchpad.net domains.

Revision history for this message
Marc Deslauriers (mdeslaur) wrote :

pycurl isn't a current dependency, but will get added as one with the update.

I don't think trying to limit the CAs is a good idea, as it will complicate things if we need to change CAs in a hurry.
If someone doesn't trust the default CA list, they can always manually check the fingerprint, or manually add the repository.

Revision history for this message
David (d--) wrote :

Fair enough.

Revision history for this message
Michael Vogt (mvo) wrote :

Thanks to both of you! The patch looks good!

I added a small test to the code its not needed for the security update but that I want to include into the precise
upload to ensure there is no regression on this.

Revision history for this message
Marc Deslauriers (mdeslaur) wrote :

I've sent the patch to Debian and proposed a CRD of 2012-01-31.

Revision history for this message
Launchpad Janitor (janitor) wrote :

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

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

  * SECURITY UPDATE: incorrect ssl certificate validation (LP: #915210)
    - softwareproperties/ppa.py: use pycurl to download the signing key
      fingerprint.
    - tests/test_lp.py: add test.
    - debian/control: add python-pycurl dependency.
    - CVE-2011-4407
 -- Marc Deslauriers <email address hidden> Thu, 26 Jan 2012 10:51:26 -0500

Revision history for this message
Launchpad Janitor (janitor) wrote :

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

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

  * SECURITY UPDATE: incorrect ssl certificate validation (LP: #915210)
    - softwareproperties/ppa.py: use pycurl to download the signing key
      fingerprint.
    - debian/control: add python-pycurl dependency.
    - CVE-2011-4407
 -- Marc Deslauriers <email address hidden> Thu, 26 Jan 2012 10:59:59 -0500

Revision history for this message
Launchpad Janitor (janitor) wrote :

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

---------------
software-properties (0.76.7.1) maverick-security; urgency=low

  * SECURITY UPDATE: incorrect ssl certificate validation (LP: #915210)
    - softwareproperties/ppa.py: use pycurl to download the signing key
      fingerprint.
    - debian/control: add python-pycurl dependency.
    - CVE-2011-4407
 -- Marc Deslauriers <email address hidden> Thu, 26 Jan 2012 11:18:52 -0500

Revision history for this message
Launchpad Janitor (janitor) wrote :

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

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

  * SECURITY UPDATE: incorrect ssl certificate validation (LP: #915210)
    - softwareproperties/ppa.py: use pycurl to download the signing key
      fingerprint.
    - debian/control: add python-pycurl dependency.
    - CVE-2011-4407
 -- Marc Deslauriers <email address hidden> Thu, 26 Jan 2012 11:20:28 -0500

Changed in software-properties (Ubuntu Lucid):
status: Confirmed → Fix Released
Changed in software-properties (Ubuntu Maverick):
status: Confirmed → Fix Released
Changed in software-properties (Ubuntu Natty):
status: Confirmed → Fix Released
Changed in software-properties (Ubuntu Oneiric):
status: Confirmed → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :

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

---------------
software-properties (0.82.3) precise; urgency=low

  [ Brian Murray ]
  * when adding new repositories use sourceslist.add instead of append thereby
    preventing duplicate entires. Thanks to Nick Russo for the patch.
    LP: #854841

  [ Robert Roth ]
  * Updated expand properties to properly expand the bottom component to avoid
    putting empty space between components. LP: #912557
  * Added symbolic link for add-apt-repository manpage under the
    apt-add-repository name, LP: #620098
  * Changed Revert button mnemonic to avoid collision with Remove, LP: #652523
  * Handle URLError from ppa pages, instruct the user to check the internet
    connection (LP: #502698)

  [ Marc Deslauriers ]
  * SECURITY UPDATE: incorrect ssl certificate validation (LP: #915210)
    - softwareproperties/ppa.py: use pycurl to download the signing key
      fingerprint.
    - tests/test_lp.py: add test.
    - debian/control: add python-pycurl dependency.
    - CVE-2011-4407
  * Wait for PPA GPG key to get imported before ending thread (LP: #888417)
 -- Marc Deslauriers <email address hidden> Fri, 03 Feb 2012 08:16:22 -0500

Changed in software-properties (Ubuntu Precise):
status: Confirmed → Fix Released
David (d--)
visibility: private → public
To post a comment you must log in.
This report contains Public Security information  
Everyone can see this security related information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.