GNOME Bugzilla – Bug 693214
CVE-2013-0240: fails to verify SSL certificates when creating accounts
Last modified: 2013-03-04 12:14:39 UTC
I discovered this vulnerability and reported it privately. It was just made public on oss-security. > it was found that Gnome Online Accounts (GOA) > did not perform SSL certificate validation, when > performing Windows Live and Facebook accounts creation. > A remote attacker could use this flaw to conduct > man-in-the-middle (MiTM) attacks, possibly leading > to their ability to obtain sensitive information. This is fixed in master, but not in 3.6 or 3.4.
Created attachment 235236 [details] [review] [3.4.x] CVE-2013-0240: Do not allow invalid SSL certificates None of the branded providers (eg., Google, Facebook and Windows Live) should ever have an invalid certificate; and in this version of GOA, that's all we have. So set "ssl-strict" on the SoupSession object being used by GoaWebView. --- Proposed patch for 3.4.x, confirmed to prevent account creation for Facebook, Google and Windows Live accounts during a man-in-the-middle attack. Please review.
Created attachment 235237 [details] [review] [3.7.x] Fix compilation with -Werror=format-security --- This fixes a problem I encountered with the original patch for 3.7.x while testing it. I'm also somewhat concerned that it appears to be an API and ABI change: it makes an incompatible change (changing function parameters) to the GoaHttpClient API in libgoabackend. *Adding* API/ABI doesn't seem like a great thing to do in a security release either, if it can be avoided... If GoaHttpClient isn't public API then the API break is OK, but otherwise it seems to me as though this would be better done by making goa_http_client_check() and goa_http_client_check_sync() reject bad certificates, and adding a new goa_http_client_check_insecure() and goa_http_client_check_insecure_sync() if necessary?
Regarding 3.6.x, do you have a patch for the 3.6 stable-branch? Distributions like Fedora and Ubuntu will need one of those, even if GNOME upstream only considers master. It shouldn't change public API, and ideally shouldn't add public API either. Debian only ships GOA 3.6 in experimental, so I'm less concerned about that version myself, and haven't developed a patch; but many distributions are going to be more concerned about 3.6 than either 3.4 or 3.7. My initial testing demonstrated that 3.6 is also vulnerable.
Steps to Reproduce: ------------------- 1. Have a GNOME machine behind a NAT you control (I used a libvirt-bin virtual machine, behind the NAT that libvirt-bin provides) 2. On the NAT, run sslsniff: sslsniff -a -c /usr/share/sslsniff/certs/wildcard -s 4433 -w ~/tmp/sslsniff.log 3. On the NAT, intercept traffic: sudo iptables -t nat -A PREROUTING -p tcp -i virbr0 --destination-port 443 -j REDIRECT --to-ports 4433 4. On the victim GNOME machine, use a web browser to visit a https address, e.g. https://lists.debian.org/, to confirm that traffic is being intercepted; it should raise a SSL warning. (I used Iceweasel.) 5. On the victim GNOME machine, use System Settings -> Online Accounts to add a Windows Live (Passport) account (use a disposable one!). 6. Also, add a Facebook account. 7. Look at ~/tmp/sslsniff.log. Actual results: ---------------- 5. Passport login proceeds, I am asked for my password, I am asked for permission for "GNOME" to use my Passport account, and at the last stage of account creation, it fails with "Error getting an Access Token: SSL handshake failed". 6. Facebook login proceeds far enough for me to enter my password, then fails with "Error getting identity: SSL handshake failed". This seems to indicate that part of the process checks certificates, but part does not. 7. ~/tmp/sslsniff.log on the NAT machine contains a lot of HTTP traffic through the MitM'd HTTPS connection, including my Passport and Facebook accounts' passwords. Expected results: ----------------- 5. Some sort of SSL error 6. Some sort of SSL error 7. sslsniff log should indicate that no HTTP transactions were made through the MitM'd HTTPS connection, and certainly none that contain my passwords.
Comment on attachment 235237 [details] [review] [3.7.x] Fix compilation with -Werror=format-security This is just the compiler being pedantic, right? I say so because apart from translation errors, we have complete control over the string in this case.
(In reply to comment #2) > I'm also somewhat concerned that it appears to be an API and ABI change: it > makes an incompatible change (changing function parameters) to the > GoaHttpClient API in libgoabackend. *Adding* API/ABI doesn't seem like a great > thing to do in a security release either, if it can be avoided... > > If GoaHttpClient isn't public API then the API break is OK, but otherwise it > seems to me as though this would be better done by making > goa_http_client_check() and goa_http_client_check_sync() reject bad > certificates, and adding a new goa_http_client_check_insecure() and > goa_http_client_check_insecure_sync() if necessary? GoaHttpClient is not public API. Although you will see that it is mentioned in goabackend/goabackendtypes.h, but most of the types mentioned in that file were never meant to be public API in the first place, and there is no way to use them either form a client program. A relic of history, which I think needs to be fixed by using the delete key. :-)
(In reply to comment #5) > (From update of attachment 235237 [details] [review]) > This is just the compiler being pedantic, right? I say so because apart from > translation errors, we have complete control over the string in this case. Correct. If you trust your translators, this is not exploitable (and if you don't trust your translators, then you have probably already lost). Distributions like to compile with -Werror=format-string because it fairly frequently catches format-string bugs that *are* exploitable. In that sense, I believe this is a false-positive.
Comment on attachment 235236 [details] [review] [3.4.x] CVE-2013-0240: Do not allow invalid SSL certificates commit 5a3d3862, will be in 3.4.3 if released
Comment on attachment 235237 [details] [review] [3.7.x] Fix compilation with -Werror=format-security commit d5d22952, will be in 3.7.5
It is good to leave a paper trail, so this is the patch that fixes the vulnerability. From edde7c63326242a60a075341d3fea0be0bc4d80e Mon Sep 17 00:00:00 2001 From: Debarshi Ray <debarshir@gnome.org> Date: Thu, 31 Jan 2013 16:45:20 +0000 Subject: Guard against invalid SSL certificates None of the branded providers (eg., Google, Facebook and Windows Live) should ever have an invalid certificate. So set "ssl-strict" on the SoupSession object being used by GoaWebView. Providers like ownCloud and Exchange might have to deal with certificates that are not up to the mark. eg., self-signed certificates. For those, show a warning when the account is being created, and only proceed if the user decides to ignore it. In any case, save the status of the certificate that was used to create the account. So an account created with a valid certificate will never work with an invalid one, and one created with an invalid certificate will not throw any further warnings. Fixes: CVE-2013-0240
Created attachment 235328 [details] [review] [3.6.x] CVE-2013-0240: Do not allow invalid SSL certificates
Comment on attachment 235328 [details] [review] [3.6.x] CVE-2013-0240: Do not allow invalid SSL certificates The goaexchangeprovider bits also need to be backported.
Created attachment 237541 [details] [review] Guard against invalid SSL certificates The last patch was insufficient. We need this one too. Anyone care to review?
I'm not an expert on SSL, Soup or GOA, but that patch looks reasonable. I don't think it's generally the done thing to "recycle" the same CVE ID for a second vulnerability that's fixed later, even if it's closely related. I think the CVE people will want to issue a second ID: * CVE-2013-0240: GOA < 3.4.x, < 3.6.y, < 3.7.z did not verify certificates for Facebook, Windows Live, ownCloud, Exchange servers * CVE-2013-nnnn: incomplete fix for CVE-2013-0240 resulted in GOA < 3.6.a, < 3.7.b submitting credentials to ownCloud, Exchange servers even if the certificate was invalid Has there been any progress on a backport to 3.6? It seems to me that it would be better to get the uncontroversial Facebook and Windows Live part fixed (e.g. Attachment #235328 [details]) with a commit message / NEWS indicating that it's a partial fix, even if the ownCloud and Exchange part remains open. (Ask security people for advice; they might want to assign a second CVE ID to represent this split, or something.) I suspect that distributions shipping 3.6 might well prefer to enable strict validation for ownCloud and Exchange servers "across the board", in fact: not being vulnerable to MitM on servers with a trusted certificate, at the cost of inability to connect insecurely to servers with a distrusted certificate, seems like a net win.
Comment on attachment 237541 [details] [review] Guard against invalid SSL certificates We will use CVE-2013-1799 for this. See bug 695106
Created attachment 237935 [details] [review] [3.6.x] CVE-2013-0240: Do not allow invalid SSL certificates
Now that we have patches against 3.4.x, 3.6.x and 3.8.x, I am closing this. Although I tested the gnome-3-6 branch, it would be good to have a 2nd opinion that the exchange code works as expected.