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

Concerns with new security features added in Riak 2.0 #433

Closed
mortman opened this issue Nov 11, 2013 · 4 comments
Closed

Concerns with new security features added in Riak 2.0 #433

mortman opened this issue Nov 11, 2013 · 4 comments

Comments

@mortman
Copy link

mortman commented Nov 11, 2013

I was reading through the code/samples that have been created in response to #355 and had two major concerns:

The first is that authentication is being done using HTTP Basic (example https://gist.github.com/Vagabond/05b7dc8ae6d3ca4af6c2). While SSL is great and all, HTTP Basic is a broken mechanism and even more so if the connection happens to be in the clear. Essentially whenever possible AuthN should be done in such a way that if it's sent in the clear the failure modes are minimal. Protocols like HTTP Request-Signing are much more robust in this scenario especially with payload signatures. @greese's book goes into much more detail on this issue.

Relatedly, when looking at https://github.com/basho/riak_core/blob/4dd84bf9541b788309a3200dbd84afc9aa2c5520/src/riak_core_ssl_util.erl

I noticed that nothing was being done to restrict which versions of SSL/TLS were being used. Given the research of the last few years, this is a great opportunity to mandate TLS 1.2 as part of the code base. There's essentially no reason to support older known broken protocols given that this is a new feature. Similarly, there's no restriction on allowed ciphers and ciphers such as des_cbc should be explicitly disallowed. Likewise, removing md5 as a hash and reviewing key_exchange algorithms is an excellent idea as well

@Vagabond
Copy link
Contributor

Yes. HTTP BASIC is not really ideal,. especially since the credentials are re-sent every request. However, one of the design constraints of Riak's security system was to allow pluggable authentication backends. Except in the case of the 'password' backend, Riak does not store any password data at all, it is passed through to the backend for validation. Since we don't control how the backend accepts passwords, and they don't all use the same hashing scheme (when they even support hashed passwords) is it not feasable to have the client send hashed passwords over the wire. Some future work is to investigate alternative approaches to this problem. Also, as of 2.0 we are moving all of the official Riak clients to only support the PB interface. HTTP is still supported, but anyone using an official Basho client will not be using it as the protocol for communicating with Riak.

As regards TLS, I completely agree. A saner cipher list and knobs for tuning the minium allowed TLS version are 2 of the things on my TODO list for 2.0. I did a writeup of the RICON talk I gave the other week on this, which you can read here:

http://vagabond.github.io/2013/11/06/ricon-west-2013-talk-writeup/

I'm not sure we can force TLS 1.2 by default, as a lot of languages have only just gained support this year for it (and TLS 1.0) but we can certainly provide the knobs for it so people with cabable clients can force it.

@bkerley
Copy link

bkerley commented Nov 11, 2013

Also, as of 2.0 we are moving all of the official Riak clients to only support the PB interface. HTTP is still supported, but anyone using an official Basho client will not be using it as the protocol for communicating with Riak.

The Python client (/cc @seancribbs ) will still support HTTP for the time being, as some Python client users have already built considerable security (authentication, auditing) infrastructure around HTTP.

@mortman
Copy link
Author

mortman commented Nov 12, 2013

@Vagabond Thanks for the prompt and great answers. It is as usual far more complex a problem that it seemed at first blush. Would it be worth adding some documentation about why HTTP Basic isn't ideal and why users might want to use other options? I'm happy to contribute to the cause.

Also thanks for the link to the writeup of your RICON talk. Will there be video posted as well?

"I'm not sure we can force TLS 1.2 by default, as a lot of languages have only just gained support this year for it (and TLS 1.0) but we can certainly provide the knobs for it so people with cabable clients can force it."'

Yay knobs! Choice is great. That would be great for ciphers/hashes generally the SSL options in general. What about setting a default of 1.2 and letting folks uncomment things or lower security though rather then making them opt-in to the better security options?

@Vagabond
Copy link
Contributor

Vagabond commented Dec 9, 2013

I've implemented the cipher work over in basho/riak_core#469 You can now set the ciphers you want to use, and even turn on 'honor cipher order' style behaviour, if you're using a Basho package (I've patched the erlang SSL application to support that option). The default cipher list is the one taken from here:

https://wiki.mozilla.org/Security/Server_Side_TLS

Vagabond added a commit to basho/riak_api that referenced this issue Dec 9, 2013
The default is to only allow TLS 1.2 and to check the CRL of any client
certificates.

This addresses some of the concerns in basho/riak#433
Vagabond added a commit to basho/riak_api that referenced this issue Dec 20, 2013
The default is to only allow TLS 1.2 and to check the CRL of any client
certificates.

This addresses some of the concerns in basho/riak#433
Vagabond added a commit to basho/riak_api that referenced this issue Jan 6, 2014
The default is to only allow TLS 1.2 and to check the CRL of any client
certificates.

This addresses some of the concerns in basho/riak#433
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants