-
Notifications
You must be signed in to change notification settings - Fork 9.2k
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
OkHostnameVerifier: Don't fall back to CN verification. #3764
OkHostnameVerifier: Don't fall back to CN verification. #3764
Conversation
The use of Common Name was deprecated in RFC 2818 (May 2000), section 3.1: Although the use of the Common Name is existing practice, it is deprecated and Certification Authorities are encouraged to use the dNSName instead. In 2017, Chrome 58, Firefox 48, and Opera 45 web browsers removed this fallback, with Chrome leaving it configurable for enterprise deployments (see https://www.chromestatus.com/feature/4981025180483584). Android is removing it in http://r.android.com/581382 .
Neat! |
I expect this to cause functionality to stop working for some existing working clients. So we should warn people upgrading and have a plan to support them. nit: Our docs still say we are RFC 2818 compliant which we are not. Should we reference the https://cabforum.org/wp-content/uploads/CA-Browser-Forum-BR-1.5.1.pdf policy instead? BTW We don't expose an additional security hole for valid certificates as we only use the CN if no alt names are present.
The chrome behaviour I assume is to fail the page unless a user clicks the advanced button and decides to continue. The android behaviour is a great sensible default. The firefox default behaviour is to fallback for certs issued before 23 August 2016. My concern here is that we will break OkHttp for various internal cases
The workaround is obviously to implement your own Hostname verifier, but I suspect many developers will just end up disabling hostname verification when they hit this. Should we support this via adding a new OkHostnameVerifier with a tolerant mode enabled to make the upgrade easier without disabling? BTW this is an interesting paper https://www.cs.utexas.edu/~shmat/shmat_ccs12.pdf |
I definitely want to call this out in our changelog. Anywhere that we break old clients is something worth bringing our users’s attention to. For user impact, I’d like to take a wait-and-see approach. My guess is that anyone using TLS in local development already has a hacked-together setup because of the trust store. For anyone else that has problems it’s tempting to create a Gist with the old hostname verifier and to encourage users to install that themselves if they need something lenient. Pasting a big hostname verifier into your code feels gross, and that’s kind of the point. |
I started out the reply as we should revert this, and ended up after research at, this is a good change but we should give people a safe way out. +1 to the gist and changelog. |
Hi there! Chromie who lead the charge for deprecation, but perhaps didn't do a good enough job explaining at the time the motivation (as it wasn't yet public) - I think if you provide it, you may want to call out the security implications of using the Legacy Hostname Verifier. The best way to see these implications is at https://nameconstraints.bettertls.com/ (which is on GitHub at https://github.com/Netflix/bettertls ). Common Name support ignores nameConstraints in certificates, unless verifiers take additional steps not specified by RFC5280 to try and transitively evaluate and apply them. This is because commonName, unlike subjectAltName, is untyped - it could be a domain name, an IP address, or just a string of characters. To that end, the naming may wish to clarify that it's an InsecureHostnameVerifier rather than a Legacy. This is especially important as more CAs begin to issue nameConstrained CA certificates - these certificates are exempt from various supervisory regimes of the browsers (by virtue of being, well, constrained), but clients that don't enforce those constraints are at risk. An important thing worth calling out here is that name constraints is part of path building inputs - namely, nameConstraints apply on instances within the Subject or Subject Alt Name (and can cause a cert to be rejected), but they do not constrain the subsequent nameConstraints (RFC 5280 4.2.1.10). Thus, it's legitimate to have a chain (leaf -> intermediate 1 -> intermediate 2 -> root) with a leaf cert for Hope that helps explain the value of restricting commonNames, and the risk that would need to be mitigated (or communicated) if restricting is undesirable. You certainly can determine if you're going to match the CN (in the absence of SAN) and how you're going to match, and then recompute the effective nameConstraints in a 5280-style validation to see if they're acceptable - and if you do that, you will have mitigated the risk, albeit with more complexity :) |
@sleevi thanks for the explanation, I saw mention in the Browser forum of people changing names legally to (something like) google.ca :) The examples I gave (proxies, legacy and self signed) are not public production, so I think a gist is fair enough, and OkHttp should favour strong security policies. But if you google how to self sign you legacy service, you still get an example using Common Names, so it's going to come up for a while. |
Right, I just mean that if you enable it for those cases, and don’t also restrict your set of trusted CAs to just those local CAs, then you’re at risk of a publicly-trusted, name-constrained CA issuing a cert that your client will trust, even when it shouldn’t. That’s the risky part :) |
The use of Common Name was deprecated in RFC 2818 (May 2000), section 3.1: Although the use of the Common Name is existing practice, it is deprecated and Certification Authorities are encouraged to use the dNSName instead. This backports upstream commit 52764cb4b9219d699b66e96ccf54db1c37c638bb from square/okhttp#3764 Bug: 70278814 Test: CtsLibcoreOkHttpTestCases Change-Id: Iefa8645b93103d70f057a872ac4332147bc2d4d2
This hasn't been used since we stopped supporting CN verification in #3764.
The use of Common Name was deprecated in RFC 2818 (May 2000), section 3.1:
In 2017, Chrome 58, Firefox 48, and Opera 45 web browsers removed this
fallback, with Chrome leaving it configurable for enterprise deployments
(see https://www.chromestatus.com/feature/4981025180483584).
Android is removing it in http://r.android.com/581382 .