Skip to content

yz_security test: forgotten CA cert for riak_core config#752

Open
llelf wants to merge 1 commit into
basho:develop-2.9from
llelf:riak_test-ca-cert
Open

yz_security test: forgotten CA cert for riak_core config#752
llelf wants to merge 1 commit into
basho:develop-2.9from
llelf:riak_test-ca-cert

Conversation

@llelf

@llelf llelf commented Mar 14, 2019

Copy link
Copy Markdown

@martinsumner

Copy link
Copy Markdown

Thanks. Just double-checking, but this isn't something you expect to change the outcome of the test (i.e. develop-2.9 will still fail, and Riak 2.2.6 will still pass)?

@llelf

llelf commented Mar 14, 2019

Copy link
Copy Markdown
Author

What do you mean? (develop-2.9 should pass with it.)

@martinsumner

Copy link
Copy Markdown

Ah. I tried, but it still failed, with the same crash caused.

The log showed that the extra cacert had been passed in though.

@martinsumner

Copy link
Copy Markdown

I will try clearing things down and doing a fresh install, in case there is some left over from an earlier test.

@martinsumner

Copy link
Copy Markdown

FYI, here are the ciphers being passed in the ssl_opts to mochiweb:

{ciphers,[<<"À'">>,<<"À#">>,<<192,19>>,<<"À\t">>,<<"À(">>,<<"À$">>,<<192,20>>,<<"À\n">>,<<0,103>>,<<0,51>>,<<0,64>>,<<0,107>>,<<0,56>>,<<0,57>>,<<192,17>>,<<192,7>>,<<192,31>>,<<192,30>>,<<0,50>>,<<"À)">>,<<"À%">>,<<192,14>>,<<192,4>>,<<0,60>>,<<0,47>>,<<"À\"">>,<<"À!">>,<<0,106>>,<<"À*">>,<<"À&">>,<<192,15>>,<<192,5>>,<<0,61>>,<<0,53>>,<<0,5>>]}

@martinsumner

Copy link
Copy Markdown

I still get this as the problem. The riak node crashes when the test attempts to start the TLS listener in Riak (with mochiweb crashing):

Reason:     {badarg,[{mochiweb_socket,'-filter_broken_cipher_suites/1-fun-0-',1,[{file,"src/mochiweb_socket.erl"},{line,41}]},{lists,'-filter/2-lc$^0/1-0-',2,[{file,"lists.erl"},{line,1271}]},{mochiweb_socket,add_unbroken_ciphers_default,1,[{file,"src/mochiweb_socket.erl"},{line,34}]},{mochiweb_socket,listen,4,[{file,"src/mochiweb_socket.erl"},{line,20}]},{mochiweb_socket_server,listen,3,[{file,"src/mochiweb_socket_server.erl"},{line,224}]},{gen_server,init_it,6,[{file,"gen_server.erl"},{line,304}]},{proc_lib,init_p_do_apply,3,[{file,"proc_lib.erl"},{line,239}]}]}

I don't think the ciphers being passed in in the ssl_opts are atoms as expected by the mochiweb filter logic.

@llelf

llelf commented Mar 14, 2019

Copy link
Copy Markdown
Author

FYI, here are the ciphers being passed in the ssl_opts to mochiweb:
{ciphers,[<<"À'">>,<<"À#">>,<<192,19>>, ⟨…⟩

They are coming from

9 Ɛ⟩ [  catch ssl_cipher:openssl_suite(C) || C <- string:tokens(riak_core_security:get_ciphers(), ":") ].     
[<<"À/">>,<<"À+">>,<<"À0">>,<<"À,">>,
  ⋮

What are you testing on? Mac?

@martinsumner

Copy link
Copy Markdown

Yes .. but I'm going to test on ubuntu now!

@llelf

llelf commented Mar 14, 2019

Copy link
Copy Markdown
Author

Ah I know why it passes for me and fails for you

filter_broken_cipher_suites(Ciphers) ->
	case proplists:get_value(ssl_app, ssl:versions()) of
		"5.3" ++ _ ->

On mac it's "8.1.3.1.1", on Ubuntu "5.3.2"

@martinsumner

Copy link
Copy Markdown

Ah. It is failing for me on ubuntu as well, but again perhaps that is a version of ubuntu thing.

I have within erl the same values for both mac osx and ubuntu e.g.

ssl:versions() returns {ssl_app,"5.3.1"} and the same cipher suites.

I will riak attach and see what I get out of riak_core_security:get_ciphers/1. Bear with me.

@llelf

llelf commented Mar 14, 2019

Copy link
Copy Markdown
Author

@martinsumner so what to change? mochiweb or riak_core?

@martinsumner

Copy link
Copy Markdown

When you said:

On mac it's "8.1.3.1.1", on Ubuntu "5.3.2"

Is that the right way round? Does it pass for you when the ssl version is 8.1.3, and hence the filtering of the ciphers doesn't happen?

@llelf

llelf commented Mar 14, 2019

Copy link
Copy Markdown
Author

Yes, doesn't make any sense. My riak_test setup is broken probably.

@llelf

llelf commented Mar 14, 2019

Copy link
Copy Markdown
Author

Alright. Now it fails for me too.

So… what to change, mochiweb or riak_core?
Because it has nothing to do with the yz test actually, just riak start will happily fail too.

@martinsumner

Copy link
Copy Markdown

I don't know where the ciphers (in the wrong format) are coming from. I had assumed that ciphers were being read from one of the pem files, and hence why the test prompted the failure. But that was just a guess.

Based on my reading of the code in riak_core_security, I can't explain why the ciphers being passed through are these short binaries.

There are some ssl tests in riak_test that are being run, and are passing. So I'm going to look at those now.

@martinsumner

Copy link
Copy Markdown

Ah. In the test suite we don't run in the mainstream tests 'riak_test/http_security', and this fails as well for the same reason. This is using the same cert files and key files.

I'm going to play around and see if I can trace how riak is fetching/passing ciphers.

@llelf

llelf commented Mar 14, 2019

Copy link
Copy Markdown
Author

Yeah, was just about to ask if ./riak_test -c rtdev -t ./ebin/http_security.beam works for you.

@llelf

llelf commented Mar 14, 2019

Copy link
Copy Markdown
Author

@martinsumner I think it's just riak_api_ssl:options/0
which does

riak_core_ssl_util:parse_ciphers(riak_core_security:get_ciphers()).

to get good/bad ciphers.

@llelf

llelf commented Mar 14, 2019

Copy link
Copy Markdown
Author

So you can go between those those ciphers “forms”:

21 Ɛ⟩ ssl_cipher:openssl_suite("ECDHE-RSA-AES128-GCM-SHA256").
<<"À/">>
22 Ɛ⟩ ssl_cipher:openssl_suite_name(<<"À/">>).
"ECDHE-RSA-AES128-GCM-SHA256"

ssl:listen is happy with either. And with {ecdhe_rsa,aes_128_gcm,null,sha256}. And with #{key_exchange => ecdhe_rsa,…}.

@martinsumner

martinsumner commented Mar 14, 2019

Copy link
Copy Markdown

Ah yes, I can see the conversion here

https://github.com/erlang/otp/blob/OTP_R16B03/lib/ssl/src/ssl_cipher.erl#L750-L857

https://github.com/erlang/otp/blob/OTP_R16B03/lib/ssl/src/ssl_cipher.hrl

so the mochiweb filter cares what form they're passed in as, and riak is converting from the text form to the binary short codes - which is the form the filter doesn't like.

I suspect we should change mochiweb to convert before it filters depending on the form it receives them?

@martinsumner

Copy link
Copy Markdown

Or perhaps we should just parse the ciphers in Riak, and remove the filter from mochiweb - avoid converting to and fro.

@llelf

llelf commented Mar 14, 2019

Copy link
Copy Markdown
Author

Doing everything in riak_core is probably better, so we don't get different cipher lists for pb and http.

@martinsumner

Copy link
Copy Markdown

So I intend to remove the filter from mochiweb, and then filter out the default ciphers from this list:

https://github.com/basho/riak_core/blob/develop-2.9/src/riak_core_security.erl#L38-L61

So if you want to pass in a broken cipher you can, but by default riak_core won't use any of the ECDH* ciphers. Does this seem reasonable?

@llelf

llelf commented Mar 14, 2019

Copy link
Copy Markdown
Author

Sounds good

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

Successfully merging this pull request may close these issues.

2 participants