From 6f75269c3ffb08fcb306db87d1d78c64529e7067 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Wed, 30 Oct 2024 15:11:01 +0100 Subject: [PATCH 1/3] Move docs from docs.wire.com to generated helper page served by brig. (#4311) Co-authored-by: Leif Battermann --- changelog.d/4-docs/fix-swagger-2 | 1 + .../api-client-perspective/swagger.md | 88 +++---------------- integration/test/Test/Swagger.hs | 2 +- .../src/Wire/API/Federation/Version.hs | 9 ++ services/brig/src/Brig/API/Public.hs | 76 +++++++++++++--- 5 files changed, 89 insertions(+), 87 deletions(-) create mode 100644 changelog.d/4-docs/fix-swagger-2 diff --git a/changelog.d/4-docs/fix-swagger-2 b/changelog.d/4-docs/fix-swagger-2 new file mode 100644 index 00000000000..118fc6d712e --- /dev/null +++ b/changelog.d/4-docs/fix-swagger-2 @@ -0,0 +1 @@ +Move docs from docs.wire.com to generated helper page served by brig \ No newline at end of file diff --git a/docs/src/understand/api-client-perspective/swagger.md b/docs/src/understand/api-client-perspective/swagger.md index 773a89fe071..a188dd166dc 100644 --- a/docs/src/understand/api-client-perspective/swagger.md +++ b/docs/src/understand/api-client-perspective/swagger.md @@ -1,37 +1,27 @@ (swagger-api-docs)= -# Swagger API documentation +# Swagger / OpenAPI documentation -Our staging system provides [Swagger / -OpenAPI](https://swagger.io/resources/open-api/) documentation of our HTTP REST -API. +Our staging system provides [OpenAPI +3.0](https://swagger.io/resources/open-api/) documentation of our HTTP +REST API under the following URL: -The swagger docs are correct by construction (compiled from the server -code), and they are complete up to bots/services and event notification -payloads (as of 2023-01-16). +[https://staging-nginz-https.zinfra.io/api/swagger-ui](https://staging-nginz-https.zinfra.io/api/swagger-ui) -There are several ways to interpret this kind of documentation: +There are several ways to interpret this documentation: - Read it as a reference - Generate client code from it - Interactively explore the API by making requests -## Swagger docs (Swagger 2.0) +To find the source code of end-points mentioned in the API, a *route +internal ID* (field `operationId` in openapi) is provided for every +end-point. See {ref}`named-and-internal-route-ids` for details and +usage. -The [Swagger / OpenAPI 2.0](https://swagger.io/specification/v2/) -documentation for endpoints depends on the API version. For a list of -all swagger docs for all supported API versions, [visit -https://staging-nginz-https.zinfra.io/api/swagger-ui](https://staging-nginz-https.zinfra.io/api/swagger-ui). +If you find anything you don't like or understand, please let us know! -To learn which versions are supported, look at -`https:///api-version`. ([See -also.](../../developer/developer/api-versioning.md)) - -If you want to get the raw json for the swagger (ie., for compiling it -into client code in typescript, kotlin, swift, ...), replace -`swagger-ui` with `swagger.json` in the above URL pattern. - -#### Example: doing it manually +## Example To get the versions a backend (`staging-nginz-https.zinfra.io` in this case) supports, execute: @@ -43,57 +33,3 @@ curl https:///api-version The URL to open in your browser for the development version `4` is `https:///v4/api/swagger-ui/`. - -### On-prem and test instances, versioning - -The above is valid for the official wire.com staging environment and -includes both all released API versions and the current development -version, which changes continuously until released. - -If you talk to any other backend, the development version may differ. -Try to ask the backend you're talking if it exposes its docs itself: - -``` -curl https://nginz-https..example.com//api/swagger-ui/ -curl https://nginz-https..example.com//api/swagger.json -``` - -### Internal endpoints - -Swagger docs for internal endpoints are served per service. I.e. there's one for -`brig`, one for `cannon`, etc.. This is because Swagger doesn't play well with -multiple actions having the same combination of HTTP method and URL path. - -Internal APIs are not under version control. - -- Unversioned: - - [`brig` - **internal** (private) - endpoints](https://staging-nginz-https.zinfra.io/api-internal/swagger-ui/brig) - - [`cannon` - **internal** (private) - endpoints](https://staging-nginz-https.zinfra.io/api-internal/swagger-ui/cannon) - - [`cargohold` - **internal** (private) - endpoints](https://staging-nginz-https.zinfra.io/api-internal/swagger-ui/cargohold) - - [`galley` - **internal** (private) - endpoints](https://staging-nginz-https.zinfra.io/api-internal/swagger-ui/galley) - - [`gundeck` - **internal** (private) - endpoints](https://staging-nginz-https.zinfra.io/api-internal/swagger-ui/gundeck) - - [`spar` - **internal** (private) - endpoints](https://staging-nginz-https.zinfra.io/api-internal/swagger-ui/spar) - -The URL pattern is similar to that of public endpoints for latest version: -`https:///api-internal/swagger-ui/`. - -If you want to get the raw json of the swagger: -`https:///api-internal/swagger-ui/-swagger.json`. - -### Federation API - -- Unversioned - - [`brig` - Federation API](https://staging-nginz-https.zinfra.io/api-federation/swagger-ui/brig) - - [`galley` - Federation API](https://staging-nginz-https.zinfra.io/api-federation/swagger-ui/galley) - - [`cargohold` - Federation API](https://staging-nginz-https.zinfra.io/api-federation/swagger-ui/cargohold) - -### Finding the source code for an end-point - -A *route internal ID* is provided for every end-point. See -{ref}`named-and-internal-route-ids` for details and usage. diff --git a/integration/test/Test/Swagger.hs b/integration/test/Test/Swagger.hs index 2fe53d487bb..571bd1ab245 100644 --- a/integration/test/Test/Swagger.hs +++ b/integration/test/Test/Swagger.hs @@ -84,7 +84,7 @@ testSwaggerToc = do get path = rawBaseRequest OwnDomain Brig Unversioned path >>= submit "GET" html :: String - html = "

please pick an api version

/v0/api/swagger-ui/
/v1/api/swagger-ui/
/v2/api/swagger-ui/
/v3/api/swagger-ui/
/v4/api/swagger-ui/
/v5/api/swagger-ui/
/v6/api/swagger-ui/
/v7/api/swagger-ui/
" + html = "

OpenAPI 3.0 docs for all Wire APIs

\n

This wire-server system provides OpenAPI 3.0 documentation of our HTTP REST API.

The openapi docs are correct by construction (compiled from the server code), and more or less complete.

Some endpoints are version-controlled. Show all supported versions. find out more.\n

Public (all available versions)

\nv0: \nswagger-ui; \nswagger.json\n
\nv1: \nswagger-ui; \nswagger.json\n
\nv2: \nswagger-ui; \nswagger.json\n
\nv3: \nswagger-ui; \nswagger.json\n
\nv4: \nswagger-ui; \nswagger.json\n
\nv5: \nswagger-ui; \nswagger.json\n
\nv6: \nswagger-ui; \nswagger.json\n
\nv7: \nswagger-ui; \nswagger.json\n
\n\n

Internal (not versioned)

\n

Openapi docs for internal endpoints are served per service. I.e. there's one for `brig`, one for `cannon`, etc.. This is because Openapi doesn't play well with multiple actions having the same combination of HTTP method and URL path.

\nbrig:
\nswagger-ui; \nswagger.json\n
\ngalley:
\nswagger-ui; \nswagger.json\n
\nspar:
\nswagger-ui; \nswagger.json\n
\ncargohold:
\nswagger-ui; \nswagger.json\n
\ngundeck:
\nswagger-ui; \nswagger.json\n
\ncannon:
\nswagger-ui; \nswagger.json\n
\nproxy:
\nswagger-ui; \nswagger.json\n
\n\n

Federated API (backend-to-backend)

\nbrig (v0):
swagger-ui; swagger.json
brig (v1):
swagger-ui; swagger.json
brig (v2):
swagger-ui; swagger.json

\ngalley (v0):
swagger-ui; swagger.json
galley (v1):
swagger-ui; swagger.json
galley (v2):
swagger-ui; swagger.json

\ncargohold (v0):
swagger-ui; swagger.json
cargohold (v1):
swagger-ui; swagger.json
cargohold (v2):
swagger-ui; swagger.json

\n\n\n" data Swagger = SwaggerPublic | SwaggerInternal Service diff --git a/libs/wire-api-federation/src/Wire/API/Federation/Version.hs b/libs/wire-api-federation/src/Wire/API/Federation/Version.hs index e3c76c36735..a7347fbeb5a 100644 --- a/libs/wire-api-federation/src/Wire/API/Federation/Version.hs +++ b/libs/wire-api-federation/src/Wire/API/Federation/Version.hs @@ -42,17 +42,23 @@ where import Control.Lens ((?~)) import Data.Aeson (FromJSON (..), ToJSON (..)) +import Data.ByteString.Char8 qualified as BS import Data.OpenApi qualified as S import Data.Schema import Data.Set qualified as Set import Data.Singletons.Base.TH import Data.Text qualified as Text import Imports +import Servant.API (ToHttpApiData (..)) data Version = V0 | V1 | V2 deriving stock (Eq, Ord, Bounded, Enum, Show, Generic) deriving (FromJSON, ToJSON) via (Schema Version) +instance ToHttpApiData Version where + toHeader = versionByteString + toUrlPiece = versionText + versionInt :: Version -> Int versionInt V0 = 0 versionInt V1 = 1 @@ -61,6 +67,9 @@ versionInt V2 = 2 versionText :: Version -> Text versionText = ("v" <>) . Text.pack . show . versionInt +versionByteString :: Version -> ByteString +versionByteString = ("v" <>) . BS.pack . show . versionInt + intToVersion :: Int -> Maybe Version intToVersion intV = find (\v -> versionInt v == intV) [minBound ..] diff --git a/services/brig/src/Brig/API/Public.hs b/services/brig/src/Brig/API/Public.hs index 42d0b5c2a7a..6580a413959 100644 --- a/services/brig/src/Brig/API/Public.hs +++ b/services/brig/src/Brig/API/Public.hs @@ -106,6 +106,7 @@ import Wire.API.Federation.API.Brig qualified as BrigFederationAPI import Wire.API.Federation.API.Cargohold qualified as CargoholdFederationAPI import Wire.API.Federation.API.Galley qualified as GalleyFederationAPI import Wire.API.Federation.Error +import Wire.API.Federation.Version qualified as Fed import Wire.API.Properties qualified as Public import Wire.API.Routes.API import Wire.API.Routes.Internal.Brig qualified as BrigInternalAPI @@ -244,17 +245,72 @@ versionedSwaggerDocsAPI Nothing = allroutes (throwError listAllVersionsResp) listAllVersionsHTML :: LByteString listAllVersionsHTML = - "

please pick an api version

" - <> mconcat - [ let url = "/" <> toQueryParam v <> "/api/swagger-ui/" - in " (fromStrict . Text.encodeUtf8 $ url) - <> "\">" - <> (fromStrict . Text.encodeUtf8 $ url) - <> "
" - | v <- [minBound :: Version ..] + LBS.unlines $ + [ "

OpenAPI 3.0 docs for all Wire APIs

", + intro, + LBS.unlines public, + LBS.unlines internal, + LBS.unlines federated, + "" + ] + where + intro = + "

This wire-server system provides OpenAPI 3.0 \ + \documentation of our HTTP REST API.

\ + \

The openapi docs are correct by construction (compiled from the server code), and more or less \ + \complete.

\ + \

Some endpoints are version-controlled. Show all supported versions. \ + \find out more." + + public :: [LByteString] + public = + ["

Public (all available versions)

"] + <> mconcat + [ [ v <> ": ", + renderLink "swagger-ui" ("/" <> v <> "/api/swagger-ui") <> "; ", + renderLink "swagger.json" ("/" <> v <> "/api/swagger.json"), + "
" + ] + | v <- versionToLByteString <$> [minBound :: Version ..] + ] + + internal :: [LByteString] + internal = + [ "

Internal (not versioned)

", + "

Openapi docs for internal endpoints are served per service. I.e. there's one for `brig`, one for `cannon`, \ + \etc.. This is because Openapi doesn't play well with multiple actions having the same combination of HTTP \ + \method and URL path.

" ] - <> "" + <> mconcat + [ [ s <> ":
", + renderLink "swagger-ui" ("/api-internal/swagger-ui/" <> s) <> "; ", + renderLink "swagger.json" ("/api-internal/swagger-ui/" <> s <> "-swagger.json"), + "
" + ] + | s <- ["brig", "galley", "spar", "cargohold", "gundeck", "cannon", "proxy"] + ] + + federated :: [LByteString] + federated = + ["

Federated API (backend-to-backend)

"] + <> [ mconcat + [ mconcat + [ s <> " (" <> v <> "):
", + renderLink "swagger-ui" ("/" <> v <> "/api-federation/swagger-ui/" <> s) <> "; ", + renderLink "swagger.json" ("/" <> v <> "/api-federation/swagger-ui/" <> s <> "-swagger.json"), + "
" + ] + | v <- versionToLByteString <$> [minBound :: Fed.Version ..] + ] + <> "
" + | s <- ["brig", "galley", "cargohold"] + ] + + versionToLByteString :: (ToHttpApiData v) => v -> LByteString + versionToLByteString = fromStrict . Text.encodeUtf8 . toQueryParam + + renderLink :: LByteString -> LByteString -> LByteString + renderLink caption url = " url <> "\">" <> caption <> "" -- | Serves Swagger docs for internal endpoints. internalEndpointsSwaggerDocsAPI :: From d3155976672edb2f554de8c24394b7a509a2a75d Mon Sep 17 00:00:00 2001 From: Akshay Mankar Date: Wed, 30 Oct 2024 16:02:34 +0100 Subject: [PATCH 2/3] Allow choosing between argon2id and scrypt as hashing algorithm (#4319) * Allow chosing between argon2id and scrypt as hashing algorithm The helm charts default to scrypt. * Update changelogs * Update docs, also add migration strategy to release notes * integration: Add test to make sure passwords keep working across hashing algorithm changes Co-authored-by: Matthias Fischmann --- .../0-release-notes/configurable-argon | 19 ++++++- .../2-features/add-config-for-pwd-hash | 2 +- changelog.d/5-internal/pwd | 1 - charts/brig/values.yaml | 9 ++-- charts/galley/values.yaml | 9 ++-- .../src/developer/reference/config-options.md | 46 ++++++++--------- hack/helm_vars/wire-server/values.yaml.gotmpl | 6 ++- integration/test/Test/User.hs | 51 +++++++++++++++++++ libs/types-common/src/Util/Options.hs | 20 +++++--- libs/wire-api/src/Wire/API/Password.hs | 4 +- libs/wire-subsystems/src/Wire/HashPassword.hs | 24 ++++----- services/brig/brig.integration.yaml | 1 + .../brig/src/Brig/CanonicalInterpreter.hs | 3 +- .../brig/test/integration/API/User/Auth.hs | 4 +- services/galley/galley.integration.yaml | 3 +- services/galley/src/Galley/App.hs | 3 +- 16 files changed, 138 insertions(+), 67 deletions(-) delete mode 100644 changelog.d/5-internal/pwd diff --git a/changelog.d/0-release-notes/configurable-argon b/changelog.d/0-release-notes/configurable-argon index b9e2a74cd8c..4a856472d06 100644 --- a/changelog.d/0-release-notes/configurable-argon +++ b/changelog.d/0-release-notes/configurable-argon @@ -1,18 +1,33 @@ -Password hashing is now done using argon2id instead of scrypt. The argon2id parameters can be configured using these options: +Password hashing can now be done using argon2id instead of scrypt. The argon2id parameters can be configured using these options: ```yaml brig: optSettings: setPasswordHashingOptions: + algorithm: argon2id iterations: ... memory: ... # memory needed in KiB parallelism: ... galley: settings: passwordHashingOptions: + algorithm: argon2id 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. +The default option is still to use scrypt as moving to argon2id might require +allocating more resources according to configured parameters. + +When configured to use argon2id, the DB will be migrated slowly over time as the +users enter their passwords (either to login or to do other operations which +require explicit password entry). This migration is **NOT** done in reverse, +i.e., if a deployment started with argon2id as the algorithm then chose to move +to scrypt, the passwords will not get rehashed automatically, instead the users +will have to reset their passwords if that is desired. + +**NOTE** It is highly recommended to move to argon2id as it will be made the + only available choice for the `algorithm` config option in future. + +(#4291, ##) \ No newline at end of file diff --git a/changelog.d/2-features/add-config-for-pwd-hash b/changelog.d/2-features/add-config-for-pwd-hash index 79ba9c55f09..3ef8e186268 100644 --- a/changelog.d/2-features/add-config-for-pwd-hash +++ b/changelog.d/2-features/add-config-for-pwd-hash @@ -1 +1 @@ -Allow configuring Argon2id parameters +Allow choosing hashing algorithm and configuring argon2id parameters (#4291, ##) diff --git a/changelog.d/5-internal/pwd b/changelog.d/5-internal/pwd deleted file mode 100644 index d0789bc9df4..00000000000 --- a/changelog.d/5-internal/pwd +++ /dev/null @@ -1 +0,0 @@ -Changed default password hashing from Scrypt to Argon2id. diff --git a/charts/brig/values.yaml b/charts/brig/values.yaml index bba7408c6a5..a7fd85eceb5 100644 --- a/charts/brig/values.yaml +++ b/charts/brig/values.yaml @@ -150,11 +150,12 @@ config: setDisabledAPIVersions: [ development ] setFederationStrategy: allowNone setFederationDomainConfigsUpdateFreq: 10 - # Options for Argon2id version 19 setPasswordHashingOptions: - iterations: 1 - parallelism: 32 - memory: 180224 # 176 MiB + algorithm: scrypt # or argon2id + # When algorithm is argon2id, these can be configured: + # iterations: + # parallelism: + # memory: smtp: passwordFile: /etc/wire/brig/secrets/smtp-password.txt proxy: {} diff --git a/charts/galley/values.yaml b/charts/galley/values.yaml index 71045b680b9..60122db2c23 100644 --- a/charts/galley/values.yaml +++ b/charts/galley/values.yaml @@ -70,11 +70,12 @@ 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 + algorithm: scrypt # or argon2id + # When algorithm is argon2id, these can be configured: + # iterations: + # parallelism: + # memory: # To disable proteus for new federated conversations: # federationProtocols: ["mls"] diff --git a/docs/src/developer/reference/config-options.md b/docs/src/developer/reference/config-options.md index a22b947c04b..74565d0dd21 100644 --- a/docs/src/developer/reference/config-options.md +++ b/docs/src/developer/reference/config-options.md @@ -719,22 +719,11 @@ optSettings: setOAuthMaxActiveRefreshTokens: 10 ``` -#### Argon2id password hashing parameters +#### Password hashing options -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. +Since release 5.6.0, wire-server can hash passwords with +[argon2id](https://datatracker.ietf.org/doc/html/rfc9106) to be stored at rest. +If you do not do anything, the deployment will still use scrypt. The password hashing options are set for brig and galley: @@ -742,29 +731,34 @@ The password hashing options are set for brig and galley: brig: optSettings: setPasswordHashingOptions: + algorithm: # argon2id or scrypt + # These options only apply to argon2id iterations: ... memory: ... # memory needed in KiB parallelism: ... galley: settings: passwordHashingOptions: + algorithm: # argon2id or scrypt + # These options only apply to argon2id 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.) +**Performance implications:** argon2id typically takes longer and uses more +memory than scrypt. So when migrating to it brig and galley pods must be +allocated more resouces according to the chosen paramters. + +When configured to use argon2id, the DB will be migrated slowly over time as the +users enter their passwords (either to login or to do other operations which +require explicit password entry). This migration is **NOT** done in reverse, +i.e., if a deployment started with argon2id as the algorithm then chose to move +to scrypt, the passwords already stored will not get rehashed automatically, +however the users will still be able to use them to login. -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. +**NOTE** It is highly recommended to move to argon2id as it will be made the + only available choice for the `algorithm` config option in future. #### Disabling API versions diff --git a/hack/helm_vars/wire-server/values.yaml.gotmpl b/hack/helm_vars/wire-server/values.yaml.gotmpl index d6db927d92d..e7f72583f39 100644 --- a/hack/helm_vars/wire-server/values.yaml.gotmpl +++ b/hack/helm_vars/wire-server/values.yaml.gotmpl @@ -136,7 +136,8 @@ brig: setOAuthMaxActiveRefreshTokens: 10 # These values are insecure, against anyone getting hold of the hash, # but its not a concern for the integration tests. - setPasswordHashingOptions: + setPasswordHashingOptions: + algorithm: argon2id iterations: 1 parallelism: 4 memory: 32 # This needs to be at least 8 * parallelism. @@ -266,7 +267,8 @@ galley: # These values are insecure, against anyone getting hold of the hash, # but its not a concern for the integration tests. - passwordHashingOptions: + passwordHashingOptions: + algorithm: argon2id iterations: 1 parallelism: 4 memory: 32 # This needs to be at least 8 * parallelism. diff --git a/integration/test/Test/User.hs b/integration/test/Test/User.hs index e0429253875..548c34b1daf 100644 --- a/integration/test/Test/User.hs +++ b/integration/test/Test/User.hs @@ -4,13 +4,17 @@ module Test.User where import API.Brig import API.BrigInternal +import API.Common import API.GalleyInternal import qualified API.Spar as Spar +import Control.Monad.Codensity +import Control.Monad.Reader import qualified Data.Aeson as Aeson import qualified Data.UUID as UUID import qualified Data.UUID.V4 as UUID import SetupHelpers import Testlib.Prelude +import Testlib.ResourcePool import Testlib.VersionedFed testSupportedProtocols :: (HasCallStack) => OneOf Domain (FedDomain 1) -> App () @@ -174,3 +178,50 @@ testActivateAccountWithPhoneV5 = do activateUserV5 dom reqBody `bindResponse` \resp -> do resp.status `shouldMatchInt` 400 resp.json %. "label" `shouldMatch` "bad-request" + +testMigratingPasswordHashingAlgorithm :: (HasCallStack) => App () +testMigratingPasswordHashingAlgorithm = do + let argon2idOpts = + object + [ "algorithm" .= "argon2id", + "iterations" .= (1 :: Int), + "memory" .= (128 :: Int), + "parallelism" .= (1 :: Int) + ] + cfgArgon2id = + def + { brigCfg = setField "settings.setPasswordHashingOptions" argon2idOpts, + galleyCfg = setField "settings.passwordHashingOptions" argon2idOpts + } + cfgScrypt = + def + { brigCfg = setField "settings.setPasswordHashingOptions.algorithm" "scrypt", + galleyCfg = setField "settings.passwordHashingOptions.algorithm" "scrypt" + } + resourcePool <- asks (.resourcePool) + runCodensity (acquireResources 1 resourcePool) $ \[testBackend] -> do + let domain = testBackend.berDomain + email1 <- randomEmail + password1 <- randomString 20 + + email2 <- randomEmail + password2 <- randomString 20 + + runCodensity (startDynamicBackend testBackend cfgScrypt) $ \_ -> do + void $ randomUser domain (def {email = Just email1, password = Just password1}) + login domain email1 password1 >>= assertSuccess + + runCodensity (startDynamicBackend testBackend cfgArgon2id) $ \_ -> do + login domain email1 password1 >>= assertSuccess + + -- Create second user to ensure that we're testing migrating back. This is + -- not really needed because the login above rehashes the password, but it + -- makes the test clearer. + void $ randomUser domain (def {email = Just email2, password = Just password2}) + login domain email2 password2 >>= assertSuccess + + -- Check that both users can still login with Scrypt in case the operator + -- wants to rollback the config. + runCodensity (startDynamicBackend testBackend cfgScrypt) $ \_ -> do + login domain email1 password1 >>= assertSuccess + login domain email2 password2 >>= assertSuccess diff --git a/libs/types-common/src/Util/Options.hs b/libs/types-common/src/Util/Options.hs index 7b6cd88b08d..9fa6117aede 100644 --- a/libs/types-common/src/Util/Options.hs +++ b/libs/types-common/src/Util/Options.hs @@ -148,7 +148,12 @@ getOptions desc mp defaultPath = do parseAWSEndpoint :: ReadM AWSEndpoint parseAWSEndpoint = readerAsk >>= maybe (error "Could not parse AWS endpoint") pure . fromByteString . fromString -data PasswordHashingOptions = PasswordHashingOptions +data PasswordHashingOptions + = PasswordHashingArgon2id Argon2idOptions + | PasswordHashingScrypt + deriving (Show, Generic) + +data Argon2idOptions = Argon2idOptions { iterations :: !Word32, memory :: !Word32, parallelism :: !Word32 @@ -157,11 +162,14 @@ data PasswordHashingOptions = PasswordHashingOptions instance FromJSON PasswordHashingOptions where parseJSON = - withObject - "PasswordHashingOptions" - ( \obj -> do + withObject "PasswordHashingOptions" $ \obj -> do + algo :: String <- obj .: "algorithm" + case algo of + "argon2id" -> do iterations <- obj .: "iterations" memory <- obj .: "memory" parallelism <- obj .: "parallelism" - pure (PasswordHashingOptions {..}) - ) + pure . PasswordHashingArgon2id $ Argon2idOptions {..} + "scrypt" -> + pure PasswordHashingScrypt + x -> fail $ "Unknown password hashing algorithm: " <> x diff --git a/libs/wire-api/src/Wire/API/Password.hs b/libs/wire-api/src/Wire/API/Password.hs index 78f2ea0697f..172c987b4d1 100644 --- a/libs/wire-api/src/Wire/API/Password.hs +++ b/libs/wire-api/src/Wire/API/Password.hs @@ -130,8 +130,8 @@ fromScrypt scryptParams = outputLength = 64 } -argon2OptsFromHashingOpts :: PasswordHashingOptions -> Argon2.Options -argon2OptsFromHashingOpts PasswordHashingOptions {..} = +argon2OptsFromHashingOpts :: Argon2idOptions -> Argon2.Options +argon2OptsFromHashingOpts Argon2idOptions {..} = Argon2.Options { variant = Argon2.Argon2id, version = Argon2.Version13, diff --git a/libs/wire-subsystems/src/Wire/HashPassword.hs b/libs/wire-subsystems/src/Wire/HashPassword.hs index c91854f4316..1f58daf794d 100644 --- a/libs/wire-subsystems/src/Wire/HashPassword.hs +++ b/libs/wire-subsystems/src/Wire/HashPassword.hs @@ -3,11 +3,11 @@ module Wire.HashPassword where -import Crypto.KDF.Argon2 qualified as Argon2 import Data.Misc import Imports import Polysemy -import Wire.API.Password (Password) +import Util.Options +import Wire.API.Password import Wire.API.Password qualified as Password data HashPassword m a where @@ -17,20 +17,18 @@ data HashPassword m a where makeSem ''HashPassword runHashPassword :: + forall r. ( Member (Embed IO) r ) => - Argon2.Options -> + PasswordHashingOptions -> InterpreterFor HashPassword r runHashPassword opts = interpret $ \case - HashPassword6 pw6 -> hashPasswordImpl opts pw6 - HashPassword8 pw8 -> hashPasswordImpl opts pw8 - -hashPasswordImpl :: - (Member (Embed IO) r) => - Argon2.Options -> - PlainTextPassword' t -> - Sem r Password -hashPasswordImpl opts pwd = do - liftIO $ Password.mkSafePassword opts pwd + HashPassword6 pw6 -> hashFunction pw6 + HashPassword8 pw8 -> hashFunction pw8 + where + hashFunction :: PlainTextPassword' t -> Sem r Password + hashFunction = case opts of + PasswordHashingArgon2id o -> Password.mkSafePassword (argon2OptsFromHashingOpts o) + PasswordHashingScrypt -> Password.mkSafePasswordScrypt diff --git a/services/brig/brig.integration.yaml b/services/brig/brig.integration.yaml index f814af8c799..ad2c8da9560 100644 --- a/services/brig/brig.integration.yaml +++ b/services/brig/brig.integration.yaml @@ -229,6 +229,7 @@ optSettings: setOAuthRefreshTokenExpirationTimeSecs: 14515200 # 24 weeks setOAuthMaxActiveRefreshTokens: 10 setPasswordHashingOptions: # in testing, we want these settings to be faster, not secure against attacks. + algorithm: argon2id iterations: 1 memory: 128 parallelism: 1 diff --git a/services/brig/src/Brig/CanonicalInterpreter.hs b/services/brig/src/Brig/CanonicalInterpreter.hs index 0ceacfba5ee..5248effd92b 100644 --- a/services/brig/src/Brig/CanonicalInterpreter.hs +++ b/services/brig/src/Brig/CanonicalInterpreter.hs @@ -33,7 +33,6 @@ import Polysemy.TinyLog (TinyLog) import Wire.API.Allowlists (AllowlistEmailDomains) import Wire.API.Federation.Client qualified import Wire.API.Federation.Error -import Wire.API.Password import Wire.ActivationCodeStore (ActivationCodeStore) import Wire.ActivationCodeStore.Cassandra (interpretActivationCodeStoreToCassandra) import Wire.AuthenticationSubsystem @@ -265,7 +264,7 @@ runBrigToIO e (AppT ma) = do . interpretIndexedUserStoreES indexedUserStoreConfig . interpretUserStoreCassandra e.casClient . interpretUserKeyStoreCassandra e.casClient - . runHashPassword (argon2OptsFromHashingOpts e.settings.passwordHashingOptions) + . runHashPassword e.settings.passwordHashingOptions . interpretFederationAPIAccess federationApiAccessConfig . rethrowHttpErrorIO . mapError propertySubsystemErrorToHttpError diff --git a/services/brig/test/integration/API/User/Auth.hs b/services/brig/test/integration/API/User/Auth.hs index f3b0ffeb4eb..a93fa99317b 100644 --- a/services/brig/test/integration/API/User/Auth.hs +++ b/services/brig/test/integration/API/User/Auth.hs @@ -55,6 +55,7 @@ import Data.ZAuth.Token qualified as ZAuth import Imports import Network.HTTP.Client (equivCookie) import Network.Wai.Utilities.Error qualified as Error +import Polysemy import Test.Tasty hiding (Timeout) import Test.Tasty.HUnit import Test.Tasty.HUnit qualified as HUnit @@ -69,6 +70,7 @@ import Wire.API.User.Auth.LegalHold import Wire.API.User.Auth.ReAuth import Wire.API.User.Auth.Sso import Wire.API.User.Client +import Wire.HashPassword -- | FUTUREWORK: Implement this function. This wrapper should make sure that -- wrapped tests run only when the feature flag 'legalhold' is set to @@ -194,7 +196,7 @@ testLoginWith6CharPassword opts brig db = do updatePassword :: (MonadClient m) => UserId -> PlainTextPassword6 -> m () updatePassword u t = do - p <- mkSafePassword (argon2OptsFromHashingOpts opts.settings.passwordHashingOptions) t + p <- liftIO $ runM . runHashPassword opts.settings.passwordHashingOptions $ hashPassword6 t retry x5 $ write userPasswordUpdate (params LocalQuorum (p, u)) userPasswordUpdate :: PrepQuery W (Password, UserId) () diff --git a/services/galley/galley.integration.yaml b/services/galley/galley.integration.yaml index c76165b1bd0..3f1e069313f 100644 --- a/services/galley/galley.integration.yaml +++ b/services/galley/galley.integration.yaml @@ -59,10 +59,11 @@ settings: ecdsa_secp521r1_sha512: test/resources/backendA/ecdsa_secp521r1_sha512.pem guestLinkTTLSeconds: 604800 passwordHashingOptions: # in testing, we want these settings to be faster, not secure against attacks. + algorithm: argon2id iterations: 1 memory: 128 parallelism: 1 - + # We explicitly do not disable any API version. Please make sure the configuration value is the same in all these configs: # brig, cannon, cargohold, galley, gundeck, proxy, spar. disabledAPIVersions: [] diff --git a/services/galley/src/Galley/App.hs b/services/galley/src/Galley/App.hs index 4bfccae43cf..6d29d6081ca 100644 --- a/services/galley/src/Galley/App.hs +++ b/services/galley/src/Galley/App.hs @@ -106,7 +106,6 @@ import UnliftIO.Exception qualified as UnliftIO import Wire.API.Conversation.Protocol import Wire.API.Error import Wire.API.Federation.Error -import Wire.API.Password import Wire.API.Team.Feature import Wire.GundeckAPIAccess (runGundeckAPIAccess) import Wire.HashPassword @@ -254,7 +253,7 @@ evalGalley e = . mapError toResponse . mapError toResponse . mapError toResponse - . runHashPassword (argon2OptsFromHashingOpts e._options._settings._passwordHashingOptions) + . runHashPassword e._options._settings._passwordHashingOptions . runInputConst e . runInputConst (e ^. cstate) . mapError toResponse -- DynError From 9445e43e1b944d7342f5ad27f877e5f1b0537994 Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Wed, 30 Oct 2024 16:19:09 +0100 Subject: [PATCH 3/3] [fix] Local federation v1 tests fixed (#4320) --- changelog.d/5-internal/fix-local-fed-v1 | 1 + .../federation-v0/nginz/conf/integration.conf | 6 ++---- .../federation-v1/nginz/conf/integration.conf | 6 ++---- deploy/dockerephemeral/run.sh | 11 +++++++---- integration/test/Test/MLS/One2One.hs | 4 ++++ 5 files changed, 16 insertions(+), 12 deletions(-) create mode 100644 changelog.d/5-internal/fix-local-fed-v1 diff --git a/changelog.d/5-internal/fix-local-fed-v1 b/changelog.d/5-internal/fix-local-fed-v1 new file mode 100644 index 00000000000..2bff4d110c1 --- /dev/null +++ b/changelog.d/5-internal/fix-local-fed-v1 @@ -0,0 +1 @@ +Local integration tests of federation version V1 fixed diff --git a/deploy/dockerephemeral/federation-v0/nginz/conf/integration.conf b/deploy/dockerephemeral/federation-v0/nginz/conf/integration.conf index fa168d16f4d..74dd1a09113 100644 --- a/deploy/dockerephemeral/federation-v0/nginz/conf/integration.conf +++ b/deploy/dockerephemeral/federation-v0/nginz/conf/integration.conf @@ -15,7 +15,5 @@ listen 8090; # But to also test tls forwarding, this port can be used. # This applies only locally, as for kubernetes (helm chart) based deployments, # TLS is terminated at the ingress level, not at nginz level -listen 8443 ssl; -listen [::]:8443 ssl; - -http2 on; +listen 8443 ssl http2; +listen [::]:8443 ssl http2; diff --git a/deploy/dockerephemeral/federation-v1/nginz/conf/integration.conf b/deploy/dockerephemeral/federation-v1/nginz/conf/integration.conf index fa168d16f4d..74dd1a09113 100644 --- a/deploy/dockerephemeral/federation-v1/nginz/conf/integration.conf +++ b/deploy/dockerephemeral/federation-v1/nginz/conf/integration.conf @@ -15,7 +15,5 @@ listen 8090; # But to also test tls forwarding, this port can be used. # This applies only locally, as for kubernetes (helm chart) based deployments, # TLS is terminated at the ingress level, not at nginz level -listen 8443 ssl; -listen [::]:8443 ssl; - -http2 on; +listen 8443 ssl http2; +listen [::]:8443 ssl http2; diff --git a/deploy/dockerephemeral/run.sh b/deploy/dockerephemeral/run.sh index f6455dacf5c..bbb6f012079 100755 --- a/deploy/dockerephemeral/run.sh +++ b/deploy/dockerephemeral/run.sh @@ -1,17 +1,20 @@ #!/usr/bin/env bash +# To start the federation v0, v1 backends, set ENABLE_FEDERATION_V0=1, ENABLE_FEDERATION_V1=1 +# in the env where this script is run + set -e # run.sh should work no matter what is the current directory -SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" DOCKER_FILE="$SCRIPT_DIR/docker-compose.yaml" FED_VERSIONS=(0 1) -opts=( "--file" "$DOCKER_FILE" ) +opts=("--file" "$DOCKER_FILE") for v in "${FED_VERSIONS[@]}"; do var="ENABLE_FEDERATION_V$v" if [[ "${!var}" == 1 ]]; then - opts+=( "--file" "$SCRIPT_DIR/federation-v$v.yaml" ) + opts+=("--file" "$SCRIPT_DIR/federation-v$v.yaml") fi done @@ -19,7 +22,7 @@ dc() { docker-compose "${opts[@]}" "$@" } -cleanup () { +cleanup() { dc down } diff --git a/integration/test/Test/MLS/One2One.hs b/integration/test/Test/MLS/One2One.hs index cbc4c1339e2..d93e5f582c2 100644 --- a/integration/test/Test/MLS/One2One.hs +++ b/integration/test/Test/MLS/One2One.hs @@ -395,6 +395,8 @@ testMLSGhostOne2OneConv = do -- still be created but only by the user whose backend hosts this conversation. -- | See Note: [Federated 1:1 MLS Conversations] +-- To run locally this test requires federation-v1 docker containers to be up and running. +-- See `deploy/dockerephemeral/run.sh` and comment on `StaticFedDomain` in `Testlib/VersionedFed.hs` for more details. testMLSFederationV1ConvOnOldBackend :: App () testMLSFederationV1ConvOnOldBackend = do alice <- randomUser OwnDomain def @@ -447,6 +449,8 @@ testMLSFederationV1ConvOnOldBackend = do parsedMsg %. "message.content.sender.External" `shouldMatchInt` 0 -- | See Note: Federated 1:1 MLS Conversations +-- To run locally this test requires federation-v1 docker containers to be up and running. +-- See `deploy/dockerephemeral/run.sh` and comment on `StaticFedDomain` in `Testlib/VersionedFed.hs` for more details. testMLSFederationV1ConvOnNewBackend :: App () testMLSFederationV1ConvOnNewBackend = do alice <- randomUser OwnDomain def