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

CN was longer than 64 bytes #1462

Closed
arkadijs opened this issue Mar 10, 2019 · 13 comments
Closed

CN was longer than 64 bytes #1462

arkadijs opened this issue Mar 10, 2019 · 13 comments
Labels
area/acme Indicates a PR directly modifies the ACME Issuer code area/api Indicates a PR directly modifies the 'pkg/apis' directory triage/support Indicates an issue that is a support question.

Comments

@arkadijs
Copy link

Using ingress shim to auto-provision certificates.
For ingress resource:

spec:
  rules:
  - host: pgweb-arkadi-pgweb-very-long-name-to-overflow-cn-limit.apps.arkadi.dev.superhub.io
    http:
      paths:
      - backend:
          serviceName: automation-hub-pgweb-pgweb-auth-svc
          servicePort: 4180
        path: /
  tls:
  - hosts:
    - pgweb-arkadi-pgweb-very-long-name-to-overflow-cn-limit.apps.arkadi.dev.superhub.io
    secretName: automation-hub-pgweb-pgweb-tls

a Certificate resource is installed (by the shim):

spec:
  acme:
    config:
    - domains:
      - pgweb-arkadi-pgweb-very-long-name-to-overflow-cn-limit.apps.arkadi.dev.superhub.io
      http01:
        ingressClass: traefik
  dnsNames:
  - pgweb-arkadi-pgweb-very-long-name-to-overflow-cn-limit.apps.arkadi.dev.superhub.io
  issuerRef:
    kind: ClusterIssuer
    name: letsencrypt-prod
  secretName: automation-hub-pgweb-pgweb-tls

That's lead to following error:

controller.go:186] orders controller: Re-queuing item "automation-hub/automation-hub-pgweb-pgweb-tls-402482467" due to error processing: error finalizing order: acme: urn:ietf:params:acme:error:malformed: Error finalizing order :: CN was longer than 64 bytes

cert-manager v0.7.0-beta.0.

/kind bug

@jetstack-bot jetstack-bot added the kind/bug Categorizes issue or PR as related to a bug. label Mar 10, 2019
@munnerz
Copy link
Member

munnerz commented Mar 11, 2019

This is working as expected - it's against spec to issue certs with CNs longer than 64 characters. You can see some discussion on this here:

https://community.letsencrypt.org/t/a-certificate-for-a-63-character-domain/78870

Right now, the advised workaround is to include a shorter common name alongside the long dns name (as let's encrypt don't allow you to leave the CN field blank).

There's some more discussion on removing this requirement here: letsencrypt/boulder#2093

@munnerz
Copy link
Member

munnerz commented Mar 11, 2019

I think the best we can do is to update this doc here: https://github.com/jetstack/cert-manager/blob/master/docs/tasks/acme/issuing-certificates.rst

to include a note stating that the max length is 64 chars 😄

@arkadijs
Copy link
Author

There is no control over CN with the ingress shim?
If the shim or some other code in-between could truncate the hostname that is put into CN (remove leading components) - that could work?

@cpu
Copy link
Contributor

cpu commented Mar 11, 2019

If the shim or some other code in-between could truncate the hostname that is put into CN (remove leading components) - that could work?

Unfortunately this won't work. The identifiers in the CSR Subj. CN/SANs need to be an exact match to the identifiers in the ACME order or the order finalization request carrying the CSR will be rejected.

@arkadijs
Copy link
Author

I just did a manual work to insert an additional 70-char long dns name into ingress that has a short name in place, and this is the cert I got:

Screen Shot 2019-03-11 at 11 19 15 PM

X509v3 Subject Alternative Name:
DNS:git-git-git-git-git-git-git-git-git-git-git.app.arkadi.dev.superhub.io, DNS:git.app.arkadi.dev.superhub.io

If Let's Encrypt requires (a shorter) CN to be validated via HTTP or DNS solver - not just SAN-s - I can do that. Let me explain a bit...

We do automation with / on top of Kubernetes. Our stacks are Kubernetes clusters with applications, CI, DNS, etc., and now TLS - hopefully with Let's Encrypt - fully automated from the code. The default DNS structure is <app name>.app.<stack>.<env>.superhub.io. The app.<stack>.<env>.superhub.io can be reasonably made shorter than 64-chars. At least we could ask our users to keep it short and document the limitation. But <app name> may blow-up past the limit due to being an autogenerated name. There could be a (dummy) ingress installed for app.<stack>.<env>.superhub.io to validate CN (if that's a requirement).

@munnerz
Copy link
Member

munnerz commented Mar 12, 2019

We do automation with / on top of Kubernetes. Our stacks are Kubernetes clusters with applications, CI, DNS, etc., and now TLS - hopefully with Let's Encrypt - fully automated from the code. The default DNS structure is .app...superhub.io. The app...superhub.io can be reasonably made shorter than 64-chars. At least we could ask our users to keep it short and document the limitation. But may blow-up past the limit due to being an autogenerated name. There could be a (dummy) ingress installed for app...superhub.io to validate CN (if that's a requirement).

We have a similar setup with review apps on GitLab, and simply truncate the branch/app name to an appropriate length before creating the Certificate resource here.

cert-manager itself can't really start manually truncating DNS names for you, as this is a violation of our API spec (i.e. user asks for a cert with a particular name, and so they should receive a cert with that name and not another).

If anything, I think we should tighten our own validation so that we bail out earlier and don't allow you to create a Certificate with a CN length >64 chars.

This isn't something we'll be able to handle automatically, as it's effectively a user configuration error.

As you've noted, it is possible to create a Certificate with a CN that is short, and a longer DNS name. I'd advise you do this for now if possible!

@munnerz munnerz added area/api Indicates a PR directly modifies the 'pkg/apis' directory triage/support Indicates an issue that is a support question. area/acme Indicates a PR directly modifies the ACME Issuer code and removed kind/bug Categorizes issue or PR as related to a bug. labels Mar 12, 2019
@arkadijs
Copy link
Author

The issue is about mechanics of ingress shim. This is not about requesting a cert and getting one with a different name - the automation does not work with Certificate resource directly.

TLS is a leaking abstraction.

As a user, effectively a software component that requires ingress, I care no about certificate/key and it's management - that's an implementation detail. I install ingress and expect it to be TLS-protected. The tls{} block and certmanager annotation(s) required are already borderline to tight coupling, to what the software wants to know about being TLS-protected. Now, despite Let's Encrypt allowing SAN-s with 64+ chars names, I cannot make use of it from ingress (conveniently). I, as a software component, must workaround deficiencies in Acme solver. This must be done for each component that installs ingress, potentially out of my control.

To me it looks like there is a combination of transformations performed by the shim that could make it work. Yes, the name that is put in CN will be shorter, but I don't care. Given a choice of (a) not having long DNS names TLS protected and (b) preserving compatibility, my choice is to get long DNS names working.

@munnerz
Copy link
Member

munnerz commented Mar 13, 2019

I understand your point here, but I'm not really sure what we can do in this instance. ingress-shim cannot reliably predict what CN should be used, in the instance where the only name on an ingress is a long one (i.e. longer than 64 chars).

Any kind of guess work, i.e. truncating at {some point}, will cause confusion for users when we guess incorrectly.

As you say, adding the annotations and tls stanza to an ingress should seamlessly secure your traffic, and users shouldn't need to be concerned about the implementation details. HOWEVER this does NOT preclude configuration errors, and in this instance there is a subtle configuration error.

The behaviour of ingress-shim can be defined as "it will create a Certificate resource that contains all the named hostnames on it, using your configured issuer, and setting the first hostname as the common name on the certificate". This is exactly what's going on here, except the first entry on your Ingress resource is technically an invalid CN, as it's greater than 64 characters.

We should better document that this is how ingress-shim behaves, but beyond that I am strongly against us attempting to automatically determine a 'safe' value to insert here.

In short:

  • ingress-shim creates Certificate resources with all the named DNS names on
  • the first DNS name will be used as the certificate's Common Name (a behaviour adopted from LE/ACME)
  • therefore, if the first DNS name is >64 chars, the common name field will be invalid

@munnerz
Copy link
Member

munnerz commented Mar 13, 2019

This isn't a deficiency in the ACME solver - it's ingress-shim working as intended, and unfortunately the first DNSName you've provided here is not valid for use as a common name.

You can quickly work around this by adding some other entry to your tls stanza before your other domain, e.g.:

spec:
  tls:
  - secretName: abc
    domains:
    - myreallyreallyreallyreallyreallyreallyreallyreallyreallyreallyreallyreallyreally.long.name.example.com

should be

spec:
  tls:
  - secretName: abc
    domains:
    - a.short.name.example.com
    - myreallyreallyreallyreallyreallyreallyreallyreallyreallyreallyreallyreallyreally.long.name.example.com

@arkadijs
Copy link
Author

Good enough, that works for me. Agree, the safe value for CN is user specific. Creating a configurable machinery to satisfy Let's Encrypt Subject: requirement is not worth the effort. Hope Let's Encrypt will depreciate must-have-a-CN check and let users decide how theirs certs should look like.

arkadijs added a commit to agilestacks/components that referenced this issue Mar 13, 2019
@munnerz munnerz closed this as completed Mar 25, 2019
@paolomainardi
Copy link

Is this solution works also when using http challenge validation ?

@arkadijs
Copy link
Author

It works for me - both with http and dns solvers. The first (shorter than 64 chars) element in hosts is resolvable in DNS and is routable to the same ingress controller. Solver pods created by cert-manager gets traffic from the Let's Encrypt and validation is performed successfully.

@paolomainardi
Copy link

Ok perfect, so the first name should be always http resolvable, it means to always carry on both domains. Thanks for letting me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/acme Indicates a PR directly modifies the ACME Issuer code area/api Indicates a PR directly modifies the 'pkg/apis' directory triage/support Indicates an issue that is a support question.
Projects
None yet
Development

No branches or pull requests

5 participants