WPB-6190 Backend should validate display name during DPoP challenge (#…

…3890)

changelog.d/2-features/WPB-6190

@@ -0,0 +1 @@
+Backend validates display name during DPoP challenge

libs/jwt-tools/src/Data/Jwt/Tools.hs

@@ -34,6 +34,7 @@ module Data.Jwt.Tools
     NowEpoch (..),
     PemBundle (..),
     Handle (..),
+    DisplayName (..),
     TeamId (..),
   )
 where
@@ -74,12 +75,15 @@ type EpochWord64 = Word64
 
 type BackendBundleCStr = CString
 
+type DisplayNameCStr = CString
+
 foreign import ccall unsafe "generate_dpop_access_token"
   generate_dpop_access_token ::
     ProofCStr ->
     UserIdCStr ->
     ClientIdWord64 ->
     HandleCStr ->
+    DisplayNameCStr ->
     TeamIdCStr ->
     DomainCStr ->
     NonceCStr ->
@@ -102,6 +106,7 @@ generateDpopAccessTokenFfi ::
   UserIdCStr ->
   ClientIdWord64 ->
   HandleCStr ->
+  DisplayNameCStr ->
   TeamIdCStr ->
   DomainCStr ->
   NonceCStr ->
@@ -112,8 +117,8 @@ generateDpopAccessTokenFfi ::
   EpochWord64 ->
   BackendBundleCStr ->
   IO (Maybe (Ptr HsResult))
-generateDpopAccessTokenFfi dpopProof user client handle tid domain nonce uri method maxSkewSecs expiration now backendKeys = do
-  ptr <- generate_dpop_access_token dpopProof user client handle tid domain nonce uri method maxSkewSecs expiration now backendKeys
+generateDpopAccessTokenFfi dpopProof user client handle displayName tid domain nonce uri method maxSkewSecs expiration now backendKeys = do
+  ptr <- generate_dpop_access_token dpopProof user client handle displayName tid domain nonce uri method maxSkewSecs expiration now backendKeys
   if ptr /= nullPtr
     then pure $ Just ptr
     else pure Nothing
@@ -138,6 +143,7 @@ generateDpopToken ::
   UserId ->
   ClientId ->
   Handle ->
+  DisplayName ->
   TeamId ->
   Domain ->
   Nonce ->
@@ -148,10 +154,11 @@ generateDpopToken ::
   NowEpoch ->
   PemBundle ->
   ExceptT DPoPTokenGenerationError m ByteString
-generateDpopToken dpopProof uid cid handle tid domain nonce uri method maxSkewSecs maxExpiration now backendPubkeyBundle = do
+generateDpopToken dpopProof uid cid handle displayName tid domain nonce uri method maxSkewSecs maxExpiration now backendPubkeyBundle = do
   dpopProofCStr <- toCStr dpopProof
   uidCStr <- toCStr uid
   handleCStr <- toCStr handle
+  displayNameCStr <- toCStr displayName
   tidCStr <- toCStr tid
   domainCStr <- toCStr domain
   nonceCStr <- toCStr nonce
@@ -165,6 +172,7 @@ generateDpopToken dpopProof uid cid handle tid domain nonce uri method maxSkewSe
   -- traceM $ "nonce = Nonce " <> show (_unNonce nonce)
   -- traceM $ "expires = ExpiryEpoch " <> show (_unExpiryEpoch maxExpiration)
   -- traceM $ "handle = Handle " <> show (_unHandle handle)
+  -- traceM $ "displayName = DisplayName " <> show (_unDisplayName displayName)
   -- traceM $ "tid = TeamId " <> show (_unTeamId tid)
 
   let before =
@@ -173,6 +181,7 @@ generateDpopToken dpopProof uid cid handle tid domain nonce uri method maxSkewSe
           uidCStr
           (_unClientId cid)
           handleCStr
+          displayNameCStr
           tidCStr
           domainCStr
           nonceCStr
@@ -273,6 +282,10 @@ newtype PemBundle = PemBundle {_unPemBundle :: ByteString}
   deriving (Eq, Show)
   deriving newtype (ToByteString)
 
+newtype DisplayName = DisplayName {_unDisplayName :: ByteString}
+  deriving (Eq, Show)
+  deriving newtype (ToByteString)
+
 data DPoPTokenGenerationError
   = NoError
   | -- | Unmapped error
@@ -359,4 +372,6 @@ data DPoPTokenGenerationError
     DpopHandleMismatch
   | -- Client team does not match the supplied team
     DpopTeamMismatch
+  | --  Client display name does not match the supplied display name
+    DpopDisplayNameMismatch
   deriving (Eq, Show, Generic, Bounded, Enum)

libs/jwt-tools/test/Spec.hs

@@ -25,15 +25,15 @@ main :: IO ()
 main = hspec $ do
   describe "generateDpopToken FFI when passing valid inputs" $ do
     it "should return an access token with the correct header" $ do
-      actual <- runExceptT $ generateDpopToken proof uid cid handle tid domain nonce uri method maxSkewSecs expires now pem
+      actual <- runExceptT $ generateDpopToken proof uid cid handle displayName tid domain nonce uri method maxSkewSecs expires now pem
       -- The actual payload of the DPoP token is not deterministic as it depends on the current time.
       -- We therefore only check the header, because if the header is correct, it means the token creation was successful.s
       let expectedHeader = "eyJhbGciOiJFZERTQSIsInR5cCI6ImF0K2p3dCIsImp3ayI6eyJrdHkiOiJPS1AiLCJjcnYiOiJFZDI1NTE5IiwieCI6ImRZSTM4VWR4a3NDMEs0UXg2RTlKSzlZZkdtLWVoblkxOG9LbUhMMllzWmsifX0"
       let actualHeader = either (const "") (head . split '.') actual
       actualHeader `shouldBe` expectedHeader
   describe "generateDpopToken FFI when passing a wrong nonce value" $ do
     it "should return BackendNonceMismatchError" $ do
-      actual <- runExceptT $ generateDpopToken proof uid cid handle tid domain (Nonce "foobar") uri method maxSkewSecs expires now pem
+      actual <- runExceptT $ generateDpopToken proof uid cid handle displayName tid domain (Nonce "foobar") uri method maxSkewSecs expires now pem
       actual `shouldBe` Left BackendNonceMismatchError
   describe "toResult" $ do
     it "should convert to correct error" $ do
@@ -74,15 +74,41 @@ main = hspec $ do
       toResult (Just 17) (Just token) `shouldBe` Left ExpMismatchError
       toResult (Just 18) Nothing `shouldBe` Left Expired
       toResult (Just 18) (Just token) `shouldBe` Left Expired
+      toResult (Just 19) (Just token) `shouldBe` Left InvalidUserId
+      toResult (Just 20) (Just token) `shouldBe` Left NotYetValid
+      toResult (Just 21) (Just token) `shouldBe` Left JwtSimpleError
+      toResult (Just 22) (Just token) `shouldBe` Left RandError
+      toResult (Just 23) (Just token) `shouldBe` Left Sec1Error
+      toResult (Just 24) (Just token) `shouldBe` Left UrlParseError
+      toResult (Just 25) (Just token) `shouldBe` Left UuidError
+      toResult (Just 26) (Just token) `shouldBe` Left Utf8Error
+      toResult (Just 27) (Just token) `shouldBe` Left Base64DecodeError
+      toResult (Just 28) (Just token) `shouldBe` Left JsonError
+      toResult (Just 29) (Just token) `shouldBe` Left InvalidJsonPath
+      toResult (Just 30) (Just token) `shouldBe` Left JsonPathError
+      toResult (Just 31) (Just token) `shouldBe` Left InvalidJwkThumbprint
+      toResult (Just 32) (Just token) `shouldBe` Left MissingDpopHeader
+      toResult (Just 33) (Just token) `shouldBe` Left MissingIssuer
+      toResult (Just 34) (Just token) `shouldBe` Left DpopChallengeMismatch
+      toResult (Just 35) (Just token) `shouldBe` Left DpopHtuMismatch
+      toResult (Just 36) (Just token) `shouldBe` Left DpopHtmMismatch
+      toResult (Just 37) (Just token) `shouldBe` Left InvalidBackendKeys
+      toResult (Just 38) (Just token) `shouldBe` Left InvalidClientId
+      toResult (Just 39) (Just token) `shouldBe` Left UnsupportedApiVersion
+      toResult (Just 40) (Just token) `shouldBe` Left UnsupportedScope
+      toResult (Just 41) (Just token) `shouldBe` Left DpopHandleMismatch
+      toResult (Just 42) (Just token) `shouldBe` Left DpopTeamMismatch
+      toResult (Just 43) (Just token) `shouldBe` Left DpopDisplayNameMismatch
       toResult Nothing Nothing `shouldBe` Left UnknownError
   where
     token = ""
