Skip to content
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.

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

OCP Ingress Support for Cert-Manager 1.4 #4247

Closed
charles-horel opened this issue Jul 23, 2021 · 13 comments
Closed

OCP Ingress Support for Cert-Manager 1.4 #4247

charles-horel opened this issue Jul 23, 2021 · 13 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. triage/support Indicates an issue that is a support question.

Comments

@charles-horel
Copy link

Is your feature request related to a problem? Please describe.
OCP 1.4 ingress relies on a very specific format for the secret created by cert-manager. With recent version changes, the root ca cert has been moved from the tls.crt key to a ca.crt key, and openshift-ingress does not pick this up. This causes degradation of the operator and the full chain is not presented by ingress services.

This is an RFE as this appears to be newer intentional behavior of cert-manager:
https://cert-manager.io/docs/concepts/certificate/
"When a certificate is issued by intermediates of the CA and the Issuer knows the intermediates, the content of tls.crt will be a resulting certificate followed by a certificate chain. The certificate chain doesn’t include a root CA certificate, as it is stored in ca.crt."

Describe the solution you'd like
A parameter on the certificate object that determines how the final cert object is formatted. This could be called legacyCertificateSecretFormat bool. True would cause the entire cert chain to be included in tls.crt instead of breaking it up into ca.crt.

Describe alternatives you've considered
The only alternatives from our end would be to submit an RFE to Redhat (which we have). We are downgrading to cert-manager 1.1 in the interim.
The only other solution would be to have a custom operator update the secret or parse it into a new secret with the format we want which could be messy to maintain. This problem will persist on all openshift ingresses that use a cert tied to cert-manager 1.4 though, it would be good to sort out long term.

Additional context

Environment details (remove if not applicable):

  • Kubernetes version: Openshift Container Platform 4.7
  • Cloud-provider/provisioner:
  • cert-manager version: 1.4.0
  • Install method: e.g. helm/static manifests: helm

/kind feature

@jetstack-bot jetstack-bot added the kind/feature Categorizes issue or PR as related to a new feature. label Jul 23, 2021
@jakexks
Copy link
Member

jakexks commented Jul 23, 2021

Hi @charles-horel

We don't use openshift day to day, so I'd be happy accept an issue if it's not working. However, I tried to reproduce the openshift ingress controller not accepting TLS certs ending in an intermediate and can't reproduce it. Could you elaborate a bit more on what error you are getting?

Which issuer are you using?

Let's say I have my own root CA, and have issued an intermediate for it. Certs inside:

tls.crt:

-----BEGIN CERTIFICATE-----
MIICkTCCAhegAwIBAgIUR9GkfKm4R6l+Vr65n2mWDaPfxqQwCgYIKoZIzj0EAwMw
QjELMAkGA1UEBhMCR0IxDzANBgNVBAcTBkxvbmRvbjEVMBMGA1UEChMMY2VydC1t
YW5hZ2VyMQswCQYDVQQDEwJJMTAeFw0yMTA3MjMxNTE4MDBaFw0yMjA3MjMxNTE4
MDBaMFYxCzAJBgNVBAYTAkdCMQ8wDQYDVQQHEwZMb25kb24xFTATBgNVBAoTDGNl
cnQtbWFuYWdlcjEfMB0GA1UEAxMWc2VydmVyLmNlcnQtbWFuYWdlci5pbzBZMBMG
ByqGSM49AgEGCCqGSM49AwEHA0IABPt95gvQqZlDzaVhdOuDkNjWtHAR1/2KMKcK
GyjsO/zBGVJjGFg2nf+nER/cZjrWh2YAKBvB5cXNeS8ImolBL0ajgdYwgdMwDgYD
VR0PAQH/BAQDAgWgMBMGA1UdJQQMMAoGCCsGAQUFBwMBMAwGA1UdEwEB/wQCMAAw
HQYDVR0OBBYEFOG8sfkhhd8lTdQapiBRlbLIk5bAMB8GA1UdIwQYMBaAFLPDfp+E
XgQeGccLkw2QJNzr/2BjMF4GA1UdEQRXMFWCFnNlcnZlci5jZXJ0LW1hbmFnZXIu
aW+CCWxvY2FsaG9zdIIqY29uc29sZS1vcGVuc2hpZnQtY29uc29sZS5hcHBzLWNy
Yy50ZXN0aW5nhwR/AAABMAoGCCqGSM49BAMDA2gAMGUCMGDf4eAGVG/PdAaQrim+
MeovBS+E0ZDkRloiBeYdQQC087aC6iLbR48i2aMKcC7BEgIxAKFCTf+4mjVzN7YK
Ozsh3jDSASOMOhQBE04MgxPhkDVyqOjYRTAOndXD0feEWFXCBw==
-----END CERTIFICATE-----
-----BEGIN CERTIFICATE-----
MIICZDCCAcagAwIBAgIUEy2SbGvo2k5hM6Qh2Jj4RWVgV+cwCgYIKoZIzj0EAwQw
WTELMAkGA1UEBhMCR0IxDzANBgNVBAcTBkxvbmRvbjEVMBMGA1UEChMMY2VydC1t
YW5hZ2VyMSIwIAYDVQQDExljZXJ0LW1hbmFnZXIgdGVzdGluZyByb290MB4XDTIx
MDcwODA5NTAwMFoXDTIyMDcwODA5NTAwMFowQjELMAkGA1UEBhMCR0IxDzANBgNV
BAcTBkxvbmRvbjEVMBMGA1UEChMMY2VydC1tYW5hZ2VyMQswCQYDVQQDEwJJMTB2
MBAGByqGSM49AgEGBSuBBAAiA2IABPrSzBhGaX5+PGLi1rh3FDLFAwvQiDJw233n
K0JWDmNauQE3PUcTWOBLXZ1+gw4pZnRcmpZpvFpEwCyyEE0+6JlhvKW1g+boW4Uj
gJ/AcfNN6MsAyyLe+FlRFZMoLT65iKNmMGQwDgYDVR0PAQH/BAQDAgGmMBIGA1Ud
EwEB/wQIMAYBAf8CAQAwHQYDVR0OBBYEFLPDfp+EXgQeGccLkw2QJNzr/2BjMB8G
A1UdIwQYMBaAFA8gWEpPc5efcU9nvhE68sP5H03UMAoGCCqGSM49BAMEA4GLADCB
hwJBAcL8CWM9oJcvHlVEnXOQ25St8OeGxuE1x/osp6/S9mFxtmSbA2POgSFOybLr
k5cKpC3YqxbhA/6zqDQl6HJupg4CQgHXYQNATIh7NewU3mdFA4Cod437Y0zzaCCG
FejbnfknbuzTPViAjWA4nWu+EtNiOjGRQFnGF0Og6eQYn3OK8Fz2xQ==
-----END CERTIFICATE-----

