LWP::Protocol::https discards 0 value for SSL_VERIFY_mode [rt.cpan.org #111517]

[oalders]

Migrated from rt.cpan.org#111517 (status was 'open')

Requestors:

• SREZIC@cpan.org
• errietta@errietta.me

From errietta@errietta.me on 2016-01-28 16:53:08:

Hello,

If you want to disable ssl cert verification, you need to use
SSL_VERIFY_NONE, which resolves to 0. LWP::Protocol::https transforms this
value to 1:

$ssl_opts{SSL_verify_mode} ||= 1;
Patch:

--- https_old.pm        2016-01-28 16:51:38.970331004 +0000
+++ https.pm    2016-01-28 16:42:22.410331004 +0000
@@ -17,7 +17,8 @@
     my $self = shift;
     my %ssl_opts = %{$self->{ua}{ssl_opts} || {}};
     if (delete $ssl_opts{verify_hostname}) {
-       $ssl_opts{SSL_verify_mode} ||= 1;
+       $ssl_opts{SSL_verify_mode} = defined $ssl_opts{SSL_verify_mode} ?
$ssl_opts{SSL_verify_mode} : 1;
+
        $ssl_opts{SSL_verifycn_scheme} = 'www';
     }
     else {
-- 
Errietta Kostala
<errietta@errietta.me>

From errietta@errietta.me on 2016-01-28 16:54:36:

Versions:
LWP::Protocol::https 6.06
This is perl 5, version 22, subversion 1 (v5.22.1) built for
x86_64-linux-gnu-thread-multi

On Thu, Jan 28, 2016 at 4:53 PM Bugs in LWP-Protocol-https via RT <
bug-LWP-Protocol-https@rt.cpan.org> wrote:

>
> Greetings,
>
> This message has been automatically generated in response to the
> creation of a trouble ticket regarding:
>         "LWP::Protocol::https discards 0 value for SSL_VERIFY_mode",
> a summary of which appears below.
>
> There is no need to reply to this message right now.  Your ticket has been
> assigned an ID of [rt.cpan.org #111517].  Your ticket is accessible
> on the web at:
>
>     https://rt.cpan.org/Ticket/Display.html?id=111517
>
> Please include the string:
>
>          [rt.cpan.org #111517]
>
> in the subject line of all future correspondence about this issue. To do
> so,
> you may reply to this message.
>
>                         Thank you,
>                         bug-LWP-Protocol-https@rt.cpan.org
>
> -------------------------------------------------------------------------
> Hello,
>
> If you want to disable ssl cert verification, you need to use
> SSL_VERIFY_NONE, which resolves to 0. LWP::Protocol::https transforms this
> value to 1:
>
> $ssl_opts{SSL_verify_mode} ||= 1;
> Patch:
>
> --- https_old.pm        2016-01-28 16:51:38.970331004 +0000
> +++ https.pm    2016-01-28 16:42:22.410331004 +0000
> @@ -17,7 +17,8 @@
>      my $self = shift;
>      my %ssl_opts = %{$self->{ua}{ssl_opts} || {}};
>      if (delete $ssl_opts{verify_hostname}) {
> -       $ssl_opts{SSL_verify_mode} ||= 1;
> +       $ssl_opts{SSL_verify_mode} = defined $ssl_opts{SSL_verify_mode} ?
> $ssl_opts{SSL_verify_mode} : 1;
> +
>         $ssl_opts{SSL_verifycn_scheme} = 'www';
>      }
>      else {
> --
> Errietta Kostala
> <errietta@errietta.me>
>
-- 
Errietta Kostala
<errietta@errietta.me>

From sune.karlsson@oru.se on 2016-05-15 21:25:35:

I can confirm this bug. In general it is of course not a good thing to turn off SSL verification but there are legitimate cases for this. This bug in combination with changed behavior in IO::Socket::SSL makes it impossible to turn off SSL verification (it used to be possible to pass a non-numerical value to IO::Socket::SSL and that would do the trick).

Fixing this would be highly appreciated!

/Sune

--
Sune Karlsson
Professor of Statistics
Handelshögskolan/Örebro University School of Business
Örebro University, SE-70182 Örebro, Sweden
Phone +46 19 301257
http://www.oru.se/hh/sune_karlsson
http://econpapers.repec.org/RAS/pka1.htm

From williamt@sonic.net on 2016-07-06 23:24:15:

Please also change

$ssl_opts{SSL_verifycn_scheme} = 'www';
to
$ssl_opts{SSL_verifycn_scheme} ||= 'www';

That way we can pass along our own verification scheme.
 For example if we want to verify a portion of the hostname or something like:
 LWP::UserAgent->new( ssl_opts => {
   SSL_verifycn_scheme => {
    callback => sub {
     if ($_[1] =~ m/^$_[0]:.*/) {
         return 1;
     }
      return 0;
     }
  }});

From williamt@sonic.net on 2016-07-06 23:38:07:

Also in the same method, shouldn't the return be

return ($self->SUPER::_extra_sock_opts, %ssl_opts);
not
return (%ssl_opts, $self->SUPER::_extra_sock_opts);

Otherwise your base class would be overriding your subclasses options.

On Wed Jul 06 19:24:15 2016, williamt@sonic.net wrote:
> Please also change
> 
> $ssl_opts{SSL_verifycn_scheme} = 'www';
> to
> $ssl_opts{SSL_verifycn_scheme} ||= 'www';
> 
> That way we can pass along our own verification scheme.
>  For example if we want to verify a portion of the hostname or
> something like:
>  LWP::UserAgent->new( ssl_opts => {
>    SSL_verifycn_scheme => {
>     callback => sub {
>      if ($_[1] =~ m/^$_[0]:.*/) {
>          return 1;
>      }
>       return 0;
>      }
>   }});

[willt]

ping

[oalders]

If this is still an issue then a pull request with tests (if possible) would help move this along.

[timlegge]

Ran into this today.

If you set:

     my $ua = LWP::UserAgent->new;

     require LWP::Protocol::https;
     $ua->ssl_opts( (
                         SSL_verify_mode => 0,
                         verify_hostname => 1, # The default is 1
                     ));

you run into this issue which makes sense because if you are verifying the hostname so you cannot disable the ssl verification. _By default verify_hostname is true so SSL_verifymode is set by default

the easiest fix for me was to:

     my $ua = LWP::UserAgent->new;

     require LWP::Protocol::https;
     $ua->ssl_opts( (
                         SSL_verify_mode => 0, #explicitly disable SSL verification
                         verify_hostname => 0, #explicitly disable hostname verification
                     ));

Unless I am missing something I am not sure that there is a code change required here. Essentially SSL_verify_mode being false also requires verify_hostname to be false.

It is possible that this is a documentation issue. It is also possible that this is not the intended functionality.

[oalders]

@timlegge maybe we should add something similar to https://github.com/libwww-perl/libwww-perl/commit/7aeb7bc0c704a9dd20802a25e43f3a5080941cac to this module?

[timlegge]

@oalders I will look at adding something this week