Re: [Bug-wget] Implement --pinnedpubkey option to pin public keys

[Tim Rühsen]

Hi Travis,

just a few thoughts about your patch resp. HPKP in general.

How do I create a pinnedpubkey file in the first place ? IMO, some examples 
using GnuTLS and OpenSSL tools should be documented.

Could you name a few sites that send a Public-Key-Pins HTTP header ?

Just as improvement... Wouldn't it be a good user experience when a HPKP 
database (e.g. a flat file) is created and maintained automatically, like with 
HSTS ? I guess max-age and includeSubdomains are relevant here, maybe report-
uri.

Regards, Tim

Am Dienstag, 23. Februar 2016, 16:10:40 schrieb moparisthebest:
> Hi Tim,
> 
> I attempted to implement your suggestions and formatting everywhere,
> though it's entirely possible I missed a place or two. :) I also added
> an entry in wget.texi.  Attached is the latest patch and it's also
> pushed up to my github repo.
> 
> Let me know when you have future comments about it, until then I'll
> await instructions about the FSF copyright assignment.
> 
> Thanks much,
> Travis
> 
> On 02/23/2016 03:23 PM, Tim Rühsen wrote:
> > Hi Travis,
> > 
> > thank you for your contribution to wget !
> > 
> > We'll take a closer look at the functionality the next days and will think
> > about automated tests.
> > 
> > Just a few comments from the first glimpse
> > - the wget options are documented in doc/wget.texi, please add an entry
> > for
> > the new option
> > - xmalloc() won't return if allocation fails, so no need for checking the
> > return value
> > - xfree() also accepts NULL values, so no need for a prior check.
> > - please use xfree() instead of free(), e.g. 'free(base64data)'.
> > - some parts of the code are 'if(expr)', please amend to 'if (expr)'
> > - we have a space between function name and (. (GNU style)
> > 
> > 
> > In order to accept your contribution, you have to sign the FSF copyrigth
> > assignment. We'll send you information on how to proceed via PM.
> > 
> > Thanks again for your work - it is highly appreciated.
> > 
> > Regards, Tim
> > 
> > Am Dienstag, 23. Februar 2016, 13:17:14 schrieb moparisthebest:
> >> Hello wget team,
> >> 
> >> The attached patch implements a --pinnedpubkey option to pin public keys
> >> for TLS/SSL.  I also pushed this to github [1].  I implemented and
> >> tested this for both the openssl and gnutls backends, and they share
> >> code which I put in util.c.
> >> 
> >> It supports a path to a single .der or .pem file public key file, or any
> >> number of base64 encoded sha256 hashes in the format of
> >> 'sha256//hashhere;sha256//secondhashhere' etc (like the HTTP HPKP
> >> standard).  This makes it behave identically to curl's option of the
> >> same name [2], which I also contributed.
> >> 
> >> I'm not sure if automated tests can be added for this functionality, or
> >> if any additional documentation needs updated or anything else? If you
> >> can point me to anything else that needs done that would make this
> >> easier to accept I'd appreciate it.
> >> 
> >> Thanks for the great tool,
> >> Travis Burtrum
> >> 
> >> [1]: https://github.com/moparisthebest/wget
> >> [2]: https://curl.haxx.se/docs/manpage.html#--pinnedpubkey

signature.asc
Description: This is a digitally signed message part.