include self-signed root CA in chain #1954
Conversation
Signed-off-by: Aron Parsons <aron@knackworks.com>
|
@knackaron: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. 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. |
jetstack-bot
added
the
dco-signoff: yes
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: knackaron The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
jetstack-bot
added
the
do-not-merge/release-note-label-needed
|
Hi @knackaron. Thanks for your PR. I'm waiting for a jetstack or cert-manager member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
jetstack-bot
added
needs-ok-to-test
|
/assign @JoshVanL |
I'm not sure I agree - if the full CA chain is included, and you implicitly trust this full chain, you aren't really verifying anything when you validate a certificate is valid. On your local system, a set of self signed root CA certificates are embedded as part of your distribution (i.e. on various linux distros, the Likewise, distributing the root of trust to the various 'nodes' in your cluster should be a separate concern. For what its worth, we do make the CA used to sign a Certificate available in the Secret resource, under the |
That is fine for public certificate authorities because most existing container images have those CAs baked in, but untrue for for corporate and isolated private networks where public certificate authorities are explicitly not trusted. At the node level, the private CA chains are distributed via configuration management tools (e.g., Puppet, custom RPMs). At the container level, those CAs are not baked in, as containers are portable between environments and networks (e.g., your prod containers don't trust any part of your dev CA chain, isolated network1 doesn't trust isolated network2). We rely on
For the application serving, yes, including the full chain in
It's not the nodes, but the containers where including additional CA certificates is typically not accounted for in the entrypoint.
So really, the root and intermediate should be included in Since the issuer secret already includes the root certificate in |
|
Solving this another way. |
|
And how did you actually solve this ? |
|
+1 @knackaron how did you solve this please ? |
The CAs are defined once for our operator that deploys charts via Helm, and all releases inherit those values. We then use a subchart to provide a |
Signed-off-by: Aron Parsons aron@knackworks.com
What this PR does / why we need it:
Currently, cert-manager explicitly excludes self-signed CA certificates from the secrets it generates, even when the operator opts to include their self-signed CA certificate in the issuer secret. It is critical to include the full CA chain so that mutual TLS authentication and verifying the whole chain work with the in-cluster secrets, versus having to include the root CA in every pod that needs it (which differs per container image and is a huge PITA to maintain).
The choice to exclude these certificates was made as part of #1077.
All the details below, including current behavior and the behavior when this PR is applied. Hopefully not too much detail, and I have commentary in between the command output to create the full story.
ClusterIssuer:
Root Certificate:
Intermediate Certificate:
I have the root CA appended to
tls.crtas per #1067 indicates that it should be included in the secrets created by the issuer.Requesting a certificate issues it, but the full chain is not included in
tls.crt, just the intermediate CA. This prevents verifying the full chain by only referencing the secret values.Certificate Request:
Details of
ca.crtin the created secret:Details of
tls.crtin the created secret:You can see that the root CA is not included in the generated secret because of this conditional: https://github.com/jetstack/cert-manager/blob/master/pkg/util/pki/csr.go#L339
I do not believe that the choice to exclude the self-signed root certificate is valid. In the case of a self-signed CA chain, not including the root CA makes including the intermediate pointless, as the whole chain can not be verified.
Below is the output of removing that check for self-signed CAs, and shows that the full chain is included in
tls.crt, as desired. I believe this is a safe change to make, because I have explicitly included my root CA intls.crtof the issuer so that it does get included; if someone did not want that, they would opt not to put their full CA chain to the secret.Patched version of cert-manager and full CA chain included, as desired: