From 6e1ab8f451bfee29f38e3dbab1fd52a4423748f3 Mon Sep 17 00:00:00 2001 From: "Kyle J. McKay" Date: Wed, 24 Feb 2021 18:04:26 -0700 Subject: [PATCH] HashUtil.pm: reverse order of hmac_sha1 arguments Change the order of the arguments passed to HashUtil::hmac_sha1 to be "text" then "key" instead of "key" then "text". Originally the hmac_sha1 provided by Girocco::HashUtil used the sane order of "key" then "text" as that's the order they are mentioned in RFC 2104. The @#%^^@* Digest::SHA module, on the other hand (which provides both hmac_sha1 and hmac_sha256), for some inscrutable reason uses the order of "text" then "key"! In an effort to avoid needless confusion, make the HashUtil implementation of hmac_sha1 take the arguments in exactly the same order as the Digest::SHA module's function of the same name. Signed-off-by: Kyle J. McKay --- Girocco/HashUtil.pm | 11 ++++++++--- Girocco/Notify.pm | 8 +------- Girocco/TimedToken.pm | 2 +- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/Girocco/HashUtil.pm b/Girocco/HashUtil.pm index 3ddec69..5e5d0c5 100644 --- a/Girocco/HashUtil.pm +++ b/Girocco/HashUtil.pm @@ -83,10 +83,15 @@ sub _xor5C {use bytes; $_[0]=~tr } # As defined in RFC 2104 for H = SHA-1 +# Note that the order of the arguments is deliberately +# $_[0] -> text +# $_[1] -> key +# To match other Perl Digest modules even though RFC 2104 +# talks about the key and then the text in that order! sub hmac_sha1 { use bytes; - my $key = shift || ''; my $text = shift || ''; + my $key = shift || ''; # HMAC is defined as H(K XOR opad, H(K XOR ipad, text)) # where ipad is always 0x36 and opad is always 0x5C @@ -158,12 +163,12 @@ sub crypt_sha1 { my $data = sprintf("%s%s%u", $salt, SHA1_MAGIC, $iterations); # Do the initial HMAC where $pw is the KEY and $data is the TEXT - $data = hmac_sha1($pw, $data); + $data = hmac_sha1($data, $pw); # Perform any additional iterations requested for (my $i = 1; $i < $iterations; ++$i) { # Again $pw is the KEY and $data is the TEXT - $data = hmac_sha1($pw, $data); + $data = hmac_sha1($data, $pw); } return SHA1_MAGIC.$iterations.'$'.$salt.'$'._encode_base64_alt($data); diff --git a/Girocco/Notify.pm b/Girocco/Notify.pm index 43f937f..37fe600 100644 --- a/Girocco/Notify.pm +++ b/Girocco/Notify.pm @@ -143,15 +143,9 @@ sub _jsonsigheaders { my $hmackey = $proj->{jsonsecret}; my @sigheaders = (); if (defined($hmackey) && $hmackey ne "") { - my $sig = "sha1=".lc(unpack('H*',hmac_sha1($hmackey, $payload))); + my $sig = "sha1=".lc(unpack('H*',hmac_sha1($payload, $hmackey))); push(@sigheaders, X_Hub_Signature => $sig); if ($have_hmac_sha256) { - # Yes, the argument order is different! #%@#%^@! - # hmac_sha1 is provided by Girocco::HashUtil and it uses the - # sane order of "key" then "text" as that's the order they are - # mentioned in RFC 2104. The @#%^^@* Digest::SHA module, on the - # other hand, is the one providing hmac_sha256 and for some - # inscrutable reason uses the order of "text" then "key"! my $sig256 = "sha256=".lc(unpack('H*',hmac_sha256($payload, $hmackey))); push(@sigheaders, X_Hub_Signature_256 => $sig256); } diff --git a/Girocco/TimedToken.pm b/Girocco/TimedToken.pm index d18b128..cb65d31 100644 --- a/Girocco/TimedToken.pm +++ b/Girocco/TimedToken.pm @@ -242,7 +242,7 @@ sub _get_raw_hmac { $period = int($period); $offset = int($offset); my $text = '$TimedToken$'.$extra.'$'.$period.'$'.$offset; - return hmac_sha1($secret, $text); + return hmac_sha1($text, $secret); } 1; -- 2.11.4.GIT