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

Ingress V1: hostname wildcard support #2138

Closed
jpeach opened this issue Jan 22, 2020 · 16 comments · Fixed by #3381
Closed

Ingress V1: hostname wildcard support #2138

jpeach opened this issue Jan 22, 2020 · 16 comments · Fixed by #3381
Assignees
Labels
area/ingress Issues or PRs related to the Ingress API.
Milestone

Comments

@jpeach
Copy link
Contributor

jpeach commented Jan 22, 2020

Support Ingress V1 hostname wildcards

https://github.com/kubernetes/enhancements/blob/master/keps/sig-network/20190125-ingress-api-group.md#hostname-wildcards

Add support for a single wildcard * as the first label in the hostname.

This may have knock-on effects on HTTPS, as a wildcard hostname probably needs a wildcard TLS certificate. Alternatively, you could support routing several non-wildcard TLS certificates to a single wildcard hostname.

@jpeach jpeach added the area/ingress Issues or PRs related to the Ingress API. label Jan 22, 2020
@jpeach jpeach mentioned this issue Jan 22, 2020
6 tasks
@sunjayBhatia sunjayBhatia self-assigned this Feb 10, 2021
@sunjayBhatia sunjayBhatia added this to the 1.13.0 milestone Feb 10, 2021
@sunjayBhatia
Copy link
Member

Using ingress conformance tests as integration tests, however we have a general issue with the conformance tests:

In #1697 and #1714 we stopped accepting Opaque type secrets that had certain keys (basically we reject those that mirror TLS secrets). The ingress conformance tests currently generate an Opaque secret so we don't set up any TLS listeners for Ingresses in the tests/the tests fail etc (see https://github.com/kubernetes-sigs/ingress-controller-conformance/blob/79bd068cbb31d77c2a060f61eea795f9791a3d48/test/kubernetes/kubernetes.go#L226-L231).