-    proof = Proof "eyJhbGciOiJFZERTQSIsImp3ayI6eyJjcnYiOiJFZDI1NTE5Iiwia3R5IjoiT0tQIiwieCI6Im5MSkdOLU9hNkpzcTNLY2xaZ2dMbDdVdkFWZG1CMFE2QzNONUJDZ3BoSHcifSwidHlwIjoiZHBvcCtqd3QifQ.eyJhdWQiOiJodHRwczovL3dpcmUuY29tL2FjbWUvY2hhbGxlbmdlL2FiY2QiLCJjaGFsIjoid2EyVnJrQ3RXMXNhdUoyRDN1S1k4cmM3eTRrbDR1c0giLCJleHAiOjE4MzE3MzcyNzEsImhhbmRsZSI6IndpcmVhcHA6Ly8lNDB2bHVwZHlwbml4dm1vdnZzeW1ndHdAZXhhbXBsZS5jb20iLCJodG0iOiJQT1NUIiwiaHR1IjoiaHR0cHM6Ly9leGFtcGxlLmNvbS9jbGllbnRzL2NjNmU2NDBlMjk2ZThiYmEvYWNjZXNzLXRva2VuIiwiaWF0IjoxNzA1NTkzMjcxLCJqdGkiOiI2ZmM1OWU3Zi1iNjY2LTRmZmMtYjczOC00ZjQ3NjBjODg0Y2EiLCJuYmYiOjE3MDU1OTMyNzEsIm5vbmNlIjoibVJDdjNKQS1TNDI0dUJyLVk2QzFndyIsInN1YiI6IndpcmVhcHA6Ly9WNVc3ZnRNeVRJNlBNYlE0Y3ZkazRnIWNjNmU2NDBlMjk2ZThiYmFAZXhhbXBsZS5jb20iLCJ0ZWFtIjoiZmZhODY1ZmEtYjI0YS00Njk3LWFhMDUtMWZjM2YzNjU0ZGI5In0.BVdawX_84Mpmvzbs3v52t3GtCgSKzxgnFDkwf4QK6AusoyfsjhK6grs9GLEe2Lfb1eDrBUJgo-nobeIWmRumBQ"
-    uid = UserId "5795bb7e-d332-4c8e-8f31-b43872f764e2"
-    nonce = Nonce "mRCv3JA-S424uBr-Y6C1gw"
-    expires = ExpiryEpoch 1831823671
-    handle = Handle "vlupdypnixvmovvsymgtw"
-    tid = TeamId "ffa865fa-b24a-4697-aa05-1fc3f3654db9"
+    proof = Proof "eyJhbGciOiJFZERTQSIsImp3ayI6eyJjcnYiOiJFZDI1NTE5Iiwia3R5IjoiT0tQIiwieCI6Im5MSkdOLU9hNkpzcTNLY2xaZ2dMbDdVdkFWZG1CMFE2QzNONUJDZ3BoSHcifSwidHlwIjoiZHBvcCtqd3QifQ.eyJhdWQiOiJodHRwczovL3dpcmUuY29tL2FjbWUvY2hhbGxlbmdlL2FiY2QiLCJjaGFsIjoid2EyVnJrQ3RXMXNhdUoyRDN1S1k4cmM3eTRrbDR1c0giLCJleHAiOjE3Mzk4ODA2NzQsImhhbmRsZSI6IndpcmVhcHA6Ly8lNDB5d2Z5ZG5pZ2Jud2h1b3pldGphZ3FAZXhhbXBsZS5jb20iLCJodG0iOiJQT1NUIiwiaHR1IjoiaHR0cHM6Ly9leGFtcGxlLmNvbS9jbGllbnRzL2NjNmU2NDBlMjk2ZThiYmEvYWNjZXNzLXRva2VuIiwiaWF0IjoxNzA4MzQ0Njc0LCJqdGkiOiI2ZmM1OWU3Zi1iNjY2LTRmZmMtYjczOC00ZjQ3NjBjODg0Y2EiLCJuYW1lIjoi5reB4qqu5KSq5rK255Kh4bKV6re14Y2q6omE6Jy16Iu17ICV54Kb66-v56qp5KqW766M6bGw6oOy6b6m57m15pWJ4LqH54et6rOj54KHIiwibmJmIjoxNzA4MzQ0Njc0LCJub25jZSI6IllWZ2dHdWlTUTZlamhQNTNFX0tPS3ciLCJzdWIiOiJ3aXJlYXBwOi8vSWZ0VzBLeFVSb2F1QWVockRremJiQSFjYzZlNjQwZTI5NmU4YmJhQGV4YW1wbGUuY29tIiwidGVhbSI6ImMxNTE5NzVlLWIxOTMtNDAwOS1hM2QyLTc0N2M5NjFmMjMzMyJ9.SHxpMzOe2yC3y6DP7lEH0l7_eOKrUZZI0OjgtnCKjO4OBD0XqKOi0y_z07-7FWc-KtThlsaZatnBNTB67GhQBw"
+    uid = UserId "21fb56d0-ac54-4686-ae01-e86b0e4cdb6c"
+    nonce = Nonce "YVggGuiSQ6ejhP53E_KOKw"
+    expires = ExpiryEpoch 1739967074
+    handle = Handle "ywfydnigbnwhuozetjagq"
+    displayName = DisplayName "\230\183\129\226\170\174\228\164\170\230\178\182\231\146\161\225\178\149\234\183\181\225\141\170\234\137\132\232\156\181\232\139\181\236\128\149\231\130\155\235\175\175\231\170\169\228\170\150\239\174\140\233\177\176\234\131\178\233\190\166\231\185\181\230\149\137\224\186\135\231\135\173\234\179\163\231\130\135"
+    tid = TeamId "c151975e-b193-4009-a3d2-747c961f2333"
 
     now = NowEpoch 1704982162
     cid = ClientId 14730821443162901434

nix/pkgs/rusty_jwt_tools_ffi/default.nix

@@ -10,12 +10,12 @@
 # Cargo.lock file in its root (not at the ffi/ subpath).
 
 let
-  version = "0.8.5";
+  version = "0.9.0";
   src = fetchFromGitHub {
     owner = "wireapp";
     repo = "rusty-jwt-tools";
-    rev = "99acb427b2169d726f356d30dec55eae83dda6b6";
-    sha256 = "sha256-x1W79spOZeFHabRbhMksz6gLtRIpl2E7WCiXuzIMoFM=";
+    rev = "60424bf7031e2fa535aac658d0b5643624d19537";
+    sha256 = "sha256-kdubK9FruZT8pbIwCHyAkxYj9yVM0q7ivNhNUNtNQCY=";
   };
   cargoLockFile = builtins.toFile "cargo.lock" (builtins.readFile "${src}/Cargo.lock");
 
@@ -29,9 +29,8 @@ rustPlatform.buildRustPackage {
     outputHashes = {
       # if any of these need updating, replace / create new key with
       # lib.fakeSha256, rebuild, and replace with actual hash.
-      "certval-0.1.4" = "sha256-mUg3Kx1I/r9zBoB7tDaZsykFkE+tsN+Rem6DjUOZbuU=";
+      "certval-0.1.4" = "sha256-gzkRC7/u/rARGPy3d37eBrAVml4XSDb6bRPpsESmttY=";
       "jwt-simple-0.12.1" = "sha256-5PAOwulL8j6f4Ycoa5Q+1dqEA24uN8rJt+i2RebL6eo=";
-      "x509-ocsp-0.2.1" = "sha256-o+r9h0CcexWqJIIoZdOgSd7hWIb91BheW6UZI98RpLA=";
     };
   };
 

nix/sources.json

@@ -24,15 +24,15 @@
         "url_template": "https://github.com/<owner>/<repo>/archive/<rev>.tar.gz"
     },
     "nixpkgs-cargo": {
-        "branch": "nixpkgs-unstable",
+        "branch": "master",
         "description": "Nix Packages collection",
         "homepage": "https://github.com/NixOS/nixpkgs",
         "owner": "NixOS",
         "repo": "nixpkgs",
-        "rev": "01441e14af5e29c9d27ace398e6dd0b293e25a54",
-        "sha256": "0yvkamjbk3aj4lvhm6vdgdk4b2j0xdv3gx9n4p7wfky52j2529dy",
+        "rev": "e236b838c71d2aff275356ade8104bbdef422117",
+        "sha256": "0zjf6b9pz3ljinwb2qxhmpix1mgiv4vakcqci7bcy5a6sv1sj1xs",
         "type": "tarball",
-        "url": "https://github.com/NixOS/nixpkgs/archive/01441e14af5e29c9d27ace398e6dd0b293e25a54.tar.gz",
+        "url": "https://github.com/NixOS/nixpkgs/archive/e236b838c71d2aff275356ade8104bbdef422117.tar.gz",
         "url_template": "https://github.com/<owner>/<repo>/archive/<rev>.tar.gz"
     }
 }

services/brig/src/Brig/API/Client.hs

@@ -532,12 +532,13 @@ createAccessToken ::
 createAccessToken luid cid method link proof = do
   let domain = tDomain luid
   let uid = tUnqualified luid
-  (tid, handle) <- do
+  (tid, handle, displayName) <- do
     mUser <- lift $ wrapClient (Data.lookupUser NoPendingInvitations uid)
     except $
-      (,)
+      (,,)
         <$> note NotATeamUser (userTeam =<< mUser)
         <*> note MissingHandle (userHandle =<< mUser)
+        <*> note MissingName (userDisplayName <$> mUser)
   nonce <- ExceptT $ note NonceNotFound <$> wrapClient (Nonce.lookupAndDeleteNonce uid (cs $ toByteString cid))
   httpsUrl <- except $ note MisconfiguredRequestUrl $ fromByteString $ "https://" <> toByteString' domain <> "/" <> cs (toUrlPiece link)
   maxSkewSeconds <- Opt.setDpopMaxSkewSecs <$> view settings
@@ -554,6 +555,7 @@ createAccessToken luid cid method link proof = do
           proof
           (ClientIdentity domain uid cid)
           handle
+          displayName
           tid
           nonce
           httpsUrl

services/brig/src/Brig/API/Error.hs

@@ -220,12 +220,14 @@ certEnrollmentError (RustError UnsupportedApiVersion) = StdError $ Wai.mkError s
 certEnrollmentError (RustError UnsupportedScope) = StdError $ Wai.mkError status400 "unsupported-scope" "Bubbling up errors"
 certEnrollmentError (RustError DpopHandleMismatch) = StdError $ Wai.mkError status400 "dpop-handle-mismatch" "Bubbling up errors"
 certEnrollmentError (RustError DpopTeamMismatch) = StdError $ Wai.mkError status400 "dpop-team-mismatch" "Bubbling up errors"
+certEnrollmentError (RustError DpopDisplayNameMismatch) = StdError $ Wai.mkError status400 "dpop-display-name-mismatch" "Bubbling up errors"
 certEnrollmentError NonceNotFound = StdError $ Wai.mkError status400 "client-token-bad-nonce" "The client sent an unacceptable anti-replay nonce"
 certEnrollmentError MisconfiguredRequestUrl = StdError $ Wai.mkError status500 "misconfigured-request-url" "The request url cannot be derived from optSettings.setFederationDomain in brig.yaml"
 certEnrollmentError KeyBundleError = StdError $ Wai.mkError status404 "no-server-key-bundle" "The key bundle required for the certificate enrollment process could not be found"
 certEnrollmentError ClientIdSyntaxError = StdError $ Wai.mkError status400 "client-token-id-parse-error" "The client id could not be parsed"
 certEnrollmentError NotATeamUser = StdError $ Wai.mkError status400 "not-a-team-user" "The user is not a team user"
 certEnrollmentError MissingHandle = StdError $ Wai.mkError status400 "missing-handle" "The user has no handle"
+certEnrollmentError MissingName = StdError $ Wai.mkError status400 "missing-name" "The user has no name"
 
 fedError :: FederationError -> Error
 fedError = StdError . federationErrorToWai

services/brig/src/Brig/API/Types.hs

@@ -214,6 +214,7 @@ data CertEnrollmentError
   | ClientIdSyntaxError
   | NotATeamUser
   | MissingHandle
+  | MissingName
 
 -------------------------------------------------------------------------------
 -- Exceptions

services/brig/src/Brig/Effects/JwtTools.hs

@@ -19,6 +19,7 @@ import Polysemy
 import Wire.API.MLS.Credential (ClientIdentity (..))
 import Wire.API.MLS.Epoch (Epoch (..))
 import Wire.API.User.Client.DPoPAccessToken (DPoPAccessToken (..), Proof (..))
+import Wire.API.User.Profile (Name (..))
 
 data JwtTools m a where
   GenerateDPoPAccessToken ::
@@ -30,6 +31,8 @@ data JwtTools m a where
     ClientIdentity ->
     -- | The user's handle
     Handle ->
+    -- The user's display name
+    Name ->
     -- | The user's team ID
     TeamId ->
     -- | The most recent DPoP nonce provided by the backend to the current client
@@ -52,7 +55,7 @@ makeSem ''JwtTools
 
 interpretJwtTools :: Member (Embed IO) r => Sem (JwtTools ': r) a -> Sem r a
 interpretJwtTools = interpret $ \case
-  GenerateDPoPAccessToken proof cid handle tid nonce uri method skew ex now pem ->
+  GenerateDPoPAccessToken proof cid handle displayName tid nonce uri method skew ex now pem ->
     mapLeft RustError
       <$> runExceptT
         ( DPoPAccessToken
@@ -61,6 +64,7 @@ interpretJwtTools = interpret $ \case
               (Jwt.UserId (toByteString' (ciUser cid)))
               (Jwt.ClientId (clientToWord64 (ciClient cid)))
               (Jwt.Handle (toByteString' (urlEncode (fromHandle (handle)))))
+              (Jwt.DisplayName (toByteString' (fromName displayName)))
               (Jwt.TeamId (toByteString' tid))
               (Jwt.Domain (toByteString' (ciDomain cid)))
               (Jwt.Nonce (toByteString' nonce))

services/brig/test/integration/API/User/Client.hs

@@ -1395,6 +1395,7 @@ data DPoPClaimsSet = DPoPClaimsSet
     claimHtu :: Text,
     claimChal :: Text,
     claimHandle :: Text,
+    claimDisplayName :: Text,
     claimTeamId :: Text
   }
   deriving (Eq, Show, Generic)
@@ -1411,6 +1412,7 @@ instance A.FromJSON DPoPClaimsSet where
       <*> o A..: "htu"
       <*> o A..: "chal"
       <*> o A..: "handle"
+      <*> o A..: "name"
       <*> o A..: "team"
 
 instance A.ToJSON DPoPClaimsSet where
@@ -1420,6 +1422,7 @@ instance A.ToJSON DPoPClaimsSet where
       & ins "htu" (claimHtu s)
       & ins "chal" (claimChal s)
       & ins "handle" (claimHandle s)
+      & ins "name" (claimDisplayName s)
       & ins "team" (claimTeamId s)
     where
       ins k v (Object o) = Object $ M.insert k (A.toJSON v) o
@@ -1456,7 +1459,16 @@ testCreateAccessToken opts n brig = do
           & claimSub ?~ fromMaybe (error "invalid sub claim") ((clientIdentity :: Text) ^? stringOrUri)
           & claimJti ?~ "6fc59e7f-b666-4ffc-b738-4f4760c884ca"
           & claimAud ?~ (maybe (error "invalid sub claim") (Audience . (: [])) (("https://wire.com/acme/challenge/abcd" :: Text) ^? stringOrUri))
-  let dpopClaims = DPoPClaimsSet claimsSet' nonceBs "POST" httpsUrl "wa2VrkCtW1sauJ2D3uKY8rc7y4kl4usH" handle (UUID.toText (toUUID tid))
+  let dpopClaims =
+        DPoPClaimsSet
+          claimsSet'
+          nonceBs
+          "POST"
+          httpsUrl
+          "wa2VrkCtW1sauJ2D3uKY8rc7y4kl4usH"
+          handle
+          (fromName u.userDisplayName)
+          (UUID.toText (toUUID tid))
   signedOrError <- fmap encodeCompact <$> liftIO (signAccessToken dpopClaims)
   case signedOrError of
     Left err -> liftIO $ assertFailure $ "failed to sign claims: " <> show err