Skip to content

Commit

Permalink
[feat] Allow configuring argon2id parameters (#4291)
Browse files Browse the repository at this point in the history
  • Loading branch information
elland authored Oct 21, 2024
1 parent 04ac98a commit b94e3c6
Show file tree
Hide file tree
Showing 47 changed files with 606 additions and 338 deletions.
6 changes: 3 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ install: init
./hack/bin/cabal-run-all-tests.sh
./hack/bin/cabal-install-artefacts.sh all

.PHONY: clean-rabbit
clean-rabbit:
.PHONY: rabbit-clean
rabbit-clean:
rabbitmqadmin -f pretty_json list queues vhost name messages | jq -r '.[] | "rabbitmqadmin delete queue name=\(.name) --vhost=\(.vhost)"' | bash

# Clean
Expand All @@ -59,7 +59,7 @@ full-clean: clean
rm -rf ~/.cache/hie-bios
rm -rf ./dist-newstyle ./.env
direnv reload
clean-rabbit
make rabbit-clean
@echo -e "\n\n*** NOTE: you may want to also 'rm -rf ~/.cabal/store \$$CABAL_DIR/store', not sure.\n"

.PHONY: clean
Expand Down
18 changes: 18 additions & 0 deletions changelog.d/0-release-notes/configurable-argon
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
Password hashing is now done using argon2id instead of scrypt. The argon2id parameters can be configured using these options:

```yaml
brig:
optSettings:
setPasswordHashingOptions:
iterations: ...
memory: ... # memory needed in KiB
parallelism: ...
galley:
settings:
passwordHashingOptions:
iterations: ...
memory: ... # memory needed in KiB
parallelism: ...
```

These have default values, which should work for most deployments. Please see documentation on config-options for more.
1 change: 1 addition & 0 deletions changelog.d/2-features/add-config-for-pwd-hash
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Allow configuring Argon2id parameters
1 change: 1 addition & 0 deletions charts/brig/templates/configmap.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -368,5 +368,6 @@ data:
{{- if .setOAuthMaxActiveRefreshTokens }}
setOAuthMaxActiveRefreshTokens: {{ .setOAuthMaxActiveRefreshTokens }}
{{- end }}
setPasswordHashingOptions: {{ toYaml .setPasswordHashingOptions | nindent 8 }}
{{- end }}
{{- end }}
7 changes: 6 additions & 1 deletion charts/brig/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ metrics:
enabled: false
# This is not supported for production use, only here for testing:
# preStop:
# exec:
# exec:
# command: ["sh", "-c", "curl http://acme.example"]
config:
logLevel: Info
Expand Down Expand Up @@ -150,6 +150,11 @@ config:
setDisabledAPIVersions: [ development ]
setFederationStrategy: allowNone
setFederationDomainConfigsUpdateFreq: 10
# Options for Argon2id version 19
setPasswordHashingOptions:
iterations: 1
parallelism: 32
memory: 180224 # 176 MiB
smtp:
passwordFile: /etc/wire/brig/secrets/smtp-password.txt
proxy: {}
Expand Down
1 change: 1 addition & 0 deletions charts/galley/templates/configmap.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ data:
{{- if .settings.guestLinkTTLSeconds }}
guestLinkTTLSeconds: {{ .settings.guestLinkTTLSeconds }}
{{- end }}
passwordHashingOptions: {{ toYaml .settings.passwordHashingOptions | nindent 8 }}
featureFlags:
sso: {{ .settings.featureFlags.sso }}
legalhold: {{ .settings.featureFlags.legalhold }}
Expand Down
5 changes: 5 additions & 0 deletions charts/galley/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,11 @@ config:
# The lifetime of a conversation guest link in seconds. Must be a value 0 < x <= 31536000 (365 days)
# Default is 31536000 (365 days) if not set
guestLinkTTLSeconds: 31536000
# Options for Argon2id version 19
passwordHashingOptions:
iterations: 1
parallelism: 32
memory: 180224 # 176 MiB
featureFlags: # see #RefConfigOptions in `/docs/reference` (https://github.com/wireapp/wire-server/)
appLock:
defaults:
Expand Down
47 changes: 47 additions & 0 deletions docs/src/developer/reference/config-options.md
Original file line number Diff line number Diff line change
Expand Up @@ -707,6 +707,53 @@ optSettings:
setOAuthMaxActiveRefreshTokens: 10
```

#### Argon2id password hashing parameters

Since release 5.6.0, wire-server hashes passwords with
[argon2id](https://datatracker.ietf.org/doc/html/rfc9106) at rest. If
you do not do anything, the default parameters will be used, which
are:

```yaml
setPasswordHashingOptions:
iterations: 1
memory: 180224 # memory needed in kibibytes (1 kibibyte is 2^10 bytes)
parallelism: 32
```

The default will be adjusted to new developments in hashing algorithm
security from time to time.

The password hashing options are set for brig and galley:

```yaml
brig:
optSettings:
setPasswordHashingOptions:
iterations: ...
memory: ... # memory needed in KiB
parallelism: ...
galley:
settings:
passwordHashingOptions:
iterations: ...
memory: ... # memory needed in KiB
parallelism: ...
```

**Performance implications:** scrypt takes ~80ms on a realistic test
system, and argon2id with default settings takes ~500ms. This is a
runtime increase by a factor of ~6. This happens every time a
password is entered by the user: during login, password reset,
deleting a device, etc. (It does **NOT** happen during any other
cryptographic operations like session key update or message
de-/encryption.)

The settings are a trade-off between resilience against brute force
attacks and password secrecy. For most systems this should be safe
and not need more hardware resources for brig, but you may want to
form your own opinion.

#### Disabling API versions

It is possible to disable one ore more API versions. When an API version is disabled it won't be advertised on the `GET /api-version` endpoint, neither in the `supported`, nor in the `development` section. Requests made to any endpoint of a disabled API version will result in the same error response as a request made to an API version that does not exist.
Expand Down
13 changes: 13 additions & 0 deletions hack/helm_vars/wire-server/values.yaml.gotmpl
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,12 @@ brig:
setOAuthEnabled: true
setOAuthRefreshTokenExpirationTimeSecs: 14515200 # 24 weeks
setOAuthMaxActiveRefreshTokens: 10
# These values are insecure, against anyone getting hold of the hash,
# but its not a concern for the integration tests.
setPasswordHashingOptions:
iterations: 1
parallelism: 4
memory: 32 # This needs to be at least 8 * parallelism.
aws:
sesEndpoint: http://fake-aws-ses:4569
sqsEndpoint: http://fake-aws-sqs:4568
Expand Down Expand Up @@ -258,6 +264,13 @@ galley:
federationDomain: integration.example.com
disabledAPIVersions: []

# These values are insecure, against anyone getting hold of the hash,
# but its not a concern for the integration tests.
passwordHashingOptions:
iterations: 1
parallelism: 4
memory: 32 # This needs to be at least 8 * parallelism.

featureFlags:
sso: disabled-by-default # this needs to be the default; tests can enable it when needed.
legalhold: whitelist-teams-and-implicit-consent
Expand Down
5 changes: 0 additions & 5 deletions libs/types-common/src/Data/Misc.hs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ module Data.Misc
fromPlainTextPassword,
plainTextPassword8Unsafe,
plainTextPassword6Unsafe,
plainTextPassword8To6,

-- * Typesafe FUTUREWORKS
FutureWork (..),
Expand Down Expand Up @@ -333,10 +332,6 @@ plainTextPassword8Unsafe = PlainTextPassword' . unsafeRange
fromPlainTextPassword :: PlainTextPassword' t -> Text
fromPlainTextPassword = fromRange . fromPlainTextPassword'

-- | Convert a 'PlainTextPassword8' to a legacy 'PlainTextPassword'.
plainTextPassword8To6 :: PlainTextPassword8 -> PlainTextPassword6
plainTextPassword8To6 = PlainTextPassword' . unsafeRange . fromPlainTextPassword

newtype PlainTextPassword' (minLen :: Nat) = PlainTextPassword'
{fromPlainTextPassword' :: Range minLen (1024 :: Nat) Text}
deriving stock (Eq, Generic)
Expand Down
19 changes: 19 additions & 0 deletions libs/types-common/src/Util/Options.hs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{-# LANGUAGE DeriveGeneric #-}
{-# LANGUAGE GeneralizedNewtypeDeriving #-}
{-# LANGUAGE OverloadedStrings #-}
{-# LANGUAGE RecordWildCards #-}
{-# LANGUAGE TemplateHaskell #-}

-- This file is part of the Wire Server implementation.
Expand Down Expand Up @@ -146,3 +147,21 @@ getOptions desc mp defaultPath = do

parseAWSEndpoint :: ReadM AWSEndpoint
parseAWSEndpoint = readerAsk >>= maybe (error "Could not parse AWS endpoint") pure . fromByteString . fromString

data PasswordHashingOptions = PasswordHashingOptions
{ iterations :: !Word32,
memory :: !Word32,
parallelism :: !Word32
}
deriving (Show, Generic)

instance FromJSON PasswordHashingOptions where
parseJSON =
withObject
"PasswordHashingOptions"
( \obj -> do
iterations <- obj .: "iterations"
memory <- obj .: "memory"
parallelism <- obj .: "parallelism"
pure (PasswordHashingOptions {..})
)
3 changes: 0 additions & 3 deletions libs/wire-api/src/Wire/API/Error/Brig.hs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ data BrigError
| LastIdentity
| NoPassword
| ChangePasswordMustDiffer
| PasswordAuthenticationFailed
| TooManyTeamInvitations
| CannotJoinMultipleTeams
| InsufficientTeamPermissions
Expand Down Expand Up @@ -254,8 +253,6 @@ type instance MapError 'NoPassword = 'StaticError 403 "no-password" "The user ha

type instance MapError 'ChangePasswordMustDiffer = 'StaticError 409 "password-must-differ" "For password change, new and old password must be different."

type instance MapError 'PasswordAuthenticationFailed = 'StaticError 403 "password-authentication-failed" "Password authentication failed."

type instance MapError 'TooManyTeamInvitations = 'StaticError 403 "too-many-team-invitations" "Too many team invitations for this team"

type instance MapError 'CannotJoinMultipleTeams = 'StaticError 403 "cannot-join-multiple-teams" "Cannot accept invitations from multiple teams"
Expand Down
43 changes: 19 additions & 24 deletions libs/wire-api/src/Wire/API/Password.hs
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ module Wire.API.Password
verifyPassword,
verifyPasswordWithStatus,
PasswordReqBody (..),
argon2OptsFromHashingOpts,

-- * Only for testing
hashPasswordArgon2idWithSalt,
hashPasswordArgon2idWithOptions,
mkSafePasswordScrypt,
parsePassword,
)
Expand All @@ -52,6 +52,7 @@ import Data.Text qualified as Text
import Data.Text.Encoding qualified as Text
import Imports
import OpenSSL.Random (randBytes)
import Util.Options

-- | A derived, stretched password that can be safely stored.
data Password
Expand Down Expand Up @@ -120,19 +121,6 @@ defaultScryptParams =
outputLength = 64
}

-- | Recommended in the RFC as the second choice: https://www.rfc-editor.org/rfc/rfc9106.html#name-parameter-choice
-- The first choice takes ~1s to hash passwords which seems like too much.
defaultOptions :: Argon2.Options
defaultOptions =
Argon2.Options
{ iterations = 1,
-- TODO: fix this after meeting with Security
memory = 2 ^ (17 :: Int),
parallelism = 32,
variant = Argon2.Argon2id,
version = Argon2.Version13
}

fromScrypt :: ScryptParameters -> Parameters
fromScrypt scryptParams =
Parameters
Expand All @@ -142,6 +130,16 @@ fromScrypt scryptParams =
outputLength = 64
}

argon2OptsFromHashingOpts :: PasswordHashingOptions -> Argon2.Options
argon2OptsFromHashingOpts PasswordHashingOptions {..} =
Argon2.Options
{ variant = Argon2.Argon2id,
version = Argon2.Version13,
iterations = iterations,
memory = memory,
parallelism = parallelism
}

-------------------------------------------------------------------------------

-- | Generate a strong, random plaintext password of length 16
Expand All @@ -154,8 +152,8 @@ genPassword =
mkSafePasswordScrypt :: (MonadIO m) => PlainTextPassword' t -> m Password
mkSafePasswordScrypt = fmap ScryptPassword . hashPasswordScrypt . Text.encodeUtf8 . fromPlainTextPassword

mkSafePassword :: (MonadIO m) => PlainTextPassword' t -> m Password
mkSafePassword = fmap Argon2Password . hashPasswordArgon2id . Text.encodeUtf8 . fromPlainTextPassword
mkSafePassword :: (MonadIO m) => Argon2.Options -> PlainTextPassword' t -> m Password
mkSafePassword opts = fmap Argon2Password . hashPasswordArgon2id opts . Text.encodeUtf8 . fromPlainTextPassword

-- | Verify a plaintext password from user input against a stretched
-- password from persistent storage.
Expand Down Expand Up @@ -190,16 +188,13 @@ encodeScryptPassword ScryptHashedPassword {..} =
Text.decodeUtf8 . B64.encode $ hashedKey
]

hashPasswordArgon2id :: (MonadIO m) => ByteString -> m Argon2HashedPassword
hashPasswordArgon2id pwd = do
hashPasswordArgon2id :: (MonadIO m) => Argon2.Options -> ByteString -> m Argon2HashedPassword
hashPasswordArgon2id opts pwd = do
salt <- newSalt 16
pure $! hashPasswordArgon2idWithSalt salt pwd

hashPasswordArgon2idWithSalt :: ByteString -> ByteString -> Argon2HashedPassword
hashPasswordArgon2idWithSalt = hashPasswordArgon2idWithOptions defaultOptions
pure $! hashPasswordArgon2idWithSalt opts salt pwd

hashPasswordArgon2idWithOptions :: Argon2.Options -> ByteString -> ByteString -> Argon2HashedPassword
hashPasswordArgon2idWithOptions opts salt pwd = do
hashPasswordArgon2idWithSalt :: Argon2.Options -> ByteString -> ByteString -> Argon2HashedPassword
hashPasswordArgon2idWithSalt opts salt pwd = do
let hashedKey = hashPasswordWithOptions opts pwd salt
in Argon2HashedPassword {..}

Expand Down
1 change: 0 additions & 1 deletion libs/wire-api/src/Wire/API/Routes/Public/Spar.hs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,6 @@ sparResponseURI (Just tid) =
type APIScim =
OmitDocs :> "v2" :> ScimSiteAPI SparTag
:<|> "auth-tokens"
:> CanThrow 'PasswordAuthenticationFailed
:> CanThrow 'CodeAuthenticationFailed
:> CanThrow 'CodeAuthenticationRequired
:> APIScimToken
Expand Down
15 changes: 11 additions & 4 deletions libs/wire-api/src/Wire/API/User.hs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ module Wire.API.User
SelfProfile (..),
-- User (should not be here)
User (..),
isSamlUser,
userId,
userDeleted,
userEmail,
Expand Down Expand Up @@ -583,6 +584,12 @@ data User = User
deriving (Arbitrary) via (GenericUniform User)
deriving (ToJSON, FromJSON, S.ToSchema) via (Schema User)

isSamlUser :: User -> Bool
isSamlUser usr = do
case usr.userIdentity of
Just (SSOIdentity (UserSSOId _) _) -> True
_ -> False

userId :: User -> UserId
userId = qUnqualified . userQualifiedId

Expand Down Expand Up @@ -1405,8 +1412,8 @@ instance (res ~ PutSelfResponses) => AsUnion res (Maybe UpdateProfileError) wher

-- | The payload for setting or changing a password.
data PasswordChange = PasswordChange
{ cpOldPassword :: Maybe PlainTextPassword6,
cpNewPassword :: PlainTextPassword8
{ oldPassword :: Maybe PlainTextPassword6,
newPassword :: PlainTextPassword8
}
deriving stock (Eq, Show, Generic)
deriving (Arbitrary) via (GenericUniform PasswordChange)
Expand All @@ -1422,9 +1429,9 @@ instance ToSchema PasswordChange where
)
. object "PasswordChange"
$ PasswordChange
<$> cpOldPassword
<$> oldPassword
.= maybe_ (optField "old_password" schema)
<*> cpNewPassword
<*> newPassword
.= field "new_password" schema

data ChangePasswordError
Expand Down
Loading

0 comments on commit b94e3c6

Please sign in to comment.