Re: [Bug-wget] [PATCH] certificate revocation lists (CRLs) #43501

[Giuseppe Scrivano]

Tim Ruehsen <address@hidden> writes:

> On Saturday 08 November 2014 13:00:13 Giuseppe Scrivano wrote:
>> Tim Ruehsen <address@hidden> writes:
>> > On Friday 07 November 2014 09:26:58 Giuseppe Scrivano wrote:
>> >> Tim Ruehsen <address@hidden> writes:
>> >> > Here is a first patch (GnuTLS only) for review and comments (and
>> >> > playing
>> >> > around).
>> >> 
>> >> I think we should fail and avoid any connection instead of printing just
>> >> a warning as it seems from the code now.  Have you tested it with some
>> >> crl file?  Would be good to add some automatic tests for this new
>> >> feature.
>> >> 
>> >> > - Should we support complete directories ?
>> >> > - Should we allow more than one --crl-file option ?
>> >> 
>> >> We can add this later, but we need to ensure that wget fails now if more
>> >> --crl-file are passed so that the user knows it is not supported now.
>> > 
>> > Amended patch.
>> 
>> thanks, the patch looks fine to me.
>
> I just moved a block of code (loading of --ca-certificate) to the right place 
> and added output on failure and success.
>
> To made up a test, I had to recreate testenv/certs. The former CN component 
> did not have the correct name, which would allow us to generate a CRL file.
> This also allows us to use the CA cert (--ca-certificate=) and remove the 
> very 
> general --no-check-certificate from the Wget command line within Test--
> https.py.
>
> The testenv/certs directory now seems somehow cleaner and better to 
> understand 
> (to me). I documented the cert/key/crl creation steps (using certtool) in 
> testenv/certs/README.
>
> Review and comments appreciated.

great work, it looks fine to me.  Feel free to push it.

Regards,
Giuseppe