Bug 1788712: publish a router-ca that can be used to verify routes in golang clients

[deads2k]

An explanation in code of how to fill in the router-ca.

Immediate mission: allow the default-ingress-ca to always contain a ca-bundle.crt that golang clients can use to validate the ingress wildcard certificate for the default ingresscontroller. To do this

1. if you are not oc get -n openshift-ingress-operator ingresscontroller/default, do not publish
2. if the spec.DefaultCertificate.Name is set, use the .data["tls.crt "] value in that secret to the router-ca.data["ca-bundle.crt"]
3. if it is not set, use the ensureRouterCASecret secret's .data.["tls.crt"]

If we can agree to this approach to solve the immediate 4.1, 4.2, and 4.3 problem, then we can update the enhancement.

Future mission: it may be useful to have a ca bundle that can terminate any ingress wildcard. This seems simple, but it has security repercussions that would effectively be elevating privilege for every ingress-controller. We have to decide if this ingress-controllers are system trusted or not. Those that are not may not update this configmap. I suspect this is already a security hole.

[Miciah]

This should be secret.Data["tls.crt"], right?

[deads2k]

fixed

[danehans]

it may be useful to have a ca bundle that can terminate any ingress wildcard. This seems simple, but it has security repercussions that would effectively be elevating privilege for every ingress-controller.

Why is the default ingress controller more secure than any other ingress controller?

[Miciah]

Why is the default ingress controller more secure than any other ingress controller?

On a typical cluster, it is more sensitive because it has the domain that is used for OAuth.

[deads2k]

it may be useful to have a ca bundle that can terminate any ingress wildcard. This seems simple, but it has security repercussions that would effectively be elevating privilege for every ingress-controller.

Why is the default ingress controller more secure than any other ingress controller?

It's created and managed by the platform. The rest are not.

[Miciah]

We have to decide if this ingress-controllers are system trusted or not.

The cluster ingress operator deployment only manages ingresscontrollers that the cluster administrator creates in the openshift-ingress-operator namespace. If in the future we allow unprivileged users to create ingresscontrollers, it will most likely be using unprivileged deployments of the ingress operator. So in the future we may want to make the operator capable of running unprivileged, but for now, it is safe to say that the ingress operator and any ingresscontrollers it manages are system-trusted.

[deads2k]

I'm planning to update this so that instead of changing the meaning of an existing configmap, we also create one named default-ingress-ca to properly represent what we're storing. We've gotten away with not including any custom wildcards into sa-token-secrets/Data[ca.crt] and I'm inclined to keep it that way.

[Miciah]

Thanks! I have updated openshift/enhancements#126 to describe the approach that David describes above (#331 (comment)).

@deads2k, we talked about always publishing the default certificate rather than the CA certificate (even in the case that the operator has generated the default certificate and therefore has the CA certificate); if take that approach, do you want to change the name of the new ConfigMap from default-ingress-ca to, say, default-ingress-cert? Let me know, and I'll update openshift/enhancements#126 again accordingly.

[deads2k]

/retest

[Miciah]

@deads2k, I see you changed currentConfigMap to return nil if it is given a nil input. I believe the following is what is happening as a result:

1. The certificate controller calls ensureRouterCAConfigMap, which calls desiredRouterCAConfigMap to get the desired router-ca configmap and passes the result to ensureConfigMap. ensureConfigMap calls currentConfigMap, determines that router-ca does not exist but is desired, and creates router-ca.
2. The E2E test configures the default ingresscontroller to use a custom default certificate.
3. The certificate controller calls ensureRouterCAConfigMap, which calls desiredRouterCAConfigMap to get the desired router-ca configmap, which is now nil. ensureRouterCAConfigMap passes the nil value to ensureConfigMap, which calls currentConfigMap, which now reports that router-ca does not exist because its input was nil. Because currentConfigMap returned nil, ensureRouterCAConfigMap does not delete router-ca.
4. The E2E test fails because router-ca does not get deleted.

[deads2k]

/retest

[deads2k]

/retest

[Miciah]

/lgtm

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, Miciah

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 [Miciah]

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

[Miciah]

/cherrypick release-4.3
The goal is to start publishing default-ingress-cert in 4.3.z, move operators to using default-ingress-cert instead of using router-ca in 4.3.(z+1), and stop publishing router-ca in 4.4.0.

[openshift-cherrypick-robot]

@Miciah: new pull request created: #336

In response to this:

/cherrypick release-4.3
The goal is to start publishing default-ingress-cert in 4.3.z, move operators to using default-ingress-cert instead of using router-ca in 4.3.(z+1), and stop publishing router-ca in 4.4.0.

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.

[Miciah]

/retitle Bug 1788712: publish a router-ca that can be used to verify routes in golang clients

[openshift-ci-robot]

@deads2k: All pull requests linked via external trackers have merged. Bugzilla bug 1788712 has been moved to the MODIFIED state.

In response to this:

Bug 1788712: publish a router-ca that can be used to verify routes in golang clients

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.