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.
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
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.
(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.
Great. Do you plan to work up a patch for this, or should I give it a stab?
This should fix it: http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=b5969af9f575e4e4b669f44e76ad01f0dbc2dd27
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.
(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.
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?
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.
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?
(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 has been assigned CVE-2013-2100.
@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.
(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.
(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.
(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.
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.
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-----
Wget fails if built with GnuTLS instead of OpenSSL. Still investigating why, as gnutls-cli works fine and returns a sha1WithRSAEncryption certificate.
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.
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.
The fixed version has been stabilized. If it is ok, please go ahead with the glsa.
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
Created attachment 362613 [details, diff] updated patch corrects comments and uses more secure tmpfile methods
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.
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.
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.
New GLSA request filed
This issue was resolved and addressed in GLSA 201507-16 at https://security.gentoo.org/glsa/201507-16 by GLSA coordinator Mikle Kolyada (Zlogene).
