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

[Tim Ruehsen]

Hi Travis,

with the latest fixes of the test suite, everything works smoothly here (just 
had to remove the now obsolete TEST_NAME from your tests).

I extended your commit messages and pushed the patches.

Thanks for your work !

Tim

On Monday 04 April 2016 12:28:47 moparisthebest wrote:
> Hi all,
> 
> I have now implemented tests for --pinnedpubkey, the first patch is
> unchanged from last time, the second patch has all the new test code.
> 
> They all pass as long as I export SSL_TESTS as an environmental variable
> (otherwise they are skipped), I see there is code in Makefile.am that
> supposedly does that, but it's not working for me, likely because I'm
> doing something wrong...
> 
> Let me know if there is anything else I can do.
> 
> Thanks,
> Travis
> 
> On 03/18/2016 02:10 AM, moparisthebest wrote:
> > Hi Tim,
> > 
> > I've implemented your suggestions below, except the python tests, and
> > rebased on top of current HEAD, attached is the patch.
> > 
> > The documentation in testenv/ says the test server doesn't support
> > https, which would be needed for this test.  Has anyone started work on
> > that?  Or would it be acceptable to just use socat or stunnel or similar
> > in front of the current test server?
> > 
> > Thanks much,
> > Travis
> > 
> > On 03/15/2016 07:50 AM, Tim Ruehsen wrote:
> >> Hi Travis,
> >> 
> >> thanks for poking. I started testing... just a few more points.
> >> 
> >> In wg_pin_peer_pubkey(), what is this loop do {...} while(0) about ?
> >> I looks like it is not supposed to loop (if it would, we had resource
> >> leaks). Maybe you can remove it and instead of 'break: do a 'goto
> >> end/cleanup/out' !?
> >> 
> >> Please consider to use wget_read_file / wget_read_file_free() for reading
> >> the contents of a file. It also allows for stdin ('-' at the command
> >> line) which makes the new option a bit more consistent with Wget's CLI
> >> standards.
> >> 
> >> Do you plan to create a python test (see testenv/) ?
> >> 
> >> Regards, Tim

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

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

[Tim Ruehsen]

> I see randomly messages like these (e.g. from Test-pinnedpubkey-der-
> https.log):

I guess the problem is using the same TEST_NAME values. These are taken to 
create the temp work directory. Using parallel tests lets work more than one 
test in the same directory which randomly fails.

@Travis: Please use unique TESTS_NAME values for each test.

@Darshit: Shouldn't we simply use the python test file name instead - that 
would be fail safe !?

Tim

On Friday 08 April 2016 12:05:03 Tim Ruehsen wrote:
> @Darshit: Please see the test suite problems below (random errors, but tests
> pass). Any idea ?
> 
> On Monday 04 April 2016 12:28:47 moparisthebest wrote:
> > Hi all,
> > 
> > I have now implemented tests for --pinnedpubkey, the first patch is
> > unchanged from last time, the second patch has all the new test code.
> > 
> > They all pass as long as I export SSL_TESTS as an environmental variable
> > (otherwise they are skipped), I see there is code in Makefile.am that
> > supposedly does that, but it's not working for me, likely because I'm
> > doing something wrong...
> 
> Here I can't reproduce this. The tests do all PASS.
> 
> Run again ./bootstrap and ./configure.
> Check that ./configure outputs
> SSL:   gnutls
> (or openssl instead of gnutls).
> (If it doesn't, install the SSL dev packages and ./configure again).
> 
> Now run 'make clean && make check'.
> 
> If you still see your tests skipping... in this case Test-hsts,
> Test--https-crl and Test--https skip as well ?
> 
> 
> 
> I see randomly messages like these (e.g. from Test-pinnedpubkey-der-
> https.log):
> ...
> Saving to: ‘File1’
>  0K   100%  616
> =0.04s 2016-04-08 12:01:06 (616 B/s) - ‘File1’ saved [24/24]
> ...
> Error: Expected file File1 not found..
> Unknown Exception while trying to remove Test Environment.
> PASS Test-pinnedpubkey-der-https.py (exit status: 0)
> 
> > Let me know if there is anything else I can do.
> > 
> > Thanks,
> > Travis
> > 
> > On 03/18/2016 02:10 AM, moparisthebest wrote:
> > > Hi Tim,
> > > 
> > > I've implemented your suggestions below, except the python tests, and
> > > rebased on top of current HEAD, attached is the patch.
> > > 
> > > The documentation in testenv/ says the test server doesn't support
> > > https, which would be needed for this test.  Has anyone started work on
> > > that?  Or would it be acceptable to just use socat or stunnel or similar
> > > in front of the current test server?
> > > 
> > > Thanks much,
> > > Travis
> > > 
> > > On 03/15/2016 07:50 AM, Tim Ruehsen wrote:
> > >> Hi Travis,
> > >> 
> > >> thanks for poking. I started testing... just a few more points.
> > >> 
> > >> In wg_pin_peer_pubkey(), what is this loop do {...} while(0) about ?
> > >> I looks like it is not supposed to loop (if it would, we had resource
> > >> leaks). Maybe you can remove it and instead of 'break: do a 'goto
> > >> end/cleanup/out' !?
> > >> 
> > >> Please consider to use wget_read_file / wget_read_file_free() for
> > >> reading
> > >> the contents of a file. It also allows for stdin ('-' at the command
> > >> line) which makes the new option a bit more consistent with Wget's CLI
> > >> standards.
> > >> 
> > >> Do you plan to create a python test (see testenv/) ?
> > >> 
> > >> Regards, Tim

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

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

[moparisthebest]

Hi all,

I have now implemented tests for --pinnedpubkey, the first patch is
unchanged from last time, the second patch has all the new test code.

They all pass as long as I export SSL_TESTS as an environmental variable
(otherwise they are skipped), I see there is code in Makefile.am that
supposedly does that, but it's not working for me, likely because I'm
doing something wrong...

Let me know if there is anything else I can do.

Thanks,
Travis

On 03/18/2016 02:10 AM, moparisthebest wrote:
> Hi Tim,
> 
> I've implemented your suggestions below, except the python tests, and
> rebased on top of current HEAD, attached is the patch.
> 
> The documentation in testenv/ says the test server doesn't support
> https, which would be needed for this test.  Has anyone started work on
> that?  Or would it be acceptable to just use socat or stunnel or similar
> in front of the current test server?
> 
> Thanks much,
> Travis
> 
> On 03/15/2016 07:50 AM, Tim Ruehsen wrote:
>> Hi Travis,
>>
>> thanks for poking. I started testing... just a few more points.
>>
>> In wg_pin_peer_pubkey(), what is this loop do {...} while(0) about ?
>> I looks like it is not supposed to loop (if it would, we had resource 
>> leaks). 
>> Maybe you can remove it and instead of 'break: do a 'goto end/cleanup/out' !?
>>
>> Please consider to use wget_read_file / wget_read_file_free() for reading 
>> the 
>> contents of a file. It also allows for stdin ('-' at the command line) which 
>> makes the new option a bit more consistent with Wget's CLI standards.
>>
>> Do you plan to create a python test (see testenv/) ?
>>
>> Regards, Tim
From ade891448c15b99112fb0ca64ed0d8492953bac3 Mon Sep 17 00:00:00 2001
From: moparisthebest 
Date: Fri, 18 Mar 2016 01:55:53 -0400
Subject: [PATCH 1/2] Implement --pinnedpubkey option to pin public keys

---
 doc/wget.texi |  12 
 src/gnutls.c  |  67 +++-
 src/init.c|   3 +
 src/main.c|   6 ++
 src/openssl.c |  72 -
 src/options.h |   5 ++
 src/utils.c   | 201 ++
 src/utils.h   |   9 +++
 8 files changed, 371 insertions(+), 4 deletions(-)

diff --git a/doc/wget.texi b/doc/wget.texi
index efebc49..c4bf7db 100644
--- a/doc/wget.texi
+++ b/doc/wget.texi
@@ -1776,6 +1776,18 @@ system-specified locations, chosen at OpenSSL installation time.
 Specifies a CRL file in @var{file}.  This is needed for certificates
 that have been revocated by the CAs.
 
+@cindex SSL Public Key Pin
+@item --pinnedpubkey=file/hashes
+Tells wget to use the specified public key file (or hashes) to verify the peer.
+This can be a path to a file which contains a single public key in PEM or DER
+format, or any number of base64 encoded sha256 hashes preceded by ``sha256//''
+and separated by ``;''
+
+When negotiating a TLS or SSL connection, the server sends a certificate
+indicating its identity. A public key is extracted from this certificate and if
+it does not exactly match the public key(s) provided to this option, wget will
+abort the connection before sending or receiving any data.
+
 @cindex entropy, specifying source of
 @cindex randomness, specifying source of
 @item --random-file=@var{file}
diff --git a/src/gnutls.c b/src/gnutls.c
index d39371f..662372e 100644
--- a/src/gnutls.c
+++ b/src/gnutls.c
@@ -37,6 +37,7 @@ as that of the covered work.  */
 #include 
 #include 
 
+#include 
 #include 
 #include 
 #include 
@@ -671,6 +672,59 @@ ssl_connect_wget (int fd, const char *hostname, int *continue_session)
   return true;
 }
 
