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

nginx: support wildcard TLS names #195

Closed
colemickens opened this issue Jan 31, 2017 · 34 comments
Closed

nginx: support wildcard TLS names #195

colemickens opened this issue Jan 31, 2017 · 34 comments
Assignees
Labels

Comments

@colemickens
Copy link
Contributor

Right now nginx-ingress-controller doesn't do anything if the domain names are omitted from the Ingress rules (with the intention of the "wildcard" TLS cert being used).

As a result, there are Ingress resources that work on GCE, but not Nginx (just using standard Ingress fields too... it's not like there's a proprietary annotation being used).

Fixing this bug would increase the number of ingress yamls in the wild that work with Nginx. As-is... the UX is pretty bad. I had to manually debug in order to figure out that the SSL rules simply weren't being written into my nginx.conf.

@aledbf
Copy link
Member

aledbf commented Jan 31, 2017

@colemickens can you post an example please?

@aledbf aledbf self-assigned this Jan 31, 2017
@colemickens
Copy link
Contributor Author

https://github.com/kubernetes/test-infra/blob/master/prow/cluster/ingress.yaml

I also had to remove the *s from the rules, or else it wouldn't work correctly. I can file a separate bug if desired, let me know.

@aledbf
Copy link
Member

aledbf commented Jan 31, 2017

@colemickens the issue here is that gce != nginx. This issue was already reported here kubernetes-retired/contrib#885

As a result, there are Ingress resources that work on GCE, but not Nginx (just using standard Ingress fields too... it's not like there's a proprietary annotation being used).

The only solution for this cases is to create a mapping of special cases but this requires changes in both ingress controllers (which makes no sense)

@bprashanth what can we do to avoid this issues?

@colemickens
Copy link
Contributor Author

