nativescript-community / https

Secure HTTP client with SSL pinning for Nativescript - iOS/Android
https://nativescript-community.github.io/https/
Other
50 stars 42 forks source link

Public key pinning support #1

Open buu700 opened 7 years ago

buu700 commented 7 years ago

I see that cert pinning is supported, but how much extra work would it be to add a method for pinning just a public key?

This would be much more convenient in most cases I think, given that key pairs are generally long-lived while certificates change on a regular basis — more so now with the rising popularity of Let's Encrypt which issues 90-day certs. With public key pining, I can just commit the public key (or hash) as a static file once and forget about it, whereas it seems like cert pinning would require me to implement additional logic to ensure that the latest version of the cert is always stored locally.

(Note: I've looked at the readme, but haven't yet used this library or done much digging into how Android, iOS, or the linked libraries handle TLS pinning; so my assumptions about what is and/or can be supported may be completely off.)

roblav96 commented 7 years ago

@buu700 currently the plugin grabs the public key from a file bundled with your app. in the future this can be done via a string or an http call.

buu700 commented 7 years ago

Got it, thanks! So can the certificate property of HttpsSSLPinningOptions be a path to a public key instead of a certificate then?

roblav96 commented 7 years ago

Your public key is the certificate, so yes.

buu700 commented 7 years ago

A public key (hash) is a subset of a certificate, not the same thing as a certificate; a certificate is a signed message from a CA assuring that a certain public key should be treated as valid for a certain host until a certain date. A public key can remain constant indefinitely, while certificates will change regularly — e.g. as in the Let's Encrypt example above, every 90 days.

That's why I was asking about pinning public keys (similarly to HPKP) rather than certificates, because in many (most?) cases I'd think it would be easier to include one static public key file in an application bundle than to come up with a way to ensure that every client always has an up-to-date certificate file.

roblav96 commented 7 years ago

Ohhh i see what you're saying. I have not looked into implementing anything like that. AFNetworking and okhttp support pinning certs out of the box which is what this plugin uses. Anything more than that I have not looked into.

buu700 commented 7 years ago

Ah, I see; thanks! Just went ahead and looked at the docs of both libraries:

roblav96 commented 7 years ago

I see. Right now the plugin supports certificate pinning and I'll plan on adding pubkey pinning as an option in the future.

buu700 commented 7 years ago

Cool, that would be really useful.

EddyVerbruggen commented 7 years ago

Looks like the plugin currently correctly implements public key pinning. iOS source. Android should already be OK because okhttp uses pubkey pinning.

buu700 commented 7 years ago

That's great to hear! Does it look like there's logic in there that dynamically chooses between public key and cert pinning based on the input like okhttp, or would some other option need to be passed in to nativescript-https to trigger that code path?

rhclayto commented 5 years ago

@buu700 :

That's great to hear! Does it look like there's logic in there that dynamically chooses between public key and cert pinning based on the input like okhttp, or would some other option need to be passed in to nativescript-https to trigger that code path?

From the code at https://github.com/gethuman/nativescript-https/blob/master/src/https.android.ts 👍

let file = new java.io.File(options.certificate)
inputStream = new java.io.FileInputStream(file)
let x509Certificate = java.security.cert.CertificateFactory.getInstance('X509').generateCertificate(inputStream)
peer.x509Certificate = x509Certificate
certificate = okhttp3.CertificatePinner.pin(x509Certificate)
inputStream.close()

For Android, this plugin is taking the certificate supplied by you, creating a new x509 certificate from it, & feeding it to okhttp3's CertificatePinner.pin method. This method actually takes a full certificate as an argument ( https://square.github.io/okhttp/3.x/okhttp/okhttp3/CertificatePinner.html#pin-java.security.cert.Certificate- ), & '[r]eturns the SHA-256 of certificate's public key. ' The code then assigns this hash to peer.certificate, which is used elsewhere to actually create the pin. So as @EddyVerbruggen says, & as the quote you provided mentions, even when you supply a certificate, okhttp3 is pinning against (the hash of) the public key within that certificate. You supply a cert, okhttp3 pins the public key, never the cert itself.

