-
Notifications
You must be signed in to change notification settings - Fork 10
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
Enforce SSL "bug?" #21
Comments
From @krancour on March 21, 2016 16:44 After further thought... I am of the opinion that enforcing HTTPS should not be contingent upon cert availability... that would make it too easy for an app admin (in the Google operational model) to circumvent the wishes of a cluster admin by simply not providing a cert for his/her custom domain. @helgi I am interested in your opinion on this one. |
From @helgi on March 22, 2016 20:28
|
From @krancour on March 22, 2016 23:19 Ok. So I think this isn't a bug, but does call for a little bit of doc to clarify this behavior. Retagging accordingly. |
From @sstarcher on May 27, 2016 11:34 So I just ran into this and it was a little painful to figure out. This was not the behavior of Deis v1 as far as I can remember. |
From @krancour on May 27, 2016 18:13 @sstarcher, the behavior of v1.x was that if an app with a custom / vanity domain didn't have its own cert, the router / platform cert got used instead. And this would always result in a cert warning. https://github.com/deis/deis/blob/master/router/rootfs/etc/confd/templates/nginx.conf#L211-L225 imo, how the v2 router handles this same scenario is an improvement. If an app with a custom / vanity domain doesn't have a cert, then it's not listening on 443 at all. (To me, this makes more sense than simply falling back to an incorrect cert.) Therefore, any request to it using HTTPS falls to the default vhost and the user gets both a cert warning (as they also would have in v1) and, if they ignore that warning, a 404. imo, 404 is the correct response for such a scenario because the end user has requested a URL that really does not exist simply by virtue of the correct cert not having been provided. The more I contemplate this issue, the more I feel the current behavior is exactly what one should expect. As I'd noted some time ago, if anything, this just needs to be documented. What I'm not clear on is where this doc addition would be most effective. @sstarcher since you ran into some of this recently... do you have any thoughts as to where a mention of this would have saved you some grief? I am contemplating here: https://github.com/deis/router/blob/master/README.md#ssl And here: https://github.com/deis/workflow/blob/master/src/applications/ssl-certificates.md Something along the lines of:
|
From @sstarcher on May 27, 2016 18:33 @krancour I relied on the behavior from v1. We have a few domains that we do not have certs for, but I still redirect over SSL and yes they certainly get cert warnings. If this is the expected behavior for Deis v2 I guess I could respecify my platform cert for everything I don't have a cert for. Basically Deis v2 if a you configure SSL and the apps name is not the prefix of the domain SSL will fail. |
From @krancour on May 27, 2016 19:32
But it's really, "works," I think. There are definitely cert warnings in those cases. From an end-user's perspective-- that's broken. fwiw, I wish those didn't work (or even "work"). It's a quirk of a regex being used to match URLs if the platform domain has not been defined because we wanted an easy install experience targeted toward first-time users, wherein providing any specific configuration was 100% optional and would work even if skipped. This forced the router's I cannot see it as being reasonable for any vhost to listen on 443 if it doesn't have an adequate certificate. Deliberately using the wrong cert doesn't seem like it really improves anything. |
From @sstarcher on May 27, 2016 20:56 @krancour That reasoning makes sense, but with the current setup of Deis and how it handles certs and domains I think you are going to make it very error prone for users in that case. I don't know about most users, but I would rather serve a site with the wrong cert and falling back to the platform cert instead of serving a 404. If Deis had inbuilt error checking when enforce HTTPS was turned on for adding domains without attached certs giving the user an easy way to be aware of the mistake that would go a long way. This is even more so the case for people coming from Deis v1. |
From @krancour on May 27, 2016 21:10
This is probably, largely, a matter of perspective. I'm deferring to the principle of least surprise. I think it's less surprising that SSL doesn't work when a appropriate cert hasn't been provided, than for it to "work" using the wrong cert. |
From @sstarcher on May 27, 2016 21:13 My understanding is that was the reason for the existence of the platform cert was as a fallback. So I would classify using the platform cert as the least surprise. Even more so since the platform cert is still used for anything that starts with the app name. Currently an exception is being made for vanity domains that is not applied to APP_NAME.* domains. |
From @sstarcher on May 27, 2016 21:16 I will say for my use case it would also cause me pains if the current functionality of APP_NAME.* domains were to break and require explicit SSL certs. Internal our primary cluster is *.cluster.private which references either *.green.private or *.blue.private After that we load over addresses from *.cluster.private to start using *.green.private. If you were to fully enforce what your talking about it would break my entire workflow. |
From @krancour on May 27, 2016 21:40
I don't know that I'd classify it as such. The platform cert (which is assumed to be a wildcard cert) has always been to provide SSL to the default domains for each app. So if your platform domain is Remove yourself from the v1 context for a moment and try to imagine you are brand new to Workflow. Approach armed only with what you know about SSL. If you haven't provided a cert for Another thing to keep in mind here. By having things not work when SSL isn't configured properly, it encourages users to incur the effort to configure SSL properly. Cert warnings are a disservice to users. If a user has to ignore a cert warning to access one of your sites under normal conditions, they won't know when there's actually a problem with their secure connection to your site. |
From @krancour on May 27, 2016 21:43 The obvious rebuttal to my last argument, of course, would be for non-production apps-- development or staging environments, etc.-- where the end users are developers who understand the cert warning and accompanying risks, but in such cases, vanity domains probably aren't required. |
From @sstarcher on May 27, 2016 22:6 @krancour I would agree with your previous statement if platform domain was a required entity. I could certainly be in the minority. |
From @krancour on May 27, 2016 22:12 Like I said... I do still wish it were required that we set that. I'm actually going to add mention of it to this doc. I feel that splits the difference between something that should be set and not requiring it to be set so as to preserve the easy installation experience. |
From @sstarcher on May 27, 2016 22:23 @krancour making the platform domain required would certainly destroy my workflow ;) |
From @krancour on May 27, 2016 22:31 It won't be. But I also don't really understand how it is your workflow (whether in v1 or v2) depends on all the ambiguity that results from the things we've discussed. |
From @sstarcher on May 28, 2016 13:34 @krancour as long as the platform domain is not required I can work around the not falling through to the platform cert. I'll just have to explicitly attach a few domains I normally let fall to the platform cert. |
From @krancour on March 21, 2016 16:32
I'm not certain this is a bug, but I thought I'd open it up to discussion.
Currently, the choice to enforce the use of HTTPS (redirect if proto is HTTP), is made at the platform / router level. I have #148 open to track a possible enhancement that makes that configurable on an app-by-app basis. Regardless of whether this were configured router-wide or on an app-by-app basis...
If
nginx.ssl.enforce: "true"
, but no cert is available for a given domain, regardless of that is a subdomain of the platform domain, or a "custom" domain, then that app has no vhost listening on 443. The end result is that the request falls to the default vhost and a 404 is returned.I'm not clear whether this is a bug (maybe enforcing HTTPS should only happen if there's a cert available for the given domain?) or if this is really just the expected behavior... i.e. "Hey... you asked me to enforce HTTPS... you gave me no cert to use... that is a hard failure."
Unless anyone has a very strong opinion that this requires code changes, my approach to this is going to be to clarify the behavior in router's documentation.
Copied from original issue: deis/router#149
The text was updated successfully, but these errors were encountered: