-
Notifications
You must be signed in to change notification settings - Fork 210
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
Loosen TLS config for pushy-server 1.x #1016
Conversation
cc @jeremiahsnapp @chef/chef-server-maintainers |
# | ||
# - AES256-GCM-SHA384 has been added to support AWS's classic ELB health check. | ||
# - AES added for pushy-server 1.x compatibility | ||
# - TLS1 and TLSv1.1 added for pushy-server 1.x compatibility |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does TLS1
mean, exactly? Can we just allow TLSv1.1
? Why do we need both?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need TLSv1 which I believe means TLS 1.0 because that is all the version of ruby we ship in pushy 1.x understands, at least in the experiments I did.
# | ||
# - AES256-GCM-SHA384 has been added to support AWS's classic ELB health check. | ||
# - AES added for pushy-server 1.x compatibility | ||
# - TLS1 and TLSv1.1 added for pushy-server 1.x compatibility |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have we verified that the cipher change alone isn't enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need the protocol change if we want the pedant tests to work.
Until opscode-push-jobs-server 2 is certified for use with Chef Automate, supporting 1.x out of the box is essential. To achieve compatibility, we need to add support for TLS 1.0 and an older set of ciphers supported both by the older versions of erlang and ruby that product ships. * Do we really need to change both the protocols and the cipher suite? I believe so, below I've recorded my observations with other possible changes. ** Out of the box Out of the box there are two points of failure I was able to find: 1. API requests fail, producing the following error in the opscode-pushy erlang application: Webmachine error at path "/organizations/testorg/pushy/jobs" : {throw, {error,{conn_failed,{error,closed}}}, [{file,"src/pushy_http_common.erl"},{line,44}]}, {pushy_org,fetch_org_id,1, [{file,"src/pushy_org.erl"},{line,38}]}, {pushy_object,fetch_org_id,1,[{file,"src/pushy_object.erl"},{line,45}]}, {pushy_wm_base,verify_request_signature,2,[{file,"src/pushy_wm_base.erl"},{line,157}]}, {pushy_wm_base,is_authorized,2,[{file,"src/pushy_wm_base.erl"},{line,135}]}, {webmachine_resource,resource_call,3,[{file,"src/webmachine_..."},...]},...]} 2. `opscode-push-jobs-server-ctl test` fails with the following error without making any requests to the pushy application: lib/pedant/core_ext/net_http.rb:61:in `connect': Connection reset by peer - SSL_connect (Errno::ECONNRESET) ** ssl_ciphers only If I add the following to chef-server.rb (newlines for readability): nginx['ssl_ciphers'] = "ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384: ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305: ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256: ECDHE-ECDSA-AES256-SHA384:ECDHE-RSA-AES256-SHA384: ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256:AES256-GCM-SHA384: AES:!aNULL:!eNULL:!EXPORT" (Note this configuration adds AES to the ssl_ciphers) We observe that: - The erlang app fails with the same error as in the out-of-the-box case. - `opscode-push-jobs-server-ctl test` fails with the same error as in the out-of-the-box case. ** ssl_protocols modified to have TLSv1.1 If I add: nginx['ssl_protocols'] = "TLSv1.1 TLSv1.2" - The erlang app fails with the same error as in the out-of-the-box case. - `opscode-push-jobs-server-ctl test` fails with the same error as in the out-of-the-box case. If I further modify the pedant config as we did with reporting using either ssl_version :TLSv1_1 or ssl_version :TLSv1_2 `opscode-push-jobs-server-ctl test` continues to fail, but with an error in the form of: lib/pedant/core_ext/net_http.rb:37:in `ssl_version=': unknown SSL method `TLSv1_1'. (ArgumentError) The only ssl_verions supported by version of ruby shipping in reporting seems to be TLSv1 or SSLv3. ** ssl_protocols modified to have TLSv1 If I add: nginx['ssl_protocols'] = "TLSv1 TLSv1.1 TLSv1.2" - The erlang application fails with a new error: SSL: hello: ssl_connection.erl:1724:Fatal error: handshake failure Webmachine error at path "/organizations/testorg/pushy/jobs" : {throw,{error,{conn_failed,{error,esslconnect}}}, [{pushy_http_common,fetch_authenticated,2,[{file,"src/pushy_http_common.erl"},{line,44}]}, {pushy_org,fetch_org_id,1,[{file,"src/pushy_org.erl"},{line,38}]}, {pushy_object,fetch_org_id,1,[{file,"src/pushy_object.erl"},{line,45}]}, {pushy_wm_base,verify_request_signature,2,[{file,"src/pushy_wm_base.erl"},{line,157}]}, {pushy_wm_base,is_authorized,2,[{file,"src/pushy_wm_base.erl"},{line,135}]}, {webmachine_resource,resource_call,3,[{file,"src/webmac..."},...]},...]} - `opscode-push-jobs-server-ctl test` fails with the a new error: lib/pedant/core_ext/net_http.rb:61:in `connect': SSL_connect returned=1 errno=0 state=SSLv3 read server hello A: sslv3 alert handshake failure It is only when I apply both the changes to ssl_protocol and ssl_ciphers that both the erlang application and pedant works. Signed-off-by: Steven Danna <[email protected]>
7ddee30
to
7960224
Compare
@marcparadise @danielsdeleo I've updated the commit message and the PR message with a bunch of my observations when testing this. I believe they point to both changes definitely being needed. I think there is probably a more restrictive ciphersuite we can add other than |
👍 I think this is the right choice for now; especially if we document how to tighten things up for those who don't need push 1.x support. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make reverting this part of the work of updating push jobs 1.x, though
Summary
Until opscode-push-jobs-server 2 is certified for use with Chef
Automate, supporting 1.x out of the box is essential.
To achieve compatibility, we need to add support for TLS 1.0 and an
older set of ciphers supported both by the older versions of erlang and
ruby that product ships.
Do we really need to change both the protocols and the cipher suite?
I believe so, below I've recorded my observations with other possible
changes.
Out of the box
Out of the box there are two points of failure I was able to
find:
API requests fail, producing the following error in the opscode-pushy
erlang application:
opscode-push-jobs-server-ctl test
fails with the following errorwithout making any requests to the pushy application:
ssl_ciphers only
If I add the following to chef-server.rb (newlines for readability):
(Note this configuration adds AES to the ssl_ciphers)
We observe that:
The erlang app fails with the same error as in the out-of-the-box case.
opscode-push-jobs-server-ctl test
fails with the same error as inthe out-of-the-box case.
ssl_protocols modified to have TLSv1.1
If I add:
The erlang app fails with the same error as in the out-of-the-box case.
opscode-push-jobs-server-ctl test
fails with the same erroras in the out-of-the-box case.
If I further modify the pedant config as we did with reporting using either
or
opscode-push-jobs-server-ctl test
continues to fail, but with an errorin the form of:
The only ssl_verions supported by version of ruby shipping in reporting
seems to be TLSv1 or SSLv3.
ssl_protocols modified to have TLSv1
If I add:
The erlang application fails with a new error:
opscode-push-jobs-server-ctl test
fails with the a new error:It is only when I apply both the changes to ssl_protocol and ssl_ciphers
that both the erlang application and pedant works.
Signed-off-by: Steven Danna [email protected]