For iOS, the same is true. As @EddyVerbruggen points out, this plugin already uses the AFSSLPinningModePublicKey. What this means is that this library pins the public key, not the certificate. See https://github.com/AFNetworking/AFNetworking/blob/6a78f1b4573d605bbe615a4e8199ae65b5e3faa0/AFNetworking/AFSecurityPolicy.h#L125-L142 : 'AFSSLPinningModePublicKey Validate host certificates against public keys of pinned certificates.'

In either case, you supply a cert, & this library pins against the public key contained within it. There appears to be no option to pin against the certificate itself at all. This is probably what is wanted for most mobile apps, & what was asked for by @buu700. So in fact this issue probably ought to be closed, since what it's asking for is already the case. However, it does raise the question: Should another issue be created for those who are seeking actual certificate pinning (which by the way is usually not a good idea, & is not possible with okhhtp3 anyway)? And should the documentation for this plug-in clarify what is actually being pinned?

PS: letsencrypt's certbot now has an option --reuse-key that will keep the same key pair in place while issuing new certificates. Check certbot --help all|less under the 'automation' section.

buu700 commented 5 years ago

Nice, thanks for looking into that! Sounds like it'll do exactly what I want it to as-is then.

I'll leave this issue open for discussion of those last two questions (clarifying the docs sounds like a great idea), but feel free to close this issue whenever it's no longer needed Rob.

On Sat, Jan 19, 2019 at 2:11 AM rhclayto notifications@github.com wrote:

@buu700 https://github.com/buu700 :

That's great to hear! Does it look like there's logic in there that dynamically chooses between public key and cert pinning based on the input like okhttp, or would some other option need to be passed in to nativescript-https to trigger that code path?

From the code at https://github.com/gethuman/nativescript-https/blob/master/src/https.android.ts 👍

let file = new java.io.File(options.certificate) inputStream = new java.io.FileInputStream(file) let x509Certificate = java.security.cert.CertificateFactory.getInstance('X509').generateCertificate(inputStream) peer.x509Certificate = x509Certificate certificate = okhttp3.CertificatePinner.pin(x509Certificate) inputStream.close()

For Android, this plugin is taking the certificate supplied by you, creating a new x509 certificate from it, & feeding it to okhttp3's CertificatePinner.pin method. This method actually takes a full certificate as an argument ( https://square.github.io/okhttp/3.x/okhttp/okhttp3/CertificatePinner.html#pin-java.security.cert.Certificate- ), & '[r]eturns the SHA-256 of certificate's public key. ' The code then assigns this hash to peer.certificate. So as @EddyVerbruggen https://github.com/EddyVerbruggen says, & as the quote you provided mentions, even when you supply a certificate, okhttp3 is pinning against (the hash of) the public key within that certificate. You supply a cert, okhttp3 pins the public key, never the cert itself.

For iOS, the same is true. As @EddyVerbruggen https://github.com/EddyVerbruggen points out, this plugin already uses the AFSSLPinningModePublicKey. What this means is that this library pins the public key, not the certificate. See https://github.com/AFNetworking/AFNetworking/blob/6a78f1b4573d605bbe615a4e8199ae65b5e3faa0/AFNetworking/AFSecurityPolicy.h#L125-L142 : 'AFSSLPinningModePublicKey Validate host certificates against public keys of pinned certificates.'

In either case, you supply a cert, & this library pins against the public key contained within it. There appears to be no option to pin against the certificate itself at all. This is probably what is wanted for most mobile apps, & what was asked for by @buu700 https://github.com/buu700. So in fact this issue probably ought to be closed, since what it's asking for is already the case. However, it does raise the question: Should another issue be created for those who are seeking actual certificate pinning (which by the way is usually not a good idea, & is not possible with okhhtp3 anyway)? And should the documentation for this plug-in clarify what is actually being pinned?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/gethuman/nativescript-https/issues/1#issuecomment-455767025, or mute the thread https://github.com/notifications/unsubscribe-auth/AAZWy8ugDXyDMIPb0M9CYMgg8k07fW5Wks5vEu9PgaJpZM4LuV2x .