Bug 1844990: Bump GoLang to 1.15 for improved TLS Security #2303

darkmuggle · 2020-12-15T00:07:33Z

This change is needed in order to fix CVE-2016-2183.

With [1] TLS 1.3 is on-by-default which improves the security posture of
the MCO.

[1] golang/go#30055

Signed-off-by: Ben Howard ben.howard@redhat.com

darkmuggle · 2020-12-15T00:37:14Z

/retest

darkmuggle · 2020-12-15T21:15:34Z

/retest

kikisdeliveryservice

Do we have to change here too @sinnykumari :

machine-config-operator/machine-config-daemon.spec

Line 16 in 522f0fa

    
           BuildRequires:  %{?go_compiler:compiler(go-compiler)}%{!?go_compiler:golang >= 1.13}

Admittedly, I'm not super knowledgeable about this go-bump... would probably perfer @runcom to take a look next week if there's time before the merge.

kikisdeliveryservice · 2020-12-15T21:27:19Z

pkg/server/api.go

@@ -60,12 +60,23 @@ func NewAPIServer(a *APIHandler, p int, is bool, c, k string) *APIServer {

 // Serve launches the API Server.
 func (a *APIServer) Serve() {


can these code changes be broken out into a separate commit from the go upgrade?

I literally just pushed that commit :)

I updated the machines-config-daemon.spec too -- it would be required.

That file is dead btw, the MCD isn't part of RHCOS anymore.

I thought the spec file was dead, but wasn't 100% sure.

I literally just pushed that commit :)

great minds!

yeah as Colin said we no longer need and build machine-config-daemon rpm package. Even if someone wants to build m-c-d rpm package it will build fine with 1.13 because the current 1.15 requirement is added mainly to MCS.

darkmuggle · 2020-12-15T21:45:45Z

Admittedly, I'm not super knowledgeable about this go-bump... would probably perfer @runcom to take a look next week if there's time before the merge.

I spent the better part of an afternoon evaluating whether bumping the GoLang version was the better path to resolving the CVE. I concluded that moving to Go1.15 was better due to:

TLS 1.3 is on by default with downgradablilty to TLS 1.2, where on GoLang 1.13 it defaults to TLS 1.2. TLS 1.3 is a better security posture.
The cypto/tls code exports mappings of Ciphers from their definition to the common name. There's an RFE for use a TLS network policy and by using a newer GoLang we map the common cipher names without custom mapping code (see https://docs.openshift.com/container-platform/4.5/networking/ingress-operator.html#nw-ingress-controller-tls-profiles_configuring-ingress)
It brings us inline with the Kube Go which was bumped.

yuqi-zhang · 2020-12-15T22:03:55Z

Should also do the equivalent of this: openshift/release#8148 I believe

darkmuggle · 2020-12-15T23:13:08Z

openshift/release#14355 filed for the CI change.

darkmuggle · 2020-12-16T16:45:50Z

/retest

darkmuggle · 2020-12-16T19:09:44Z

/retest

The MCD is not longer distributed as an RPM.

darkmuggle · 2020-12-16T22:59:40Z

I dropped the TLS config for now. Turns out there is no easy way to have HTTP/2 and provide a list of approved ciphers. However, since we are using TLS and HTTP/2 only approved ciphers are allowed so this will drop the SHA hashing and 3DES ciphers.

darkmuggle · 2020-12-17T01:24:52Z

/retest

openshift-merge-robot · 2020-12-17T05:29:39Z

@darkmuggle: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/e2e-aws	`5aba0a9`	link	`/test e2e-aws`
ci/prow/okd-e2e-aws	`5aba0a9`	link	`/test okd-e2e-aws`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

sinnykumari · 2020-12-17T08:57:30Z

Too get this into 4.7, we will need a bug for go 1.15 version bump.
LGTM.
/approve

darkmuggle · 2020-12-21T16:58:04Z

/retest

sinnykumari · 2021-01-04T09:33:19Z

/retest

darkmuggle · 2021-01-04T15:27:44Z

/bugzilla refresh

openshift-ci-robot · 2021-01-04T15:27:45Z

@darkmuggle: No Bugzilla bug is referenced in the title of this pull request.
To reference a bug, add 'Bug XXX:' to the title of this pull request and request another bug refresh with /bugzilla refresh.

In response to this:

/bugzilla refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot · 2021-01-04T15:28:21Z

@darkmuggle: This pull request references Bugzilla bug 1844990, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.7.0) matches configured target release for branch (4.7.0)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1844990: Bump GoLang to 1.15 for improved TLS Security

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

darkmuggle · 2021-01-04T15:28:29Z

/bugzilla refresh

openshift-ci-robot · 2021-01-04T15:28:32Z

@darkmuggle: This pull request references Bugzilla bug 1844990, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.7.0) matches configured target release for branch (4.7.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

/bugzilla refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

yuqi-zhang

/lgtm

openshift-ci-robot · 2021-01-04T20:52:29Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: darkmuggle, sinnykumari, yuqi-zhang

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:

~~OWNERS~~ [sinnykumari,yuqi-zhang]

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

openshift-ci-robot · 2021-01-04T23:21:19Z

@darkmuggle: All pull requests linked via external trackers have merged:

Bugzilla bug 1844990 has been moved to the MODIFIED state.

In response to this:

Bug 1844990: Bump GoLang to 1.15 for improved TLS Security

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot requested review from kikisdeliveryservice and yuqi-zhang December 15, 2020 00:07

darkmuggle changed the title ~~GoLang: Bump from 1.13 to 1.15~~ Disable insecure ciphers and bump Go to 1.15 Dec 15, 2020

darkmuggle force-pushed the pr/CVE-2016-2183 branch from 6b9c30a to 87a2ae3 Compare December 15, 2020 01:08

darkmuggle force-pushed the pr/CVE-2016-2183 branch from 87a2ae3 to 7fa6056 Compare December 15, 2020 21:30

kikisdeliveryservice reviewed Dec 15, 2020

View reviewed changes

vendor: bump deps for moving to Go 1.15

b4237ca

darkmuggle force-pushed the pr/CVE-2016-2183 branch from 7fa6056 to 9c60b12 Compare December 15, 2020 21:37

darkmuggle mentioned this pull request Dec 15, 2020

MCO: bump golang to 1.15 in 4.7 and above release openshift/release#14355

Merged

Ben Howard added 2 commits December 16, 2020 15:55

machine-config-daemon.spec: drop dead file

ab2baba

The MCD is not longer distributed as an RPM.

Dockerfile: use GoLang 1.15

5aba0a9

darkmuggle force-pushed the pr/CVE-2016-2183 branch from 9c60b12 to 5aba0a9 Compare December 16, 2020 22:57

darkmuggle changed the title ~~Disable insecure ciphers and bump Go to 1.15~~ Bump GoLang to 1.15 for improved TLS Security Dec 16, 2020

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 17, 2020

darkmuggle changed the title ~~Bump GoLang to 1.15 for improved TLS Security~~ Bugzilla 1844990: Bump GoLang to 1.15 for improved TLS Security Dec 21, 2020

darkmuggle changed the title ~~Bugzilla 1844990: Bump GoLang to 1.15 for improved TLS Security~~ Bug 1844990: Bump GoLang to 1.15 for improved TLS Security Jan 4, 2021

openshift-ci-robot added bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Jan 4, 2021

yuqi-zhang approved these changes Jan 4, 2021

View reviewed changes

openshift-ci-robot assigned yuqi-zhang Jan 4, 2021

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 4, 2021

openshift-merge-robot merged commit d00d93c into openshift:master Jan 4, 2021

openshift-ci-robot mentioned this pull request Feb 25, 2021

Bug 1844990: server/api: Disable unsafe ciphers for CVE-2016-2183 #2420

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug 1844990: Bump GoLang to 1.15 for improved TLS Security #2303

Bug 1844990: Bump GoLang to 1.15 for improved TLS Security #2303

darkmuggle commented Dec 15, 2020 •

edited

darkmuggle commented Dec 15, 2020

darkmuggle commented Dec 15, 2020

kikisdeliveryservice left a comment

kikisdeliveryservice Dec 15, 2020

darkmuggle Dec 15, 2020

darkmuggle Dec 15, 2020

cgwalters Dec 15, 2020

darkmuggle Dec 15, 2020

kikisdeliveryservice Dec 15, 2020

sinnykumari Dec 16, 2020

darkmuggle commented Dec 15, 2020 •

edited

yuqi-zhang commented Dec 15, 2020

darkmuggle commented Dec 15, 2020

darkmuggle commented Dec 16, 2020

darkmuggle commented Dec 16, 2020

darkmuggle commented Dec 16, 2020

darkmuggle commented Dec 17, 2020

openshift-merge-robot commented Dec 17, 2020

sinnykumari commented Dec 17, 2020 •

edited

darkmuggle commented Dec 21, 2020

sinnykumari commented Jan 4, 2021

darkmuggle commented Jan 4, 2021

openshift-ci-robot commented Jan 4, 2021

openshift-ci-robot commented Jan 4, 2021

darkmuggle commented Jan 4, 2021

openshift-ci-robot commented Jan 4, 2021

yuqi-zhang left a comment

openshift-ci-robot commented Jan 4, 2021

openshift-ci-robot commented Jan 4, 2021

		@@ -60,12 +60,23 @@ func NewAPIServer(a APIHandler, p int, is bool, c, k string) APIServer {

		// Serve launches the API Server.
		func (a *APIServer) Serve() {

Bug 1844990: Bump GoLang to 1.15 for improved TLS Security #2303

Bug 1844990: Bump GoLang to 1.15 for improved TLS Security #2303

Conversation

darkmuggle commented Dec 15, 2020 • edited

darkmuggle commented Dec 15, 2020

darkmuggle commented Dec 15, 2020

kikisdeliveryservice left a comment

Choose a reason for hiding this comment

kikisdeliveryservice Dec 15, 2020

Choose a reason for hiding this comment

darkmuggle Dec 15, 2020

Choose a reason for hiding this comment

darkmuggle Dec 15, 2020

Choose a reason for hiding this comment

cgwalters Dec 15, 2020

Choose a reason for hiding this comment

darkmuggle Dec 15, 2020

Choose a reason for hiding this comment

kikisdeliveryservice Dec 15, 2020

Choose a reason for hiding this comment

sinnykumari Dec 16, 2020

Choose a reason for hiding this comment

darkmuggle commented Dec 15, 2020 • edited

yuqi-zhang commented Dec 15, 2020

darkmuggle commented Dec 15, 2020

darkmuggle commented Dec 16, 2020

darkmuggle commented Dec 16, 2020

darkmuggle commented Dec 16, 2020

darkmuggle commented Dec 17, 2020

openshift-merge-robot commented Dec 17, 2020

sinnykumari commented Dec 17, 2020 • edited

darkmuggle commented Dec 21, 2020

sinnykumari commented Jan 4, 2021

darkmuggle commented Jan 4, 2021

openshift-ci-robot commented Jan 4, 2021

openshift-ci-robot commented Jan 4, 2021

darkmuggle commented Jan 4, 2021

openshift-ci-robot commented Jan 4, 2021

yuqi-zhang left a comment

Choose a reason for hiding this comment

openshift-ci-robot commented Jan 4, 2021

openshift-ci-robot commented Jan 4, 2021

darkmuggle commented Dec 15, 2020 •

edited

darkmuggle commented Dec 15, 2020 •

edited

sinnykumari commented Dec 17, 2020 •

edited