+static bool
+pkp_pin_peer_pubkey (gnutls_x509_crt_t cert, const char *pinnedpubkey)
+{
+  /* Scratch */
+  size_t len1 = 0, len2 = 0;
+  char *buff1 = NULL;
+
+  gnutls_pubkey_t key = NULL;
+
+  /* Result is returned to caller */
+  int ret = 0;
+  bool result = false;
+
+  /* if a path wasn't specified, don't pin */
+  if (NULL == pinnedpubkey)
+return true;
+
+  if (NULL == cert)
+return result;
+
+  /* Begin Gyrations to get the public key */
+  gnutls_pubkey_init ();
+
+  ret = gnutls_pubkey_import_x509 (key, cert, 0);
+  if (ret < 0)
+goto cleanup; /* failed */
+
+  ret = gnutls_pubkey_export (key, GNUTLS_X509_FMT_DER, NULL, );
+  if (ret != GNUTLS_E_SHORT_MEMORY_BUFFER || len1 == 0)
+goto cleanup; /* failed */
+
+  buff1 = xmalloc (len1);
+
+  len2 = len1;
+
+  ret = gnutls_pubkey_export (key, GNUTLS_X509_FMT_DER, buff1, );
+  if (ret < 0 || len1 != len2)
+goto cleanup; /* failed */
+
+  /* End Gyrations */
+
+  /* The one good exit point */
+  result = wg_pin_peer_pubkey (pinnedpubkey, buff1, len1);
+
+ cleanup:
+  if (NULL != key)
+gnutls_pubkey_deinit (key);
+
+  xfree (buff1);
+
+  return result;
+}
+
 #define _CHECK_CERT(flag,msg) \
   if (status & (flag))\
 {\
@@ -691,9 +745,10 @@ ssl_check_certificate (int fd, const char *host)
  him about problems with the server's certificate.  */
   const char *severity = opt.check_cert ? _("ERROR") : 

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

[moparisthebest]

Hi Tim,

I've implemented your suggestions below, except the python tests, and
rebased on top of current HEAD, attached is the patch.

The documentation in testenv/ says the test server doesn't support
https, which would be needed for this test.  Has anyone started work on
that?  Or would it be acceptable to just use socat or stunnel or similar
in front of the current test server?

Thanks much,
Travis

On 03/15/2016 07:50 AM, Tim Ruehsen wrote:
> Hi Travis,
>
> thanks for poking. I started testing... just a few more points.
>
> In wg_pin_peer_pubkey(), what is this loop do {...} while(0) about ?
> I looks like it is not supposed to loop (if it would, we had resource leaks). 
> Maybe you can remove it and instead of 'break: do a 'goto end/cleanup/out' !?
>
> Please consider to use wget_read_file / wget_read_file_free() for reading the 
> contents of a file. It also allows for stdin ('-' at the command line) which 
> makes the new option a bit more consistent with Wget's CLI standards.
>
> Do you plan to create a python test (see testenv/) ?
>
> Regards, Tim
>
> On Monday 14 March 2016 12:57:52 moparisthebest wrote:
>> Hi all,
>>
>> Just checking back in about this patch, is there anything else you are
>> waiting on from me before integrating it?
>>
>> Thanks much,
>> Travis

From ade891448c15b99112fb0ca64ed0d8492953bac3 Mon Sep 17 00:00:00 2001
From: moparisthebest 
Date: Fri, 18 Mar 2016 01:55:53 -0400
Subject: [PATCH] Implement --pinnedpubkey option to pin public keys

---
 doc/wget.texi |  12 
 src/gnutls.c  |  67 +++-
 src/init.c|   3 +
 src/main.c|   6 ++
 src/openssl.c |  72 -
 src/options.h |   5 ++
 src/utils.c   | 201 ++
 src/utils.h   |   9 +++
 8 files changed, 371 insertions(+), 4 deletions(-)

diff --git a/doc/wget.texi b/doc/wget.texi
index efebc49..c4bf7db 100644
--- a/doc/wget.texi
+++ b/doc/wget.texi
@@ -1776,6 +1776,18 @@ system-specified locations, chosen at OpenSSL installation time.
 Specifies a CRL file in @var{file}.  This is needed for certificates
 that have been revocated by the CAs.
 
+@cindex SSL Public Key Pin
+@item --pinnedpubkey=file/hashes
+Tells wget to use the specified public key file (or hashes) to verify the peer.
+This can be a path to a file which contains a single public key in PEM or DER
+format, or any number of base64 encoded sha256 hashes preceded by ``sha256//''
+and separated by ``;''
+
+When negotiating a TLS or SSL connection, the server sends a certificate
+indicating its identity. A public key is extracted from this certificate and if
+it does not exactly match the public key(s) provided to this option, wget will
+abort the connection before sending or receiving any data.
+
 @cindex entropy, specifying source of
 @cindex randomness, specifying source of
 @item --random-file=@var{file}
diff --git a/src/gnutls.c b/src/gnutls.c
index d39371f..662372e 100644
--- a/src/gnutls.c
+++ b/src/gnutls.c
@@ -37,6 +37,7 @@ as that of the covered work.  */
 #include 
 #include 
 
+#include 
 #include 
 #include 
 #include 
@@ -671,6 +672,59 @@ ssl_connect_wget (int fd, const char *hostname, int *continue_session)
   return true;
 }
 
