search

openshift / origin-server

OpenShift 2 (deprecated)
889 stars 516 forks source link

Remove RC4 from allowed SSL ciphers #6349

Closed tiwillia closed 8 years ago

tiwillia commented 8 years ago

Bug 1299014 https://bugzilla.redhat.com/show_bug.cgi?id=1299014

Added a configuration option in the node-web-proxy json to specify the list of ciphers. The list of ciphers specified by default is the same cipher suite list that is used on the broker's proxy as well as both provided frontends:

broker/httpd/000002_openshift_origin_broker_proxy.conf:  
SSLCipherSuite kEECDH:+kEECDH+SHA:kEDH:+kEDH+SHA:+kEDH+CAMELLIA:kECDH:+kECDH+SHA:kRSA:+kRSA+SHA:+kRSA+CAMELLIA:!aNULL:!eNULL:!SSLv2:!RC4:!DES:!EXP:!SEED:!IDEA:+3DES

plugins/frontend/apache-vhost/httpd/000001_openshift_origin_frontend_vhost.conf:
SSLCipherSuite kEECDH:+kEECDH+SHA:kEDH:+kEDH+SHA:+kEDH+CAMELLIA:kECDH:+kECDH+SHA:kRSA:+kRSA+SHA:+kRSA+CAMELLIA:!aNULL:!eNULL:!SSLv2:!RC4:!DES:!EXP:!SEED:!IDEA:+3DES

/plugins/frontend/apache-mod-rewrite/httpd/000001_openshift_origin_node.conf:
SSLCipherSuite kEECDH:+kEECDH+SHA:kEDH:+kEDH+SHA:+kEDH+CAMELLIA:kECDH:+kECDH+SHA:kRSA:+kRSA+SHA:+kRSA+CAMELLIA:!aNULL:!eNULL:!SSLv2:!RC4:!DES:!EXP:!SEED:!IDEA:+3DES

Additionally, disabling forcing the use of sslv2_3, since sslv2 has been known to be insecure for some time.

tiwillia commented 8 years ago

[test][extended:node]

@Miciah PTAL @thrasher-redhat fyi

sferich888 commented 8 years ago

@tiwillia so were going to limit the node-web-proxy to only AES256-GCM-SHA384 ciphers, and not allow or fix the issue define in https://bugzilla.redhat.com/show_bug.cgi?id=1299014 to make the cipher that this service uses by default, configurable?

rjhowe commented 8 years ago

The ciphers need to be all listed if we are defining these in the code.

After the change adding ssl_opts.ciphers = "AES256-GCM-SHA384:!RC4"

[root@node1 ~]# nmap --script ssl-cert,ssl-enum-ciphers -p 8443 localhost

Starting Nmap 5.51 ( http://nmap.org ) at 2016-01-19 18:01 EST
Nmap scan report for localhost (127.0.0.1)
Host is up (0.00014s latency).
Other addresses for localhost (not scanned): 127.0.0.1
PORT     STATE SERVICE
8443/tcp open  https-alt
| ssl-cert: Subject: commonName=*.example.com/organizationName=OpenShift Enterprise default/stateOrProvinceName=SomeState/countryName=XX
| Issuer: commonName=*.example.com/organizationName=OpenShift Enterprise default/stateOrProvinceName=SomeState/countryName=XX
| Public Key type: rsa
| Public Key bits: 2048
| Not valid before: 2015-10-15 19:18:36
| Not valid after:  2025-10-12 19:18:36
| MD5:   0a64 16e5 29e1 af48 6c02 c78c b155 1d1a
|_SHA-1: 795b a647 13b8 9367 7c00 bc74 b769 d3af 894c 5e92
| ssl-enum-ciphers: 
|   TLSv1.2
|     Ciphers (1)
|       TLS_RSA_WITH_AES_256_GCM_SHA384
|     Compressors (1)
|_      uncompressed

Nmap done: 1 IP address (1 host up) scanned in 1.70 seconds

Before the Change 

[root@node1 ~]# nmap --script ssl-cert,ssl-enum-ciphers -p 8443 localhost

Starting Nmap 5.51 ( http://nmap.org ) at 2016-01-19 17:59 EST
Nmap scan report for localhost (127.0.0.1)
Host is up (0.00017s latency).
Other addresses for localhost (not scanned): 127.0.0.1
PORT     STATE SERVICE
8443/tcp open  https-alt
| ssl-cert: Subject: commonName=*.example.com/organizationName=OpenShift Enterprise default/stateOrProvinceName=SomeState/countryName=XX
| Issuer: commonName=*.example.com/organizationName=OpenShift Enterprise default/stateOrProvinceName=SomeState/countryName=XX
| Public Key type: rsa
| Public Key bits: 2048
| Not valid before: 2015-10-15 19:18:36
| Not valid after:  2025-10-12 19:18:36
| MD5:   0a64 16e5 29e1 af48 6c02 c78c b155 1d1a
|_SHA-1: 795b a647 13b8 9367 7c00 bc74 b769 d3af 894c 5e92
| ssl-enum-ciphers: 
|   TLSv1.0
|     Ciphers (6)
|       TLS_RSA_WITH_3DES_EDE_CBC_SHA
|       TLS_RSA_WITH_AES_128_CBC_SHA
|       TLS_RSA_WITH_AES_256_CBC_SHA
|       TLS_RSA_WITH_CAMELLIA_128_CBC_SHA
|       TLS_RSA_WITH_CAMELLIA_256_CBC_SHA
|       TLS_RSA_WITH_RC4_128_SHA
|     Compressors (1)
|       uncompressed
|   TLSv1.1
|     Ciphers (6)
|       TLS_RSA_WITH_3DES_EDE_CBC_SHA
|       TLS_RSA_WITH_AES_128_CBC_SHA
|       TLS_RSA_WITH_AES_256_CBC_SHA
|       TLS_RSA_WITH_CAMELLIA_128_CBC_SHA
|       TLS_RSA_WITH_CAMELLIA_256_CBC_SHA
|       TLS_RSA_WITH_RC4_128_SHA
|     Compressors (1)
|       uncompressed
|   TLSv1.2
|     Ciphers (10)
|       TLS_RSA_WITH_3DES_EDE_CBC_SHA
|       TLS_RSA_WITH_AES_128_CBC_SHA
|       TLS_RSA_WITH_AES_128_CBC_SHA256
|       TLS_RSA_WITH_AES_128_GCM_SHA256
|       TLS_RSA_WITH_AES_256_CBC_SHA
|       TLS_RSA_WITH_AES_256_CBC_SHA256
|       TLS_RSA_WITH_AES_256_GCM_SHA384
|       TLS_RSA_WITH_CAMELLIA_128_CBC_SHA
|       TLS_RSA_WITH_CAMELLIA_256_CBC_SHA
|       TLS_RSA_WITH_RC4_128_SHA
|     Compressors (1)
|_      uncompressed

Nmap done: 1 IP address (1 host up) scanned in 1.67 seconds
tiwillia commented 8 years ago

@rjhowe 

# nmap --script ssl-cert,ssl-enum-ciphers -p 8443 localhost

Starting Nmap 5.51 ( http://nmap.org ) at 2016-01-20 10:48 EST
Nmap scan report for localhost (127.0.0.1)
Host is up (0.000040s latency).
rDNS record for 127.0.0.1: localhost.localdomain
PORT     STATE SERVICE
8443/tcp open  https-alt
| ssl-cert: Subject: commonName=ip-172-18-15-112/organizationName=SomeOrganization/stateOrProvinceName=SomeState/countryName=--
| Issuer: commonName=ip-172-18-15-112/organizationName=SomeOrganization/stateOrProvinceName=SomeState/countryName=--
| Public Key type: rsa
| Public Key bits: 2048
| Not valid before: 2016-01-15 07:49:45
| Not valid after:  2017-01-14 07:49:45
| MD5:   46cb f275 d7f3 cd22 9764 cfee 20a8 5ad9
|_SHA-1: 9078 75b5 3a17 fff1 0350 e124 c086 4441 2b6e 4ace
| ssl-enum-ciphers: 
|   TLSv1.0
|     Ciphers (5)
|       TLS_RSA_WITH_3DES_EDE_CBC_SHA
|       TLS_RSA_WITH_AES_128_CBC_SHA
|       TLS_RSA_WITH_AES_256_CBC_SHA
|       TLS_RSA_WITH_CAMELLIA_128_CBC_SHA
|       TLS_RSA_WITH_CAMELLIA_256_CBC_SHA
|     Compressors (1)
|       uncompressed
|   TLSv1.1
|     Ciphers (5)
|       TLS_RSA_WITH_3DES_EDE_CBC_SHA
|       TLS_RSA_WITH_AES_128_CBC_SHA
|       TLS_RSA_WITH_AES_256_CBC_SHA
|       TLS_RSA_WITH_CAMELLIA_128_CBC_SHA
|       TLS_RSA_WITH_CAMELLIA_256_CBC_SHA
|     Compressors (1)
|       uncompressed
|   TLSv1.2
|     Ciphers (9)
|       TLS_RSA_WITH_3DES_EDE_CBC_SHA
|       TLS_RSA_WITH_AES_128_CBC_SHA
|       TLS_RSA_WITH_AES_128_CBC_SHA256
|       TLS_RSA_WITH_AES_128_GCM_SHA256
|       TLS_RSA_WITH_AES_256_CBC_SHA
|       TLS_RSA_WITH_AES_256_CBC_SHA256
|       TLS_RSA_WITH_AES_256_GCM_SHA384
|       TLS_RSA_WITH_CAMELLIA_128_CBC_SHA
|       TLS_RSA_WITH_CAMELLIA_256_CBC_SHA
|     Compressors (1)
|_      uncompressed

Nmap done: 1 IP address (1 host up) scanned in 0.52 seconds
tiwillia commented 8 years ago

@Miciah @sferich888 Whats the consensus on this? Is it ready for merge?

Do you think we should be marking this as a configuration file in the rpm spec from here-on-out?

Miciah commented 8 years ago

If we add %config this release, we will need to make sure web-proxy-config.json still gets updated, or that ciphers gets a reasonable default value even if it isn't set in web-proxy-config.json.

My preference is that we specify a default value in the assignment to ssl_opts.ciphers in http-utils.js. It would be nice also to specify %config web-proxy-config.json in the spec file.

tiwillia commented 8 years ago

Specifying a default for just this parameter is inconsistent, but I agree that it is the only option if we are going to mark this as a configuration file. I'll work on that, thanks.

tiwillia commented 8 years ago

[test]

tiwillia commented 8 years ago

@Miciah could you give my latest changes a look when you have a chance?

tiwillia commented 8 years ago

@Miciah should be ready for your review one more time.

Miciah commented 8 years ago

Looks good! I can tag it for merge after the tests run.

openshift-bot commented 8 years ago

Evaluated for online test up to 7dec6165f2e8d3ac3c7bf3b8b5dfaf7adfc7aa6e

openshift-bot commented 8 years ago

Online Test Results: SUCCESS (https://ci.dev.openshift.redhat.com/jenkins/job/test_pull_requests/9169/) (Extended Tests: node)

Miciah commented 8 years ago

Please [merge]!

openshift-bot commented 8 years ago

Online Merge Results: SUCCESS (https://ci.dev.openshift.redhat.com/jenkins/job/merge_pull_requests/6695/) (Image: devenv_5757)

openshift-bot commented 8 years ago

Evaluated for online merge up to 7dec6165f2e8d3ac3c7bf3b8b5dfaf7adfc7aa6e

tiwillia commented 8 years ago

@Miciah Bug opened for the configuration file issue: https://bugzilla.redhat.com/show_bug.cgi?id=1302787

Miciah commented 8 years ago

Thanks! ++@tiwillia