Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bundled excon ca certificates used by default #212

Closed
johnl opened this issue Mar 27, 2013 · 13 comments
Closed

bundled excon ca certificates used by default #212

johnl opened this issue Mar 27, 2013 · 13 comments

Comments

@johnl
Copy link
Contributor

johnl commented Mar 27, 2013

Commit f3b4e58 bundles a set of ca certificates and makes using them the default.

I presume this is to fix something on a badly behaving OS somewhere, but it's causing some problems for us on Debian/Ubuntu (and I presume other distros) and I think will cause future problems too.

Debian/Ubuntu already put a lot of work into distributing a bundle of ca certificates - they do it securely and keep it up to date. They also provide a simple way to add in custom certificates for your system. Their ssl library installation looks to default to using this (it just works for nearly everything, system-wide).

This commit makes excon use it's own bundle of ca certs. It means any custom certificates installed by the user are ignored, but it also ties the excon code to the ca bundle - there isn't a simple way to update the excon ca cert bundle without updating the excon library (and you'd have to do it for every version of excon installed too, and do it every time excon is reinstalled).

It's also much less secure (at least, until everyone is using rubygems signature verification).

We've noticed that the Debian package of excon patches out this behaviour (which is partly why it's taken us so long to notice a problem): http://anonscm.debian.org/gitweb/?p=pkg-ruby-extras/ruby-excon.git;a=blob;f=debian/patches/01_use_ca-certificate.patch;h=da56c80fa894f69b253c2e475b6670603a7a6596;hb=43b1a78a8c686ae33b3a05531fbb064f0b07fe91

So I don't think that excon should be specifying a ca bundle by default - it should just default to whatever the system library decides.

If there are some OSes that need help here (and it's deemed excon should be helping), I think excon should explicitly detect those OSes rather than just change the default for everyone. It rather depends on what this commit was originally for though.

Unfortunately, this commit is over a year old now - it's quite possible changing it now will have lots of unintended effects. So I'm not certain what is best now.

@geemus
Copy link
Contributor

geemus commented Mar 27, 2013

@johnl - it's been a while but I think my initial attempt was for the sake of consistency. Prior different things would fail (or succeed) quite differently across OSes. It would "just work" on mac, totally fail on windows and sometimes work on linux.

I think in usage at least one can change the ca file without having to change the source code, but changing the source code is probably the easiest at the distribution level. I would be open to making this easier, perhaps checking for DEFAULT_CA_FILE (or similar) in the environment and then falling back to the bundled version would be workable?

I do think changing this is likely to cause issues. The hope with bundled CA was that it would work, without modifications in all (or nearly all) cases. That is something that I would like to continue to be the case, though I would also like to support your different use case also.

Hopefully that clarifies, certainly happy to discuss further though.

@raggi
Copy link

raggi commented Apr 2, 2013

If you're going to distribute CA certs then you're taking on the responsibility to ensure that they're up to date, that they're valid, and that all of the CA's in the bundle are trustworthy.

You've failed to do this for the current bundle. In my opinion, you're doing a disservice to your users by doing this.

I do realize that it will have reduced the number of incoming issues from users who don't know how to install ca bundles on their systems, and that users generally do equally silly things in terms of failing to keep them up to date. Despite this, you really must realize that by taking the action you have, you assumed responsibility for their SSL safety. If you're willing to do that, so be it, but I for one don't trust you to do that, even though I do really appreciate your hard work in producing this software.

@halorgium
Copy link
Contributor

@geemus an immediate "patch" would be to update the file with a more up-to-date set of certs.
It definitely wouldn't solve the overarching problem of excon hijacking the CA certificate chain on the host system.

/cc @raggi

@geemus
Copy link
Contributor

geemus commented Apr 2, 2013

@raggi - it seemed that my options at the time were to have a consistent experience for users by defaulting to secure connections (and ensuring this was going to work) or defaulting to insecure connections (which didn't work for many users). Having a working cert seemed better than not verifying at all, even if it wasn't best (which would be users/systems maintaining their own). So I don't want to get in the way of people who will do it better but I also like the idea of more secure by default.

@halorgium - good point, I'll at least do that for now as it is quick/easy and shouldn't have behavior change issues.

geemus added a commit that referenced this issue Apr 2, 2013
@johnl
Copy link
Contributor Author

johnl commented Apr 3, 2013

This came to my attention because we use cacert.org certs for our staging system, whose ca is installed by Debian and Ubuntu but apparently not in the excon bundle. I don't really want to ask you to put the cacert cert in the excon bundle as it just opens the floodgates for further requests (though it may nicely highlight the problem :)

For what it's worth, curl appears to ship a script that can make a cabundle (which basically download's mozilla's). They don't ship their own bundle.

http://curl.haxx.se/docs/caextract.html

The curl package on ubuntu doesn't come with a bundle - it uses the system default.

Interestingly too, stracing curl shows it not reading one big bundle but doing some kind of id or hash lookup, which means it only has to read and parse one certificate - so there are performance benefits to be had here too I expect:

$ strace -f curl https://api.gb1.brightbox.com  2>&1 | grep cert
stat("/etc/ssl/certs/578d5c04.0", {st_mode=S_IFREG|0644, st_size=1143, ...}) = 0
open("/etc/ssl/certs/578d5c04.0", O_RDONLY) = 4

excon used to do this hash lookup behaviour, so it looks to be the openssl default. We're definitely losing out in many ways by using the bundled CA here (at least on Debian/Ubuntu).

I've still more investigation to do here, but I've run out of time right now. I think the key here is figuring out how best to decide when to use the ssl library defaults and when to explicitly set our own bundle.

Frankly though, I'm not certain this is excon's problem - problems with bad default configs for ssl libraries should be fixed upstream, ideally in the ssl library packaging or perhaps, in this case, in the ruby packages. I think it really depends where this was actually broken to begin with I suppose :)

@raggi
Copy link

raggi commented Apr 3, 2013

@geemus I absolutely sympathize with your goals. The problem is, if it's out of date, then it's the opposite. It's not much harder to mitm someone with previously exposed CA certs than it is to mitm someone using plain. I gave you praise some years ago for using the default system store, this really feels like a regression. If you setup some way to update these (safely - worth noting that haxx.se does not respond to SSL, and does not have a trusted DNS server), and keep them up to date, I would have less of a problem. It would also make more sense I think to only fall back to this cert chain if the system cert store is empty.

@johnl see openssl will never ship a cert store.

@geemus
Copy link
Contributor

geemus commented Apr 3, 2013

@raggi - yeah. I'd definitely be up for only falling back to this if nothing else is available, I just never had a ton of luck figuring out how to do that properly in the past. I'd certainly welcome any advice and/or code you might have in this realm. I had hoped that doing something like this would be better than no verification at all, but keeping it up to date is definitely a concern. I'd also welcome suggestions on how/where I should be pulling this cert from instead and I'd be happy to add a test that checks it's contents against that so we could get CI failures if/when it changes (still not ideal, but would notify us in relatively short order at least.

At the end of the day I don't particularly want to be in the business of doing this, it just seemed like somebody needed to make it less likely that people would just turn verification off (see for instance the huge number of google hits you get around net::http verification errors that advice you to remap the verification constant to turn off peer verification). Rubyists seem not to care about this as much as they ought to, so I was trying to help out by forcing the point, so I certainly appreciate any/all help I can get to make this more reasonable/better for everyone.

@geemus
Copy link
Contributor

geemus commented Jun 17, 2013

@johnl - I think/hope this addresses your concern well (and falls back in a way that allows it to still work on Windows). This also makes verification on-by-default, even on windows (unlike previously) which seems better. Let me know if this still doesn't do the right-thing-for you, but I'm hoping that by relying on the OS defaults as defined by it's openssl install we should be good.

@halorgium
Copy link
Contributor

@geemus one further thing to consider is logging when your certs are used.
Perhaps then users could be made aware of their lack of OS-provided cert bundle.

@geemus
Copy link
Contributor

geemus commented Jun 17, 2013

@halorgium - true. I think it should almost always file OS certs. The exception is railsinstaller+windows, or at least my current installation of said thing. It has a default config file path which points to a non-existent file. That said, perhaps a warning when it falls back is in order. For now I was just aiming for getting this in and getting some external validation that it fixes it on machines-other-than-my-own.

@johnl
Copy link
Contributor Author

johnl commented Jun 17, 2013

Ah, excellent - a good compromise I think. This is what I was trying to figure out how to do originally, but I couldn't find how to actually use OpenSSL::Config::DEFAULT_CONFIG_FILE. Nice one.

This appears to do the right thing on my laptop running Ubuntu 13.04 (under ruby1.8, 1.9.3 and 2.0).

For anyone else following along, you can test this in isolation with this little script: https://gist.github.com/johnl/5795909 - it raises an OpenSSL exception if there is a problem.

Thanks @geemus!

@geemus
Copy link
Contributor

geemus commented Jun 17, 2013

@johnl - yeah. I was basically on the same page (I wanted to do something like this, but couldn't figure out for the life of me how to get it to play along). Here was the missing link for me: http://stackoverflow.com/a/9238221/967276

@geemus
Copy link
Contributor

geemus commented Jul 18, 2013

@johnl - unfortunately some folks had other issues with that just not working, so I'm continuing to tweak, wanted to give you a heads up. In particular SSL_CERT_FILE and/or SSL_CERT_DIR should be respected now and my bundled cert will always be added (but since it doesn't get added until after setting default paths, I don't believe it will be reached until/unless the default ones fall through). I'm hoping this will "do the right thing" in more cases, but wanted to give you a heads up. More details:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants