Use SecureTLSConfig() everywhere

[tiran]

The function SecureTLSConfig() of the name github.com/openshift/origin/pkg/cmd/server/crypto is used in several places to set safe and sane default settings of crypto/tls.TLSConfig. The default TLS version is only 1.2 instead of TLS 1.0, 1.1, and 1.2. Since PR #15400 SecureTLSConfig() also disables cipher suites with 3DES.

A couple of places in Origin's code base do not use the function, for example https://github.com/openshift/origin/blob/master/pkg/auth/ldaputil/client.go#L21 and https://github.com/openshift/origin/blob/master/pkg/cmd/server/kubernetes/master/master_config.go#L514 . To make it easier to apply a common policy for TLS settings and to increase security, Origin should use SecureTLSConfig() all over the place.

In order to apply default settings to all tls.Config structs, some refactoring is required. Otherwise we'd end up with a dependency cycle between pkg/cmd/util and cmd/server/crypto. IMHO cmd/util/crypto.go is a good place for SecureTLSConfig() and a couple of other functions. After all the are used outside the cmd/server namespace or just used by SecureTLSConfig():

• TLSVersion
• ValidTLSVersions
• DefaultTLSVersion
• CipherSuite
• ValidCipherSuites
• DefaultCiphers
• SecureTLSConfig

I found out about the issue while I was working on openshift/openshift-docs#4946 / https://trello.com/c/a9egV9TF/62-1-sccfsi-publicly-document-the-key-configurations-and-crypto-levels-in-openshifts-default-configuration

CC @openshift/sig-security

[liggitt]

That function is for setting TLS config for the server endpoints we start. The call sites you linked are for clients. We need to be cautious about tightening TLS config in clients that speak to servers we don't control (like the LDAP client)

[tiran]

Would it make sense to create a utils.SecureTLSClientConfig() function for all client side configurations? That way we can at least tighten cipher suite selection and make it easier to drop TLS 1.0 and 1.1 at a later point.

I was also concerned about LDAP. In fact I have an experimental branch on disk that uses VersionTLS10 for LDAP. Or would you rather see a new option in LDAPPasswordIdentityProvider?

[liggitt]

Would it make sense to create a utils.SecureTLSClientConfig() function for all client side configurations?

possibly. most of our clients' tls config are built using upstream methods, so I would start the discussion there, rather than try to wrap/fork the client builders.

I was also concerned about LDAP. In fact I have an experimental branch on disk that uses VersionTLS10 for LDAP. Or would you rather see a new option in LDAPPasswordIdentityProvider?

We definitely have to expose the option if we use non-defaults... there are some terribly old LDAP stacks people rely on.

[tiran]

@liggitt I discussed your suggestion with @simo5, @pweil- and @enj. We came to an agreement. I'll create an upstream issue and propose a mechanism to define a policy for TLS connections.

I'm also going to create an issue to look into LDAP settings for Origin.

[openshift-bot]

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

[openshift-bot]

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale