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

[Thomas Lamprecht]

On 6/17/19 2:15 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>
> ---
>  src/PVE/Ticket.pm | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/src/PVE/Ticket.pm b/src/PVE/Ticket.pm
> index 5935ba5..629ec11 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,10 +33,13 @@ sub verify_csrf_prevention_token {
>  	my $timestamp = $1;
>  	my $ttime = hex($timestamp);
>  
> +	my $hmac_digest = Digest::SHA::hmac_sha256_base64("$timestamp:$username", $secret);
> +
> +	# fallback
>  	my $digest = Digest::SHA::sha1_base64("$timestamp:$username", $secret);

maybe we could use the length if $sig to determine which one we
probably need to check? Else one _always_ computes both, which isn't
ideal...

We then could also back port this one to stable-5 so that we ensure
less problems on upgrade, e.g., if one does an upgrade of Node B but
is connected (proxied) through Node A, or the like.

>  
>  	my $age = time() - $ttime;
> -	return 1 if ($digest eq $sig) && ($age > $min_age) &&
> +	return 1 if (($digest eq $sig) or ($hmac_digest eq $sig)) && ($age > $min_age) &&
>  	    ($age < $max_age);
>      }
>  
>