On 4/21/2016 3:25 PM, moparisthebest wrote:
Hi all,

I was talking to a friend using OSX about --pinnedpubkey support, I
mentioned that it wasn't supported in the darwinssl/SecureTransport back
end, but the option was there and he tried it and successfully connected
with a bad key.  This got me thinking about failure modes:

$ echo bob > bad.key
$ curl --pinnedpubkey bad.key https://google.com/
curl: (90) SSL: public key does not match pinned public key!

Is how it behaves when --pinnedpubkey support is there, from
https://github.com/curl/curl/blob/master/docs/libcurl/opts/CURLOPT_PINNEDPUBLICKEY.3
today:

Added in 7.39.0 for OpenSSL, GnuTLS and GSKit. Added in 7.43.0 for
NSS and wolfSSL/CyaSSL. Added for mbedtls in 7.47.0, sha256 support
added in 7.44.0 for OpenSSL, GnuTLS, NSS and wolfSSL/CyaSSL. Other
SSL backends not supported.
So wherever there is no support, it 'fails open', I guess, in that it
just goes ahead and connects whether the public key matches or not.

There is one backend, GSKit, that supports der/pem files but not sha256
hashes (no curlssl_sha256sum defined), that 'fails closed' because it
thinks the hashes are a file, can't find them (or they don't match), and
it fails to connect.

As an aside, I notice it doesn't really say if mbedtls supports sha256
hashes, but I see it does in the code.

So:
1. Should unsupported backends 'fail open', if not is this a security issue?
2. Should --pinnedpubkey just not show up and be an unknown option if
compiled with an unsupported backend?
3. Should it just be left as-is today? (know your curl?)

Thanks for feedback,
Travis Burtrum
-------------------------------------------------------------------
List admin: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:  https://curl.haxx.se/mail/etiquette.html

In the future if you think you have a security issue e-mail curl-security at haxx.se.

I see what you're saying. The easiest way to fix it would be require a symbol that is defined only when the feature is available and put that in the header of each vtls that is supported, and wrap all the pubkey code. Only axtls, darwin and schannel are not supported I think. (Someone was working on schannel though so that may change.) Like this

#define have_curlssl_pinnedpubkey

Then in each file guard it with that and in setopt do something like:

diff --git a/lib/url.c b/lib/url.c
index 792ce77..9700373 100644
--- a/lib/url.c
+++ b/lib/url.c
@@ -2072,12 +2072,16 @@ CURLcode Curl_setopt(struct SessionHandle *data, CURLopt
 #endif
     break;
   case CURLOPT_PINNEDPUBLICKEY:
+#ifdef have_curlssl_pinnedpubkey /* not supported by all backends */
     /*
      * Set pinned public key for SSL connection.
      * Specify file name of the public key in DER format.
      */
     result = setstropt(&data->set.str[STRING_SSL_PINNEDPUBLICKEY],
                        va_arg(param, char *));
+#else
+    result = CURLE_NOT_BUILT_IN;
+#endif
     break;
   case CURLOPT_CAINFO:
     /*


The error looks like
curl: (4) A requested feature, protocol or option was not found built-in in this libcurl due to a build-time decision.

-------------------------------------------------------------------
List admin: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:  https://curl.haxx.se/mail/etiquette.html

Reply via email to