#1471 CURLOPT_PINNEDPUBLICKEY documentation incomplete

closed-fixed
None
5
2015-02-17
2015-01-16
Kyle
No

The documentation for CURLOPT_PINNEDPUBLICKEY available at:

http://curl.haxx.se/libcurl/c/CURLOPT_PINNEDPUBLICKEY.html

Does not state what happens when
BOTH CURLOPT_SSL_VERIFYHOST == 0 AND CURLOPT_SSL_VERIFYPEER == 0.

This is bad because (at least vtls/openssl.c) ignores the pinned public
key (other than to emit a verbose mode message) when VERIFYHOST and VERIFYPEER
are off.

For example (using curl 7.40 built with OpenSSL):

cd /tmp
openssl genrsa | openssl rsa -pubout > dummykey.pem
curl -vI --pinnedpubkey dummykey.pem https://github.com/

This appears in the output:

* SSL: public key does not match pinned public key!
curl: (90) SSL: public key does not match pinned public key!

and curl's exit status is 90. However, if we repeat like so:

curl -vI -k --pinnedpubkey dummykey.pem https://github.com/

Then only this appears in the output:

* SSL: public key does not match pinned public key!

And curl's exit status is 0.

That is completely unexpected and not mentioned anywhere in the docs for CURLOPT_PINNEDPUBLICKEY, so either it's a bug or the docs are wrong. And while you might also want to VERIFYHOST when using a pinned public key, that shouldn't be required to use one.

Please take appropriate action. I'm inclined to believe it's a bug because the whole point of using pinned public keys is so that you can completely ignore trusted root certificates etc. in favor of a pinned public key.

Discussion

  • Daniel Stenberg

    Daniel Stenberg - 2015-01-16
    • status: open --> open-confirmed
     
  • Daniel Stenberg

    Daniel Stenberg - 2015-01-16

    Thanks for your report!

    In general we try to make options as independent from each other as possible and I don't see any reason for this option to depend on the other. I think you're right in that it can be made to work even with -k, so we should make it so. At least where possible - I'm stating it carefully because some TLS backend may make it hard.

     
  • Kyle

    Kyle - 2015-01-16

    In that case, here's a suggested fix for vtls/openssl.c:

    diff --git a/lib/vtls/openssl.c b/lib/vtls/openssl.c
    index a68d88ea..efbdc17f 100644
    --- a/lib/vtls/openssl.c
    +++ b/lib/vtls/openssl.c
    @@ -2671,8 +2671,11 @@ static CURLcode ossl_connect_step3(struct connectdata *conn, int sockindex)
    * operations.
    */
    
    -  if(!data->set.ssl.verifypeer && !data->set.ssl.verifyhost)
    -    (void)servercert(conn, connssl, FALSE);
    +  if(!data->set.ssl.verifypeer && !data->set.ssl.verifyhost) {
    +    CURLcode check = servercert(conn, connssl, FALSE);
    +    if(check == CURLE_SSL_PINNEDPUBKEYNOTMATCH)
    +      result = check;
    +  }
       else
         result = servercert(conn, connssl, TRUE);
    

    Or something along those lines.

     
  • Kyle

    Kyle - 2015-01-19

    Yes, just tried that on top of 7.40.0. It seems to work very nicely. :)

     
  • Daniel Stenberg

    Daniel Stenberg - 2015-01-19
    • status: open-confirmed --> closed-fixed
     
  • Daniel Stenberg

    Daniel Stenberg - 2015-01-19

    Thanks for that, merged in commit be57f689b. Cased closed!

     

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks