Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 469888 (CVE-2013-2100) - <sys-apps/portage-2.1.12.2 : urlopen() does not verify SSL certificates (CVE-2013-2100)
Summary: <sys-apps/portage-2.1.12.2 : urlopen() does not verify SSL certificates (CVE-...
Status: RESOLVED FIXED
Alias: CVE-2013-2100
Product: Gentoo Security
Classification: Unclassified
Component: Vulnerabilities (show other bugs)
Hardware: All Linux
: Normal major (vote)
Assignee: Portage team
URL:
Whiteboard: A2 [glsa cve]
Keywords:
Depends on: 470054 472540 482870
Blocks:
  Show dependency tree
 
Reported: 2013-05-14 21:56 UTC by Jason A. Donenfeld
Modified: 2015-07-10 13:14 UTC (History)
5 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments
fix to reimplement caching for https (portage_https_fetchhack_caching_fix.patch,1.93 KB, patch)
2013-11-05 09:26 UTC, Blu3
no flags Details | Diff
updated patch corrects comments and uses more secure tmpfile methods (portage_https_fetchhack_caching_fix.patch,2.18 KB, patch)
2013-11-05 10:04 UTC, Blu3
no flags Details | Diff
handle newly set PORTAGE_BINHOST environments (portage_https_fetchhack_caching_fix.patch,2.39 KB, patch)
2013-11-13 02:41 UTC, Blu3
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jason A. Donenfeld archtester Gentoo Infrastructure gentoo-dev Security 2013-05-14 21:56:08 UTC
Various functions call urlopen() in pym/portage/util/_urlopen.py, including the code which fetches the binary package list from a specified binhost.

urlopen() does not include any logic for validating ssl certificates. 


def urlopen(url, if_modified_since=None):
...
                request = urllib_request.Request(url)
...
                opener = urllib_request.build_opener(auth_handler)
...
                hdl = opener.open(request)
...
                return hdl



SSL certificates must be validated. Otherwise a man in the middle may tamper with data which is then used by portage.
Comment 1 Zac Medico gentoo-dev 2013-05-14 22:53:10 UTC
There are some code examples here that may be useful:

http://stackoverflow.com/questions/1087227/validate-ssl-certificates-with-python
https://gist.github.com/schlamar/2993700
Comment 2 Jason A. Donenfeld archtester Gentoo Infrastructure gentoo-dev Security 2013-05-14 23:49:14 UTC
An intermediate solution to solve the binary package issue might be to use FETCHCOMMAND in bintree.py. Right now the code reads:

def _populate(self, getbinpkgs=0):
...
    try:
...
        f = _urlopen(url, if_modified_since=local_timestamp)
...
    except IOError as err:
...
        if parsed_url.scheme == 'sftp':
...
        elif parsed_url.scheme == 'ssh':
...
        else:
            setting = 'FETCHCOMMAND_' + parsed_url.scheme.upper()
            fcmd = self.settings.get(setting)
            if not fcmd:
                raise
            fd, tmp_filename = tempfile.mkstemp()
            tmp_dirname, tmp_basename = os.path.split(tmp_filename)
            os.close(fd)
            success = portage.getbinpkg.file_get(url,
                tmp_dirname, fcmd=fcmd, filename=tmp_basename)
            if not success:
                raise EnvironmentError("%s failed" % (setting,))
            f = open(tmp_filename, 'rb')


FETCHCOMMAND has the nice property that by default it uses wget, which does things right, and furthermore, it can be overridden by the user in make.conf for matching against custom certificate authorities. 

As seen in the code above, this function already makes use of FETCHCOMMAND_*, which is different from vanilla FETCHCOMMAND, but is essentially the same logic. 

The proposed fix here would be to first try using FETCHCOMMAND, instead of urlopen, before falling back to the various alternatives.
Comment 3 Zac Medico gentoo-dev 2013-05-14 23:55:59 UTC
(In reply to comment #2)
> An intermediate solution to solve the binary package issue might be to use
> FETCHCOMMAND in bintree.py.

Yeah, that sounds reasonable, at least for https protocol. For plain http, it would be nice to keep the existing urlopen code, since it's optimized check for modifications with the If-Modified-Since header.
Comment 4 Jason A. Donenfeld archtester Gentoo Infrastructure gentoo-dev Security 2013-05-15 02:42:06 UTC
Great. Do you plan to work up a patch for this, or should I give it a stab?
Comment 6 Jason A. Donenfeld archtester Gentoo Infrastructure gentoo-dev Security 2013-05-15 11:10:05 UTC
A CVE has been requested for the binhost bug: http://seclists.org/oss-sec/2013/q2/332 .


We still need to audit all additional uses of urlopen() to see if a similar mistake is repeated elsewhere.
Comment 7 Agostino Sarubbo gentoo-dev 2013-05-15 12:41:14 UTC
(In reply to comment #5)
> This should fix it:
> 
> http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;
> h=b5969af9f575e4e4b669f44e76ad01f0dbc2dd27

@Zac, you need to release a new version and we need to stabilize it.
Comment 8 Jason A. Donenfeld archtester Gentoo Infrastructure gentoo-dev Security 2013-05-15 18:03:11 UTC
Let's go one by one.

zx2c4@thinkpad ~/portage-HEAD-5425900 $ grep -R -l urlopen
bin/repoman
pym/portage/glsa.py
pym/portage/util/_urlopen.py
pym/portage/dbapi/bintree.py

* _urlopen.py: N/A
* bintree.py: fixed by b5969af9
* glsa.py: only uses file:// -- not vulnerable


* repoman: metadata_dtd_uri hard-coded as http:// -- this means it's not relevent for this SSL validation issue, but, more generally, is this a bad thing? Apparently the fetched file is passed to xmllint. Will xmllint execute code in DTDs? Will it resolve external entities? I really have no idea. More generally, in assessing this, we essentially have to consider metadata.dtd to be untrusted input. Assuming this, are we safe? If not, perhaps we should be using https and checking certificates.

Anybody better versed care to weigh in on this?
Comment 9 Zac Medico gentoo-dev 2013-05-15 22:33:31 UTC
I've fixed repoman to use FETCHCOMMAND:

http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=155b31eb91da4e6007a3a72c8027d66caf9fadb8

If infra enables https for www.gentoo.org/dtd/metadata.dtd, then we can easily update repoman to use https.
Comment 10 Jason A. Donenfeld archtester Gentoo Infrastructure gentoo-dev Security 2013-05-15 23:15:20 UTC
Adding infra to this bug report.

It might be wise to enable SSL for gentoo.org for metadata.dtd. What is involved to make this happen? What's the deal with certificates?
Comment 11 Alec Warner archtester Gentoo Infrastructure gentoo-dev Security 2013-05-16 02:27:50 UTC
(In reply to comment #10)
> Adding infra to this bug report.
> 
> It might be wise to enable SSL for gentoo.org for metadata.dtd. What is
> involved to make this happen? What's the deal with certificates?

Please file a separate bug.

I don't believe https is enabled for anything on www.gentoo.org.

We have not yet purchased certificates, so we would rely on cacert to generate certs for www (or use the wildcard cert.)

-A
Comment 12 Jason A. Donenfeld archtester Gentoo Infrastructure gentoo-dev Security 2013-05-16 10:30:03 UTC
This has been assigned CVE-2013-2100.
Comment 13 Jason A. Donenfeld archtester Gentoo Infrastructure gentoo-dev Security 2013-05-16 11:22:03 UTC
@Zac: for b5969af9, it might be nice to suppress the wget output somehow. It's totally up to you, but IMHO, it's a bit ugly. Same functionality either way though.
Comment 14 Zac Medico gentoo-dev 2013-05-16 13:48:52 UTC
(In reply to comment #13)
> @Zac: for b5969af9, it might be nice to suppress the wget output somehow.
> It's totally up to you, but IMHO, it's a bit ugly. Same functionality either
> way though.

Well, maybe it's ugly, but it could also provide some useful feedback to the user (especially if it times out while connecting to the server).

(In reply to comment #7)
> @Zac, you need to release a new version and we need to stabilize it.

It's release in portage-2.1.12.
Comment 15 Jason A. Donenfeld archtester Gentoo Infrastructure gentoo-dev Security 2013-05-16 17:31:46 UTC
(In reply to comment #14)
> Well, maybe it's ugly, but it could also provide some useful feedback to the
> user (especially if it times out while connecting to the server).

Yea, this is a legitimate reason. Though, to be fair, the urlopen() http:// handler doesn't show any information.
Comment 16 Alex Xu (Hello71) 2013-08-29 01:58:17 UTC
(In reply to Alec Warner from comment #11)
> (In reply to comment #10)
> > Adding infra to this bug report.
> > 
> > It might be wise to enable SSL for gentoo.org for metadata.dtd. What is
> > involved to make this happen? What's the deal with certificates?
> 
> Please file a separate bug.
> 
> I don't believe https is enabled for anything on www.gentoo.org.
> 
> We have not yet purchased certificates, so we would rely on cacert to
> generate certs for www (or use the wildcard cert.)
> 
> -A

This appears to have been done, but with CACert certificates. Conveniently, app-misc/ca-certificates installs the CACert roots.
Comment 17 Zac Medico gentoo-dev 2013-08-29 04:51:02 UTC
If we change repoman to fetch metadata.dtd via https, then we need to provide some instruction detailed instructions for how to configure it. For example, here's what happens with my current configuration:

$ wget https://www.gentoo.org/dtd/metadata.dtd
--2013-08-28 21:46:35--  https://www.gentoo.org/dtd/metadata.dtd
Resolving www.gentoo.org (www.gentoo.org)... 89.16.167.134, 2001:41c8:0:936::139, 2001:41c8:0:936::136
Connecting to www.gentoo.org (www.gentoo.org)|89.16.167.134|:443... connected.
ERROR: The certificate of ‘www.gentoo.org’ is not trusted.
Comment 18 Robin Johnson archtester Gentoo Infrastructure gentoo-dev Security 2013-08-29 06:03:34 UTC
That host is using DigiCert, not CA-Cert.

$ wget https://www.gentoo.org/dtd/metadata.dtd
--2013-08-28 23:01:50--  https://www.gentoo.org/dtd/metadata.dtd
Resolving www.gentoo.org... 89.16.167.134, 2001:41c8:0:936::136, 2001:41c8:0:936::139
Connecting to www.gentoo.org|89.16.167.134|:443... connected.
HTTP request sent, awaiting response... 200 OK
Length: 4720 (4.6K) [application/xml-dtd]
Saving to: ‘metadata.dtd’
...

robbat2@bohr-int:~ $ openssl s_client -connect www.gentoo.org:443 -showcerts -servername www.gentoo.org
CONNECTED(00000003)
depth=2 C = US, O = DigiCert Inc, OU = www.digicert.com, CN = DigiCert High Assurance EV Root CA
verify error:num=20:unable to get local issuer certificate
verify return:0
---
Certificate chain
 0 s:/C=US/ST=New Mexico/L=Albuquerque/O=GENTOO Foundation, Inc./CN=*.gentoo.org
   i:/C=US/O=DigiCert Inc/OU=www.digicert.com/CN=DigiCert High Assurance CA-3
-----BEGIN CERTIFICATE-----
Comment 19 Alex Xu (Hello71) 2013-08-29 15:13:10 UTC
Wget fails if built with GnuTLS instead of OpenSSL.

Still investigating why, as gnutls-cli works fine and returns a sha1WithRSAEncryption certificate.
Comment 20 Alex Xu (Hello71) 2013-08-29 16:13:46 UTC
Fails with gnutls-2.12.23-r1 (stable on most arches) but works with gnutls-3.2.3 (~ on most arches).

Why? No idea. Maybe nettle is related.
Comment 21 Zac Medico gentoo-dev 2013-08-29 16:52:49 UTC
Maybe we should just file a stable request for net-libs/gnutls-3.2.3, since it's been in the tree for 30 days now.
Comment 22 Agostino Sarubbo gentoo-dev 2013-11-04 09:55:26 UTC
The fixed version has been stabilized. If it is ok, please go ahead with the glsa.
Comment 23 Blu3 2013-11-05 09:26:38 UTC
Created attachment 362609 [details, diff]
fix to reimplement caching for https

attached is a method of utilizing wget with -N option so local cache checking can be done. currently cache checking is voided with https due to the use of wget with redirection (-O option negates -N)

the attached patch creates a temporary directory instead of a temp file and copies the pkgindex file to it preserving timestamps. chdir is used so wget infers the unqualified basename, e.g. "Packages", to do the timestamp checks.

'-O "${DISTDIR}/${FILE}"' in fcmd is replaced with '-N' to undo the redirect notion within wget.

after fetching (or not), and timestamps check, tmp_filename is set to tmp_dirname so the later cleanup with os.unlink() will remove both the temp directory and file

p.s. regarding the original implementation, there's still a race condition if a tmpfile is created and the opened file descriptor is discarded. malware watching for the tmpfile can quickly unlink it and link a target to the intended tmpfile for destruction or employ other DoS means. the use of a tmpfile is only secure if the returned file descriptor is used
Comment 24 Blu3 2013-11-05 10:04:51 UTC
Created attachment 362613 [details, diff]
updated patch corrects comments and uses more secure tmpfile methods
Comment 25 Blu3 2013-11-13 02:41:58 UTC
Created attachment 363172 [details, diff]
handle newly set PORTAGE_BINHOST environments

this version of the patch now cleanly handles the situation where the PORTAGE_BINHOST factory has not yet stored a Packages file.
Comment 26 GLSAMaker/CVETool Bot gentoo-dev 2014-10-11 00:38:49 UTC
CVE-2013-2100 (http://nvd.nist.gov/nvd.cfm?cvename=CVE-2013-2100):
  The urlopen function in pym/portage/util/_urlopen.py in Gentoo Portage
  2.1.12, when using HTTPS, does not verify X.509 certificates from SSL
  servers, which allows man-in-the-middle attackers to spoof servers and
  modify binary package lists via a crafted certificate.
Comment 27 Brian Dolbec gentoo-dev 2014-10-11 01:48:15 UTC
I don't know how much opposition I'll get, but the new ssl-fetch lib code I created from layman's code to do https verified fetching.  I was planning on possibly implementing it's use in portage in the near future.

It will be installed as a dep with the gentoo-keys project, layman and mirrorselect,  It wraps the urllib3 and dev-python/requests libs for the verified fetching from api.gentoo.org or any other url.

With gentoo-keys (soon to be released ;) and in operation, portage will get a module to make use of it for gpg verification of files too when signatures are available.

Repoman is currently undergoing a rewrite.  It would be a good time to introduce the ssl-fetch code there which can also easily be swapped out for a gkeys call which will use ssl-fetch to retrieve the file as well as a signature file if available and do the gpg verification.

We are currently working on the automation scripts for gkeys infra deployment.  So more files will be served via api.gentoo.org strictly via https and many with gpg signatures for further verification.
Comment 28 Kristian Fiskerstrand (RETIRED) gentoo-dev 2015-05-11 16:43:08 UTC
New GLSA request filed
Comment 29 GLSAMaker/CVETool Bot gentoo-dev 2015-07-10 13:14:33 UTC
This issue was resolved and addressed in
 GLSA 201507-16 at https://security.gentoo.org/glsa/201507-16
by GLSA coordinator Mikle Kolyada (Zlogene).