[pve-devel] [PATCH v2 common] use hmac_sha256 instead of sha1 for csrf token

[Oguz Bektas]

hi!

On Tue, Jun 18, 2019 at 03:01:17PM +0200, Thomas Lamprecht wrote:
> On 6/18/19 2:41 PM, Oguz Bektas wrote:
> > now generates & verifies with hmac_sha1. also left the old digest format
> > for backwards compatibility during verification, to be removed at some
> > later time.
> > 
> > Signed-off-by: Oguz Bektas <o.bektas at proxmox.com>
> > ---
> > v1 -> v2:
> > * only calculate the needed hash, based on the length of the signature (thx thomas)
> > 
> >  src/PVE/Ticket.pm | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/PVE/Ticket.pm b/src/PVE/Ticket.pm
> > index 5935ba5..6c83ec7 100644
> > --- a/src/PVE/Ticket.pm
> > +++ b/src/PVE/Ticket.pm
> > @@ -20,7 +20,7 @@ sub assemble_csrf_prevention_token {
> >  
> >      my $timestamp = sprintf("%08X", time());
> >  
> > -    my $digest = Digest::SHA::sha1_base64("$timestamp:$username", $secret);
> > +    my $digest = Digest::SHA::hmac_sha256_base64("$timestamp:$username", $secret);
> >  
> >      return "$timestamp:$digest";
> >  }
> > @@ -33,7 +33,7 @@ sub verify_csrf_prevention_token {
> >  	my $timestamp = $1;
> >  	my $ttime = hex($timestamp);
> >  
> > -	my $digest = Digest::SHA::sha1_base64("$timestamp:$username", $secret);
> > +	my $digest = (length($sig) == 27) ? Digest::SHA::sha1_base64("$timestamp:$username", $secret) : Digest::SHA::hmac_sha256_base64("$timestamp:$username", $secret);
> 
> Really? This is a >160 character line... NAK! 
sorry.
>Rather something like:
> 
> my $digest;
> if (length($sig) == 27) {
>     # detected SHA1 from older proxy -> fallback. FIXME: remove with 7.0!
>     $digest = Digest::SHA::sha1_base64("$timestamp:$username", $secret);
> ...
> 
> Please also sent all patches with a next version, at least if it's a
> small series (if it was a series with >10 patches and only one changed,
> OK, then /maybe/ not, but as mails are pretty cheap even then it may be
> better :))
alright. i'll send v3.
> 
> Also, the PMG uses that too, so it could be good if you add, test and
> send a patch for it too, so that we don't forget it. Thanks! :)
ok. i'll send for pmg later today.
> 
> >  
> >  	my $age = time() - $ttime;
> >  	return 1 if ($digest eq $sig) && ($age > $min_age) &&
> > 
>