Skip to content

OCPBUGS-1694: pkg/cvo/availableupdates: Clear TLS config for *.cluster.local Cincinnati #927

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wking wants to merge 1 commit into from
Closed

OCPBUGS-1694: pkg/cvo/availableupdates: Clear TLS config for *.cluster.local Cincinnati #927

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions pkg/cvo/availableupdates.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,10 @@ func calculateAvailableUpdatesStatus(ctx context.Context, clusterID string, tran
}
}

if strings.HasSuffix(upstreamURI.Hostname(), ".cluster.local") {
transport.TLSClientConfig = nil
}

Comment on lines +237 to +240

@DavidHurta DavidHurta Aug 30, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may instead of this "hack" utilize the ConfigMap default-ingress-cert from the openshift-config-managed namespace. For more information, here's the enhancement describing the ConfigMap (for even more curious folks, the Ingress certificates Documentation contains some general information and workflow).

From the enhancement:

The ingress operator publishes the default certificate of the default IngressController in a ConfigMap for other operators to consume. This ConfigMap is named default-ingress-cert and exists in the openshift-config-managed namespace. The intended consumers are other operators that need to incorporate the default certificate into their trust bundles in order to connect to Route resources.

Regarding a potential fix to our bug:

The standard way to validate a certificate is to verify that the certificate is signed by a trusted CA certificate. Consumers therefore may expect the default-ingress-cert ConfigMap to include the CA certificate that signed the default certificate rather than the default certificate itself.

For Go-based clients, this is not a problem as the Go TLS implementation has looser certificate validation that can be satisfied by configuring the certificate itself in the trusted certificates pool. As the ConfigMap is not intended to be used outside of OpenShift's own operators, which are Go-based, publishing the certificate itself should not pose a problem. Furthermore, the default-ingress-cert ConfigMap is an internal API, and to the extent that we document it at all, we should document that it has the default certificate, not the signing CA certificate.

So maybe we can configure the func (optr *Operator) getTLSConfig() (*tls.Config, error) method to include the default ingress CA bundle in the root CAs.

Something like this could be added to the method (not tested):

	cm, err = optr.cmConfigManagedLister.Get("default-ingress-cert")
	if apierrors.IsNotFound(err) {
		return nil, nil
	}
	if err != nil {
		return nil, err
	}

	if cm.Data["ca-bundle.crt"] != "" {
		if ok := certPool.AppendCertsFromPEM([]byte(cm.Data["ca-bundle.crt"])); !ok {
			return nil, fmt.Errorf("unable to add ca-bundle.crt certificates")
		}
	} else {
		return nil, nil
	}

Testing the idea out using curl seems to work.

$ # setup env variables
$ NAMESPACE=openshift-update-service
$ NAME=sample
$ POLICY_ENGINE_GRAPH_URI="$(oc -n "${NAMESPACE}" get -o jsonpath='{.status.policyEngineURI}/api/upgrades_info/v1/graph{"\n"}' updateservice "${NAME}")"
$
$ # extract the default ingress CA bundle to the local machine
$ oc extract -n openshift-config-managed configmap/default-ingress-cert 
$
$ # copy the default ingress's CA bundle to the CVO container
$ oc cp ca-bundle.crt "openshift-cluster-version/$(oc get po -n openshift-cluster-version -o=name | sed "s/pod\///g"):/tmp/default-ingress-cert-ca-bundle.crt"
$
$ # running curl without specifying a CA bundle - self signed certificate in certificate chain error
$ oc exec -n openshift-cluster-version deployments/cluster-version-operator -- curl   "$POLICY_ENGINE_GRAPH_URI?channel=stable-4.10" 
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
curl: (60) SSL certificate problem: self signed certificate in certificate chain
More details here: https://curl.haxx.se/docs/sslcerts.html

curl failed to verify the legitimacy of the server and therefore could not
establish a secure connection to it. To learn more about this situation and
how to fix it, please visit the web page mentioned above.
command terminated with exit code 60
$ 
$ # running curl using the default ingress CA bundle - success
$ oc exec -n openshift-cluster-version deployments/cluster-version-operator -- curl --cacert /tmp/default-ingress-cert-ca-bundle.crt  "$POLICY_ENGINE_GRAPH_URI?channel=stable-4.10" 
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0{"version":1,"nodes":[{"version":"4.9.28","payload":"quay.io/openshift-release-dev/ocp-release@sha256:4084d94969b186e20189649b5affba7da59f7d1943e4e5bc7ef78b981eafb7a8","metadata":{"io.openshift.upgrades.graph.release.channels":"candidate-4.10,fast-4.10,stable-4.10,candidate-4.9,fast-4.9,stable-4.9","io.openshift.upgrades.graph.release.manifestref":"sha256:4084d94969b186e20189649b5affba7da59f7d1943e4e5bc7ef78b981eafb7a8","url":"https://access.redhat.com/errata/RHBA-2022:1245"}},{"version":"4.10.20","payload":"quay.io/openshift-release-dev/ocp-release@sha256:b89ada9261a1b257012469e90d7d4839d0d2f99654f5ce76394fa3f06522b600","metadata":{"io.openshift.upgrades.graph.release.channels":"candidate-4.10,eus-4.10,fast-4.10,stable-4.10,candidate-4.11,fast-4.11,stable-4.11,eus-4.12","io.openshift.upgrades.graph.release.manifestref":"sha256:b89ada9261a1b257012469e90d7d4839d0d2f99654f5ce76394fa3f06522b600","url":"https://access.redhat.com/errata/RHBA-2022:5172"}},{"version":"4.9.21","payload":"quay.io/openshift-release-dev/ocp-release@sha256:fd96300600f9585e5847f5855ca14e2b3cafbce12aefe3b3f52c5da10c4476eb","metadata":{"io.openshift.upgrades.graph.previous.remove_regex":"4\\.8\\..*","io.openshift.upgrades.graph.release.channels":"candidate-4.10,fast-4.10,stable-4.10,candidate-4.9...

Worth pointing out from the commit message:

It seems like this would be a generic issue that could be sorted at the network operator's (c), for all Proxy-consuming resources who might want to reach back into the cluster via the Ingress router. But as a cheap hack until something like that happens, users can use:

http://${POLICY_ENGINE_SERVICE_NAME}.${NAMESPACE}.svc.cluster.local/api/upgrades_info/graph

to bypass TLS. Or, if they'd rather use encrypted communication, this commit allows them to use:

 https://${POLICY_ENGINE_SERVICE_NAME}.${NAMESPACE}.svc.cluster.local/api/upgrades_info/graph

We have lost the ability to resolve service DNS names in #920. Unfortunately, this wouldn't probably work now 😢.

$ oc exec -n openshift-cluster-version deployments/cluster-version-operator -- curl   "https://sample-policy-engine.openshift-update-service.svc.cluster.local:80/api/upgrades_info/graph?channel=stable-4.10" 
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0curl: (6) Could not resolve host: sample-policy-engine.openshift-update-service.svc.cluster.local
command terminated with exit code 6

uuid, err := uuid.Parse(string(clusterID))
if err != nil {
return cvoCurrent, nil, nil, configv1.ClusterOperatorStatusCondition{
Expand Down