Bugzilla – Attachment 53487 Details for
Bug 16929
Prevent opac-memberentry waiting for random chars
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 16929: Prevent opac-memberentry waiting for random chars
Bug-16929-Prevent-opac-memberentry-waiting-for-ran.patch (text/plain), 10.30 KB, created by
Marcel de Rooy
on 2016-07-19 13:49:33 UTC
(
hide
)
Description:
Bug 16929: Prevent opac-memberentry waiting for random chars
Filename:
MIME Type:
Creator:
Marcel de Rooy
Created:
2016-07-19 13:49:33 UTC
Size:
10.30 KB
patch
obsolete
>From 535e987fed093505f98b245b20ac891fe70affb7 Mon Sep 17 00:00:00 2001 >From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >Date: Fri, 15 Jul 2016 14:16:07 +0200 >Subject: [PATCH] Bug 16929: Prevent opac-memberentry waiting for random chars >Content-Type: text/plain; charset=utf-8 > >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. >--- > 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 0000000..ddec453 >--- /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 de157ef..9468a06 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 C4::Auth; > use C4::Output; >@@ -33,6 +32,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 0000000..642342f >--- /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" ); >-- >1.7.10.4
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 16929
:
53418
|
53487
|
53552
|
53809
|
54140
|
54141
|
54142
|
54143
|
54144
|
54145
|
54151
|
54152
|
54153
|
54154
|
54216