Hacker News Reader

Comment by tptacek | Hacker News Reader

The worst example from this paper is Curl's API.

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.

View on HN

While you're attacking curl you (intentionally?) forget to mention that the default value is 2 that is perfectly safe. I don't think it's "reasonable to assume" that the parameter is a boolean value while it's well documented. If programmers are lazy, they make many kind of mistakes.

Yes, yes, I know, stupid Paypal, stupid Amazon, we get it. VERIFYHOST=1 doing nothing makes perfect sense, you just have to read the documentation. Of course.

Yes, Amazon et.al. made some mistakes due being lazy and PHP being the nice language that it is. They have fixed it. Let's go ahead and find another non-default, documented parameter value in a library and call it a vulnerability.

Ok. Your turn. Go!

Policy vs Mechanism.

I think it's highly questionable that a default value of 2 is appropriate for something that is critical functionality. DISABLE_HOST_VERIFICATION would have been much better, similar to the example code.

http://curl.haxx.se/libcurl/c/https.html

It's bad API design. Yeah, I get it, you read your docs properly and would never make that mistake. But most programmers are not as gifted and good API design is about mitigating inevitable human error.

Most programmers are not gifted enough to read the docs? Well there's the problem.

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.

Well, maybe it's 6 of one and half a dozen of the other.

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.

Or rename the parameter to CURL_SSL_VERIFICATION_LEVEL.

No! Because CURL_SSL_VERIFICATION_LEVEL 0 is the same as CURL_SSL_VERIFICATION_LEVEL 1: meaningless.

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.

>Most programmers are not gifted enough to read the docs? Well there's the problem.

Which one's easier - fixing an API, or fixing most programmers?

I don't know if it's fair to blame laziness. Acting in haste, maybe? A ternary implementation for something that intuitively seems like a binary choice is a little... well.. unintuitive. That said, when building an application of significant scale, yeah, you should probably read the documentation of anything you aren't familiar with.

Reading the documentation is important, but tests are even more so. A simple test running with a bad certificate would have caught a lot of these issues. Small devs often push out software without having much time to read docs or make tests, but these are big players with lots of resources, and real money at stake.

I think the real problem is the name of the option, not the range of its accepted values. Should they have named it HOST_VERIFICATION_LEVEL or something similar, no-one would think it's boolean.

In that case "TRUE" shouldn't be a valid value.

I think that's because in the weakly typed language used, TRUE is a constant with a value of 1.

Perhaps this is a dumb question, but when would VERIFYHOST=1 be useful?

On Twitter today, Daniel Stenberg, Curl's maintainer, said:

@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.

When it comes to API design, a good rule of thumb is that parameters that can be either 0, 1 or 2 will cause bugs.

More generally: "when it comes to API design, a good rule of thumb is that parameters will cause bugs."

Your more general adage is useless. The one you responded to is useful. A more useful, also general adage would be "the more choice in selecting parameters to an API, the more likely there will be bugs consuming that API."

No, I was actually trying to be specific. There's usually nothing wrong with 0/1 or 1-9.

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.

It's akin to http://c2.com/cgi/wiki?ZeroOneInfinityRule. If you started out thinking it was just a switch, you should be very reluctant to add a third state without rethinking what you really need to express, or you end up with my favorite from TheDailyWTF:

  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.

In this case I think it wouldn't cause bugs if the effect of 1 and 2 would be switched. Then everybody would use it like a boolean and that would be fine, only the few people who really want the name-check disabled would use that option.

> and that VERIFYHOST=1 exists primarily to give people the feeling of having configured SSL/TLS well without making them actually do that.

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...

I think we are all clear that if library users carefully read the documentation, this is an avoidable flaw. I'm not sure what that has to do with my point. If even the Curl author thinks VERIFYHOST=true isn't useful, why does it work that way? The answer is: it shouldn't.

"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.

I don't think this shouldn't be changed, but surely it's not one that can be made without thought or concern either. Changing the behavior overnight will undoubtedly break servers and scripts.

Changing the behavior overnight will undoubtedly break servers and scripts.

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.

> I'm not sure what that has to do with my point.

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.

Oh yeah? Interesting. Tell me, what is the point of checking to see if a certificate has a common name field in it but then not checking to see if that common name is related to the connection bearing the certificate? Make it clear for me, will you?

That option exists to punish those evil people who don't read the documentation.

I'm kidding... I think.

Of course, who it really punishes is the user.

Some developers seem to believe that more options are always better, even if removing the bad options makes the software better and easier to use.

> Put simply: It does not exist primarily for that purpose.

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.

This is definitely NOT a vulnerability in curl. I hate it when security people treat everything black and white. There ARE other things to consider.

Such as what? When VERIFYHOST=1, curl has basically turned the security part of SSL/TLS off. That seems like kind of a big deal for an HTTPS/SSL API.

> I hate it when security people treat everything black and white.

That's because experience shows if things are "grey" then someone can almost certainly break in.

You do realize that the curl API says this:

> 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.

It's easy to get much clearer than that. Instead of "1", use a constant with a name like "VERIFY_HOST_SETTING_INSECURE_IGNORE_COMMON_NAME".

Networks where users have failed to set up proper SSL CAs for internal use; Lots of customers I've seen with failed CA setups, self-signed certs everywhere, etc.

Yes, but VERIFYHOST=0 handles that case, and does what users expect.

At least the default value is 2.

Which, as the authors show in section 7.1 and 7.2 of their paper, many PHP developers, like the authors of the Amazon Flexible Payments Service SDK and the PayPal Payments Standard SDK, will happily override with true.

You would have to read the docs to find the parameter you wish to overwrite would you not? I don't see how the curl behaviour can be faulted. If you are overwriting a default you are responsible. And you are reading the docs anyway.

Of course the developers are at fault here, but good API design would be to not invite developers to mess things up.

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.

This is like saying any API is reasonable as long as it's documented. If you read the example, they found numerous examples of people who clearly weren't intending to disable SSL certificate validation who nonetheless overrode that default.

No, not any API. But if the defaults err on the side of caution, which is the case here, and the non-default documentation is complete and coherent, I think that's pretty good. I don't think I would call this situation unreasonable, though I do agree that it could be improved.

There's a simple concept that one should bear in mind when designing APIs: "pit of success" (credit to Rico Mariani).

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.

I would say that by having the default be the safer option, the API is in line with that philosophy.

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.

The effort it takes to do something that's unsafe should be laborious and not simply require adding a single line of code.

This API doesn't fit with the philosophy because it takes a negligible amount of work to walk down the unsafe path.

Many times as a developer I have followed other people's example code. And if I see someone's code say

    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.

I will sometimes override default values in my code even when the defaults do what I want.

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.

Reply on news.ycombinator.com