Browse Source

Bug 16929: Prevent opac-memberentry waiting for random chars

Move calls to WWW::CSRF to Koha::Token.
Send a safe random string to WWW::CSRF instead of letting CSRF make a
blocking call to Bytes::Random::Secure. If your server has not enough
entropy, opac-memberentry will hang waiting for more characters in
dev/random. Koha::Token uses Bytes::Random::Secure with the NonBlocking
flag.

Test plan:
[1] Do not yet apply this patch.
[2] If your server has not enough entropy, calling opac-memberentry may
    take a while. But this not may be the case for you (no worries).
[3] Apply this patch.
[4] Verify that opac-memberentry still works as expected.
[5] Run t/Token.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Yes, my server had entropy trouble (reason for finding the problem).
This patch resolves the delay.

Tested all 3 patches together, works as expected.
Signed-off-by: Marc <veron@veron.ch>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
16.11.x
Marcel de Rooy 8 years ago
committed by Kyle M Hall
parent
commit
523d0be9dc
  1. 161
      Koha/Token.pm
  2. 29
      opac/opac-memberentry.pl
  3. 45
      t/Token.t

161
Koha/Token.pm

@ -0,0 +1,161 @@
package Koha::Token;
# Created as wrapper for CSRF tokens, but designed for more general use
# Copyright 2016 Rijksmuseum
#
# This file is part of Koha.
#
# Koha is free software; you can redistribute it and/or modify it under the
# terms of the GNU General Public License as published by the Free Software
# Foundation; either version 3 of the License, or (at your option) any later
# version.
#
# Koha is distributed in the hope that it will be useful, but WITHOUT ANY
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
# A PARTICULAR PURPOSE. See the GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License along
# with Koha; if not, write to the Free Software Foundation, Inc.,
# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
=head1 NAME
Koha::Token - Tokenizer
=head1 SYNOPSIS
use Koha::Token;
my $tokenizer = Koha::Token->new;
my $token = $tokenizer->generate({ length => 20 });
# safely generate a CSRF token (nonblocking)
my $csrf_token = $tokenizer->generate({
CSRF => 1, id => $id, secret => $secret,
});
# or check a CSRF token
my $result = $tokenizer->check({
CSRF => 1, id => $id, secret => $secret, token => $token,
});
=head1 DESCRIPTION
Designed for providing general tokens.
Created due to the need for a nonblocking call to Bytes::Random::Secure
when generating a CSRF token.
=cut
use Modern::Perl;
use base qw(Class::Accessor);
use constant HMAC_SHA1_LENGTH => 20;
=head1 METHODS
=head2 new
Create object (via Class::Accessor).
=cut
sub new {
my ( $class ) = @_;
return $class->SUPER::new();
}
=head2 generate
my $token = $tokenizer->generate({ length => 20 });
my $csrf_token = $tokenizer->generate({
CSRF => 1, id => $id, secret => $secret,
});
Generate several types of tokens. Now includes CSRF.
Room for future extension.
=cut
sub generate {
my ( $self, $params ) = @_;
if( $params->{CSRF} ) {
$self->{lasttoken} = _gen_csrf( $params );
} else {
$self->{lasttoken} = _gen_rand( $params );
}
return $self->{lasttoken};
}
=head2 check
my $result = $tokenizer->check({
CSRF => 1, id => $id, secret => $secret, token => $token,
});
Check several types of tokens. Now includes CSRF.
Room for future extension.
=cut
sub check {
my ( $self, $params ) = @_;
if( $params->{CSRF} ) {
return _chk_csrf( $params );
}
return;
}
# --- Internal routines ---
sub _gen_csrf {
# Since WWW::CSRF::generate_csrf_token does not use the NonBlocking
# parameter of Bytes::Random::Secure, we are passing random bytes from
# a non blocking source to WWW::CSRF via its Random parameter.
my ( $params ) = @_;
return if !$params->{id} || !$params->{secret};
require Bytes::Random::Secure;
require WWW::CSRF;
my $randomizer = Bytes::Random::Secure->new( NonBlocking => 1 );
# this is most fundamental: do not use /dev/random since it is
# blocking, but use /dev/urandom !
my $random = $randomizer->bytes( HMAC_SHA1_LENGTH );
my $token = WWW::CSRF::generate_csrf_token(
$params->{id}, $params->{secret}, { Random => $random },
);
return $token;
}
sub _chk_csrf {
my ( $params ) = @_;
return if !$params->{id} || !$params->{secret} || !$params->{token};
require WWW::CSRF;
my $csrf_status = WWW::CSRF::check_csrf_token(
$params->{id},
$params->{secret},
$params->{token},
);
return $csrf_status == WWW::CSRF::CSRF_OK();
}
sub _gen_rand {
my ( $params ) = @_;
my $length = $params->{length} || 1;
$length = 1 unless $length > 0;
require String::Random;
return String::Random::random_string( '.' x $length );
}
=head1 AUTHOR
Marcel de Rooy, Rijksmuseum Amsterdam, The Netherlands
=cut
1;

29
opac/opac-memberentry.pl

@ -20,7 +20,6 @@ use Modern::Perl;
use CGI qw ( -utf8 );
use Digest::MD5 qw( md5_base64 md5_hex );
use String::Random qw( random_string );
use WWW::CSRF qw(generate_csrf_token check_csrf_token CSRF_OK);
use HTML::Entities;
use C4::Auth;
@ -34,6 +33,7 @@ use C4::Scrubber;
use Email::Valid;
use Koha::DateUtils;
use Koha::Patron::Images;
use Koha::Token;
my $cgi = new CGI;
my $dbh = C4::Context->dbh;
@ -182,8 +182,13 @@ if ( $action eq 'create' ) {
elsif ( $action eq 'update' ) {
my $borrower = GetMember( borrowernumber => $borrowernumber );
my $csrf_status = check_csrf_token($borrower->{userid}, md5_base64(C4::Context->config('pass')), scalar $cgi->param('csrf_token'));
die "Wrong CSRF token" unless ($csrf_status == CSRF_OK);
die "Wrong CSRF token"
unless Koha::Token->new->check({
CSRF => 1,
id => $borrower->{userid},
secret => md5_base64( C4::Context->config('pass') ),
token => scalar $cgi->param('csrf_token'),
});
my %borrower = ParseCgiForBorrower($cgi);
@ -197,7 +202,11 @@ elsif ( $action eq 'update' ) {
empty_mandatory_fields => \@empty_mandatory_fields,
invalid_form_fields => $invalidformfields,
borrower => \%borrower,
csrf_token => generate_csrf_token($borrower->{userid}, md5_base64(C4::Context->config('pass'))),
csrf_token => Koha::Token->new->generate({
CSRF => 1,
id => $borrower->{userid},
secret => md5_base64( C4::Context->config('pass') ),
}),
);
$template->param( action => 'edit' );
@ -229,7 +238,11 @@ elsif ( $action eq 'update' ) {
action => 'edit',
nochanges => 1,
borrower => GetMember( borrowernumber => $borrowernumber ),
csrf_token => generate_csrf_token($borrower->{userid}, md5_base64(C4::Context->config('pass')))
csrf_token => Koha::Token->new->generate({
CSRF => 1,
id => $borrower->{userid},
secret => md5_base64( C4::Context->config('pass') ),
}),
);
}
}
@ -249,7 +262,11 @@ elsif ( $action eq 'edit' ) { #Display logged in borrower's data
borrower => $borrower,
guarantor => scalar Koha::Patrons->find($borrowernumber)->guarantor(),
hidden => GetHiddenFields( $mandatory, 'modification' ),
csrf_token => generate_csrf_token($borrower->{userid}, md5_base64(C4::Context->config('pass')))
csrf_token => Koha::Token->new->generate({
CSRF => 1,
id => $borrower->{userid},
secret => md5_base64( C4::Context->config('pass') ),
}),
);
if (C4::Context->preference('OPACpatronimages')) {

45
t/Token.t

@ -0,0 +1,45 @@
#!/usr/bin/perl
# tests for Koha::Token
# Copyright 2016 Rijksmuseum
#
# This file is part of Koha.
#
# Koha is free software; you can redistribute it and/or modify it under the
# terms of the GNU General Public License as published by the Free Software
# Foundation; either version 3 of the License, or (at your option) any later
# version.
#
# Koha is distributed in the hope that it will be useful, but WITHOUT ANY
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
# A PARTICULAR PURPOSE. See the GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License along
# with Koha; if not, write to the Free Software Foundation, Inc.,
# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
use Modern::Perl;
use Test::More tests => 6;
use Koha::Token;
my $tokenizer = Koha::Token->new;
is( length( $tokenizer->generate ), 1, "Generate without parameters" );
my $token = $tokenizer->generate({ length => 20 });
is( length($token), 20, "Token $token has 20 chars" );
my $id = $tokenizer->generate({ length => 8 });
my $secr = $tokenizer->generate({ length => 32 });
my $csrftoken = $tokenizer->generate({ CSRF => 1, id => $id, secret => $secr });
isnt( length($csrftoken), 0, "Token $csrftoken should not be empty" );
is( $tokenizer->check, undef, "Check without any parameters" );
my $result = $tokenizer->check({
CSRF => 1, id => $id, secret => $secr, token => $csrftoken,
});
is( $result, 1, "CSRF token verified" );
$result = $tokenizer->check({
CSRF => 1, id => $id, secret => $secr, token => $token,
});
isnt( $result, 1, "This token is no CSRF token" );
Loading…
Cancel
Save