I think I understand why the treatment of /blah /blah/ and /blah/* are different... The path I can fix for both nginx and GCE by just changing the suffix.

But what about the TLS cert? I don't see why the nginx template generation couldn't do the same logic that the GCE controller does...

@aledbf aledbf added the nginx label Feb 1, 2017
@aledbf
Copy link
Member

aledbf commented Feb 1, 2017

But what about the TLS cert? I don't see why the nginx template generation couldn't do the same logic that the GCE controller does...

You are right, in the absence of hosts in the TLS section we should use the information available on the certificate and generate a server.

@aledbf
Copy link
Member

aledbf commented Feb 7, 2017

@bprashanth how we should handle this case? (empty hosts in the TLS section).
Writing PR 236 the code becomes more complex than should be

@bprashanth
Copy link
Contributor

Hmm, figuring out the hostname from a cert should be relatively straightforward I think.

func main() {
	var k, c bytes.Buffer
	if err := generateRSACerts("foo.bar.com", true, &k, &c); err != nil {
		log.Fatal(err)
	}
	log.Printf("Hostname %v", getHostnameOrDie(c.Bytes()))
}

func getHostnameOrDie(cert []byte) string {
	block, _ := pem.Decode(cert)
	if block == nil {
		log.Fatalf("Failed to decode cert")
	}
	parsed, err := x509.ParseCertificate(block.Bytes)
	if err != nil {
		log.Fatalf("%v", err)
	}
	return strings.Join(parsed.DNSNames, ",")
}

So maybe try and figure out the hostname before failing?
Shouldn't be rules evaluate according to longest prefix so it's not possible for someone specifying a wildcard to somehow hijack the requests from someone specifying a hostname?

@aledbf
Copy link
Member

aledbf commented Feb 7, 2017

@bprashanth that's easy ^^.
I meant what's the correct behavior for Ingress rules with empty hosts in the TLS section and a valid secretName? The controller should create one server per common name and check for wildcards?

@bprashanth
Copy link
Contributor

It should create a server per hostname in the rules section, and use the cert from the secret for each.

If there are no hostnames in the rules section, and no hostname in the tls section, we can do 1 of 3 things:

  1. use a catchall hostname and the given cert - this would catch all traffic for the default backend. Maybe this is ok if we document it correctly, it's the cost of muxing all ingresses onto 1 webeserver. If there are many ingresses like this, we sort them based on creation timestamp. Maybe we only allow this behavior if the controller was NOT started with --default-backend, if it was, we assume that overrules everything and do (3)
  2. create a server per dns name in the cert - probably the best option, but the user can still force 1 by applying a wildcard as the common name.
  3. nothing, just send an event saying user needs to mention hostname

I'm fine doing 1 adding a doc, filing a bug for 2 and waiting for people to complain.

@rothgar
Copy link
Member

rothgar commented Aug 1, 2017

Can someone clarify for me to make sure I understand how this will change.

If someone has a certificate that has the hostnames foo.example and bar.example the controller will automatically add them to

spec:
  tls:
    - hosts:
      - foo.example
      - bar.example

and if a user has a certificate with a wildcard cert with no hosts specified (e.g., *.example) they can manually add the hosts they want to use that certificate.

Is that correct?

The current ingress controller I'm trying (gcr.io/google_containers/nginx-ingress-controller:0.9.0-beta.11) doesn't appear to be friendly with wildcard certs. I receive a notice in the logs ssl certificate $NAMESPACE/$CERTIFICATE does not contain a common name for host $HOST and keep getting routed to the default backend even though I have specified hosts to use with that certificate.

@keyosk
Copy link

keyosk commented Aug 9, 2017

@aledbf I'm also running into the same issue @rothgar is seeing. I'm getting routed to the default backend with a host match error for my wildcard certifcate. Do you have any update on the status for a fix here?

@aledbf
Copy link
Member

aledbf commented Aug 10, 2017

@rothgar @keyosk please use quay.io/aledbf/nginx-ingress-controller:0.174 that contains several fixes for ssl certificates.

@keyosk
Copy link

keyosk commented Aug 10, 2017

@aledbf We've just completed testing quay.io/aledbf/nginx-ingress-controller:0.174 and are seeing the same behavior for our wildcard certificate.

Our ingress:

  -
    apiVersion: extensions/v1beta1
    kind: Ingress
    metadata:
      name: example-internal-ingress
      namespace: example
      labels:
        app: example
    spec:
      tls:
      - hosts:
        - example.bronze.mydomain.com
        secretName: example-tls-secret
      rules:
      - host: example.bronze.mydomain.com
        http:
          paths:
            - path: /
              backend:
                serviceName: example-frontend-service
                servicePort: 80

Log from ingress controller:

W0810 00:43:18.602120       7 controller.go:1108] ssl certificate example/example-tls-secret does not contain a common name for host example.bronze.mydomain.com

I've changed the subdomain and top level domain for privacy reasons. But the domain depth is identical to what we're actually using. The certificate has a common name of *.mydomain.com

@aledbf
Copy link
Member

aledbf commented Aug 10, 2017

@keyosk thank you for the log and example. Let me reproduce this.
The cert only contains one cn for *.mydomain.com ?

@keyosk
Copy link

keyosk commented Aug 10, 2017

@aledbf That's correct. The cert only contains a common name for *.mydomain.com

@keyosk
Copy link

keyosk commented Aug 10, 2017

@aledbf If it helps, in this certificate there is a Subject Alternative Name which is filled in by *.bronze.mydomain.com and *.mydomain.com and mydomain.com (Just to reiterate, the Common Name field itself is simply *.mydomain.com

@rothgar
Copy link
Member

rothgar commented Aug 10, 2017

I also wanted to follow up an this. I tried with 0.173 yesterday (didn't get a chance to test 0.174 today) but I saw the same behavior still.

@keyosk
Copy link

keyosk commented Aug 10, 2017

Just tested quay.io/aledbf/nginx-ingress-controller:0.175 to great success! Thanks for the fast help @aledbf 👍

@gurvindersingh
Copy link

gurvindersingh commented Sep 27, 2017

@aledbf @keyosk well i tried quay.io/aledbf/nginx-ingress-controller:0.175 and gcr.io/google_containers/nginx-ingress-controller:0.9.0-beta.13 both of them give me this error W0927 18:32:11.527281 7 controller.go:1109] ssl certificate test-ns/wildcard-tls does not contain a Common Name or Subject Alternative Name for host test.example.world.com The wildcard-tls has CN as *.world.com wondering if i am missing something here.

@SaaldjorMike
Copy link
Contributor

@gurvindersingh IIRC *.world.com is not valid for sub-sub domains, like test.example.world.com but only one-level sub domains like example.world.com

@gurvindersingh
Copy link

@SaaldjorMike yup that seems to be the case :)

@aledbf aledbf closed this as completed Oct 11, 2017
@schulzp
Copy link

schulzp commented Dec 6, 2017

@aledbf just tested with quay.io/aledbf/nginx-ingress-controller:0.274 and am still running in this issue: default/<tls-secret-name> does not contain a Common Name or Subject Alternative Name for f.q.d.n :-( Is this a regression?

@aledbf
Copy link
Member

aledbf commented Dec 6, 2017

@schulzp please update to the latest version quay.io/kubernetes-ingress-controller/nginx-amd64:0.9.0

@schulzp
Copy link

schulzp commented Dec 6, 2017

@aledbf do you mean https://quay.io/repository/kubernetes-ingress-controller/nginx-amd64 tag 0.30? If I use that none of my ingresses work at all, I always end up on 'Welcome to nginx'.

@aledbf
Copy link
Member

aledbf commented Dec 6, 2017

@schulzp quay.io/kubernetes-ingress-controller/nginx-ingress-controller:0.9.0

@aledbf
Copy link
Member

aledbf commented Dec 6, 2017

@schulzp apologies, I sent the wrong image in the first link

@schulzp
Copy link

schulzp commented Dec 6, 2017

@aledbf, thanks! However, now I'm getting:

W1206 12:48:24.723194       7 controller.go:1063] Validating certificate against DNS names. This will be deprecated in a future version.
W1206 12:48:24.723201       7 controller.go:1068] ssl certificate default/tls-secret does not contain a Common Name or Subject Alternative Name for host foo.bar.azure.company.de. Reason: x509: certificate is valid for *.company.de, company.de, not foo.bar.azure.company.de

But that's expected, as explained by @SaaldjorMike

@Oleksii-Terekhov
Copy link

Oleksii-Terekhov commented Dec 24, 2017

same issue:
cert with Subject Alternative Name:
DNS:*.companyname.com, DNS:companyname.com
error on rabbitmq.namespace.k8s.companyname.com

@Nowaker
Copy link

Nowaker commented Jan 26, 2018

@Oleksii-Terekhov *.whatever is valid for this.whatever, not this.that.whatever.

@Oleksii-Terekhov
Copy link

-_- ok... but why apache/nginx/firefox/IE/chome and other soft use my "not valid" cert without troubles?

@Nowaker
Copy link

Nowaker commented Jan 27, 2018

Apache/nginx will serve whatever it's told to. Firefox/IE/Chrome will warn if invalid cert is served.

It's Ingress that rejects the certificate - and therefore, won't pass to nginx.

W0927 18:32:11.527281 7 controller.go:1109] ssl certificate test-ns/wildcard-tls does not contain a Common Name or Subject Alternative Name for host test.example.world.com 

@yxxhero
Copy link
Member

yxxhero commented Aug 12, 2019

any answer???

@tonyturino
Copy link

I'm running into this exact same issue with our wildcard certificate. Would really appreciate some kind of workaround or fix on this one..

@Leonardo-Ferreira
Copy link

Having the same issue... created a new question about it: #7349

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.