Skip to content

Commit

Permalink
Merge branch 'wireapp:develop' into develop
Browse files Browse the repository at this point in the history
  • Loading branch information
offsoc authored Oct 22, 2024
2 parents f5130c8 + b94e3c6 commit 4bc8b51
Show file tree
Hide file tree
Showing 86 changed files with 1,195 additions and 655 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/1-api-changes/add-columns-to-export
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
The team CSV export endpoint has gained two extra columns: `last_active` and `status`. The streaming behaviour has also been improved.
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
1 change: 1 addition & 0 deletions integration/integration.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ library
API.GundeckInternal
API.Nginz
API.Spar
API.Stern
MLS.Util
Notifications
RunAllTests
Expand Down
8 changes: 8 additions & 0 deletions integration/test/API/Stern.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
module API.Stern where

import Testlib.Prelude

getTeamActivity :: (HasCallStack, MakesValue domain) => domain -> String -> App Response
getTeamActivity domain tid =
baseRequest domain Stern Unversioned (joinHttpPath ["team-activity-info", tid])
>>= submit "GET"
45 changes: 32 additions & 13 deletions integration/test/Test/Teams.hs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
{-# OPTIONS -Wno-ambiguous-fields #-}
-- This file is part of the Wire Server implementation.
--
-- Copyright (C) 2024 Wire Swiss GmbH <[email protected]>
Expand All @@ -22,6 +23,7 @@ import qualified API.BrigInternal as I
import API.Common
import API.Galley (getTeam, getTeamMembers, getTeamMembersCsv, getTeamNotifications)
import API.GalleyInternal (setTeamFeatureStatus)
import API.Gundeck
import Control.Monad.Codensity (Codensity (runCodensity))
import Control.Monad.Extra (findM)
import Control.Monad.Reader (asks)
Expand Down Expand Up @@ -284,16 +286,28 @@ testUpgradePersonalToTeamAlreadyInATeam = do
-- for additional tests of the CSV download particularly with SCIM users, please refer to 'Test.Spar.Scim.UserSpec'
testTeamMemberCsvExport :: (HasCallStack) => App ()
testTeamMemberCsvExport = do
(owner, tid, members) <- createTeam OwnDomain 10
let numClients = [0, 1, 2] <> repeat 0
modifiedMembers <- for (zip numClients (owner : members)) $ \(n, m) -> do
handle <- randomHandle
putHandle m handle >>= assertSuccess
replicateM_ n $ addClient m def
void $ I.putSSOId m def {I.scimExternalId = Just "foo"} >>= getBody 200
setField "handle" handle m
>>= setField "role" (if m == owner then "owner" else "member")
>>= setField "num_clients" (show n)
(owner, tid, members) <- createTeam OwnDomain 5

modifiedMembers <- for
( zip
([0, 1, 2] <> repeat 0)
(owner : members)
)
$ \(n, m) -> do
handle <- randomHandle
putHandle m handle >>= assertSuccess
clients <-
replicateM n
$ addClient m def
>>= getJSON 201
>>= (%. "id")
>>= asString
for_ (listToMaybe clients) $ \c ->
getNotifications m def {client = Just c}
void $ I.putSSOId m def {I.scimExternalId = Just "foo"} >>= getBody 200
setField "handle" handle m
>>= setField "role" (if m == owner then "owner" else "member")
>>= setField "num_clients" n

memberMap :: Map.Map String Value <- fmap Map.fromList $ for (modifiedMembers) $ \m -> do
uid <- m %. "id" & asString
Expand All @@ -302,14 +316,16 @@ testTeamMemberCsvExport = do
bindResponse (getTeamMembersCsv owner tid) $ \resp -> do
resp.status `shouldMatchInt` 200
let rows = sort $ tail $ B8.lines $ resp.body
length rows `shouldMatchInt` 10
length rows `shouldMatchInt` 5
for_ rows $ \row -> do
let cols = B8.split ',' row
let uid = read $ B8.unpack $ cols !! 11
let mem = memberMap Map.! uid

ownerId <- owner %. "id" & asString
let ownerMember = memberMap Map.! ownerId
now <- formatTime defaultTimeLocale "%Y-%m-%d" <$> liftIO getCurrentTime
numClients <- mem %. "num_clients" & asInt

let parseField = unquote . read . B8.unpack . (cols !!)

Expand All @@ -319,12 +335,15 @@ testTeamMemberCsvExport = do
role <- mem %. "role" & asString
parseField 3 `shouldMatch` role
when (role /= "owner") $ do
now <- formatTime defaultTimeLocale "%Y-%m-%d" <$> liftIO getCurrentTime
take 10 (parseField 4) `shouldMatch` now
parseField 5 `shouldMatch` (ownerMember %. "handle")
parseField 7 `shouldMatch` "wire"
parseField 9 `shouldMatch` "foo"
parseField 12 `shouldMatch` (mem %. "num_clients")
parseField 12 `shouldMatch` show numClients
(if numClients > 0 then shouldNotMatch else shouldMatch)
(parseField 13)
""
parseField 14 `shouldMatch` "active"
where
unquote :: String -> String
unquote ('\'' : x) = x
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 {..})
)
2 changes: 2 additions & 0 deletions libs/wire-api/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
, iso3166-country-codes
, iso639
, jose
, kan-extensions
, lens
, lib
, memory
Expand Down Expand Up @@ -165,6 +166,7 @@ mkDerivation {
iso3166-country-codes
iso639
jose
kan-extensions
lens
memory
metrics-wai
Expand Down
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
Loading

0 comments on commit 4bc8b51

Please sign in to comment.