From 523d0be9dc795a676aae907736394e64e33350a3 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Fri, 15 Jul 2016 14:16:07 +0200 Subject: [PATCH] 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 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 Signed-off-by: Jonathan Druart Signed-off-by: Kyle M Hall --- Koha/Token.pm | 161 +++++++++++++++++++++++++++++++++++++++ opac/opac-memberentry.pl | 29 +++++-- t/Token.t | 45 +++++++++++ 3 files changed, 229 insertions(+), 6 deletions(-) create mode 100644 Koha/Token.pm create mode 100644 t/Token.t diff --git a/Koha/Token.pm b/Koha/Token.pm new file mode 100644 index 0000000000..ddec453d37 --- /dev/null +++ b/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; diff --git a/opac/opac-memberentry.pl b/opac/opac-memberentry.pl index 61cfaa18b2..399fea6f3b 100755 --- a/opac/opac-memberentry.pl +++ b/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')) { diff --git a/t/Token.t b/t/Token.t new file mode 100644 index 0000000000..642342f756 --- /dev/null +++ b/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" ); -- 2.39.5