I made a PR to change the secrets generated by the tests to a TLS certificate (kubernetes-sigs/ingress-controller-conformance#88) but should we reconsider allowing Opaque certs? Has there been any clamor for this?

@sunjayBhatia
Copy link
Member

sunjayBhatia commented Feb 12, 2021

Also it seems this test: https://github.com/kubernetes-sigs/ingress-controller-conformance/blob/79bd068cbb31d77c2a060f61eea795f9791a3d48/features/host_rules.feature#L78-L82 is failing when wildcards are enabled (all other wildcard tests pass)

(The test sets up a wildcard *.foo.com)

                    {
                        "keyword": "When ",
                        "name": "I send a \"GET\" request to \"http://baz.bar.foo.com\"",
                        "line": 81,
                        "match": {
                            "location": "feature.go:112"
                        },
                        "result": {
                            "status": "passed",
                            "duration": 9144200
                        }
                    },
                    {
                        "keyword": "Then ",
                        "name": "the response status-code must be 404",
                        "line": 82,
                        "match": {
                            "location": "feature.go:134"
                        },
                        "result": {
                            "status": "failed",
                            "error_message": "expected status code 404 but 200 was returned",
                            "duration": 69000
                        }
                    }

Envoy might be doing too aggressive of matching, not the per-label wildcard matching we want

@youngnick
Copy link
Member

So, envoy may match more than one label as part of the *? We definitely need to check that. We may have to do something more clever with the pattern we send to get the matching behavior we want.

@sunjayBhatia
Copy link
Member

So, envoy may match more than one label as part of the *? We definitely need to check that. We may have to do something more clever with the pattern we send to get the matching behavior we want.

Yeah, from looking at Envoy code it looks like wildcard matching on virtualhost domain is a simple prefix (*.foo.com is somewhat like .+\.foo\.com in regex) or postfix (foo.* is somewhat like foo\..* in regex) match.

We can do what we want to do using a regex match on the Host header rather than the above

@skriss
Copy link
Member

skriss commented Feb 17, 2021

See #3180

@sunjayBhatia
Copy link
Member

Ive got the plain HTTP stuff working for this (worth making its own PR?)

Now for Ingresses with TLS enabled there are a few cases:

apiVersion: networking.k8s.io/v1
kind: Ingress
...
spec:
  tls:
  - hosts:
      - *.foo.com
    secretName: wildcard
  rules:
  - host: *.foo.com
     ...
  • Ingress with a TLS secret specified for the wildcard domain and an Ingress rule with a specific host
    • The tricky part would be associating the wildcard with the specific secure virtualhost in the DAG but that can be sorted out
apiVersion: networking.k8s.io/v1
kind: Ingress
...
spec:
  tls:
  - hosts:
      - *.foo.com
    secretName: wildcard
  rules:
  - host: bar.foo.com
     ...
  • Ingress with a TLS secret specified that matches a host under the wildcard domain (more specific secret than Ingress rule)
    • cc @youngnick this could be considered an error case, interpreted as a misconfiguration
    • A point against the above ^ would be if someone wanted to configure a wildcard Ingress rule and supply many specific hosts a secret applies to and not have to write a bunch of duplicated host rules, somewhat related to Please add support for wildcard virtual hosts #1228 (though it would probably just make more sense to configure a wildcard cert instead)
    • If we do support this:
      • Regardless of whether there is a bar.foo.com Ingress rule (separate from the wildcard) I would say the wildcard filter chain/transport socket should include the more specific secret
      • In the case there is a bar.foo.com rule, the cert may be unused since requests to bar.foo.com will always match that filter chain and not the wildcard
apiVersion: networking.k8s.io/v1
kind: Ingress
...
spec:
  tls:
  - hosts:
      - bar.foo.com
    secretName: bar
  rules:
  - host: *.foo.com
     ...

@youngnick
Copy link
Member

Ive got the plain HTTP stuff working for this (worth making its own PR?)
I think there's enough complexity in the TLS bits that we shouldn't bring the insecure parts in first, sadly.

  • Ingress with a TLS secret specified for the wildcard domain
    ...
    • we would need to modify the Lua script we add to account for the wildcard hostname I think
      Yes, I think we'll need to check the SNI binding stuff very carefully as we make this change. We'll definitely need more checks in the integration test to cover all these new cases.
  • Ingress with a TLS secret specified that matches a host under the wildcard domain (more specific secret than Ingress rule)

    • cc @youngnick this could be considered an error case, interpreted as a misconfiguration
    • A point against the above ^ would be if someone wanted to configure a wildcard Ingress rule and supply many specific hosts a secret applies to and not have to write a bunch of duplicated host rules, somewhat related to Please add support for wildcard virtual hosts #1228 (though it would probably just make more sense to configure a wildcard cert instead)
      I'd argue that, because Ingress objects aren't required to be unique on hostname at any level, we should recommend that people who want to do something like this have one Ingress per host, that refers to the specific hostname for rules and TLS.

I think that allowing this usecase at all will make the SNI binding much more difficult. I am not convinced it's worth all the extra effort, feels like YAGNI.

Thanks for the deep thought on this @sunjayBhatia!

@sunjayBhatia
Copy link
Member

Having some trouble with the wildcard filter chain and SNI/:authority validation:

With no code changes to the Lua filter we add to HTTPS filter chains, requests that match a wildcard host are all rejected with code 421 since nothing will literally equal the wildcard hostname. Obviously we can get around this, but a further issue is that I can't seem to get the requested server name from TLS/SNI in the Lua code. It's a little funny, I think I should be able to get it since the access logger has access to this in the %REQUESTED_SERVER_NAME% variable. Since the Lua filter has to be configured before the router, any HTTP headers we add in the route configuration are not present when we inspect a request with Lua, so setting something with a dynamic variable doesn't work as the custom header is added to the request later than the Lua filter is run.

Mostly for my own notes around test cases:

With a wildcard host and cert for *.foo.com we get a filter chain that matches on *.foo.com

  • Requests with :authority and SNI that match under the *.foo.com wildcard are good (e.g. a.foo.com)
  • Requests with :authority and SNI that match with more than one DNS label should be rejected w/ a 404 by the header match policy we set (e.g. a.b.foo.com)
  • Requests with :authority and SNI that do not match the wildcard should get an EOF from Envoy, or match a default/other filter chain since they don't match any filter chain (e.g. bar.com)
  • This is the problematic one: I cannot yet figure out how to detect requests with mismatched :authority and SNI (e.g. a.foo.com and b.foo.com) since the Lua interface doesn't seem to give any way to do so, these requests should return a 421 code

@jpeach
Copy link
Contributor Author

jpeach commented Feb 19, 2021

Couple of points that may be relevant

  • the security goal is to preserve the policy for hosts that is in the HTTPConnectionManager; if that's the same for multiple hosts (i.e. SNI and auth doesn't match) maybe that's OK
  • it might help to generate different Lua filters depending on context

@youngnick
Copy link
Member

Yes, I think that @jpeach's first point is a great one - the important thing we are aiming for here is that you can't specify a client cert in the TLS config, then swing between virtualhosts. I think it's fair to make the argument that if you are specifying a client cert, it's specified at the virtualhost level, so any routes under that will require the client cert.

In that case, your last concern doesn't matter, since any two hostnames that match the FQDN have the same security posture.

@sunjayBhatia
Copy link
Member

sunjayBhatia commented Feb 19, 2021

So with this Ingress:

apiVersion: networking.k8s.io/v1
kind: Ingress
...
spec:
  tls:
  - hosts:
      - "*.foo.com"
    secretName: wildcard
  - hosts:
      - bar.foo.com
    secretName: bar
  rules:
  - host: "*.foo.com"
     ...
  - host: bar.foo.com

Couldn’t we get misrouted requests, following the logic in envoyproxy/envoy#6767 (comment) (assuming a request first to foo.foo.com then another from the same browser to bar.foo.com)

@youngnick
Copy link
Member

🤦 I was thinking about this as a HTTPProxy thing, d'oh. I see what you are talking about now. I will need to think about this a bit.

@sunjayBhatia
Copy link
Member

See envoyproxy/envoy#15142

@xaleeks
Copy link

xaleeks commented Feb 22, 2021

So with this Ingress:

apiVersion: networking.k8s.io/v1
kind: Ingress
...
spec:
  tls:
  - hosts:
      - "*.foo.com"
    secretName: wildcard
  - hosts:
      - bar.foo.com
    secretName: bar
  rules:
  - host: "*.foo.com"
     ...
  - host: bar.foo.com

Couldn’t we get misrouted requests, following the logic in envoyproxy/envoy#6767 (comment) (assuming a request first to foo.foo.com then another from the same browser to bar.foo.com)

That was the use case I was thinking about in my last comment in #1228. Whatever approach we go with for using wildcard TLS certs, can we make sure it works the same way for HTTProxy as it does for Ingress? and in the future for httproutes.

@sunjayBhatia
Copy link
Member

sunjayBhatia commented Feb 23, 2021

That was the use case I was thinking about in my last comment in #1228. Whatever approach we go with for using wildcard TLS certs, can we make sure it works the same way for HTTProxy as it does for Ingress? and in the future for httproutes.

I think it is a lot simpler with HTTPProxy since there is only one virtual host configuration (FQDN + TLS config) at the root of a tree of HTTPProxies (here) and included "child" HTTPProxies cannot have virtual host configuration. Unlike Ingress where a single resource can be for multiple host names and multiple TLS secrets.

In general, Gateway APIs seems simpler as well since we don't have to match the hostname a secret is meant for to the hostname a listener/route is meant for. There is also a specific mechanism for a Gateway to say that TLS settings can be configured via Routes or not.

@sunjayBhatia sunjayBhatia modified the milestones: 1.14.0, 1.15.0 Mar 30, 2021
sunjayBhatia added a commit that referenced this issue Apr 26, 2021
- Wildcard virtualhosts now allowed
- Envoy's virtualhost matching does not match on a single DNS label like
we want to implement
- For wildcard host routes, we add a header match rule
to match on the ":authority" header that is more strict and will only
match one DNS label wildcard
- Adds an integration test to validate behavior with a wildcard
certificate
- Passes ingress conformance tests for wildcards
- Adds the "regex" header match strategy (only internal for now)
- Ensure regex header matches are sorted correctly (after exact, before
contains matches)

Fixes #2138
Updates #2139

Signed-off-by: Sunjay Bhatia <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ingress Issues or PRs related to the Ingress API.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants