The most dangerous code in the world
crypto.stanford.edu
crypto.stanford.edu
Curl has an option, CURL_SSL_VERIFYHOST. When VERIFYHOST=0, Curl does what you'd expect: it effectively doesn't validate SSL certificates.
When VERIFYHOST=2, Curl does what you'd expect: it verifies SSL certificates, ensuring that one of the hosts attested by the certificate matches the host presenting it.
When VERIFYHOST=1, or, in some popular languages, when VERIFYHOST=TRUE, Curl does something very strange. It checks to see if the certificate attests to any hostnames, and then accepts the certificate no matter who presents it.
Developers reasonably assume parameters like "VERIFYHOST" are boolean; either we're verifying or we're not. So they routinely set VERIFYHOST to 1 or "true" (which can promote to 1). Because Curl has this weird in-between setting, which does not express any security policy I can figure out, they're effectively not verifying certificates.
When designing the Curl API, it should have been obvious that VERIFYHOST is likely be set incorrectly, especially once it's used in languages that coerce true to 1.
That is, failure should take effort. Dangerous options should be more laborious to express. The happy path should be the most concise and straightforward path.
It takes more effort to override the default and shoot yourself in the foot.
Granted, it could be better. I guess that strange option could be a completely separate command parameter with a very obvious name. Then again, it's such a strange behaviour, maybe it just shouldn't even be an option at all.
This API doesn't fit with the philosophy because it takes a negligible amount of work to walk down the unsafe path.
curl_setopt($ch, CURLOPT_SSL_VERIFYPEER, TRUE);
I and any normal developer is going to think "oh, this is important, I'd better make sure to leave it in there."Configuration audits aren't as expensive as code audits, but they are still expensive.
Sometimes it's just because I like to be explicit and sometimes I'm concerned about the default being changed in a future version of the library or due to a server with a different config.
Not to mention that a lot of code ends up on the internet and is often blindly copied and pasted by programmers in a rush.
@pmjordan @tqbf for test, and also for the same reason browser-users just click yes yes yes on cert dialogues
He seems like a nice guy and being the sole maintainer of Curl seems like a tough and probably unrewarding job. But that said, it's hard to shake the impression that VERIFYHOST=1 exists because people want to believe they're doing SSL/TLS securely, but have trouble getting it configured properly, and that VERIFYHOST=1 exists primarily to give people the feeling of having configured SSL/TLS well without making them actually do that.
Empirically, 0/1/2 either means no/yes/yes-and-something-obscure-on-top-of-that, or, worse, as in this case, no/kind-of-but-not-really/yes.
enum Bool {
True,
False,
FileNotFound
};
This would have been much safer as separate AUTHENTICATE_CERT (which checks the CA sig and expiry and revocation) and VERIFY_CERT_COMMON_NAME options, both of which default to true.API doc: "When the value is 1, the certificate must contain a Common Name field, but it doesn't matter what name it says. (This is not ordinarily a useful setting)."
[emphasis mine] It's hard to get clearer than that.
http://curl.haxx.se/libcurl/c/curl_easy_setopt.html#CURLOPTS...
"Here's an option that you could reasonably expect to be boolean. And lo! the code appears to work if you treat it as a boolean. But, just to mess with you, I introduced an apparently useless option to handle the 'true' case in that boolean, and introduced an 'even truer' value that does what you expected".
This is simply a terrible, dangerous design. It is, like I said on Twitter today, a vulnerability in Curl, and one that should be fixed. The option expressed today by VERIFYHOST=1 should instead be the case when VERIFYHOST=-1 or VERIFYHOST=666. Meanwhile, VERIFYHOST=1 should, tomorrow if possible, do exactly what VERIFYHOST=2 does today.
It will only break servers and scripts that currently operate under a false sense of security. The maintainer should file a CVE, push the fixed version and let the distros sort out the rest. And he should do it ASAP.
The point of yours that I addressed was: "that VERIFYHOST=1 exists primarily to give people the feeling of having configured SSL/TLS well without making them actually do that."
Put simply: It does not exist primarily for that purpose.
I can't make it any clearer than that.
I'm kidding... I think.
You fail to provide any proof for that statement. The API doc merely states what the setting does, it says absolutely nothing about about its purpose, and neither do you.
That's because experience shows if things are "grey" then someone can almost certainly break in.
> The default value for this option is 2.
In other words, there is absolutely no issue here. It does the right thing out of the box.
Also "The most dangerous code in the world" is a bit misleading. I would say the code that runs a Predator drone is a little higher in the danger department.
But it's certainly thoughtless API design. The parameter sounds boolean (am I going to verify the SSL host or not?), and essentially it is, except that not verifying it for some reason takes two forms, and a true value turns out to specify one of them.
Whereas I doubt that the giftedness of the programmer has much to do with their reading the docs. A gifted programmer might assume that the API designer would do the same thing they would, which is to specify this with a boolean argument.
The problem isn't simply that VERIFY_HOST is documented as a scale from 0-2 that people are mistaking for a boolean by not reading the docs. The problem is that the scale is pointless, because the value it is expressing actually is a boolean: you are either verifying SSL certificates properly or you might as well not verify them at all. There is no real scale to express. The whole option actually documents a misunderstanding of SSL/TLS on the part of curl's author.
Which one's easier - fixing an API, or fixing most programmers?
They cast a really wide net, looking for as many examples as possible where non-browser applications fail to do SSL validation correctly, but then conclude that this will result in a security compromise without fully examining the implications.
For instance, they point out that many SDKs for Amazon FPS don't validate certificates correctly. But I didn't see them mention that the FPS protocol does its own signature-based authentication and that credentials are never transmitted in the clear: it was essentially designed to operate over an insecure transport to begin with.
Likewise, they point out an "unsafe" construction that an Android application that I wrote (TextSecure) uses. But they don't mention that this is for communication with an MMSC, that this is how it has to be (many don't present CA-signed certificates), and that the point of TextSecure is that an OTR-like secure protocol is layered on top of base transport layer (be it SMS or MMS).
So I think the paper would be a lot stronger if they weren't overstating their position so much.
Don't confuse a paper having 30+ references as being generous. If they could have supported and justified their own work with less citations, they would have.
https://docs.google.com/document/pub?id=1roBIeSJsYq3Ntpf6N0P...
For example, broken SSL in Amazon FPS allows a MITM attacker to forge instant payment notifications and defraud merchants who use vulnerable SDKs. This is a real vulnerability, acknowledged by Amazon.
I'll have to take a second look at the instant payment notification forgery. Last I used FPS, I remember the transaction notifications being signed. Maybe "instant payments" are a new thing.
SSL and the Future of Authenticity
http://www.youtube.com/watch?v=Z7Wl2FW2TcA
What's the status of the Convergence SSL alternatives that were going to be built into Chrome/FF?
The internet draft and reference code can be found here: http://tack.io
The obvious problem with that is that you have to call in a favor from Google to get that level of security. TACK is a mechanism that allows any site to get something comparable.
Another attractive thing about TACK is that it follows a model that other security features in the browser already use. For instance, the HSTS header is a widely-supported feature that allows websites to instruct browsers to remember that a site is intended to be reached only via HTTPS. TACK does something similar, but with a much more useful assertion.
TACK is designed to alleviate that pain for the site owner by providing flexibility, and by eliminating even the CAs the site uses from its scope of exposure.
The latter helps the former by making it easier to deploy. If TACK were the norm, then the only purpose CAs would serve is to introduce clients to websites they have never seen before (rather than authenticating every single connection to a website during every page load to that website).
By taking a bite out of the problem, we feel the remainder will be easier to solve. And yeah, hopefully we can position convergence as that solution.
It's also easier to get TACK done with browser vendors, simply because it's well encapsulated as a TLS extension, is fairly uncontroversial, and requires them to write less code. Basically, we feel it's a good first step.
However what happens if the MITM attack is on the other end, in other words somebody has got into a hosting providers network and is MITMing a bunch of traffic to some of their servers.
They could use this to pass back bullshit certs/public keys to all clients (including notaries) who connect to servers they have MITMd.
One way to prevent this of course would be to have the server keep it's own list of notaries and self-check every so often and alert clients if something appears wrong.
However here you are relying on server administrators keeping this configured and working. I could imagine less scrupulous administrators on strict SLAs disabling this and letting it fail in a way that is silent to the end user to avoid downtime. This would be more difficult to do with the traditional CA structure since the attacker would need a valid cert for the site or would need to SSL strip everything (which would eventually get noticed).
Or do I have this wrong and it is intended to augment the existing CA structure rather than replace it?
edit: http://blog.cryptographyengineering.com/2012/05/tack.html helps (i was missing that it is in addition to tls, so it's like perspectives / network notaries, but over (limited) time, for a single client, rather than over multiple clients)
schemeRegistry.register(new Scheme("http", PlainSocketFactory.getSocketFactory(), 80)); schemeRegistry.register(new Scheme("https", SSLSocketFactory.getSocketFactory(), 443)); ... HttpHost target = new HttpHost(hostUrl.getHost(), hostUrl.getPort(), HttpHost.DEFAULT_SCHEME_NAME); ... HttpResponse response = client.execute(target, request);
Viewing the code sample from the paper it can be seen that an SSLSocket was meant to be used if the connection was over HTTPS. However this use of the API results in a request being sent over HTTP instead of HTTPS. The argument for CAs not having correct certs makes less sense here in conjunction with the use of SSL API.
We clearly qualify in the paper that this is not exploitable directly.
Many security flaws found in commonly used SSL libraries.
Other than that, it is a great find.
<base href="http://crypto.stanford.edu/~dabo/pubs/pubs.html>;
This causes the page to throw an HTTPS warning: "this page loads insecure content" due to the css loaded over HTTP.
"Not the most interesting technically, but perhaps the most devastating (because of the ease of exploitation) bug is the broken certificate validation in the Chase mobile banking app on Android. Even a primitive network attacker—for example, someone in control of a malicious Wi-Fi access point—can exploit this vulnerability to harvest the login credentials of Chase mobile banking customers."
public final void checkServerTrusted(X509Certificate[]
paramArrayOfX509Certificate, String paramString)
{
if ((paramArrayOfX509Certificate != null) && (
paramArrayOfX509Certificate.length == 1))
paramArrayOfX509Certificate[0].checkValidity();
while (true)
{
return;
this.a.checkServerTrusted(
paramArrayOfX509Certificate, paramString);
}
}
Good to know this isn't used for anything critical, just for, um, mobile banking. void foo(X509Certificate[] a, String unused) {
if( a != null && a.length == 1) a[0].checkValidity();
while(true){
return;
/* never executed stuff */
foo(a, unused);
}
}
X509Certificate.checkValidity()'s javadoc:
http://docs.oracle.com/javase/1.5.0/docs/api/javax/security/...Selections from above link:
Checks that the certificate is currently valid. It is if the current date and time are within the validity period given in the certificate.
Throws:
CertificateExpiredException - if the certificate has expired.
CertificateNotYetValidException - if the certificate is not yet valid.
There has got to be more to their code than this. It does nothing useful except make sure a random certificate is not expired. If there is no certificate, it doesn't fail or return false or anything. What is this!?Could this be a profiler-optimized set of code that really had a LOT more code to it? But then why would the bit of random junk after the return statement exist? Did no one do a proper code-review of this???
[edit: I used JavaSE 1.5 above; but I just checked Android's version and it looks to be the same, just less verbage in the docs.]
I can imagine some poor developer, unable to figure out why it seems to always throw a StackOverflowException, putting that return statement in in frustration, and forgetting about it.
I'm not saying that the conclusion of the article is wrong, I'm merely trying to explain why the code blob above looks so weird.
In either case, we have the bytecode; and shouldn't what is written above compile to the same bytecode?
public class C { public int foo() { return 1; int b = 1 + 2; } }
Then compile it (javac C.java). You will get the following:
C.java:4: unreachable statement int b = 1 + 2; ^ C.java:5: missing return statement } ^ 2 errors
Therefore this code is not compileable, thus telling us that the initial code blob could never have run in the first place.
I'm not sure why the decompiler returns non-compiling java code, I just know it does.
http://docs.python-requests.org/en/latest/
It validates SSL certificates correctly by default. How about other languages?
I suppose my web browser has an extended list of CA that my OSX lion does not know about.
i'm not saying that this would solve all the problems, or that you should develop critical financial software by having people that don't understand much writing tests. but tests are pretty much common culture now; you'd think people would have considered this. and the argument the paper makes is not that the programmers are clueless, but that they are confused by the API, so they should be able to think up some useful tests...
of course, integration testing with sockets is a bit more complicated than unit tests (perhaps something toolkit apis should support is a way to allow testing without sockets?), but it's not super-hard. [edit: hmm. although testing for unreliable dns is going to be more tricky.]
At any rate, here is a pull request for PHP which attempts to address the issue:
Some, lynx for example, seem to have been fixed a long time ago.
I can see turning off validation for stuff where you're sending non-important data to a third party which may or may not be encrypted (asyncronous API pings where you say "something happened, ask us [securely] what it was") - but that's only OK if you're just as willing to send the info over an http connection as well.
If you turn off cert validation in your app to your own API, something is seriously wrong. Unfortunately, it often comes down to simply buying a more expensive certificate that's signed by a root CA with wider trust. Given spending $50 or CURLOPT_SSL_VERIFYPEER=false, a scary number of people will choose the latter.
These libraries should provide an easy way to specify an additional root CA when validating the chain of trust - that way you could even use a self-signed cert without disabling the chain of trust check, all you'd have to do is embed your self-signing public key.
Probably more important, there should be clearer error messages when the SSL chain verification. Cryptic "verify the CA cert is OK" messages don't help much; they should say "SSL CA [name] is not in the installed trusted certificates, add trust for it with CURLOPT_CAINFO, details at example.com/ca.html" or something along those lines.
It may cause issues with root CA revocation (DigiNotar anyone?), but it's still better than disabling the checks entirely.
Now I will say that, most common apps (Apache HTTPD + mod_ssl for example), do not make this process as clear as it should be. Specifying cert order wrong in the Intermediate Chain file will still break cert validation for some apps, because it's sent out exactly as the server admin configures (and this fact is often lost of folks who maybe touch their certs once a year when they expire).
My point was that the more widely-trusted issuers (meaning installed by default on the most OSes) can get away with charging more because they're more compatible.
There's nothing stopping you from setting up your own root CA and self-signing, maintaining a full chain of trust if you build the certs right (I've done it; once you have the process documented well it only takes a few minutes) but unless the clients trust the lowest link in the chain (not the case for self-signed, and some cheaper issuers) you'll still get validation errors.
What is a trusted and reputable CA that offers cheap certificates ? Is this even a possibility or do all the CAs charge according to how reputable they are ?
Any issuer with a widely installed root CA can basically charge whatever they want.
Sent me to: https://payments.amazon.com/sdui/sdui/about?nodeId=201033780...
It is destined to be flawed as long as insecurity is allowed. Only when every exploit is exploited continously will people be vigilant.
https://github.com/getlantern/lantern/blob/master/src/main/j...
? This code is intended for deployment in potentially dangerous regions for getting around government censors.
Thanks.
You can see it here: https://github.com/rails/rails/blob/3-2-stable/activeresourc...
I'm pointing it out as it was not mentioned in the paper.
Edit: It looks like it has been that way since SSL was first implemented in Connection.
require 'always_verify_ssl_certificates' AlwaysVerifySSLCertificates.ca_file = "/path/path/path/cacert.pem"
http= Net::HTTP.new('https://some.ssl.site, 443) http.use_ssl = true req = Net::HTTP::Get.new('/') response = http.request(req)
http://www.rubyinside.com/how-to-cure-nethttps-risky-default...
<falls off chair>