NE-310 Enhancement proposal for HSTS route admission plugin #749
Conversation
candita
changed the title
openshift-ci-robot
added
the
do-not-merge/work-in-progress
candita
changed the title
candita
force-pushed
the
NE-310-global-admission-hsts
branch
from
920c8ac
to
0d17500
Compare
candita
force-pushed
the
NE-310-global-admission-hsts
branch
from
0d17500
to
538b3c4
Compare
|
@deads2k @mcurry-rh @Miciah please take a look |
candita
force-pushed
the
NE-310-global-admission-hsts
branch
3 times, most recently
from
ea3c088
to
f4108f9
Compare
| and this period of time elapses before the client makes another request for the same host, the | ||
| HSTS policy will expire on that client | ||
| - `includeSubDomains`: if present, applies to any subdomains of the host's domain name, when the | ||
| route admission policy allows wildcards. If wildcard routes aren't allowed, this directive has no |
There was a problem hiding this comment.
If wildcard routes aren't allowed, this directive has no effect
Clarify this for me. If foo.company.com returns an HSTS header that says includeSubDomains, even if our router doesn't serve, superimportant-http-resource.foo.company.com will suddenly become inaccessible, right? That's part of the risk we're looking to mitigate.
There was a problem hiding this comment.
Thanks! I updated this section, code samples, and the risks and mitigation sections.
There was a problem hiding this comment.
If the route admission policy with host foo.com allows subdomain wildcards it should use the includeSubDomains directive only if all subdomains of that route should share the same HSTS Policy. There cannot be a mix of plain HTTP and HTTPS routes covered by the route.
If the route admission policy with host foo.com does not allow subdomain wildcards, it may not use the includeSubDomains directive, because it would unintentionally convey its HSTS Policy to any routes having foo.com as a superdomain. A route configured without allowing subdomain wildcards will be rejected if it contains includeSubdomains in its HSTS Policy.
There was a problem hiding this comment.
And to be clear, while we only have visibility to route based ingress into a single cluster, there is no actual restriction that our router owns an entire subdomain. Traffic can be intercepted in front of it using ServerName from the TCP requests themselves to have a subdomain of foo.com served by something that isn't even openshift based.
There was a problem hiding this comment.
In the end, there will be no changes in whether a route is accepted/rejected. I will document all the ways things could go wrong, for the end user, and let them do what they choose. I listed this in Design Details in what I hope is the last update of this EP, plus added a few corrections.
candita
force-pushed
the
NE-310-global-admission-hsts
branch
10 times, most recently
from
34fc7e2
to
1250e46
Compare
| - If the route admission policy with host `foo.com` allows subdomain wildcards it should use the | ||
| `includeSubDomains` directive only if all subdomains of that route should share the same HSTS Policy. There | ||
| cannot be a mix of plain HTTP and HTTPS routes covered by the route. |
There was a problem hiding this comment.
I'm not sure what "the route admission policy with host foo.com means; a router has an admission policy, and a route has a host and wildcard policy.
The behavior with respect to wildcard routes is tricky because the route API is a little wonky: If a route has spec.host: foo.com and spec.wildcardPolicy: Subdomain, then really the route matches *.com. Thus a more realistic example would be a route with spec.host: bar.foo.com and spec.wildcardPolicy: Subdomain.
Furthermore, the router's host admitter should not admit a route with a host under domain foo.com if there is a wildcard route for bar.foo.com, so there shouldn't ever be any routes that use subdomains of a wildcard route's domain.
There was a problem hiding this comment.
Thanks for the guidance. I have updated this, and the conclusion I've added is:
...if a route with host of bar.foo.com allows a WildcardPolicy of Subdomain, then it exclusively serves all the hosts ending in foo.com. Another route could not be created that ends in foo.com. Furthermore, if a route with host of bar.foo.com allows a WildcardPolicy of Subdomain, AND specifies a HSTS Policy with includeSubDomains, then there would be no way to add HSTS Policy to foo.com or def.foo.com
This introduces a new issue we should discuss, and I added it to the Open Questions section.
There was a problem hiding this comment.
I updated the Design Details to clarify the issue.
| - it is possible for a HSTS host to offer unsecured services on alternate ports or different subdomains, if the | ||
| HSTS Policy is set to `includeSubDomains` but there are non-matching or non-existent HSTS Policies required | ||
| for those different subdomains |
There was a problem hiding this comment.
This is confusing. Do we need to worry about "alternate ports" in the context of OpenShift? The router only handles routes on ports 80 and 443. What is an HSTS host, exactly, in this context? If a route specifies an HSTS policy with includeSubDomains, then once a client uses that route, the client should use the route's HSTS policy for any connections involving the route's host or any host that is a subdomain of the route's host.
There was a problem hiding this comment.
This is describing what could happen if the mitigations weren't applied, but this section is still under development based on other comments.
| To mitigate issues, it is recommended to refrain from using `includeSubDomains` in the case that HTTPS and HTTP | ||
| route share the same domain, or to ensure HTTP based services are also offered via HTTPS for the same subdomain. | ||
| In the case that different web applications are offered on different subdomains of a HSTS host, and some use HTTP | ||
| while others use HTTPS, each domain should be configured separately instead of using `includeSubDomains` on a superdomain. | ||
| This will be incorporated into validation code that enforces the use of a Wildcard Policy for subdomains for each | ||
| route that attempts to use `includeSubDomains` in its HSTS Policy. If a route does not have a Wildcard Policy of | ||
| subdomain, it will not be admitted if it tries to use a HSTS Policy with `includeSubDomains`. |
There was a problem hiding this comment.
Is the validation code going to reject any attempt to do the thing that you say here it is recommended to refrain from doing? It seems redundant and misleading to recommend refraining from doing a thing if any attempt to do the thing is rejected anyway.
There was a problem hiding this comment.
I don't think we can reject all cases since there is no guarantee that the entire domain is served by a single openshift cluster.
There was a problem hiding this comment.
I will add a note to the Open Questions.
candita
force-pushed
the
NE-310-global-admission-hsts
branch
from
1250e46
to
e43bec8
Compare
|
|
||
| ### User Stories | ||
|
|
||
| #### As a cluster administrator, I want to verify HSTS globally, for all TLS routes in domain `foo.com` |
There was a problem hiding this comment.
@soltysh have a think on what capabilities oc and/or our existing route fields selectors are missing to make the selection below easier to manage.
I think we're looking for
- all routes with a particular annotation set
- all routes with a particular unset
- http/https field selector that can be used in conjunction with 1 or 2
- a namespace label selector to constrain namespaces in conjunction with 3
This feature may be important enough to require an oc routes hsts-something command. On the whole, I suspect the actions are predictable and the golang will be easier than bash.
There was a problem hiding this comment.
- all routes with a particular annotation set
- all routes with a particular unset
We don't have any annotation selectors, only label selectors, since that's what api server allows. Do we want to add that capability to oc/kubectl, it still will be client-side, though.
- http/https field selector that can be used in conjunction with 1 or 2
That should be possible already.
- a namespace label selector to constrain namespaces in conjunction with 3
We are iterating over namspaces for -A so we could theoretically filter out based on label selector, the question is only UX.
candita
force-pushed
the
NE-310-global-admission-hsts
branch
from
e43bec8
to
ac4340f
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
candita
force-pushed
the
NE-310-global-admission-hsts
branch
from
ac4340f
to
ab63ad1
Compare
candita
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
|
/assign @stevekuznetsov |
| @@ -29,6 +29,7 @@ see-also: | |||
| replaces: | |||
|
|
|||
| superseded-by: | |||
| - enhancements/enhancements/ingress/global-admission-hsts.md | |||
There was a problem hiding this comment.
| - enhancements/enhancements/ingress/global-admission-hsts.md | |
| - enhancements/ingress/global-admission-hsts.md |
candita
force-pushed
the
NE-310-global-admission-hsts
branch
from
ab63ad1
to
1631abc
Compare
|
There have been some minor revisions in openshift/api#958 to the API proposed here, but nothing too substantial, so let's merge this PR now and revise later. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Miciah The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
NE-310 enhancement proposal supersedes previous NE-310 enhancement proposal: https://github.com/openshift/enhancements/blob/master/enhancements/ingress/global-options-enable-hsts.md