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

Increase nginx security by adapting ssl_ciphers. #3606

Merged
merged 2 commits into from
May 3, 2018

Conversation

cbickel
Copy link
Contributor

@cbickel cbickel commented May 3, 2018

This PR adapts the allowed encryption methods of nginx to the recommandation of Mozilla.
See https://wiki.mozilla.org/Security/Server_Side_TLS

Allowed ciphers are now:

openssl ciphers -v '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'
ECDHE-ECDSA-AES256-GCM-SHA384 TLSv1.2 Kx=ECDH     Au=ECDSA Enc=AESGCM(256) Mac=AEAD
ECDHE-RSA-AES256-GCM-SHA384 TLSv1.2 Kx=ECDH     Au=RSA  Enc=AESGCM(256) Mac=AEAD
ECDHE-ECDSA-CHACHA20-POLY1305 TLSv1.2 Kx=ECDH     Au=ECDSA Enc=ChaCha20-Poly1305 Mac=AEAD
ECDHE-RSA-CHACHA20-POLY1305 TLSv1.2 Kx=ECDH     Au=RSA  Enc=ChaCha20-Poly1305 Mac=AEAD
ECDHE-ECDSA-AES128-GCM-SHA256 TLSv1.2 Kx=ECDH     Au=ECDSA Enc=AESGCM(128) Mac=AEAD
ECDHE-RSA-AES128-GCM-SHA256 TLSv1.2 Kx=ECDH     Au=RSA  Enc=AESGCM(128) Mac=AEAD
ECDHE-ECDSA-AES256-SHA384 TLSv1.2 Kx=ECDH     Au=ECDSA Enc=AES(256)  Mac=SHA384
ECDHE-RSA-AES256-SHA384 TLSv1.2 Kx=ECDH     Au=RSA  Enc=AES(256)  Mac=SHA384
ECDHE-ECDSA-AES128-SHA256 TLSv1.2 Kx=ECDH     Au=ECDSA Enc=AES(128)  Mac=SHA256
ECDHE-RSA-AES128-SHA256 TLSv1.2 Kx=ECDH     Au=RSA  Enc=AES(128)  Mac=SHA256

Description

Related issue and scope

  • I opened an issue to propose and discuss this change (#????)

My changes affect the following components

  • API
  • Controller
  • Message Bus (e.g., Kafka)
  • Loadbalancer
  • Invoker
  • Intrinsic actions (e.g., sequences, conductors)
  • Data stores (e.g., CouchDB)
  • Tests
  • Deployment
  • CLI
  • General tooling
  • Documentation

Types of changes

  • Bug fix (generally a non-breaking change which closes an issue).
  • Enhancement or new feature (adds new functionality).
  • Breaking change (a bug fix or enhancement which changes existing behavior).

Checklist:

  • I signed an Apache CLA.
  • I reviewed the style guides and followed the recommendations (Travis CI will check :).
  • I added tests to cover my changes.
  • My changes require further changes to the documentation.
  • I updated the documentation where necessary.

@sven-lange-last Could you please review this?

@cbickel cbickel added the review Review for this PR has been requested and yet needs to be done. label May 3, 2018
@cbickel cbickel changed the title Increase nginx security by adapting ss_ciphers. Increase nginx security by adapting ssl_ciphers. May 3, 2018
This PR adapts the allowed encryption methods of nginx to the recommandation of Mozilla.
See https://wiki.mozilla.org/Security/Server_Side_TLS

Allowed ciphers are now:
openssl ciphers -v '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'
ECDHE-ECDSA-AES256-GCM-SHA384 TLSv1.2 Kx=ECDH     Au=ECDSA Enc=AESGCM(256) Mac=AEAD
ECDHE-RSA-AES256-GCM-SHA384 TLSv1.2 Kx=ECDH     Au=RSA  Enc=AESGCM(256) Mac=AEAD
ECDHE-ECDSA-CHACHA20-POLY1305 TLSv1.2 Kx=ECDH     Au=ECDSA Enc=ChaCha20-Poly1305 Mac=AEAD
ECDHE-RSA-CHACHA20-POLY1305 TLSv1.2 Kx=ECDH     Au=RSA  Enc=ChaCha20-Poly1305 Mac=AEAD
ECDHE-ECDSA-AES128-GCM-SHA256 TLSv1.2 Kx=ECDH     Au=ECDSA Enc=AESGCM(128) Mac=AEAD
ECDHE-RSA-AES128-GCM-SHA256 TLSv1.2 Kx=ECDH     Au=RSA  Enc=AESGCM(128) Mac=AEAD
ECDHE-ECDSA-AES256-SHA384 TLSv1.2 Kx=ECDH     Au=ECDSA Enc=AES(256)  Mac=SHA384
ECDHE-RSA-AES256-SHA384 TLSv1.2 Kx=ECDH     Au=RSA  Enc=AES(256)  Mac=SHA384
ECDHE-ECDSA-AES128-SHA256 TLSv1.2 Kx=ECDH     Au=ECDSA Enc=AES(128)  Mac=SHA256
ECDHE-RSA-AES128-SHA256 TLSv1.2 Kx=ECDH     Au=RSA  Enc=AES(128)  Mac=SHA256
@codecov-io
Copy link

codecov-io commented May 3, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@495a006). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #3606   +/-   ##
=========================================
  Coverage          ?   74.39%           
=========================================
  Files             ?      125           
  Lines             ?     5947           
  Branches          ?      381           
=========================================
  Hits              ?     4424           
  Misses            ?     1523           
  Partials          ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 495a006...8c73c41. Read the comment docs.

Copy link
Member

@sven-lange-last sven-lange-last left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

I suggest to add a comment directly to the nginx config template with a link to https://wiki.mozilla.org/Security/Server_Side_TLS such that future authors / readers have the background information.

Copy link
Contributor

@markusthoemmes markusthoemmes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@markusthoemmes markusthoemmes merged commit 0df22e9 into apache:master May 3, 2018
@cbickel cbickel deleted the tls branch May 3, 2018 14:12
@ningyougang
Copy link
Contributor

ningyougang commented May 11, 2018

oh. @cbickel @markusthoemmes , after rebased, some test cases can't be passed in my local build env. for example

apigw.healthtests.ApiGwRestEndToEndTests > initializationError FAILED
    org.scalatest.exceptions.TestFailedException: The future returned an exception of type: javax.net.ssl.SSLException, with message: Received fatal alert: handshake_failure.
        at org.scalatest.concurrent.Futures$FutureConcept$class.tryTryAgain$1(Futures.scala:523)
        at org.scalatest.concurrent.Futures$FutureConcept$class.futureValueImpl(Futures.scala:550)
        at org.scalatest.concurrent.ScalaFutures$$anon$1.futureValueImpl(ScalaFutures.scala:275)
        at org.scalatest.concurrent.Futures$FutureConcept$class.futureValue(Futures.scala:476)
        at org.scalatest.concurrent.ScalaFutures$$anon$1.futureValue(ScalaFutures.scala:275)
        at common.rest.RunWskRestCmd.requestEntity(WskRest.scala:1279)
        at common.rest.WskRestNamespace.list(WskRest.scala:888)
        at common.rest.WskRestNamespace.whois(WskRest.scala:901)
        at apigw.healthtests.ApiGwEndToEndTests.<init>(ApiGwEndToEndTests.scala:58)
        at apigw.healthtests.ApiGwRestEndToEndTests.<init>(ApiGwRestEndToEndTests.scala:34)

        Caused by:
        javax.net.ssl.SSLException: Received fatal alert: handshake_failure

If revert this commit, all test cases can be passed in my local build env.

So why in travis-ci is good, but in my local env has problem, i don't know the reason, do you know?
Need to install some package in our local build env.?
here is build env

$ uname -a
Linux xxxxxx 4.11.3-1.el7.elrepo.x86_64 #1 SMP Fri May 26 09:19:46 EDT 2017 x86_64 x86_64 x86_64 GNU/Linux

$ lsb_release -a
LSB Version:    :core-4.1-amd64:core-4.1-noarch
Distributor ID: CentOS
Description:    CentOS Linux release 7.2.1511 (Core) 
Release:        7.2.1511
Codename:       Core

$ java -version
openjdk version "1.8.0_102"
OpenJDK Runtime Environment (build 1.8.0_102-b14)
OpenJDK 64-Bit Server VM (build 25.102-b14, mixed mode)

@sven-lange-last
Copy link
Member

sven-lange-last commented May 11, 2018

@ningyougang you are using OpenJDK 1.8.0 update 102 which is from July 2016. It may not support the required ciphers and TLS v1.2 yet. Can you please try the latest update 172?

You can also find discussions in www around removed EC ciphers on RHEL / CentOS. This may also be the root cause of your problems.

@ningyougang
Copy link
Contributor

ningyougang commented May 16, 2018

@sven-lange-last ,thinks. already fixed.

Why that problem happened?
because the random key which is generated by client is very long, it is failed to use like ECDHE-ECDSA-AES256-GCM-SHA384 to decrypt it.
So should upgrade jdk version from jdk1.8.0_102 to jdk1.8.0_161(or more high version),there has a Critical configuration: crypto.policy=unlimited in <java_Home>/jre/lib/security/java.security (old jdk version didn't has this configuration)

refer to:
https://stackoverflow.com/questions/38203971/javax-net-ssl-sslhandshakeexception-received-fatal-alert-handshake-failure
https://confluence.atlassian.com/jirakb/sslhandshakeexception-received-fatal-alert-handshake_failure-due-to-no-overlap-in-cipher-suite-943544397.html

BillZong pushed a commit to BillZong/openwhisk that referenced this pull request Nov 18, 2019
Adapts the allowed encryption methods of nginx to the recommendation of Mozilla.
See https://wiki.mozilla.org/Security/Server_Side_TLS

Allowed ciphers are now:
openssl ciphers -v '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'
ECDHE-ECDSA-AES256-GCM-SHA384 TLSv1.2 Kx=ECDH     Au=ECDSA Enc=AESGCM(256) Mac=AEAD
ECDHE-RSA-AES256-GCM-SHA384 TLSv1.2 Kx=ECDH     Au=RSA  Enc=AESGCM(256) Mac=AEAD
ECDHE-ECDSA-CHACHA20-POLY1305 TLSv1.2 Kx=ECDH     Au=ECDSA Enc=ChaCha20-Poly1305 Mac=AEAD
ECDHE-RSA-CHACHA20-POLY1305 TLSv1.2 Kx=ECDH     Au=RSA  Enc=ChaCha20-Poly1305 Mac=AEAD
ECDHE-ECDSA-AES128-GCM-SHA256 TLSv1.2 Kx=ECDH     Au=ECDSA Enc=AESGCM(128) Mac=AEAD
ECDHE-RSA-AES128-GCM-SHA256 TLSv1.2 Kx=ECDH     Au=RSA  Enc=AESGCM(128) Mac=AEAD
ECDHE-ECDSA-AES256-SHA384 TLSv1.2 Kx=ECDH     Au=ECDSA Enc=AES(256)  Mac=SHA384
ECDHE-RSA-AES256-SHA384 TLSv1.2 Kx=ECDH     Au=RSA  Enc=AES(256)  Mac=SHA384
ECDHE-ECDSA-AES128-SHA256 TLSv1.2 Kx=ECDH     Au=ECDSA Enc=AES(128)  Mac=SHA256
ECDHE-RSA-AES128-SHA256 TLSv1.2 Kx=ECDH     Au=RSA  Enc=AES(128)  Mac=SHA256
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review Review for this PR has been requested and yet needs to be done.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants