Documentation for Federation Test Cases

changelog.d/5-internal/map-federation-tests

@@ -0,0 +1 @@
+Tag integration tests for security audit.

services/brig/docs/swagger.md

@@ -54,7 +54,7 @@ An error in this category likely indicates an issue with the configuration of fe
 
  - **Federation unavailable** (status: 500, label: `federation-not-available`): Federation is configured for this backend, but the local federator cannot be reached. This can be transient, so clients should retry the request.
  - **Federation not implemented** (status: 500, label: `federation-not-implemented`): Federated behaviour for a certain endpoint is not yet implemented.
- - **Federator discovery failed** (status: 500, label: `discovery-failure`): A DNS error occurred during discovery of a remote backend. This can be transient, so clients should retry the request.
+ - **Federator discovery failed** (status: 400, label: `discovery-failure`): A DNS error occurred during discovery of a remote backend. This can be transient, so clients should retry the request.
  - **Local federation error** (status: 500, label: `federation-local-error`): An error occurred in the communication between this backend and its local federator. These errors are most likely caused by bugs in the backend, and should be reported as such.
 
 ### Remote federation errors

services/federator/src/Federator/Discovery.hs

@@ -49,7 +49,7 @@ instance AsWai DiscoveryFailure where
     where
       (status, label) = case e of
         DiscoveryFailureSrvNotAvailable _ -> (HTTP.status422, "invalid-domain")
-        DiscoveryFailureDNSError _ -> (HTTP.status500, "discovery-failure")
+        DiscoveryFailureDNSError _ -> (HTTP.status400, "discovery-failure")
   waiErrorDescription :: DiscoveryFailure -> Text
   waiErrorDescription (DiscoveryFailureSrvNotAvailable msg) =
     "srv record not found: " <> Text.decodeUtf8 msg

services/federator/src/Federator/Validation.hs

@@ -150,6 +150,7 @@ validateDomain ::
 validateDomain Nothing _ = throw NoClientCertificate
 validateDomain (Just encodedCertificate) unparsedDomain = do
   targetDomain <- parseDomain unparsedDomain
+  ensureCanFederateWith targetDomain
 
   -- run discovery to find the hostname of the client federator
   certificate <-
@@ -160,7 +161,7 @@ validateDomain (Just encodedCertificate) unparsedDomain = do
   unless (any null validationErrors) $
     throw $ AuthenticationFailure validationErrors
 
-  ensureCanFederateWith targetDomain $> targetDomain
+  pure targetDomain
 
 -- | Match a hostname against the domain names of a certificate.
 --

services/federator/test/integration/Test/Federator/IngressSpec.hs

@@ -46,6 +46,8 @@ import Wire.API.Federation.Domain
 import Wire.API.User
 import Wire.Network.DNS.SRV
 
+-- | This module contains tests for the interface between federator and ingress.  Ingress is
+-- mocked with nginz.
 spec :: TestEnv -> Spec
 spec env = do
   describe "Ingress" $ do
@@ -68,7 +70,16 @@ spec env = do
           responseStatusCode resp `shouldBe` HTTP.status200
           actualProfile `shouldBe` (Just expectedProfile)
 
-  it "should not be accessible without a client certificate" $
+  -- @SF.Federation @TSFI.RESTfulAPI @S2 @S3 @S7
+  --
+  -- This test was primarily intended to test that federator is using the API right (header
+  -- name etc.), but it is also effectively testing that federator rejects clients without
+  -- certificates that have been validated by ingress.
+  --
+  -- We can't test end-to-end here: the TLS termination happens in k8s, and would have to be
+  -- tested there (and with a good emulation of the concrete configuration of the prod
+  -- system).
+  it "rejectRequestsWithoutClientCertIngress" $
     runTestFederator env $ do
       brig <- view teBrig <$> ask
       user <- randomUser brig
@@ -93,6 +104,10 @@ spec env = do
           expectationFailure "Expected client certificate error, got remote error"
         Left (RemoteErrorResponse _ status _) -> status `shouldBe` HTTP.status400
 
+-- FUTUREWORK: ORMOLU_DISABLE
+-- @END
+-- ORMOLU_ENABLE
+
 runTestSem :: Sem '[Input TestEnv, Embed IO] a -> TestFederator IO a
 runTestSem action = do
   e <- ask

services/federator/test/integration/Test/Federator/InwardSpec.hs

@@ -39,8 +39,11 @@ import Wire.API.Federation.Domain
 import Wire.API.User
 
 -- FUTUREWORK(federation): move these tests to brig-integration (benefit: avoid duplicating all of the brig helper code)
+-- FUTUREWORK(fisx): better yet, reorganize integration tests (or at least the helpers) so
+-- they don't spread out over the different sevices.
 
--- | Path covered by this test
+-- | This module contains tests for the interface between federator and brig.  The tests call
+-- federator directly, circumnventing ingress:
 --
 --  +----------+
 --  |federator-|          +------+--+
@@ -72,6 +75,14 @@ spec env =
             <!! const 200 === statusCode
         liftIO $ bdy `shouldBe` expectedProfile
 
+    -- @SF.Federation @TSFI.RESTfulAPI @S2 @S3 @S7
+    --
+    -- (This is tested in unit tests; search for
+    -- 'validateDomainCertInvalid' and 'testDiscoveryFailure'.)
+    it "shouldRejectMissmatchingOriginDomainInward" $
+      runTestFederator env $ pure ()
+    -- @END
+
     it "should be able to call cargohold" $
       runTestFederator env $ do
         inwardCall "/federation/cargohold/get-asset" (encode ())
@@ -98,9 +109,11 @@ spec env =
         inwardCall "/i/users" (encode o)
           !!! const 403 === statusCode
 
-    -- Matching client certificates against domain names is better tested in
-    -- unit tests.
-    it "should reject requests without a client certificate" $
+    -- @SF.Federation @TSFI.RESTfulAPI @S2 @S3 @S7
+    --
+    -- See related tests in unit tests (for matching client certificates against domain names)
+    -- and "IngressSpec".
+    it "rejectRequestsWithoutClientCertInward" $
       runTestFederator env $ do
         originDomain <- cfgOriginDomain <$> view teTstOpts
         hdl <- randomHandle
@@ -110,6 +123,10 @@ spec env =
           (encode hdl)
           !!! const 403 === statusCode
 
+-- TODO: ORMOLU_DISABLE
+-- @END
+-- ORMOLU_ENABLE
+
 inwardCallWithHeaders ::
   (MonadIO m, MonadHttp m, MonadReader TestEnv m, HasCallStack) =>
   ByteString ->
@@ -132,15 +149,24 @@ inwardCall ::
   LBS.ByteString ->
   m (Response (Maybe LByteString))
 inwardCall requestPath payload = do
+  originDomain :: Text <- cfgOriginDomain <$> view teTstOpts
+  inwardCallWithOriginDomain (toByteString' originDomain) requestPath payload
+
+inwardCallWithOriginDomain ::
+  (MonadIO m, MonadHttp m, MonadReader TestEnv m, HasCallStack) =>
+  ByteString ->
+  ByteString ->
+  LBS.ByteString ->
+  m (Response (Maybe LByteString))
+inwardCallWithOriginDomain originDomain requestPath payload = do
   Endpoint fedHost fedPort <- cfgFederatorExternal <$> view teTstOpts
-  originDomain <- cfgOriginDomain <$> view teTstOpts
   clientCertFilename <- clientCertificate . optSettings . view teOpts <$> ask
   clientCert <- liftIO $ BS.readFile clientCertFilename
   post
     ( host (encodeUtf8 fedHost)
         . port fedPort
         . path requestPath
         . header "X-SSL-Certificate" (HTTP.urlEncode True clientCert)
-        . header originDomainHeaderName (toByteString' originDomain)
+        . header originDomainHeaderName originDomain
         . bytes (toByteString' payload)
     )

services/federator/test/unit/Test/Federator/InternalServer.hs

@@ -99,6 +99,9 @@ federatedRequestSuccess =
     body <- Wai.lazyResponseBody res
     body @?= "\"bar\""
 
+-- @SF.Federation @TSFI.RESTfulAPI @S2 @S3 @S7
+--
+-- Refuse to send outgoing request to non-included domain when allowlist is configured.
 federatedRequestFailureAllowList :: TestTree
 federatedRequestFailureAllowList =
   testCase "should not make a call when target domain not in the allowList" $ do
@@ -134,3 +137,5 @@ federatedRequestFailureAllowList =
         . runInputConst settings
         $ callOutward request
     eith @?= Left (FederationDenied targetDomain)
+
+-- @END

services/federator/test/unit/Test/Federator/Options.hs

@@ -171,7 +171,8 @@ testSettings =
             assertFailure $
               "expected failure for non-existing client certificate, got: "
                 <> show (tlsSettings ^. creds),
-      testCase "fail on invalid certificate" $ do
+      -- @SF.Federation @TSFI.RESTfulAPI @S3 @S7
+      testCase "failToStartWithInvalidServerCredentials" $ do
         let settings =
               defRunSettings
                 "test/resources/unit/invalid.pem"
@@ -193,6 +194,7 @@ testSettings =
             assertFailure $
               "expected failure for invalid client certificate, got: "
                 <> show (tlsSettings ^. creds),
+      -- @END
       testCase "fail on invalid private key" $ do
         let settings =
               defRunSettings

services/federator/test/unit/Test/Federator/Response.hs

@@ -72,7 +72,7 @@ testDiscoveryFailure =
         throw (DiscoveryFailureDNSError "mock error")
     body <- Wai.lazyResponseBody resp
     let merr = Aeson.decode body
-    Wai.responseStatus resp @?= HTTP.status500
+    Wai.responseStatus resp @?= HTTP.status400
     fmap Wai.label merr @?= Just "discovery-failure"
 
 testRemoteError :: TestTree

services/federator/test/unit/Test/Federator/Validation.hs

@@ -113,6 +113,9 @@ validateDomainAllowListFailSemantic =
         $ validateDomain (Just exampleCert) "invalid//.><-semantic-&@-domain"
     res @?= Left (DomainParseError "invalid//.><-semantic-&@-domain")
 
+-- @SF.Federation @TSFI.RESTfulAPI @S2 @S3 @S7
+--
+-- Refuse to send outgoing request to non-included domain when allowlist is configured.
 validateDomainAllowListFail :: TestTree
 validateDomainAllowListFail =
   testCase "allow list validation" $ do
@@ -127,6 +130,8 @@ validateDomainAllowListFail =
         $ validateDomain (Just exampleCert) "localhost.example.com"
     res @?= Left (FederationDenied (Domain "localhost.example.com"))
 
+-- @END
+
 validateDomainAllowListSuccess :: TestTree
 validateDomainAllowListSuccess =
   testCase "should give parsed domain if in the allow list" $ do
@@ -153,6 +158,7 @@ validateDomainCertMissing =
         $ validateDomain Nothing "foo.example.com"
     res @?= Left NoClientCertificate
 
+-- @SF.Federation @TSFI.RESTfulAPI @S2 @S3 @S7
 validateDomainCertInvalid :: TestTree
 validateDomainCertInvalid =
   testCase "should fail if the client certificate is invalid" $ do
@@ -164,6 +170,12 @@ validateDomainCertInvalid =
         $ validateDomain (Just "not a certificate") "foo.example.com"
     res @?= Left (CertificateParseError "no certificate found")
 
+-- @END
+
+-- @SF.Federation @TSFI.RESTfulAPI @S3 @S7
+--
+-- Reject request if the infrastructure domain in the client cert does not match the backend
+-- domain in the `Wire-origin-domain` header.
 validateDomainCertWrongDomain :: TestTree
 validateDomainCertWrongDomain =
   testCase "should fail if the client certificate has a wrong domain" $ do
@@ -176,6 +188,8 @@ validateDomainCertWrongDomain =
         $ validateDomain (Just exampleCert) "foo.example.com"
     res @?= Left (AuthenticationFailure (pure [X509.NameMismatch "foo.example.com"]))
 
+-- @END
+
 validateDomainCertCN :: TestTree
 validateDomainCertCN =
   testCase "should succeed if the certificate has subject CN but no SAN" $ do

services/spar/test-integration/Test/Spar/APISpec.hs

@@ -482,7 +482,9 @@ specFinalizeLogin = do
               (cs . fromJust . responseBody $ sparresp) `shouldContain` "<title>wire:sso:error:forbidden</title>"
         check mkareq mkaresp submitaresp checkresp
 
+    -- {- ORMOLU_DISABLE -} -- FUTUREWORK: try a newer release of ormolu?
     -- @END
+    -- {- ORMOLU_ENABLE -}
 
     context "IdP changes response format" $ do
       it "treats NameId case-insensitively" $ do