Is the ignore of spec.tls.caCertificate on purpose ?

[strima]

As far as i can see in the code the spec.tls.caCertificate on the route object is nowhere considered to be set

• Is this on purpose ?
  • If yes, then how is this intended to be populated ?
  • If yes, would it make sense to add a short note regarding the intended setting of that in the documentation ?
• If no, how could a suitable PR look like
  • Since cert-manager populates the secret with the necessary additional ca certs i would have taken them from there and use that as value(s) for the spec.tls.caCertificate field

Looking forward to the answers

[Bengrunt]

Hello, I can confirm this is an issue and that it also affects us on clusters running openshift-routes 0.7+

I believe that it may originate from an intent to make things rights with regards of what the spec.tls.certificate field should contain. Indeed, if you run:

$ oc explain route.spec.tls.certificate
GROUP:      route.openshift.io
KIND:       Route
VERSION:    v1

FIELD: certificate <string>


DESCRIPTION:
    certificate provides certificate contents. This should be a single serving
    certificate, not a certificate chain. Do not include a CA certificate.

Notice the end of the description "Do not include a CA certificate".

However, on openshift-routes prior to 0.7.0 the full certificate chain was inserted in the spec.tls.certificate field and thus all TLS clients were happy. Now, all clients that do not support TLS Authority Information Access (AIA) will fail to verify the provided certificate since the Route has no data in spec.tls.caCertificate and does not provide the certificate chain.

Would be nice to have either:

• an option to enable such a behavior
• automatically fill the spec.tls.caCertificate field this the chain contents of the certificate secret

Thanks a lot !

[strima]

However, on openshift-routes prior to 0.7.0 the full certificate chain was inserted in the spec.tls.certificate field and thus all TLS clients were happy. Now, all clients that do not support TLS Authority Information Access (AIA) will fail to verify the provided certificate since the Route has no data in spec.tls.caCertificate and does not provide the certificate chain.

just wanted to explicitely state that the scenario i was talking about is NOT filling the route.spec.tls.certificate with the whole chain but rather filling route.spec.tls.certificate with the one and only cert (this already is like it should be) but ADDITIONALLY filling the route.spec.tls.caCertificate with the (potentially) existing certs from the cert-manager manged secret

[Bengrunt]

However, on openshift-routes prior to 0.7.0 the full certificate chain was inserted in the spec.tls.certificate field and thus all TLS clients were happy. Now, all clients that do not support TLS Authority Information Access (AIA) will fail to verify the provided certificate since the Route has no data in spec.tls.caCertificate and does not provide the certificate chain.

just wanted to explicitely state that the scenario i was talking about is NOT filling the route.spec.tls.certificate with the whole chain but rather filling route.spec.tls.certificate with the one and only cert (this already is like it should be) but ADDITIONALLY filling the route.spec.tls.caCertificate with the (potentially) existing certs from the cert-manager manged secret

I agree, that would be best.

After a little digging in the code, it seems to me that the full certificate chain is retrieved from the secret (here) but then, only the first certificate is encoded in the Route spec.tls.certificate field since the EncodeX509 function (defined here) is used. Using the EncodeX509Chain function (defined here) instead would probably work as prior to v0.7.0 but that would miss the target of properly using spec.tls.certificate and spec.tls.caCertificate.

In the utilpki lib I did not find any function that would allow us to easily strip the host certificate from the chain bundle in order to set the spec.tls.caCertificate field cleanly, so I assume such a goal would require a little bit more work.

PS: my golang skills are mostly at the "beginner/reading" level so maybe I missed something or did not understand the code properly.

[Bengrunt]

My colleague and I found a solution, we will work on a PR to fix this.

[strima]

thx for your PR - but regarding code - i made two comments regarding the implementation

[SgtCoDFish]

Hey, thanks for raising this. What's the use case for this field (caCertificate) here?

As far as I can see, it's informational. If that's the case, it would probably be a similar case to https://cert-manager.io/docs/faq/#why-isnt-my-certificates-chain-in-my-issued-secrets-cacrt

In other words, leaving caCertificate out would be intentional and we wouldn't consider adding it because it would be an easy way to cause an outage down the road.

If it's required for something we could consider adding it.

[Bengrunt]

Thanks for the reply @SgtCoDFish.

The main issue for us is that the openshift-routes controller only sets the first certificate of the chain in the spec.tls.certificate field of the Route instead of setting the whole chain that is contained in the TLS Secret associated to the Certificate resource. This is not what was done before openshift-routes 0.7+, when the whole chain was added to the Route spec.

For us this is a breaking change because not all HTTP clients support Authority Information Access (AIA) that would allow for the retrieval of the missing intermediate certificates. In such an event, HTTPS requests would fail because of the missing certificates. Modern web browser do support AIA but tools like curl or some programming language libraries don't.

I hope this helps clarify the issue at hand.

[SgtCoDFish]

I want to make sure I'm understanding! (I'm still jet lagged from KubeCon!)

Let's say we have a chain with three certs, R -> I -> L (root issues intermediate, intermediate issues leaf).

A cert-manager Secret would have L + I in tls.crt. A server using that cert would present both L and I to clients trying to connect. Clients would have advance knowledge of R, which is how they would trust the server's chain.

The main issue for us is that the openshift-routes controller only sets the first certificate of the chain in the spec.tls.certificate field of the Route instead of setting the whole chain that is contained in the TLS Secret associated to the Certificate resource.

I tested locally and confirmed that we're setting tls.certificate to just L in the above scenario, which is just a straight up bug as far as I can see.

I've raised #122 to address that. I think that should fix the issue without us having to set caCertificate (which hopefully we can avoid setting, since it's hard to be sure that we can set a value for that)

[Bengrunt]

I hope you had a blast at Kubecon :)

You understood correctly.
However, "fixing" that bug in this fashion would infringe the following:

$ oc explain route.spec.tls.certificate
GROUP:      route.openshift.io
KIND:       Route
VERSION:    v1

FIELD: certificate <string>


DESCRIPTION:
    certificate provides certificate contents. This should be a single serving
    certificate, not a certificate chain. Do not include a CA certificate.

At first, I thought that was the reason for this change of behavior in 0.7.0.

[ctml91]

Despite the documentation saying not to include it in the certificate field, even OpenShift routes controller manager will copy both leaf and intermediate certs from secret/TLS.crt when using an ingress to generate a route and stick them in the certificate field.

I wonder if there is a good reason for splitting them, and if so why they do not enforce that themselves. I suppose we'd have to check the OpenShift ingress code to see what it's doing with the caCertificate field.

[maelvls]

From this OpenShift issue opened in 2021, it seems like it's always been OK to put both the leaf followed by the intermediates in spec.certificate as long as you didn't use termination: reencrypt. The issue indicates that in order for re-encryption to work, the certificate field must only contain a single certificate, and the intermediates must be given in caCertificate:

I think the documentation should be clear that the caCertificate field is for intermediate CA certificates, not the root CA certificate, and that there may be multiple. I think the doc (and API doc) should be updated to be clear about what is intended for each field - specifically to say just put the leaf cert in spec.tls.certificate and just put intermediate cert(s) in spec.tls.caCertificate.

Now, with regards to OpenShift's route controller manager that creates routes by shoving the contents of the Ingress's Secret's tls.crt into spec.certificate, it works alright because re-encryption isn't needed in this case. There is no way to configure Ingress resources to generate a Route that uses re-encryption.

tl;dr: this project should put the leaf in the certificate field and intermediates in caCertificate so that it's possible to use termination: reencrypt.

[ctml91]

From this OpenShift issue opened in 2021, it seems like it's always been OK to put both the leaf followed by the intermediates in spec.certificate as long as you didn't use termination: reencrypt. The issue indicates that in order for re-encryption to work, the certificate field must only contain a single certificate, and the intermediates must be given in caCertificate:

I think the documentation should be clear that the caCertificate field is for intermediate CA certificates, not the root CA certificate, and that there may be multiple. I think the doc (and API doc) should be updated to be clear about what is intended for each field - specifically to say just put the leaf cert in spec.tls.certificate and just put intermediate cert(s) in spec.tls.caCertificate.

Now, with regards to OpenShift's route controller manager that creates routes by shoving the contents of the Ingress's Secret's tls.crt into spec.certificate, it works alright because re-encryption isn't needed in this case. There is no way to configure Ingress resources to generate a Route that uses re-encryption.

tl;dr: this project should put the leaf in the certificate field and intermediates in caCertificate so that it's possible to use termination: reencrypt.

Hmm I don't think that is entirely correct. It is possible to configure an ingress to generate a reencrypt route using the route.openshift.io/termination annotation on the ingress and setting to reencrypt (or edge for that matter, whatever termination type you want). The destinationCaCertificate field can also be setting using the below which would be important for the re-encrypt part.

https://docs.openshift.com/container-platform/4.14/networking/routes/route-configuration.html#creating-re-encrypt-route-with-custom-certificate_route-configuration

Since there are separate fields to handle the backend certificate, such as destinationCaCertificate I am not sure how the certificate field plays a role at all and it still works correctly when placing both in the leaf certificate on reencrypt routes. I do have ingress generated routes with reencrypt, where it still puts both leaf + intermediate in certificate and it works correctly. Now maybe there is still a good reason to not do that, however openshift-routes-controller-manager does it (even for reencrypt routes) and aside from the API field description haven't seen a good reason why it shouldn't be.

[SgtCoDFish]

Thanks all for the discussion!

At first, I thought that was the reason for this change of behavior in 0.7.0.

Looking back, I'm not actually sure if there was a change of behavior? Are you sure it used to populate caCertificate?

The code in v0.7.1 seems clearly to only parse one cert: https://github.com/cert-manager/openshift-routes/blob/v0.6.1/internal/controller/sync.go#L649

So it doesn't seem like v0.7.x broke anything? Am I missing something?

I'm inclined to think that @maelvls 's approach is the way to go:

tl;dr: this project should put the leaf in the certificate field and intermediates in caCertificate so that it's possible to use termination: reencrypt.

It seems odd to me that OpenShift would be designed to require that, but I can believe it at least.

[Bengrunt]

At first, I thought that was the reason for this change of behavior in 0.7.0.

Looking back, I'm not actually sure if there was a change of behavior? Are you sure it used to populate caCertificate?

The code in v0.7.1 seems clearly to only parse one cert: https://github.com/cert-manager/openshift-routes/blob/v0.6.1/internal/controller/sync.go#L649

So it doesn't seem like v0.7.x broke anything? Am I missing something?

No I did not mean that spec.tls.caCertificate was being populated before 0.7.x but that spec.tls.certificate was being populated with the whole chain instead of just the leaf certificate. Sorry for being unclear and, I believe, to maybe have mixed up this issue within @strima 's issue.

In clusters running 0.6 we do have the whole chain contained in spec.tls.certificate and that's OK for every HTTP clients (so far).

[maelvls]

It is possible to configure an ingress to generate a reencrypt route using the route.openshift.io/termination annotation on the ingress and setting to reencrypt

Thanks for pointing that out!

It still works correctly when placing both in the leaf certificate on reencrypt routes

I am so confused now. I'm not sure why the OpenShift project wants the leaf in the certificate field. If it works to put the leaf followed by the intermediates in certificate, regardless of the termination... why do they say to only put the leaf in there?

I think we need someone from Red Hat to help us out 😅 @TrilokGeer Do you know who we can contact to get some clarity? Thanks!

[nate-duke]

there's some conversation related to this in https://bugzilla.redhat.com/show_bug.cgi?id=2013238 if you feel like reading up while you figure out who to get in touch with at Redhat.

[ctml91]

there's some conversation related to this in https://bugzilla.redhat.com/show_bug.cgi?id=2013238 if you feel like reading up while you figure out who to get in touch with at Redhat.

Yeah that issue is what we were discussing above, @maelvls linked to it here. It sounds like from that discussion, although it works chaining them in tls.certificate it's not the intended way the API is designed to be used but they still allow it today to avoid breaking existing setups that chain it because people do use it like that (including openshifts own routes controller manager for ingress-to-route).

Now I wonder about the new external certificate API, there is no callout of including intermediate cert in the tls.crt. In the case of externally managed certs, you aren't going to get that level of control over the secret contents so I assume it's acceptable to have them chained in that case.

https://docs.openshift.com/container-platform/4.16/networking/routes/secured-routes.html#nw-ingress-route-secret-load-external-cert_secured-routes

If there is no downside to chaining and putting both in tls.crt then why not continue doing that since it's what was done before, especially if routes-controller-manager does that today. But, if there is no downside to using the API "as designed" and splitting them you could make the same argument I guess.