Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Argon2 v10 fix #56

Merged
merged 4 commits into from
Mar 8, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 38 additions & 25 deletions password/src/Data/Password/Argon2.hs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ import Data.Maybe (fromMaybe)
import Data.Semigroup ((<>))
#endif
import Data.Text (Text)
import qualified Data.Text as T (intercalate, length, split, splitAt)
import qualified Data.Text as T (intercalate, split, splitAt, stripPrefix)
import Data.Word (Word32)

import Data.Password.Internal (
Expand Down Expand Up @@ -277,30 +277,40 @@ checkPassword :: Password -> PasswordHash Argon2 -> PasswordCheck
checkPassword pass (PasswordHash passHash) =
fromMaybe PasswordCheckFail $ do
let paramList = T.split (== '$') passHash
guard $ Prelude.length paramList == 6
let [ _,
variantT,
versionT,
parametersT,
salt64,
hashedKey64 ] = paramList
argon2Variant <- parseVariant variantT
argon2Version <- parseVersion versionT
(argon2MemoryCost, argon2TimeCost, argon2Parallelism) <- parseParameters parametersT
salt <- from64 $ unsafePad64 salt64
hashedKey <- from64 $ unsafePad64 hashedKey64
let argon2OutputLength = fromIntegral $ B.length hashedKey -- only here because of warnings
producedKey = hashPasswordWithSalt' Argon2Params{..} (Salt salt) pass
(argon2Params, salt, hashedKey) <- parseArgon2Params paramList
let producedKey = hashPasswordWithSalt' argon2Params salt pass
guard $ hashedKey `constEq` producedKey
return PasswordCheckSuccess

parseArgon2Params :: [Text] -> Maybe (Argon2Params, Salt Argon2, ByteString)
-- vp - version or params
-- ps - params or salt
-- sh - salt or hash
parseArgon2Params (_:variantT:vp:ps:sh:rest) = do
variant <- parseVariant variantT
case rest of
-- If there is a 6th part, we'll assume the version is included
[hashedKey64] -> do
version <- parseVersion vp
parseAll variant version ps sh hashedKey64
-- If there are only 5 parts, we'll assume the version is 'Version10'
[] -> parseAll variant Version10 vp ps sh
-- Any other amount of parts means the provided hash is malformed
_ -> Nothing
where
argon2Salt = 16 -- only here because of warnings
parseVariant = splitMaybe "argon2" letterToVariant
parseVersion = splitMaybe "v=" numToVersion
parseParameters params = do
let ps = T.split (== ',') params
guard $ Prelude.length ps == 3
go ps (Nothing, Nothing, Nothing)
parseAll argon2Variant argon2Version parametersT salt64 hashedKey64 = do
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a nitpick, but I felt like I wanted a type signature on this function. I thought it might be a little easier to see exactly what it is doing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parseAll :: Argon2.Variant -> Argon2.Version -> Text -> Text -> Text -> Maybe (Argon2Params, Salt Argon2, ByteString)

Sure, I'll add this in before I merge it.

(argon2MemoryCost, argon2TimeCost, argon2Parallelism) <- parseParameters parametersT
salt <- from64 $ unsafePad64 salt64
hashedKey <- from64 $ unsafePad64 hashedKey64
let argon2OutputLength = fromIntegral $ B.length hashedKey -- only here because of warnings
argon2Salt = 16 -- only here because of warnings
pure (Argon2Params{..}, Salt salt, hashedKey)
parseParameters paramsT = do
let paramsL = T.split (== ',') paramsT
guard $ Prelude.length paramsL == 3
go paramsL (Nothing, Nothing, Nothing)
where
go [] (Just m, Just t, Just p) = Just (m, t, p)
go [] _ = Nothing
Expand All @@ -310,11 +320,14 @@ checkPassword pass (PasswordHash passHash) =
("t=", i) -> go xs (m, readT i, p)
("p=", i) -> go xs (m, t, readT i)
_ -> Nothing
splitMaybe :: Text -> (Text -> Maybe a) -> Text -> Maybe a
splitMaybe match f t =
case T.splitAt (T.length match) t of
(m, x) | m == match -> f x
_ -> Nothing
-- If there are less than 5 parts, the hash is malformed
parseArgon2Params _ = Nothing

-- | Strips the given 'match' if it matches and uses
-- the function on the remainder of the given text.
splitMaybe :: Text -> (Text -> Maybe a) -> Text -> Maybe a
splitMaybe match f t =
T.stripPrefix match t >>= f

-- | Generate a random 16-byte @Argon2@ salt
--
Expand Down
31 changes: 27 additions & 4 deletions password/test/tasty/Argon2.hs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{-# LANGUAGE OverloadedStrings #-}
module Argon2 where
module Argon2 (testArgon2) where

import Test.Tasty
import Test.Tasty (TestTree, testGroup)
import Test.Tasty.HUnit (assertBool, assertEqual, testCase)

import Data.Password.Argon2
Expand All @@ -20,6 +20,7 @@ testArgon2 = testGroup "Argon2"
, testWithParams "Argon2 (Argon2i)" $ fastParams{ argon2Variant = Argon2i }
, testWithParams "Argon2 (Argon2d)" $ fastParams{ argon2Variant = Argon2d }
, paddingTests
, omittedVersionTest
]
where
testWithParams s params =
Expand Down Expand Up @@ -47,18 +48,40 @@ hashWithPadding, hashWithoutPadding :: PasswordHash Argon2
hashWithPadding = PasswordHash "$argon2id$v=19$m=65536,t=2,p=1$YWJjZGVmZ2hpamtsbW5vcA==$BztdyfEefG5V18ZNlztPrfZaU5duVFKZiI6dJeWht0o="
hashWithoutPadding = PasswordHash "$argon2id$v=19$m=65536,t=2,p=1$YWJjZGVmZ2hpamtsbW5vcA$BztdyfEefG5V18ZNlztPrfZaU5duVFKZiI6dJeWht0o"

-- Very old hashes might not have version parts, so infer as version 1.0
omittedVersionTest :: TestTree
omittedVersionTest = testGroup "Version 1.0"
[ go "version 1.0 part in hash (placebo)" "testtest" v10Hash
, go "no version part in hash == version 1.0" "testtest" v10HashWithoutVersion
, go "version 1.3 part in hash (reference)" "password" referenceHash
, testCase "no version 1.3 part in hash should fail" $
assertEqual "check passed!?" PasswordCheckFail $
checkPassword "password" referenceHashWithoutVersion
]
where
go s p = testCase s
. assertEqual "check failed" PasswordCheckSuccess
. checkPassword p

-- Reference check using the Command-line Utility output example
-- from: https://github.com/P-H-C/phc-winner-argon2
referenceTest :: TestTree
referenceTest = testCase "PHC Argon2 reference" $
assertEqual "output hash is wrong" expected $
assertEqual "output hash is wrong" referenceHash $
hashPasswordWithSalt params salt pwd
where
expected = PasswordHash "$argon2i$v=19$m=65536,t=2,p=4$c29tZXNhbHQ$RdescudvJCsgt3ub+b+dWRWJTmaaJObG"
salt = Salt "somesalt"
pwd = mkPassword "password"
params = defaultParams {
argon2Variant = Argon2i,
argon2Parallelism = 4,
argon2OutputLength = 24
}

-- Weirdly lined out to show it's exactly the same, except the 'v=' part is missing.
referenceHash, referenceHashWithoutVersion :: PasswordHash Argon2
referenceHash = PasswordHash "$argon2i$v=19$m=65536,t=2,p=4$c29tZXNhbHQ$RdescudvJCsgt3ub+b+dWRWJTmaaJObG"
referenceHashWithoutVersion = PasswordHash "$argon2i$m=65536,t=2,p=4$c29tZXNhbHQ$RdescudvJCsgt3ub+b+dWRWJTmaaJObG"
v10Hash, v10HashWithoutVersion :: PasswordHash Argon2
v10Hash = PasswordHash "$argon2i$v=16$m=65536,t=2,p=1$Kx1BEcpIg0Ey5GyXq5do2w$0qRfWHw09EdqQkSsaG57O/ou8v/E6Vc83w"
v10HashWithoutVersion = PasswordHash "$argon2i$m=65536,t=2,p=1$Kx1BEcpIg0Ey5GyXq5do2w$0qRfWHw09EdqQkSsaG57O/ou8v/E6Vc83w"
8 changes: 4 additions & 4 deletions stack.yaml.lock
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
packages: []
snapshots:
- completed:
size: 567241
url: https://raw.githubusercontent.com/commercialhaskell/stackage-snapshots/master/lts/17/10.yaml
sha256: 321b3b9f0c7f76994b39e0dabafdc76478274b4ff74cc5e43d410897a335ad3b
original: lts-17.10
size: 587821
url: https://raw.githubusercontent.com/commercialhaskell/stackage-snapshots/master/lts/18/24.yaml
sha256: 06d844ba51e49907bd29cb58b4a5f86ee7587a4cd7e6cf395eeec16cba619ce8
original: lts-18.24