On Tue, 26 Aug 2014, moparisthebest wrote:

> This patch implements public key pinning (currently only for OpenSSL) in
> curl by providing a path to a public key in DER format. The command line
> option is --pinnedpubkey and if it isn't provided, curl functions just like
> it did before.

Lovely! Thanks for helping out improving libcurl!

> Please let me know what else I need to do to get this accepted into curl.
> As far as I can find, there are currently no command line tools that support
> certificate or public key pinning, so it'd be great to have support in curl
> first.

First off, I think this is a really good first take. There are several
smaller formalities we need to get sorted in this patch:

  1 - build with ./configure --enable-debug and gcc will help you point out a
      lot of C90 non-compliant mistakes, like mixing code and variable
      declarations and using // comments. Personally I also use --enable-werror
      to make really sure I don't miss a warning...

  2 - a tiny detail but you use 4 letters indent-level in your new code while
      all curl code otherwise use 2

  3 - the "Arbitrary size" in pkp_pin_peer_pubkey() is not explained much but
      is set to 2048. How about making it a define, putting it somewhere at the
      top and explaining some reasoning why 2048 might be suitable?

Then, in order for us to accept this properly into the project there are two
more things I would like the patch to provide:

  A - documentation for the new option. It would end up a new man page since it
      is a new option. Remember that TLS and certificates etc are used in
      security sensitive areas so it is a good thing if users fully understand
      what the options do!

  B - a test case or two that shows that the feature is actually working. I can
      help out with that if you just provide files we can use to test with. One
      good test and one bad test would be a sufficient first step I'd say!

Finally, I just want to say that we won't merge this feature into git until
after the pending release anyway.

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