+static bool
+pkp_pin_peer_pubkey (gnutls_x509_crt_t cert, const char *pinnedpubkey)
+{
+  /* Scratch */
+  size_t len1 = 0, len2 = 0;
+  char *buff1 = NULL;
+
+  gnutls_pubkey_t key = NULL;
+
+  /* Result is returned to caller */
+  int ret = 0;
+  bool result = false;
+
+  /* if a path wasn't specified, don't pin */
+  if (NULL == pinnedpubkey)
+return true;
+
+  if (NULL == cert)
+return result;
+
+  /* Begin Gyrations to get the public key */
+  gnutls_pubkey_init ();
+
+  ret = gnutls_pubkey_import_x509 (key, cert, 0);
+  if (ret < 0)
+goto cleanup; /* failed */
+
+  ret = gnutls_pubkey_export (key, GNUTLS_X509_FMT_DER, NULL, );
+  if (ret != GNUTLS_E_SHORT_MEMORY_BUFFER || len1 == 0)
+goto cleanup; /* failed */
+
+  buff1 = xmalloc (len1);
+
+  len2 = len1;
+
+  ret = gnutls_pubkey_export (key, GNUTLS_X509_FMT_DER, buff1, );
+  if (ret < 0 || len1 != len2)
+goto cleanup; /* failed */
+
+  /* End Gyrations */
+
+  /* The one good exit point */
+  result = wg_pin_peer_pubkey (pinnedpubkey, buff1, len1);
+
+ cleanup:
+  if (NULL != key)
+gnutls_pubkey_deinit (key);
+
+  xfree (buff1);
+
+  return result;
+}
+
 #define _CHECK_CERT(flag,msg) \
   if (status & (flag))\
 {\
@@ -691,9 +745,10 @@ ssl_check_certificate (int fd, const char *host)
  him about problems with the server's certificate.  */
   const char *severity = opt.check_cert ? _("ERROR") : _("WARNING");
   bool success = true;
+  bool pinsuccess = opt.pinnedpubkey == NULL;
 
   /* The user explicitly said to not check for the certificate.  */
-  if (opt.check_cert == CHECK_CERT_QUIET)
+  if (opt.check_cert == CHECK_CERT_QUIET && pinsuccess)
 return success;
 
   err = gnutls_certificate_verify_peers2 

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

[moparisthebest]

And of course NOW I see the Test--https.py file and that https tests are
indeed supported.  I'll write up some tests and send them shortly.

On 03/18/2016 02:10 AM, moparisthebest wrote:
> The documentation in testenv/ says the test server doesn't support
> https, which would be needed for this test.  Has anyone started work on
> that?  Or would it be acceptable to just use socat or stunnel or similar
> in front of the current test server?
>



signature.asc
Description: OpenPGP digital signature

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

[moparisthebest]

Hi Tim,

On 03/15/2016 07:50 AM, Tim Ruehsen wrote:
> In wg_pin_peer_pubkey(), what is this loop do {...} while(0) about ?
> I looks like it is not supposed to loop (if it would, we had resource leaks). 
> Maybe you can remove it and instead of 'break: do a 'goto end/cleanup/out' !?

Ah yea that was exactly to fill the place of a goto, I can change that.

> Please consider to use wget_read_file / wget_read_file_free() for reading the 
> contents of a file. It also allows for stdin ('-' at the command line) which 
> makes the new option a bit more consistent with Wget's CLI standards.

I didn't know about those but it sounds nicer, I'll have a go at
changing it to use them.

> Do you plan to create a python test (see testenv/) ?

I'll give that a shot as well.

Thanks much,
Travis



signature.asc
Description: OpenPGP digital signature

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

[Tim Ruehsen]

Hi Travis,

thanks for poking. I started testing... just a few more points.

In wg_pin_peer_pubkey(), what is this loop do {...} while(0) about ?
I looks like it is not supposed to loop (if it would, we had resource leaks). 
Maybe you can remove it and instead of 'break: do a 'goto end/cleanup/out' !?

Please consider to use wget_read_file / wget_read_file_free() for reading the 
contents of a file. It also allows for stdin ('-' at the command line) which 
makes the new option a bit more consistent with Wget's CLI standards.

Do you plan to create a python test (see testenv/) ?

Regards, Tim

On Monday 14 March 2016 12:57:52 moparisthebest wrote:
> Hi all,
> 
> Just checking back in about this patch, is there anything else you are
> waiting on from me before integrating it?
> 
> Thanks much,
> Travis

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

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

[moparisthebest]

Hi all,

Just checking back in about this patch, is there anything else you are
waiting on from me before integrating it?

Thanks much,
Travis



signature.asc
Description: OpenPGP digital signature

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

[moparisthebest]

Hi Tim,

So here are some examples on how to get/create keys in various formats
(for my website, moparisthebest.com):

# pem format
openssl s_client -connect www.moparisthebest.com:443 2>&1 < /dev/null |
sed -n '/-BEGIN/,/-END/p' | openssl x509 -noout -pubkey >
www.moparisthebest.com.pem

# der format
openssl s_client -connect www.moparisthebest.com:443 2>&1 < /dev/null |
sed -n '/-BEGIN/,/-END/p' | openssl x509 -noout -pubkey |
openssl asn1parse -noout -inform pem -out /dev/stdout >
www.moparisthebest.com.der

# sha256 HPKP pin format
openssl s_client -connect www.moparisthebest.com:443 2>&1 < /dev/null |
sed -n '/-BEGIN/,/-END/p' | openssl x509 -noout -pubkey |
openssl asn1parse -noout -inform pem -out /dev/stdout | openssl dgst
-sha256 -binary | openssl base64

I'm not sure where or if this should be documented for wget anywhere?

And yes, a flat-file type system that is updated when receiving a
Public-Key-Pins header is an ultimate end-goal, I just viewed it as kind
of a next step to this.  You want manual-pinning for some use-cases,
like scripts and such.  Automatic pinning would increase security in the
generic case like web scraping, or maybe even a crowd-sourced list could
be gathered and shared?

Thanks,
Travis

On 02/29/2016 02:24 PM, Tim Rühsen wrote:
> 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: OpenPGP digital signature

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.

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

[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
From c2bb101729092a1fdc3e2f8f70bf3dd58e9c9e23 Mon Sep 17 00:00:00 2001
From: moparisthebest 
Date: Mon, 22 Feb 2016 23:01:36 -0500
Subject: [PATCH] Implement --pinnedpubkey option to pin public keys

---
 doc/wget.texi |  12 
 src/gnutls.c  |  71 ++-
 src/init.c|   3 +
 src/main.c|   6 ++
 src/openssl.c |  74 +++-
 src/options.h |   5 ++
 src/utils.c   | 219 ++
 src/utils.h   |   9 +++
 8 files changed, 394 insertions(+), 5 deletions(-)

diff --git a/doc/wget.texi b/doc/wget.texi
index f3925ca..a58e498 100644
--- a/doc/wget.texi
+++ b/doc/wget.texi
@@ -1778,6 +1778,18 @@ system-specified locations, chosen at OpenSSL installation time.
 Specifies a CRL file in @var{file}.  This is needed for certificates
 that have been revocated by the CAs.
 
+@cindex SSL Public Key Pin
+@item --pinnedpubkey=file/hashes
+Tells wget to use the specified public key file (or hashes) to verify the peer.
+This can be a path to a file which contains a single public key in PEM or DER
+format, or any number of base64 encoded sha256 hashes preceded by ``sha256//''
+and separated by ``;''
+
+When negotiating a TLS or SSL connection, the server sends a certificate
+indicating its identity. A public key is extracted from this certificate and if
+it does not exactly match the public key(s) provided to this option, wget will
+abort the connection before sending or receiving any data.
+
 @cindex entropy, specifying source of
 @cindex randomness, specifying source of
 @item --random-file=@var{file}
diff --git a/src/gnutls.c b/src/gnutls.c
index d39371f..99713bb 100644
--- a/src/gnutls.c
+++ b/src/gnutls.c
@@ -37,6 +37,7 @@ as that of the covered work.  */
 #include 
 #include 
 
+#include 
 #include 
 #include 
 #include 
@@ -671,6 +672,61 @@ ssl_connect_wget (int fd, const char *hostname, int *continue_session)
   return true;
 }
 
+static bool
+pkp_pin_peer_pubkey (gnutls_x509_crt_t cert, const char *pinnedpubkey)
+{
+  /* Scratch */
+  size_t len1 = 0, len2 = 0;
+  char *buff1 = NULL;
+
+  gnutls_pubkey_t key = NULL;
+
+  /* Result is returned to caller */
+  int ret = 0;
+  bool result = false;
+
+  /* if a path wasn't specified, don't pin */
+  if (NULL == pinnedpubkey)
+return true;
+
+  if (NULL == cert)
+return result;
+
+  do
+{
+  /* Begin Gyrations to get the public key */
+  gnutls_pubkey_init ();
+
+  ret = gnutls_pubkey_import_x509 (key, cert, 0);
+  if (ret < 0)
+break; /* failed */
+
+  

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

[Tim Rühsen]

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.