Setting TLS1.2 minimum because TLS1.0 and TLS1.1 are vulnerable #26169

victorgp · 2016-05-24T14:38:28Z

TLS1.0 is known as vulnerable since it can be downgraded to SSL
https://blog.varonis.com/ssl-and-tls-1-0-no-longer-acceptable-for-pci-compliance/

TLS1.1 can be vulnerable if cipher RC4-SHA is used, and in Kubernetes it is, you can check it with
openssl s_client -cipher RC4-SHA -connect apiserver.k8s.example.com:443

https://www.globalsign.com/en/blog/poodle-vulnerability-expands-beyond-sslv3-to-tls/

Test suites like Qualys are reporting this Kubernetes issue as a level 3 vulnerability, they recommend to upgrade to TLS1.2 that is not affected, quoting Qualys:

RC4 should not be used where possible. One reason that RC4 was still being used was BEAST and Lucky13 attacks against CBC mode ciphers in SSL and TLS. However, TLSv 1.2 or later address these issues.

k8s-bot · 2016-05-24T14:38:49Z

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

k8s-bot · 2016-05-24T14:39:21Z

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

k8s-bot · 2016-05-24T14:40:24Z

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

lavalamp · 2016-05-24T22:45:12Z

cmd/kubelet/app/server.go

@@ -413,8 +413,8 @@ func InitializeTLS(s *options.KubeletServer) (*server.TLSOptions, error) {
 	}
 	tlsOptions := &server.TLSOptions{
 		Config: &tls.Config{
-			// Change default from SSLv3 to TLSv1.0 (because of POODLE vulnerability).
-			MinVersion: tls.VersionTLS10,
+			// Change default from SSLv3 to TLSv1.2 (because of POODLE vulnerability).


Please expand comment to list what is wrong with each version. i.e.

// Can't use SSLv3 because of POODLE // Can't use TLSv1.0 because of xxxx // Can't use TLSv1.1 because of xxxx

lavalamp · 2016-05-24T22:45:21Z

ok to test

lavalamp · 2016-05-24T22:45:40Z

@smarterclayton will this change break any openshift clients?

smarterclayton · 2016-05-24T22:49:47Z

No, we've been running this way for a while. We left an env var escape
hatch if an older cipher / protocol was required, but so far no one has
needed it. @liggitt.

On May 24, 2016, at 6:46 PM, Daniel Smith notifications@github.com wrote:

@smarterclayton https://github.com/smarterclayton will this change break
any openshift clients?

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#26169 (comment)

lavalamp · 2016-05-24T22:51:12Z

In that case, fix the comment and LGTM :)

On Tue, May 24, 2016 at 3:50 PM, Clayton Coleman notifications@github.com
wrote:

No, we've been running this way for a while. We left an env var escape
hatch if an older cipher / protocol was required, but so far no one has
needed it. @liggitt.

On May 24, 2016, at 6:46 PM, Daniel Smith notifications@github.com
wrote:

@smarterclayton https://github.com/smarterclayton will this change break
any openshift clients?

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
<
#26169 (comment)

—
You are receiving this because you were assigned.
Reply to this email directly or view it on GitHub
#26169 (comment)

victorgp · 2016-05-25T02:11:41Z

@lavalamp done!

liggitt · 2016-05-25T02:13:07Z

cmd/kubelet/app/server.go

+			// Can't use SSLv3 because of POODLE and BEAST
+			// Can't use TLSv1.0 because of POODLE and BEAST using CBC cipher
+			// Can't use TLSv1.1 because of RC4 cipher usage
+			MinVersion: tls.VersionTLS12,


MHBauer · 2016-05-25T05:39:56Z

Good comment.

smarterclayton · 2016-05-25T15:43:04Z

Looks good, can you squash?

Adding comments to explain what is wrong with each version

victorgp · 2016-05-25T16:15:41Z

@smarterclayton sure! done!

lavalamp · 2016-05-25T17:03:55Z

LGTM

lavalamp · 2016-05-25T17:04:42Z

I set the release note label-- we should announce this change.

wojtek-t · 2016-05-29T11:09:23Z

@k8s-bot e2e test this please issue: #IGNORE (there were problems with Jenkins at that time)

k8s-github-robot · 2016-05-29T19:35:04Z

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

k8s-bot · 2016-05-29T20:19:07Z

GCE e2e build/test passed for commit d3f3e6c.

k8s-github-robot · 2016-05-29T20:24:46Z

Automatic merge from submit-queue

dims · 2016-05-31T10:39:02Z

pkg/client/transport/transport.go

@@ -63,8 +63,10 @@ func TLSConfigFor(c *Config) (*tls.Config, error) {
 	}

 	tlsConfig := &tls.Config{
-		// Change default from SSLv3 to TLSv1.0 (because of POODLE vulnerability)
-		MinVersion:         tls.VersionTLS10,
+		// Can't use SSLv4 because of POODLE and BEAST


SSLv3. right? (minor nit)

Oops good catch! I just created this PR to fix it #26562

Automatic merge from submit-queue Minor typo in comment, SSLv3 instead of SSLv4 Minor fix in a comment from this PR #26169

In the following PR, TLS was set to 1.2 as the minimum because TLS1.0 and TLS1.1 are vulnerable: kubernetes#26169 However the scripts that used curl were not updated to match the TLS version. Since --tlsv1.2 was introduced in curl 7.34.0, we should check the version before using the option. Fixes kubernetes#28888

googlebot added the cla: yes label May 24, 2016

k8s-github-robot assigned vishh May 24, 2016

k8s-github-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. release-note-label-needed labels May 24, 2016

vishh assigned lavalamp and unassigned vishh May 24, 2016

lavalamp reviewed May 24, 2016
View reviewed changes

liggitt reviewed May 25, 2016
View reviewed changes

k8s-github-robot added the needs-ok-to-merge label May 25, 2016

smarterclayton added ok-to-merge and removed needs-ok-to-merge labels May 25, 2016

smarterclayton added this to the v1.3 milestone May 25, 2016

smarterclayton assigned smarterclayton and unassigned lavalamp May 25, 2016

Setting TLS1.2 minimum because TLS1.0 and TLS1.1 are vulnerable

d3f3e6c

Adding comments to explain what is wrong with each version

victorgp force-pushed the master branch from 2a9b3a9 to d3f3e6c Compare May 25, 2016 16:15

lavalamp added lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels May 25, 2016

lavalamp removed the release-note-label-needed label May 25, 2016

wojtek-t added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. labels May 29, 2016

k8s-github-robot merged commit 0fc5732 into kubernetes:master May 29, 2016

dims reviewed May 31, 2016
View reviewed changes

victorgp mentioned this pull request May 31, 2016

Minor typo in comment, SSLv3 instead of SSLv4 #26562

Merged

k8s-github-robot pushed a commit that referenced this pull request Jun 1, 2016

Merge pull request #26562 from victorgp/master

2976e89

Automatic merge from submit-queue Minor typo in comment, SSLv3 instead of SSLv4 Minor fix in a comment from this PR #26169

dims mentioned this pull request Jul 13, 2016

Use TLS 1.2 to access healthz API #28895

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Setting TLS1.2 minimum because TLS1.0 and TLS1.1 are vulnerable #26169

Setting TLS1.2 minimum because TLS1.0 and TLS1.1 are vulnerable #26169

victorgp commented May 24, 2016

k8s-bot commented May 24, 2016

k8s-bot commented May 24, 2016

k8s-bot commented May 24, 2016

lavalamp May 24, 2016

lavalamp commented May 24, 2016

lavalamp commented May 24, 2016

smarterclayton commented May 24, 2016

lavalamp commented May 24, 2016

victorgp commented May 25, 2016

liggitt May 25, 2016

MHBauer commented May 25, 2016

smarterclayton commented May 25, 2016

victorgp commented May 25, 2016

lavalamp commented May 25, 2016

lavalamp commented May 25, 2016

wojtek-t commented May 29, 2016

k8s-github-robot commented May 29, 2016

k8s-bot commented May 29, 2016

k8s-github-robot commented May 29, 2016

dims May 31, 2016

victorgp May 31, 2016

Setting TLS1.2 minimum because TLS1.0 and TLS1.1 are vulnerable #26169

Setting TLS1.2 minimum because TLS1.0 and TLS1.1 are vulnerable #26169

Conversation

victorgp commented May 24, 2016

k8s-bot commented May 24, 2016

k8s-bot commented May 24, 2016

k8s-bot commented May 24, 2016

lavalamp May 24, 2016

Choose a reason for hiding this comment

lavalamp commented May 24, 2016

lavalamp commented May 24, 2016

smarterclayton commented May 24, 2016

lavalamp commented May 24, 2016

victorgp commented May 25, 2016

liggitt May 25, 2016

Choose a reason for hiding this comment

MHBauer commented May 25, 2016

smarterclayton commented May 25, 2016

victorgp commented May 25, 2016

lavalamp commented May 25, 2016

lavalamp commented May 25, 2016

wojtek-t commented May 29, 2016

k8s-github-robot commented May 29, 2016

k8s-bot commented May 29, 2016

k8s-github-robot commented May 29, 2016

dims May 31, 2016

Choose a reason for hiding this comment

victorgp May 31, 2016

Choose a reason for hiding this comment