Possible cipher suites values and tls versions in help for apiserver and kubelet #58920

victorgp · 2018-01-27T20:24:14Z

What this PR does / why we need it:
Addresses a suggestion made by @lavalamp to list the possible TLS cipher suites in the kubelet and apiserver helps: #48859 (comment)

Which issue(s) this PR fixes
NONE

Special notes for your reviewer:
This pull request only adds to the help message the possible values of the TLS Cipher suites for Kubelet and api server.
It is an addition to the already merged PR #48859

The help output looks like this:

      --tls-cert-file string                                    File containing the default x509 Certificate for HTTPS. (CA cert, if any, concatenated after server cert). If HTTPS serving is enabled, and --tls-cert-file and --tls-private-key-file are not provided, a self-signed certificate and key are generated for the public address and saved to the directory specified by --cert-dir.
      --tls-cipher-suites strings                               Comma-separated list of cipher suites for the server. Values are from tls package constants (https://golang.org/pkg/crypto/tls/#pkg-constants). If omitted, the default Go cipher suites will be use.  Possible values: TLS_ECDHE_ECDSA_WITH_RC4_128_SHA,TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA,TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256,TLS_RSA_WITH_RC4_128_SHA,TLS_RSA_WITH_AES_256_CBC_SHA,TLS_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA,TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_RSA_WITH_3DES_EDE_CBC_SHA,TLS_RSA_WITH_AES_128_CBC_SHA,TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305,TLS_RSA_WITH_AES_128_CBC_SHA256,TLS_ECDHE_RSA_WITH_RC4_128_SHA,TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,TLS_RSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA
      --tls-min-version string                                  Minimum TLS version supported. Value must match version names from https://golang.org/pkg/crypto/tls/#pkg-constants.
      --tls-private-key-file string                             File containing the default x509 private key matching --tls-cert-file.

Release note:

NONE

liggitt · 2018-01-27T20:41:43Z

staging/src/k8s.io/apiserver/pkg/util/flag/ciphersuites_flag.go

@@ -48,6 +50,15 @@ var ciphers = map[string]uint16{
 	"TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305":  tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305,
 }

+func TLSCipherPossibleValues() string {
+	cipherKeys := reflect.ValueOf(ciphers).MapKeys()


Prefer gathering this without reflection. Iterate, insert into a sets.NewString(), and return the result of calling List() on the string set. That returns a []string and lets the caller decide how to render the list (prefer joining with ", " to allow wrapping help)

While you're in here, can you do the same for the TLSMinVersion flags?

liggitt · 2018-01-27T20:46:48Z

cmd/kubelet/app/options/options.go

@@ -442,10 +442,13 @@ func AddKubeletConfigFlags(fs *pflag.FlagSet, c *kubeletconfig.KubeletConfigurat
 		"If --tls-cert-file and --tls-private-key-file are not provided, a self-signed certificate and key "+
 		"are generated for the public address and saved to the directory passed to --cert-dir.")
 	fs.StringVar(&c.TLSPrivateKeyFile, "tls-private-key-file", c.TLSPrivateKeyFile, "File containing x509 private key matching --tls-cert-file.")
+
+	tlsCipherPossibleValues := flag.TLSCipherPossibleValues()
 	fs.StringSliceVar(&c.TLSCipherSuites, "tls-cipher-suites", c.TLSCipherSuites,
 		"Comma-separated list of cipher suites for the server. "+
 			"Values are from tls package constants (https://golang.org/pkg/crypto/tls/#pkg-constants). "+


If we're enumerating the valid values, I think we can drop the golang package reference completely

victorgp · 2018-01-30T06:14:07Z

@liggitt changes done, for both cipher suites and tls versions

liggitt · 2018-01-31T04:18:15Z

thanks
/ok-to-test
/lgtm

liggitt · 2018-01-31T04:58:15Z

/lgtm

liggitt · 2018-03-13T21:05:24Z

yes, once master opens up after 1.10

liggitt · 2018-03-23T13:03:31Z

@derekwaynecarr friendly ping

anjuls · 2018-05-30T03:48:43Z

Hi Guys,
is this going to be closed anytime soon? I am facing issue with security and I have to replace DES cipher from kubelet/api-server. I am using 1.10 right now.

Thanks
Anjul

victorgp · 2018-05-30T04:38:06Z

This is only the help improvement but the actual code to set custom ciphers was already merged
@liggitt what version will it be included in?

liggitt · 2018-05-30T04:44:48Z

v1.10.0 contains the cipher-setting flags

victorgp · 2018-05-30T04:59:19Z

It seems that the flag is only in the api-server but not the kubelet, how could it get lost? the code is (or was at least) there

liggitt · 2018-05-30T05:11:35Z

It seems that the flag is only in the api-server but not the kubelet, how could it get lost? the code is (or was at least) there

1.10 also enabled running the kubelet via a config file, and marked many flags as deprecated in the process, which hid them from help, though they are still functional

see #62505 for resolution of showing those flags in help (a backport to fix the help display in 1.10 is pending in #63448)

liggitt · 2018-05-30T05:14:19Z

@victorgp can you rebase this? I'll work on getting it approved this week.

…and kubelet

victorgp · 2018-05-30T05:25:16Z

rebased

victorgp · 2018-05-30T06:17:16Z

/retest

derekwaynecarr · 2018-05-30T20:04:52Z

/approve

liggitt · 2018-05-30T20:07:07Z

/lgtm

k8s-ci-robot · 2018-05-30T20:07:16Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: derekwaynecarr, liggitt, victorgp

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~cmd/kubelet/OWNERS~~ [derekwaynecarr]
~~staging/src/k8s.io/apiserver/OWNERS~~ [liggitt]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

liggitt · 2018-05-30T20:09:04Z

/milestone v1.11
/status approved-for-milestone
/sig node
/sig api-machinery

this is closing a documentation gap in the custom cipher/tls config added in 1.10

k8s-github-robot · 2018-05-30T20:09:17Z

[MILESTONENOTIFIER] Milestone Pull Request Labels Incomplete

@derekwaynecarr @liggitt @victorgp

Action required: This pull request requires label changes. If the required changes are not made within 3 days, the pull request will be moved out of the v1.11 milestone.

kind: Must specify exactly one of kind/bug, kind/cleanup or kind/feature.
priority: Must specify exactly one of priority/critical-urgent, priority/important-longterm or priority/important-soon.

Help

k8s-github-robot · 2018-05-30T23:09:19Z

Automatic merge from submit-queue (batch tested with PRs 58920, 58327, 60577, 49388, 62306). If you want to cherry-pick this change to another branch, please follow the instructions here.

k8s-ci-robot requested review from enj, tmrts and wojtek-t January 27, 2018 20:24

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jan 27, 2018

victorgp mentioned this pull request Jan 27, 2018

Support for custom tls cipher suites in api server and kubelet #48859

Merged

liggitt reviewed Jan 27, 2018

View reviewed changes

victorgp force-pushed the master branch from 668fd3e to 77e17cd Compare January 30, 2018 06:06

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 30, 2018

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 30, 2018

victorgp force-pushed the master branch from 77e17cd to a450204 Compare January 30, 2018 06:13

k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 30, 2018

victorgp changed the title ~~Possible cipher values in help~~ Possible cipher suites values and tls versions in help for apiserver and kubelet Jan 30, 2018

k8s-ci-robot assigned liggitt Jan 31, 2018

victorgp force-pushed the master branch from a450204 to 3b421bd Compare January 31, 2018 04:42

k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 31, 2018

victorgp force-pushed the master branch from 3b421bd to e062bec Compare January 31, 2018 04:48

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 31, 2018

victorgp force-pushed the master branch from e062bec to 9aeb546 Compare January 31, 2018 05:47

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 26, 2018

Possible cipher suites values and tls versions in help for apiserver …

3dfa22e

…and kubelet

victorgp force-pushed the master branch from 9aeb546 to 3dfa22e Compare May 30, 2018 05:24

k8s-ci-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels May 30, 2018

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 30, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 30, 2018

k8s-ci-robot added status/approved-for-milestone sig/node Categorizes an issue or PR as relevant to SIG Node. labels May 30, 2018

k8s-ci-robot added this to the v1.11 milestone May 30, 2018

k8s-ci-robot added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label May 30, 2018

k8s-github-robot added the milestone/incomplete-labels label May 30, 2018

k8s-github-robot merged commit 22919ae into kubernetes:master May 30, 2018

liggitt mentioned this pull request Jul 17, 2018

TLS Configuration #2799

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Possible cipher suites values and tls versions in help for apiserver and kubelet #58920

Possible cipher suites values and tls versions in help for apiserver and kubelet #58920

victorgp commented Jan 27, 2018 •

edited by liggitt

Loading

liggitt Jan 27, 2018 •

edited

Loading

liggitt Jan 27, 2018

liggitt Jan 27, 2018

victorgp commented Jan 30, 2018

liggitt commented Jan 31, 2018

liggitt commented Jan 31, 2018

liggitt commented Mar 13, 2018

liggitt commented Mar 23, 2018

anjuls commented May 30, 2018 •

edited

Loading

victorgp commented May 30, 2018

liggitt commented May 30, 2018

victorgp commented May 30, 2018 •

edited

Loading

liggitt commented May 30, 2018 •

edited

Loading

liggitt commented May 30, 2018

victorgp commented May 30, 2018

victorgp commented May 30, 2018

derekwaynecarr commented May 30, 2018

liggitt commented May 30, 2018

k8s-ci-robot commented May 30, 2018

liggitt commented May 30, 2018

k8s-github-robot commented May 30, 2018

k8s-github-robot commented May 30, 2018

Possible cipher suites values and tls versions in help for apiserver and kubelet #58920

Possible cipher suites values and tls versions in help for apiserver and kubelet #58920

Conversation

victorgp commented Jan 27, 2018 • edited by liggitt Loading

liggitt Jan 27, 2018 • edited Loading

Choose a reason for hiding this comment

liggitt Jan 27, 2018

Choose a reason for hiding this comment

liggitt Jan 27, 2018

Choose a reason for hiding this comment

victorgp commented Jan 30, 2018

liggitt commented Jan 31, 2018

liggitt commented Jan 31, 2018

liggitt commented Mar 13, 2018

liggitt commented Mar 23, 2018

anjuls commented May 30, 2018 • edited Loading

victorgp commented May 30, 2018

liggitt commented May 30, 2018

victorgp commented May 30, 2018 • edited Loading

liggitt commented May 30, 2018 • edited Loading

liggitt commented May 30, 2018

victorgp commented May 30, 2018

victorgp commented May 30, 2018

derekwaynecarr commented May 30, 2018

liggitt commented May 30, 2018

k8s-ci-robot commented May 30, 2018

liggitt commented May 30, 2018

k8s-github-robot commented May 30, 2018

k8s-github-robot commented May 30, 2018

victorgp commented Jan 27, 2018 •

edited by liggitt

Loading

liggitt Jan 27, 2018 •

edited

Loading

anjuls commented May 30, 2018 •

edited

Loading

victorgp commented May 30, 2018 •

edited

Loading

liggitt commented May 30, 2018 •

edited

Loading