From 95f4492fdee1eb31a69d2e75e18971a6e6291367 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Fri, 15 Jul 2016 14:16:07 +0200 Subject: [PATCH] Bug 16929: Wrapper around CSRF call Content-Type: text/plain; charset=utf-8 Just in concept --- Koha/Token.pm | 82 ++++++++++++++++++++++++++++++++++++++++++++++++ opac/opac-memberentry.pl | 28 +++++++++++++---- t/Token.t | 13 ++++++++ 3 files changed, 117 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..d67cb19 --- /dev/null +++ b/Koha/Token.pm @@ -0,0 +1,82 @@ +package Koha::Token; + +# Created as wrapper for CSRF tokens, but designed for more general use + +use Modern::Perl; + +use base qw(Class::Accessor); + +use constant HMAC_SHA1_LENGTH => 20; + +sub new { + my ( $class ) = @_; + return $class->SUPER::new(); +} + +sub generate { + my ( $self, $params ) = @_; + if( $params->{CSRF} ) { + $self->{lasttoken} = _gen_csrf( $params ); + } else { + $self->{lasttoken} = _gen_rand( $params ); + } + return $self->{lasttoken}; +} + +sub check { + my ( $self, $params ) = @_; + if( $params->{CSRF} ) { + return _chk_csrf( $params ); + } + return; +} + +#--- Internal routines + +sub _gen_csrf { + my ( $params ) = @_; + return if !$params->{id} || !$params->{secret}; + + require WWW::CSRF; + my $token; + local $SIG{ALRM} = sub { die "alarm\n"; }; + alarm 2; + eval { + $token = WWW::CSRF::generate_csrf_token( + $params->{id}, $params->{secret}, + ); + }; + alarm 0; + warn "L49 alarm" if $@ =~ /alarm/; + if( !$token ) { + my $mask = _gen_rand({ length => HMAC_SHA1_LENGTH }); + $token = WWW::CSRF::generate_csrf_token( + $params->{id}, $params->{secret}, { Random => $mask }, + ); + } + 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 ); +} + +1; diff --git a/opac/opac-memberentry.pl b/opac/opac-memberentry.pl index de157ef..35110dc 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,12 @@ 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( + id => $borrower->{userid}, + secret => md5_base64( C4::Context->config('pass') ), + token => scalar $cgi->param('csrf_token'), + ); my %borrower = ParseCgiForBorrower($cgi); @@ -197,7 +201,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 +237,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 +261,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..70ba20e --- /dev/null +++ b/t/Token.t @@ -0,0 +1,13 @@ +use Koha::Token; + +my $tokenizer = Koha::Token->new; +my $token = $tokenizer->generate({ length => 20 }); +print "$token\n"; +my $token = $tokenizer->generate({ CSRF => 1, id => 'ed', secret => 'x@#' }); +print "$token\n"; +my $r = $tokenizer->check({ CSRF => 1, id => 'ed', secret => 'x@#', token => $token }); +print "r=$r\n"; +substr($token, -2) = '12'; +$r = $tokenizer->check({ CSRF => 1, id => 'ed', secret => 'x@#', token => $token }); +print "r=$r\n"; + -- 2.1.4