Deprecate SSL pinning and trust chain verification. #534
Conversation
| @@ -13,6 +13,9 @@ | |||
|
|
|||
| NS_ASSUME_NONNULL_BEGIN | |||
|
|
|||
| // NOTE: While publicly, SocketRocket does not support configuring the security policy with pinned certificates, | |||
There was a problem hiding this comment.
Nit: Use a section comment aka /** ... */ as well as document the class itself?
There was a problem hiding this comment.
It is a private class as of right now, I don't think full documentation is required at this time. I will change to using /** comments though.
| - (void)setSR_SSLPinnedCertificates:(nullable NSArray *)SR_SSLPinnedCertificates | ||
| { | ||
| [NSURLProtocol setProperty:[SR_SSLPinnedCertificates copy] forKey:SRSSLPinnnedCertificatesKey inRequest:self]; | ||
| [NSException raise:NSGenericException |
There was a problem hiding this comment.
Nit: Not sure if Generic or Parameter exception? You sure about Generic? Not that it matters much tho...
Oh boy. Here's a controversial change.  Let's give a bit of backstory. A few weeks ago, Facebook was contacted by a whitehat hacker (the good guys) about a security vulnerability here in SocketRocket. For those of you who are truly interested in what that security flaw was, it is essentially the same flaw as outlined here: https://www.synopsys.com/blogs/software-security/ineffective-certificate-pinning-implementations/ So, we were faced with a choice - quietly push out a patch, and hope that eventually existing applications updated, or be transparent and admit we screwed up. This is us admititng we screwed up. And while yes, we could probably fix the implementation. But we talked internally, and decided that the best approach here is to completely remove the option for pinning. For all of our existing users that use certificate pinning, while we understand that in the past there has been a very large barrier to entry with getting a CA to issue a certificate. However, since the rollout of CAs like LetsEncrypt, there's become an ever-dwindling reason to actually use self-signed or unsigned certificates. For this reason, we're going to go ahead and deprecate the APIs that allow SSL pinning and disabling trust chain verification. The pinning APIs are now going to throw an exception when invoked, and the trust chain APIs have deprecation warnings. If you are a user of these APIs, and you for some reason **CANNOT** use a trust chain validated certificate, PLEASE speak up. While we cannot think of any reason to use those kinds of certificates, it's entirely possible we overlooked something. We'll leave this pullrequest unmerged for a two week period (Monday, August 28th, 2017), at which point, unless we have feedback convincing us otherwise, we will go ahead with this change.
richardjrossiii
force-pushed
the
richardross.pinning
branch
from
3c53f16
to
1649f71
Compare
Deprecate SSL pinning and trust chain verification. (facebookincubator#534)
|
I know this is already merged, but here is an example where you would need self signed certificate and you don't have a possibility of using CA to make valid trusted certificate. Imagine this, you have a device which has REST API and it's only accessible locally over LAN. Since it is local device, it has a local IP address, but doesn't have domain. You still want to use SSL for this REST API, that is, access it over HTTPS. Since it has only local IP address and no domain, it's impossible for CA to issue certificate and our only option is to use self signed certificate. |
|
In the wake of Project Atlas and the lack of adoption of HPKP, wouldn't certificate pinning be useful in preventing eavesdropping by using a rogue root certificate? Or is there a way to enforce Certificate Transparency inside my iOS/Android app? |
Oh boy. Here's a controversial change.
Let's give a bit of backstory.
A few weeks ago, Facebook was contacted by a whitehat hacker (the good
guys) about a security vulnerability here in SocketRocket.
For those of you who are truly interested in what that security flaw
was, it is essentially the same flaw as outlined here:
https://www.synopsys.com/blogs/software-security/ineffective-certificate-pinning-implementations/
So, we were faced with a choice - quietly push out a patch, and hope
that eventually existing applications updated, or be transparent and
admit we screwed up.
This is us admititng we screwed up. And while yes, we could probably fix
the implementation. But we talked internally, and decided that the best
approach here is to completely remove the option for pinning.
For all of our existing users that use certificate pinning, while we
understand that in the past there has been a very large barrier to entry
with getting a CA to issue a certificate.
However, since the rollout of CAs like LetsEncrypt, there's become an
ever-dwindling reason to actually use self-signed or unsigned
certificates.
For this reason, we're going to go ahead and deprecate the APIs that
allow SSL pinning and disabling trust chain verification. The pinning
APIs are now going to throw an exception when invoked, and the trust
chain APIs have deprecation warnings.
If you are a user of these APIs, and you for some reason CANNOT use
a trust chain validated certificate, PLEASE speak up. While we cannot
think of any reason to use those kinds of certificates, it's entirely
possible we overlooked something. We'll leave this pullrequest unmerged
for a two week period (Monday, August 28th, 2017), at which point,
unless we have feedback convincing us otherwise, we will go ahead with
this change.