New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 1 user
Status: Verified
Owner:
Last visit > 30 days ago
Closed: Sep 2014
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug-Security



Sign in to add a comment
libcurl: Wildcard IP in cert's CN field can allow server spoof
Reported by cmasone@chromium.org, Aug 25 2014 Back to list
Advisory: CVE-2014-0139
  Details: http://vomit.googleplex.com/advisory?id=CVE/CVE-2014-0139
  CVSS severity score: 5.8/10.0
  Confidence: high
  Description:

cURL and libcurl 7.1 before 7.36.0, when using the OpenSSL, axtls, qsossl or gskit libraries for TLS, recognize a wildcard IP address in the subject's Common Name (CN) field of an X.509 certificate, which might allow man-in-the-middle attackers to spoof arbitrary SSL servers via a crafted certificate issued by a legitimate Certification Authority.

If this impacts update_engine, then I'm pretty concerned. If not, then I think medium is fine.

Sosa, do you know what extra checks we do on the HTTPS connection over which we fetch update payload signatures? or know who'd know? If you can find someone to answer this question and then reassign the bug to me, that'd be helpful.
 
Comment 1 by de...@chromium.org, Aug 25 2014
The HTTPS settings are done by the LibcurlHttpFetcher::SetCurlOptionsForHttps() (
https://chromium.googlesource.com/chromiumos/platform2/+/master/update_engine/libcurl_http_fetcher.cc line 196). I don't know how does that translate to vulnerable or not, but we could add a test (my guess from the CVE is that it is vulnerable to that). We use version 7.31 and +curl_ssl_openssl.
I don't think there are curl settings that'd protect, no, but I was wondering if there were any further checks we do on the cert more manually after the connection succeeds. Does anyone know?
Cc: zeuthen@chromium.org
Cc: cmasone@chromium.org
The way I read update_engine's usage of libcurl is that as long as the other end has a certificate issued by one of the few CAs that we trust (see the chromeos-ca-certificates ebuild), we'll accept it. IOW, we don't use certificate pinning (so what I said in  Issue 406546  and  Issue 406548  is not entirely true). So I agree with deymo@ and cmasone@ that we're vulnerable to this.

As for extra checks, I don't think we do anything on the connection.... but we do a bunch of checks on the payload and we even have autoupdate_CatchBadSignatures to check that we properly reject payloads not signed by Google.

Owner: de...@chromium.org
Deymo, Sosa said you could take this on? Ideally, we'd eventually backport this to all the branches if it's a pretty easy merge...
Comment 7 by de...@chromium.org, Aug 26 2014
Labels: Iteration-112
hmm... we are at curl-7.31.0, should we just bump the curl version or check that we don't get wildcard IP in the cert?
Bumping the version would be ideal.

It'd also be nice to verify that there are some other defenses that'd prevent us from applying a bad update even in the event that the HTTPS connection to Omaha is vulnerable to spoofing. This would be in case there's pushback against pushing the libcurl rev to beta/stable, we'd at least be making an informed decision about the impact.
Comment 9 by de...@chromium.org, Aug 27 2014
Status: Started
Will update to curl-7.36.0, which is upstream stable. Turns out we don't have any modifications for curl in chromiumos-overlay.
> It'd also be nice to verify that there are some other defenses that'd prevent us from applying a bad update even in the event that the HTTPS connection to Omaha is vulnerable to spoofing.

autoupdate_CatchBadSignatures should give you that assurance ... the autotest checks that update_engine rejects payload not signed by the right key. IOW, in addition to MITMing the HTTPS connection to Omaha, an attacker would also need to forge signatures so they look like they're signed by the private key owned by Google.
Project Member Comment 11 by bugdroid1@chromium.org, Aug 28 2014
Project: chromiumos/overlays/portage-stable
Branch : master
Author : Alex Deymo <deymo@chromium.org>
Commit : 105ed036a907f326d4d84ef81b4992fef86ae731

Code-Review  0 : Alex Deymo, chrome-internal-fetch
Code-Review  +2: Mike Frysinger
Commit-Queue 0 : Mike Frysinger, chrome-internal-fetch
Commit-Queue +1: Alex Deymo
Verified     0 : Mike Frysinger, chrome-internal-fetch
Verified     +1: Alex Deymo
Change-Id      : Ib15fae9497b572e14ea0e52f2a95f0ca1fd2976b
Reviewed-at    : https://chromium-review.googlesource.com/214420

curl: upgraded package to upstream

Upgraded net-misc/curl to version 7.36.0 on amd64, arm, x86

BUG= chromium:407235 
TEST=`FEATURES=test emerge-link curl` works.
TEST=cbuildbot x86-generic-full amd64-generic-full chromiumos-sdk

metadata/md5-cache/net-misc/curl-7.36.0
net-misc/curl/Manifest
net-misc/curl/curl-7.36.0.ebuild
net-misc/curl/files/curl-7.30.0-configure.patch
net-misc/curl/files/curl-7.30.0-prefix.patch
net-misc/curl/files/curl-7.35.0-tests.patch
net-misc/curl/files/curl-7.36.0-hostcheck.patch
net-misc/curl/files/curl-7.37.0-host-krb5-config.patch
net-misc/curl/files/curl-fix-gnutls-nettle.patch
net-misc/curl/files/curl-respect-cflags-3.patch
net-misc/curl/metadata.xml
Comment 12 by de...@chromium.org, Aug 28 2014
In production, the payload metadata signature sent by omaha decrypted with "/usr/share/update_engine/update-payload-key.pub.pem" should match the hash of the payload manifest. So yeah, even if you break HTTPS you still need to provide a metadata signature signed by Google. The code that does this is in DeltaPerformer::ValidateMetadataSignature() (delta_performer.cc).

For official builds we don't allow using other public key than the one in that file (in test images we can pass it in the devserver response).

Should we mark this as fixed?

Labels: -Security_Severity-High Security_Severity-Medium
I think we should backport this to beta and stable, because this vulnerability would allow an attacker to subvert time settings, which would allow expired CA certs to be re-used in browser sessions.

Downgrading to medium, though, as update isn't immediately effective.

So...whatever the backport process requires, let's get the labels set thusly and request merges.
 Issue 407234  has been merged into this issue.
Labels: -Iteration-112 Iteration-113
Project Member Comment 16 by bugdroid1@chromium.org, Sep 2 2014
Project: chromiumos/overlays/chromiumos-overlay
Branch : master
Author : Alex Deymo <deymo@chromium.org>
Commit : fbf2af862aabe4788e6e4f8640d159587c50f0b2

Code-Review  0 : Alex Deymo, chrome-internal-fetch
Code-Review  +2: Chris Masone, Mike Frysinger
Commit-Queue 0 : Chris Masone, Mike Frysinger, chrome-internal-fetch
Commit-Queue +1: Alex Deymo
Verified     0 : Chris Masone, Mike Frysinger, chrome-internal-fetch
Verified     +1: Alex Deymo
Change-Id      : I7a145aafcb56d54cdc5bb18b9e7a109d1fecb1a3
Reviewed-at    : https://chromium-review.googlesource.com/215413

net-misc/curl: Moved to portage-stable.

The latest version 7.36 is being installed from portage-stable.

BUG= chromium:407235 
TEST=emrege-link curl

net-misc/curl/Manifest
net-misc/curl/curl-7.31.0-r1.ebuild
net-misc/curl/curl-7.31.0.ebuild
net-misc/curl/files/0001-ossl_recv-check-for-an-OpenSSL-error-don-t-assume.patch
net-misc/curl/files/curl-7.18.2-prefix.patch
net-misc/curl/files/curl-7.19.7-test241.patch
net-misc/curl/files/curl-7.30.0-prefix.patch
net-misc/curl/files/curl-fix-gnutls-nettle.patch
net-misc/curl/files/curl-respect-cflags-3.patch
net-misc/curl/metadata.xml
Labels: M-38 Merge-Requested
Change in #11 landed last week and canaries are happy. Requesting merge.
Labels: -Merge-Requested Merge-Approved
Merge approved for M38.
Project Member Comment 19 by bugdroid1@chromium.org, Sep 5 2014
Project: chromiumos/overlays/portage-stable
Branch : release-R38-6158.B
Author : Alex Deymo <deymo@chromium.org>
Commit : f6609447a103973b13e95dc75b2469e194f41f5d

Code-Review  +2: Alex Deymo
Verified     +1: Alex Deymo
Commit Queue   : Chumped
Change-Id      : Ib15fae9497b572e14ea0e52f2a95f0ca1fd2976b
Reviewed-at    : https://chromium-review.googlesource.com/216477

curl: upgraded package to upstream

Upgraded net-misc/curl to version 7.36.0 on amd64, arm, x86

BUG= chromium:407235 
TEST=`FEATURES=test emerge-link curl` works.
TEST=cbuildbot x86-generic-full amd64-generic-full chromiumos-sdk

Previous-Reviewed-on: https://chromium-review.googlesource.com/214420
(cherry picked from commit 105ed036a907f326d4d84ef81b4992fef86ae731)

metadata/md5-cache/net-misc/curl-7.36.0
net-misc/curl/Manifest
net-misc/curl/curl-7.36.0.ebuild
net-misc/curl/files/curl-7.30.0-configure.patch
net-misc/curl/files/curl-7.30.0-prefix.patch
net-misc/curl/files/curl-7.35.0-tests.patch
net-misc/curl/files/curl-7.36.0-hostcheck.patch
net-misc/curl/files/curl-7.37.0-host-krb5-config.patch
net-misc/curl/files/curl-fix-gnutls-nettle.patch
net-misc/curl/files/curl-respect-cflags-3.patch
net-misc/curl/metadata.xml
Labels: -Merge-Approved Merge-Merged
I'll test a beta build before requesting M-37 merge
Labels: -M-38 -Merge-Merged M-37 Merge-Requested
Comment 22 by josa...@google.com, Sep 10 2014
Have you confirmed this change on Dev/Beta?
Comment 23 by de...@chromium.org, Sep 10 2014
I tested it on link-beta 6158.25.0.
The curl command and update_engine (uses libcurl) seem to work to at least talk to omaha.

Comment 24 by de...@chromium.org, Sep 16 2014
josafat, can we still merge this to stable?
Comment 25 by de...@chromium.org, Sep 16 2014
Labels: iteration-114
Labels: -M-37 -Merge-Requested M-38 Merge-Merged
Removing M37 since we are not planning another M37 Stable. 
Comment 27 by de...@chromium.org, Sep 24 2014
Status: Fixed
Then I'll just close this.
Labels: Release-0-M38
Project Member Comment 29 by ClusterFuzz, Dec 31 2014
Labels: -Restrict-View-SecurityTeam
Bulk update: removing view restriction from closed bugs.
Labels: VerifyIn-41
Status: Verified
Project Member Comment 32 by sheriffbot@chromium.org, Oct 1 2016
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member Comment 33 by sheriffbot@chromium.org, Oct 1 2016
Labels: Restrict-View-SecurityNotify
Project Member Comment 34 by sheriffbot@chromium.org, Oct 2 2016
Labels: -Restrict-View-SecurityNotify
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: allpublic
Sign in to add a comment