ca.crt:

-----BEGIN CERTIFICATE-----
MIICfTCCAd+gAwIBAgIURpC9vOVIx9NTWfl/zk7PlBfmL/QwCgYIKoZIzj0EAwQw
WTELMAkGA1UEBhMCR0IxDzANBgNVBAcTBkxvbmRvbjEVMBMGA1UEChMMY2VydC1t
YW5hZ2VyMSIwIAYDVQQDExljZXJ0LW1hbmFnZXIgdGVzdGluZyByb290MB4XDTIx
MDcwODA5NTAwMFoXDTI2MDcwNzA5NTAwMFowWTELMAkGA1UEBhMCR0IxDzANBgNV
BAcTBkxvbmRvbjEVMBMGA1UEChMMY2VydC1tYW5hZ2VyMSIwIAYDVQQDExljZXJ0
LW1hbmFnZXIgdGVzdGluZyByb290MIGbMBAGByqGSM49AgEGBSuBBAAjA4GGAAQA
2wZl6AU8W5TqJpMVhiG4nnDkMMeHboUPaPRM0VCD5yKxKcqAgecGwAJ6BM06QU8l
QE7HGUvjWNnDQB61JBLHAncAVVwanwtF/JWwIvWv2M48GbR0q/haZi7419ETxaUs
wUT+SXQtIBAbt1iRqeG35dTNcZC3S60LGcAg1wITY75FW9mjQjBAMA4GA1UdDwEB
/wQEAwIBBjAPBgNVHRMBAf8EBTADAQH/MB0GA1UdDgQWBBQPIFhKT3OXn3FPZ74R
OvLD+R9N1DAKBggqhkjOPQQDBAOBiwAwgYcCQWXkeHdI+VXtBdNU1JIzEZQoIGjN
6Ej2CH3oHIp3fnkEyEm33eO8uqzP86rMvh+wPbqmIC6JKhdMDfjiW3OICqfxAkIA
nr/x0fZeQ7VHak3wpIG55bNW6nxUKri51joJ/hFCyGxkcB/nDT+eIRCv0chMjNFh
7eEKjJw0uVFhdsFlngiaHE0=
-----END CERTIFICATE-----

Checking my local openshift install:

❯ crc version
WARN A new version (1.29.1) has been published on https://cloud.redhat.com/openshift/create/local
CodeReady Containers version: 1.27.0+3d6bc39d
OpenShift version: 4.7.11 (not embedded in executable)

Following this documentation for configuring Ingress controllers: https://docs.openshift.com/container-platform/4.7/networking/ingress-operator.html#nw-ingress-setting-a-custom-default-certificate_configuring-ingress

oc --namespace openshift-ingress create secret tls custom-certs-default --cert=tls.crt --key=tls.key
oc patch --type=merge --namespace openshift-ingress-operator ingresscontrollers/default \
  --patch '{"spec":{"defaultCertificate":{"name":"custom-certs-default"}}}'
ingresscontroller.operator.openshift.io/default patched

Trying to see if the cert works and is valid:

curl -vvv --cacert ca.crt https://console-openshift-console.apps-crc.testing/
*   Trying ::1...
* TCP_NODELAY set
* Connected to console-openshift-console.apps-crc.testing (::1) port 443 (#0)
* ALPN, offering h2
* ALPN, offering http/1.1
* successfully set certificate verify locations:
*   CAfile: ca.crt
  CApath: none
* TLSv1.2 (OUT), TLS handshake, Client hello (1):
* TLSv1.2 (IN), TLS handshake, Server hello (2):
* TLSv1.2 (IN), TLS handshake, Certificate (11):
* TLSv1.2 (IN), TLS handshake, Server key exchange (12):
* TLSv1.2 (IN), TLS handshake, Server finished (14):
* TLSv1.2 (OUT), TLS handshake, Client key exchange (16):
* TLSv1.2 (OUT), TLS change cipher, Change cipher spec (1):
* TLSv1.2 (OUT), TLS handshake, Finished (20):
* TLSv1.2 (IN), TLS change cipher, Change cipher spec (1):
* TLSv1.2 (IN), TLS handshake, Finished (20):
* SSL connection using TLSv1.2 / ECDHE-ECDSA-AES128-GCM-SHA256
* ALPN, server did not agree to a protocol
* Server certificate:
*  subject: C=GB; L=London; O=cert-manager; CN=server.cert-manager.io
*  start date: Jul 23 15:18:00 2021 GMT
*  expire date: Jul 23 15:18:00 2022 GMT
*  subjectAltName: host "console-openshift-console.apps-crc.testing" matched cert's "console-openshift-console.apps-crc.testing"
*  issuer: C=GB; L=London; O=cert-manager; CN=I1
*  SSL certificate verify ok.

Note the ingress controller has no need for ca.crt (which is just provided for informational purposes). Clients need to already trust the root CA through another distribution method (e.g, including the cert in your docker images trust store or, mounting it from a well-known secret) or you don't really have TLS at all!

@jakexks
Copy link
Member

jakexks commented Jul 23, 2021

/triage support

@jetstack-bot jetstack-bot added the triage/support Indicates an issue that is a support question. label Jul 23, 2021
@charles-horel
Copy link
Author

charles-horel commented Jul 23, 2021 via email

@charles-horel
Copy link
Author

Note we opened an RFE with Redhat as well to accept/leverage the new cert format:

https://issues.redhat.com/browse/RFE-2023

@absynth76
Copy link

absynth76 commented Aug 27, 2021

Hi, the main issue is that ingress controller certificate is also used by openshift as a trust bundle in other operators, like the authentication one. So even if the router/ingress controller does not complain about a certificate chain not containing the Root CA, The other operators that consume the same secret are going in a degrade state and so openshift cluster is consequently degraded too. I really do not understand why you can't accept the PR feature that @SgtCoDFish created above, this could have be a potential solution to make cert-manager fully compatible with openshift.

@charles-horel
Copy link
Author

charles-horel commented Aug 30, 2021

I think it's possible all that is needed in order to resolve the authentication operator is to get the trust added for the root CA. I did a bit more digging and saw this, which indicates the full cert should never need be presented if the root is a trusted entity.

https://cert-manager.io/docs/concepts/certificate/
cert-manager intentionally avoids adding root certificates to tls.crt, because they are useless in a situation where TLS is being done securely. For more information, see RFC 5246 section 7.4.2 which contains the following explanation:

Because certificate validation requires that root keys be distributed independently, the self-signed certificate that specifies the root certificate authority MAY be omitted from the chain, under the assumption that the remote end must already possess it in order to validate it in any case.

I am reading this to believe that we should be adding trust to whatever store is managed by authentication. If Openshift operators aren't leveraging the cluster trust store, they should. This would mean that this issue is not an issue, and more of a configuration issue with Openshift...

To corroborate this, we did notice that our browsers (which trust the root CA) did not complain about the console certificate even though it only contained the intermediate and end entities!

@absynth76
Copy link

And again I'll repeat myself, MAY goes in two directions, it means can or cannot, it means a flag should be implemented to make the CA appended. Redhat decided to trust an internal (tls) secret to the cluster for their operation, they did an error assuming the CA will always be present in the certificate chain, but if you can't trust the internal objects, where the automation can start? Should I remember here it WAS working prior to cert-manager 1.3 or something like that, it means it's a regression that must be solved imho.

@charles-horel
Copy link
Author

charles-horel commented Aug 31, 2021 via email

@absynth76
Copy link

Don't waste your time, I did test via a machine config assuming if the CA is in the host trust store it should work. It did not. It means redhat use only that tls.crt as a trust store for that specific flow as far as I understand.

@jetstack-bot
Copy link
Collaborator

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close.
Send feedback to jetstack.
/lifecycle stale

@jetstack-bot jetstack-bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 29, 2021
@jetstack-bot
Copy link
Collaborator

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close.
Send feedback to jetstack.
/lifecycle rotten
/remove-lifecycle stale

@jetstack-bot jetstack-bot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Dec 29, 2021
@jetstack-bot
Copy link
Collaborator

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.
Send feedback to jetstack.
/close

@jetstack-bot
Copy link
Collaborator

@jetstack-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.
Send feedback to jetstack.
/close

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. triage/support Indicates an issue that is a support question.
Projects
None yet
Development

No branches or pull requests

4 participants