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

2016-02-23 Thread 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 32fc47886a7b7314262efa2df0649c088772f9d2 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

---
 src/gnutls.c  |  73 ++-
 src/init.c|   3 +
 src/main.c|   6 ++
 src/openssl.c |  73 ++-
 src/options.h |   5 ++
 src/utils.c   | 229 ++
 src/utils.h   |  10 +++
 7 files changed, 394 insertions(+), 5 deletions(-)

diff --git a/src/gnutls.c b/src/gnutls.c
index d39371f..cf7cb72 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,63 @@ 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(&key);
+
+ret = gnutls_pubkey_import_x509(key, cert, 0);
+if(ret < 0)
+  break; /* failed */
+
+ret = gnutls_pubkey_export(key, GNUTLS_X509_FMT_DER, NULL, &len1);
+if(ret != GNUTLS_E_SHORT_MEMORY_BUFFER || len1 == 0)
+  break; /* failed */
+
+buff1 = xmalloc(len1);
+if(NULL == buff1)
+  break; /* failed */
+
+len2 = len1;
+
+ret = gnutls_pubkey_export(key, GNUTLS_X509_FMT_DER, buff1, &len2);
+if(ret < 0 || len1 != len2)
+  break; /* failed */
+
+/* End Gyrations */
+
+/* The one good exit point */
+result = wg_pin_peer_pubkey(pinnedpubkey, buff1, len1);
+  } while(0);
+
+  if(NULL != key)
+gnutls_pubkey_deinit(key);
+
+  if(NULL != buff1)
+xfree(buff1);
+
+  return result;
+}
+
 #define _CHECK_CERT(flag,msg) \
   if (status & (flag))\
 {\
@@ -691,9 +749,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 (ctx->session, &status);
@@ -727,7 +786,6 @@ ssl_check_certificate (int fd, const char *host)
   success = false;
   goto out;
 }
-
   cert_list = gnutls_certificate_get_peers (ctx->session, &cert_list_size);
   if (!cert_list)
 {
@@ -743,6 +801,7 @@ ssl_check_certificate (int fd, const char *host)
   success = false;
   goto crt_deinit;
 }
+
   if (now < gnutls_x509_crt_get_activation_time (cert))
 {
   logprintf (LOG_NOTQUIET, _("The certificate has not yet been activated\n"));
@@ -760,6 +819,13 @@ ssl_check_certificate (int fd, const char *host)
  quote (host));
   success = false;
 }
+
+  pinsuccess = pkp_pin_peer_pubkey(cert, opt.pinnedpubkey);
+  if (!pinsuccess)
+{
+  logprintf (LOG_ALWAYS, _("The public key does not match pinned public key!\n"));
+  success = false;
+}
  crt_deinit:
   gnutls_x509_crt_deinit (cert);
 }
@@ -770,5 +836,6 @@ ssl_check_certificate (int fd, const char *host)
 }
 
  out:
-  return opt.check_cert == CHECK_CERT_ON ? success : true;
+  /* never return true if pinsuccess fails

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

2016-02-23 Thread 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 specifi

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

2016-02-29 Thread 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

2016-03-14 Thread 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

2016-03-15 Thread 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

2016-03-18 Thread 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

2016-03-19 Thread 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 (&key);
+
+  ret = gnutls_pubkey_import_x509 (key, cert, 0);
+  if (ret < 0)
+goto cleanup; /* failed */
+
+  ret = gnutls_pubkey_export (key, GNUTLS_X509_FMT_DER, NULL, &len1);
+  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, &len2);
+  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 fo

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

2016-04-04 Thread 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 (&key);
+
+  ret = gnutls_pubkey_import_x509 (key, cert, 0);
+  if (ret < 0)
+goto cleanup; /* failed */
+
+  ret = gnutls_pubkey_export (key, GNUTLS_X509_FMT_DER, NULL, &len1);
+  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, &len2);
+  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

Re: [Bug-wget] Wget - acess list bypass / race condition PoC

2016-08-15 Thread moparisthebest
Hello,

I find it extremely hard to call this a wget vulnerability when SO many
other things are wrong with that 'vulnerable code' implementation it
isn't even funny:

1. The image_importer.php script takes a single argument, why would it
download with the recursive switch turned on?  Isn't that clearly a bug
in the php script?  Has a php script like this that downloads all files
from a website of a particular extension ever been observed in the wild?

2. A *well* configured server would have a whitelist of .php files it
will execute, making it immune to this.  A *decently* configured server
would always at a minimum make sure they don't execute code in
directories with user provided uploads in them.  So it's additionally a
bug in the server configuration. (incidentally every php package I've
downloaded has at minimum a .htaccess in upload directories to prevent
this kind of thing with apache)

It seems to me like there has always been plenty of ways to shoot
yourself in the foot with PHP, and this is just another iteration on a
theme.

Just my 2 cents,
Thanks!