From 56b457ec82c0507de00041f7da6de54017d56070 Mon Sep 17 00:00:00 2001
From: Paolo Capriotti <paolo@capriotti.io>
Date: Mon, 26 Jul 2021 08:13:35 +0200
Subject: [PATCH 01/41] Require client certificates in demo nginz

---
 deploy/services-demo/conf/nginz/nginx.conf | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/deploy/services-demo/conf/nginz/nginx.conf b/deploy/services-demo/conf/nginz/nginx.conf
index f4249995d7..2f837d52f6 100644
--- a/deploy/services-demo/conf/nginz/nginx.conf
+++ b/deploy/services-demo/conf/nginz/nginx.conf
@@ -130,6 +130,8 @@ http {
     ssl_certificate integration-leaf.pem;
     ssl_certificate_key integration-leaf-key.pem;
 
+    ssl_verify_client on;
+    ssl_client_certificate integration-ca.pem;
     ######## TLS/SSL block end ##############
 
     zauth_keystore resources/zauth/pubkeys.txt;

From aa445d5118d27ca8007a348ae148bc103ef31e65 Mon Sep 17 00:00:00 2001
From: Paolo Capriotti <paolo@capriotti.io>
Date: Mon, 26 Jul 2021 08:14:26 +0200
Subject: [PATCH 02/41] Wrap CA store in a TLSSettings structure

---
 services/federator/src/Federator/Env.hs            | 6 +++++-
 services/federator/src/Federator/InternalServer.hs | 4 ++--
 services/federator/src/Federator/Run.hs            | 1 +
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/services/federator/src/Federator/Env.hs b/services/federator/src/Federator/Env.hs
index e9d2457499..17f3f29393 100644
--- a/services/federator/src/Federator/Env.hs
+++ b/services/federator/src/Federator/Env.hs
@@ -31,6 +31,9 @@ import qualified Network.HTTP.Client as HTTP
 import qualified System.Logger.Class as LC
 import Wire.API.Federation.GRPC.Types
 
+data TLSSettings = TLSSettings
+  {_caStore :: CertificateStore}
+
 data Env = Env
   { _metrics :: Metrics,
     _applog :: LC.Logger,
@@ -39,7 +42,8 @@ data Env = Env
     _runSettings :: RunSettings,
     _service :: Component -> RPC.Request,
     _httpManager :: HTTP.Manager,
-    _caStore :: CertificateStore
+    _tls :: TLSSettings
   }
 
+makeLenses ''TLSSettings
 makeLenses ''Env
diff --git a/services/federator/src/Federator/InternalServer.hs b/services/federator/src/Federator/InternalServer.hs
index 1a95a1d3f1..5cf69f436c 100644
--- a/services/federator/src/Federator/InternalServer.hs
+++ b/services/federator/src/Federator/InternalServer.hs
@@ -28,7 +28,7 @@ import qualified Data.Text.Encoding as Text
 import Data.X509.CertificateStore
 import Federator.App (Federator, runAppT)
 import Federator.Discovery (DiscoverFederator, LookupError (LookupErrorDNSError, LookupErrorSrvNotAvailable), runFederatorDiscovery)
-import Federator.Env (Env, applog, caStore, dnsResolver, runSettings)
+import Federator.Env (Env, applog, caStore, dnsResolver, runSettings, tls)
 import Federator.Options (RunSettings)
 import Federator.Remote (Remote, RemoteError (..), discoverAndCall, interpretRemote)
 import Federator.Utils.PolysemyServerError (absorbServerError)
@@ -112,7 +112,7 @@ serveOutward env port = do
     transformer action =
       runAppT env
         . runM -- Embed Federator
-        . Polysemy.runReader (view caStore env) -- Reader CertificateStore
+        . Polysemy.runReader (view (tls . caStore) env) -- Reader CertificateStore
         . Polysemy.runReader (view runSettings env) -- Reader RunSettings
         . embedToMonadIO @Federator -- Embed IO
         . absorbServerError
diff --git a/services/federator/src/Federator/Run.hs b/services/federator/src/Federator/Run.hs
index 6faa0be985..2fd14d9279 100644
--- a/services/federator/src/Federator/Run.hs
+++ b/services/federator/src/Federator/Run.hs
@@ -98,6 +98,7 @@ newEnv o _dnsResolver = do
       _service Galley = mkEndpoint (Opt.galley o)
   _httpManager <- initHttpManager
   _caStore <- mkCAStore _runSettings
+  let _tls = TLSSettings {..}
   return Env {..}
   where
     mkEndpoint s = RPC.host (encodeUtf8 (s ^. epHost)) . RPC.port (s ^. epPort) $ RPC.empty

From bcbf93b5533218dad911f5224f560a2956a429b7 Mon Sep 17 00:00:00 2001
From: Paolo Capriotti <paolo@capriotti.io>
Date: Mon, 26 Jul 2021 08:39:21 +0200
Subject: [PATCH 03/41] Add settings for client certificate

---
 services/federator/package.yaml               | 33 ++++++++++---------
 services/federator/src/Federator/Env.hs       |  6 +++-
 services/federator/src/Federator/Options.hs   | 14 +++++++-
 services/federator/src/Federator/Run.hs       | 21 ++++++++----
 .../unit/Test/Federator/ExternalServer.hs     | 11 +++----
 .../unit/Test/Federator/InternalServer.hs     | 19 +++++------
 .../test/unit/Test/Federator/Remote.hs        | 28 +++++++++++++---
 .../test/unit/Test/Federator/Validation.hs    |  2 +-
 8 files changed, 87 insertions(+), 47 deletions(-)

diff --git a/services/federator/package.yaml b/services/federator/package.yaml
index acc6c7280d..bd38ce129d 100644
--- a/services/federator/package.yaml
+++ b/services/federator/package.yaml
@@ -11,19 +11,22 @@ license: AGPL-3
 extra-source-files: test/resources/**/*
 dependencies:
 - aeson
-- http-types
-- either
 - base
 - bilge
 - bytestring
 - data-default
 - dns
 - dns-util
+- either
 - exceptions
 - extended
-- http-client
+- HsOpenSSL
+- HsOpenSSL-x509-system
 - http2-client
 - http2-client-grpc
+- http-client
+- http-client-openssl
+- http-types
 - imports
 - lens
 - metrics-core
@@ -32,29 +35,27 @@ dependencies:
 - mu-grpc-client
 - mu-grpc-server
 - mu-rpc
+- network-uri
+- polysemy
+- polysemy-wire-zoo
+- retry
 - servant
 - servant-server
 - string-conversions
 - text
-- tls
-- x509-store
-- x509-system
 - tinylog
+- tls
+- transformers
 - types-common
+- unliftio
+- uri-bytestring
 - uuid
+- wai-utilities
 - wire-api
 - wire-api-federation
-- polysemy
-- polysemy-wire-zoo
-- retry
-- HsOpenSSL
-- HsOpenSSL-x509-system
-- http-client-openssl
-- unliftio
-- wai-utilities
-- network-uri
-- uri-bytestring
 - x509
+- x509-store
+- x509-system
 - x509-validation
 
 library:
diff --git a/services/federator/src/Federator/Env.hs b/services/federator/src/Federator/Env.hs
index 17f3f29393..0a6e68a896 100644
--- a/services/federator/src/Federator/Env.hs
+++ b/services/federator/src/Federator/Env.hs
@@ -26,13 +26,17 @@ import Control.Lens (makeLenses)
 import Data.Metrics (Metrics)
 import Data.X509.CertificateStore
 import Federator.Options (RunSettings)
+import Imports
 import Network.DNS.Resolver (Resolver)
 import qualified Network.HTTP.Client as HTTP
+import qualified Network.TLS as TLS
 import qualified System.Logger.Class as LC
 import Wire.API.Federation.GRPC.Types
 
 data TLSSettings = TLSSettings
-  {_caStore :: CertificateStore}
+  { _caStore :: CertificateStore,
+    _creds :: Maybe TLS.Credential
+  }
 
 data Env = Env
   { _metrics :: Metrics,
diff --git a/services/federator/src/Federator/Options.hs b/services/federator/src/Federator/Options.hs
index 42f5b11c89..d489777331 100644
--- a/services/federator/src/Federator/Options.hs
+++ b/services/federator/src/Federator/Options.hs
@@ -62,10 +62,22 @@ data RunSettings = RunSettings
   { -- | Would you like to federate with everyone or only with a select set of other wire-server installations?
     federationStrategy :: FederationStrategy,
     useSystemCAStore :: Bool,
-    remoteCAStore :: Maybe FilePath
+    remoteCAStore :: Maybe FilePath,
+    clientCertificate :: Maybe FilePath,
+    clientPrivateKey :: Maybe FilePath
   }
   deriving (Show, Generic)
 
+defRunSettings :: RunSettings
+defRunSettings =
+  RunSettings
+    { federationStrategy = AllowAll,
+      useSystemCAStore = True,
+      remoteCAStore = Nothing,
+      clientCertificate = Nothing,
+      clientPrivateKey = Nothing
+    }
+
 instance FromJSON RunSettings
 
 data Opts = Opts
diff --git a/services/federator/src/Federator/Run.hs b/services/federator/src/Federator/Run.hs
index 2fd14d9279..4ac5b99bfb 100644
--- a/services/federator/src/Federator/Run.hs
+++ b/services/federator/src/Federator/Run.hs
@@ -34,6 +34,7 @@ where
 import qualified Bilge as RPC
 import Control.Exception (throw)
 import Control.Lens ((^.))
+import Control.Monad.Trans.Maybe (runMaybeT)
 import Data.Default (def)
 import qualified Data.Metrics.Middleware as Metrics
 import Data.Text.Encoding (encodeUtf8)
@@ -45,6 +46,7 @@ import Federator.Options as Opt
 import Imports
 import qualified Network.DNS as DNS
 import qualified Network.HTTP.Client as HTTP
+import qualified Network.TLS as TLS
 import qualified Polysemy
 import qualified Polysemy.Error as Polysemy
 import qualified System.Logger.Class as Log
@@ -83,10 +85,12 @@ run opts =
 -------------------------------------------------------------------------------
 -- Environment
 
-newtype InvalidCAStore = InvalidCAStore FilePath
+data FederationSetupError
+  = InvalidCAStore FilePath
+  | InvalidClientCertificate String
   deriving (Show)
 
-instance Exception InvalidCAStore
+instance Exception FederationSetupError
 
 newEnv :: Opts -> DNS.Resolver -> IO Env
 newEnv o _dnsResolver = do
@@ -98,6 +102,7 @@ newEnv o _dnsResolver = do
       _service Galley = mkEndpoint (Opt.galley o)
   _httpManager <- initHttpManager
   _caStore <- mkCAStore _runSettings
+  _creds <- mkCreds _runSettings
   let _tls = TLSSettings {..}
   return Env {..}
   where
@@ -114,17 +119,21 @@ mkCAStore settings = do
       else pure mempty
   pure (customCAStore <> systemCAStore)
 
+mkCreds :: RunSettings -> IO (Maybe TLS.Credential)
+mkCreds settings = runMaybeT $ do
+  cert <- maybe mzero pure (clientCertificate settings)
+  key <- maybe mzero pure (clientPrivateKey settings)
+  lift (TLS.credentialLoadX509 cert key) >>= \case
+    Left e -> lift (throw (InvalidClientCertificate e))
+    Right x -> pure x
+
 closeEnv :: Env -> IO ()
 closeEnv e = do
   Log.flush $ e ^. applog
   Log.close $ e ^. applog
 
--- | Copied (and adjusted) from brig, do we want to put this somehwere common?
--- FUTUREWORK(federation): review certificate and protocol security setting for this TLS
--- manager
 initHttpManager :: IO HTTP.Manager
 initHttpManager =
-  -- See Note [SSL context]
   HTTP.newManager
     HTTP.defaultManagerSettings
       { HTTP.managerConnCount = 1024,
diff --git a/services/federator/test/unit/Test/Federator/ExternalServer.hs b/services/federator/test/unit/Test/Federator/ExternalServer.hs
index bcd08702c0..854f7bc331 100644
--- a/services/federator/test/unit/Test/Federator/ExternalServer.hs
+++ b/services/federator/test/unit/Test/Federator/ExternalServer.hs
@@ -22,7 +22,7 @@ module Test.Federator.ExternalServer where
 import Data.Domain (Domain (..))
 import Data.String.Conversions (cs)
 import Federator.ExternalServer (callLocal)
-import Federator.Options (FederationStrategy (AllowAll), RunSettings (..))
+import Federator.Options (defRunSettings)
 import Federator.Service (Service)
 import Imports
 import qualified Network.HTTP.Types as HTTP
@@ -53,7 +53,7 @@ requestBrigSuccess =
       mockServiceCallReturns @IO (\_ _ _ _ -> pure (HTTP.ok200, Just "response body"))
       let request = Request Brig "/federation/get-user-by-handle" "\"foo\"" exampleDomain
 
-      res :: InwardResponse <- mock @Service @IO . TinyLog.discardLogs . Polysemy.runReader allowAllSettings $ callLocal request
+      res :: InwardResponse <- mock @Service @IO . TinyLog.discardLogs . Polysemy.runReader defRunSettings $ callLocal request
       actualCalls <- mockServiceCallCalls @IO
       let expectedCall = (Brig, "federation/get-user-by-handle", "\"foo\"", aValidDomain)
       embed $ assertEqual "one call to brig should be made" [expectedCall] actualCalls
@@ -66,7 +66,7 @@ requestBrigFailure =
       mockServiceCallReturns @IO (\_ _ _ _ -> pure (HTTP.notFound404, Just "response body"))
       let request = Request Brig "/federation/get-user-by-handle" "\"foo\"" exampleDomain
 
-      res <- mock @Service @IO . TinyLog.discardLogs . Polysemy.runReader allowAllSettings $ callLocal request
+      res <- mock @Service @IO . TinyLog.discardLogs . Polysemy.runReader defRunSettings $ callLocal request
 
       actualCalls <- mockServiceCallCalls @IO
       let expectedCall = (Brig, "federation/get-user-by-handle", "\"foo\"", aValidDomain)
@@ -82,15 +82,12 @@ requestGalleySuccess =
       mockServiceCallReturns @IO (\_ _ _ _ -> pure (HTTP.ok200, Just "response body"))
       let request = Request Galley "federation/get-conversations" "{}" exampleDomain
 
-      res :: InwardResponse <- mock @Service @IO . TinyLog.discardLogs . Polysemy.runReader allowAllSettings $ callLocal request
+      res :: InwardResponse <- mock @Service @IO . TinyLog.discardLogs . Polysemy.runReader defRunSettings $ callLocal request
       actualCalls <- mockServiceCallCalls @IO
       let expectedCall = (Galley, "federation/get-conversations", "{}", aValidDomain)
       embed $ assertEqual "one call to brig should be made" [expectedCall] actualCalls
       embed $ assertEqual "response should be success with correct body" (InwardResponseBody "response body") res
 
-allowAllSettings :: RunSettings
-allowAllSettings = RunSettings AllowAll True Nothing
-
 exampleDomain :: Text
 exampleDomain = "some.example.com"
 
diff --git a/services/federator/test/unit/Test/Federator/InternalServer.hs b/services/federator/test/unit/Test/Federator/InternalServer.hs
index 6b363eeb6f..691b573c33 100644
--- a/services/federator/test/unit/Test/Federator/InternalServer.hs
+++ b/services/federator/test/unit/Test/Federator/InternalServer.hs
@@ -22,7 +22,7 @@ module Test.Federator.InternalServer (tests) where
 import Data.Domain (Domain (Domain))
 import Federator.Discovery (LookupError (LookupErrorDNSError, LookupErrorSrvNotAvailable))
 import Federator.InternalServer (callOutward)
-import Federator.Options (AllowedDomains (..), FederationStrategy (..), RunSettings (..))
+import Federator.Options (AllowedDomains (..), FederationStrategy (..), RunSettings (..), defRunSettings)
 import Federator.Remote (Remote, RemoteError (RemoteErrorDiscoveryFailure))
 import Imports
 import Mu.GRpc.Client.Record
@@ -53,10 +53,7 @@ tests =
 
 settingsWithAllowList :: [Domain] -> RunSettings
 settingsWithAllowList domains =
-  RunSettings (AllowList (AllowedDomains domains)) True Nothing
-
-allowAllSettings :: RunSettings
-allowAllSettings = RunSettings AllowAll True Nothing
+  defRunSettings {federationStrategy = AllowList (AllowedDomains domains)}
 
 federatedRequestSuccess :: TestTree
 federatedRequestSuccess =
@@ -65,7 +62,7 @@ federatedRequestSuccess =
       mockDiscoverAndCallReturns @IO (const $ pure (Right (GRpcOk (InwardResponseBody "success!"))))
       let federatedRequest = FederatedRequest validDomainText (Just validLocalPart)
 
-      res <- mock @Remote @IO . Polysemy.runReader allowAllSettings $ callOutward federatedRequest
+      res <- mock @Remote @IO . Polysemy.runReader defRunSettings $ callOutward federatedRequest
 
       actualCalls <- mockDiscoverAndCallCalls @IO
       let expectedCall = ValidatedFederatedRequest (Domain validDomainText) validLocalPart
@@ -81,7 +78,7 @@ federatedRequestFailureTMC =
       mockDiscoverAndCallReturns @IO (const $ pure (Right (GRpcTooMuchConcurrency (TooMuchConcurrency 2))))
       let federatedRequest = FederatedRequest validDomainText (Just validLocalPart)
 
-      res <- mock @Remote @IO . Polysemy.runReader allowAllSettings $ callOutward federatedRequest
+      res <- mock @Remote @IO . Polysemy.runReader defRunSettings $ callOutward federatedRequest
 
       actualCalls <- mockDiscoverAndCallCalls @IO
       let expectedCall = ValidatedFederatedRequest (Domain validDomainText) validLocalPart
@@ -96,7 +93,7 @@ federatedRequestFailureErrCode =
       mockDiscoverAndCallReturns @IO (const $ pure (Right (GRpcErrorCode 77))) -- TODO: Maybe use some legit HTTP2 error code?
       let federatedRequest = FederatedRequest validDomainText (Just validLocalPart)
 
-      res <- mock @Remote @IO . Polysemy.runReader allowAllSettings $ callOutward federatedRequest
+      res <- mock @Remote @IO . Polysemy.runReader defRunSettings $ callOutward federatedRequest
 
       actualCalls <- mockDiscoverAndCallCalls @IO
       let expectedCall = ValidatedFederatedRequest (Domain validDomainText) validLocalPart
@@ -111,7 +108,7 @@ federatedRequestFailureErrStr =
       mockDiscoverAndCallReturns @IO (const $ pure (Right (GRpcErrorString "some grpc error")))
       let federatedRequest = FederatedRequest validDomainText (Just validLocalPart)
 
-      res <- mock @Remote @IO . Polysemy.runReader allowAllSettings $ callOutward federatedRequest
+      res <- mock @Remote @IO . Polysemy.runReader defRunSettings $ callOutward federatedRequest
 
       actualCalls <- mockDiscoverAndCallCalls @IO
       let expectedCall = ValidatedFederatedRequest (Domain validDomainText) validLocalPart
@@ -126,7 +123,7 @@ federatedRequestFailureNoRemote =
       mockDiscoverAndCallReturns @IO (const $ pure (Left $ RemoteErrorDiscoveryFailure (Domain "example.com") (LookupErrorSrvNotAvailable "_something._tcp.example.com")))
       let federatedRequest = FederatedRequest validDomainText (Just validLocalPart)
 
-      res <- mock @Remote @IO . Polysemy.runReader allowAllSettings $ callOutward federatedRequest
+      res <- mock @Remote @IO . Polysemy.runReader defRunSettings $ callOutward federatedRequest
 
       actualCalls <- mockDiscoverAndCallCalls @IO
       let expectedCall = ValidatedFederatedRequest (Domain validDomainText) validLocalPart
@@ -141,7 +138,7 @@ federatedRequestFailureDNS =
       mockDiscoverAndCallReturns @IO (const $ pure (Left $ RemoteErrorDiscoveryFailure (Domain "example.com") (LookupErrorDNSError "No route to 1.1.1.1")))
       let federatedRequest = FederatedRequest validDomainText (Just validLocalPart)
 
-      res <- mock @Remote @IO . Polysemy.runReader allowAllSettings $ callOutward federatedRequest
+      res <- mock @Remote @IO . Polysemy.runReader defRunSettings $ callOutward federatedRequest
 
       actualCalls <- mockDiscoverAndCallCalls @IO
       let expectedCall = ValidatedFederatedRequest (Domain validDomainText) validLocalPart
diff --git a/services/federator/test/unit/Test/Federator/Remote.hs b/services/federator/test/unit/Test/Federator/Remote.hs
index e83869a805..ba8d4c5439 100644
--- a/services/federator/test/unit/Test/Federator/Remote.hs
+++ b/services/federator/test/unit/Test/Federator/Remote.hs
@@ -37,14 +37,24 @@ testValidatesCertificateSuccess =
     "can get response with valid certificate"
     [ testCase "when hostname=localhost and certificate-for=localhost" $ do
         bracket (startMockServer certForLocalhost) (\(serverThread, _) -> Async.cancel serverThread) $ \(_, port) -> do
-          caStore <- mkCAStore (RunSettings AllowAll False (Just "test/resources/unit/unit-ca.pem"))
+          caStore <-
+            mkCAStore $
+              defRunSettings
+                { useSystemCAStore = False,
+                  remoteCAStore = Just "test/resources/unit/unit-ca.pem"
+                }
           eitherClient <- Polysemy.runM . TinyLog.discardLogs . Polysemy.runReader caStore $ mkGrpcClient (SrvTarget "localhost" (fromIntegral port))
           case eitherClient of
             Left err -> assertFailure $ "Unexpected error: " <> show err
             Right _ -> pure (),
       testCase "when hostname=localhost. and certificate-for=localhost" $ do
         bracket (startMockServer certForLocalhost) (\(serverThread, _) -> Async.cancel serverThread) $ \(_, port) -> do
-          caStore <- mkCAStore (RunSettings AllowAll False (Just "test/resources/unit/unit-ca.pem"))
+          caStore <-
+            mkCAStore $
+              defRunSettings
+                { useSystemCAStore = False,
+                  remoteCAStore = Just "test/resources/unit/unit-ca.pem"
+                }
           eitherClient <- Polysemy.runM . TinyLog.discardLogs . Polysemy.runReader caStore $ mkGrpcClient (SrvTarget "localhost." (fromIntegral port))
           case eitherClient of
             Left err -> assertFailure $ "Unexpected error: " <> show err
@@ -52,7 +62,12 @@ testValidatesCertificateSuccess =
       -- This is a limitation of the TLS library, this test just exists to document that.
       testCase "when hostname=localhost. and certificate-for=localhost." $ do
         bracket (startMockServer certForLocalhostDot) (\(serverThread, _) -> Async.cancel serverThread) $ \(_, port) -> do
-          caStore <- mkCAStore (RunSettings AllowAll False (Just "test/resources/unit/unit-ca.pem"))
+          caStore <-
+            mkCAStore $
+              defRunSettings
+                { useSystemCAStore = False,
+                  remoteCAStore = Just "test/resources/unit/unit-ca.pem"
+                }
           eitherClient <-
             Polysemy.runM . TinyLog.discardLogs . Polysemy.runReader caStore $
               mkGrpcClient (SrvTarget "localhost." (fromIntegral port))
@@ -67,7 +82,12 @@ testValidatesCertificateWrongHostname =
     "refuses to connect with server"
     [ testCase "when the server's certificate doesn't match the hostname" $
         bracket (startMockServer certForWrongDomain) (Async.cancel . fst) $ \(_, port) -> do
-          caStore <- mkCAStore (RunSettings AllowAll False (Just "test/resources/unit/unit-ca.pem"))
+          caStore <-
+            mkCAStore $
+              defRunSettings
+                { useSystemCAStore = False,
+                  remoteCAStore = Just "test/resources/unit/unit-ca.pem"
+                }
           eitherClient <-
             Polysemy.runM . TinyLog.discardLogs . Polysemy.runReader caStore $
               mkGrpcClient (SrvTarget "localhost." (fromIntegral port))
diff --git a/services/federator/test/unit/Test/Federator/Validation.hs b/services/federator/test/unit/Test/Federator/Validation.hs
index 02927ace86..112fe89cef 100644
--- a/services/federator/test/unit/Test/Federator/Validation.hs
+++ b/services/federator/test/unit/Test/Federator/Validation.hs
@@ -177,4 +177,4 @@ expectErr expectedType (Left err) =
 
 settingsWithAllowList :: [Domain] -> RunSettings
 settingsWithAllowList domains =
-  RunSettings (AllowList (AllowedDomains domains)) False Nothing
+  defRunSettings {federationStrategy = AllowList (AllowedDomains domains)}

From 3f98c493190c44e9e75dcefbf67d7a18a76e2c98 Mon Sep 17 00:00:00 2001
From: Paolo Capriotti <paolo@capriotti.io>
Date: Mon, 26 Jul 2021 08:52:26 +0200
Subject: [PATCH 04/41] Access TLSSettings in Remote

---
 .../federator/src/Federator/InternalServer.hs |  6 ++--
 services/federator/src/Federator/Remote.hs    | 31 ++++++++++++-------
 services/federator/src/Federator/Run.hs       | 12 ++++---
 .../integration/Test/Federator/IngressSpec.hs |  4 +--
 .../test/integration/Test/Federator/Util.hs   |  8 ++---
 .../test/unit/Test/Federator/Remote.hs        | 26 ++++++++--------
 6 files changed, 50 insertions(+), 37 deletions(-)

diff --git a/services/federator/src/Federator/InternalServer.hs b/services/federator/src/Federator/InternalServer.hs
index 5cf69f436c..49d77c860b 100644
--- a/services/federator/src/Federator/InternalServer.hs
+++ b/services/federator/src/Federator/InternalServer.hs
@@ -28,7 +28,7 @@ import qualified Data.Text.Encoding as Text
 import Data.X509.CertificateStore
 import Federator.App (Federator, runAppT)
 import Federator.Discovery (DiscoverFederator, LookupError (LookupErrorDNSError, LookupErrorSrvNotAvailable), runFederatorDiscovery)
-import Federator.Env (Env, applog, caStore, dnsResolver, runSettings, tls)
+import Federator.Env (Env, TLSSettings, applog, caStore, dnsResolver, runSettings, tls)
 import Federator.Options (RunSettings)
 import Federator.Remote (Remote, RemoteError (..), discoverAndCall, interpretRemote)
 import Federator.Utils.PolysemyServerError (absorbServerError)
@@ -104,7 +104,7 @@ serveOutward env port = do
            Polysemy.Error ServerError,
            Embed IO,
            Polysemy.Reader RunSettings,
-           Polysemy.Reader CertificateStore,
+           Polysemy.Reader TLSSettings,
            Embed Federator
          ]
         a ->
@@ -112,7 +112,7 @@ serveOutward env port = do
     transformer action =
       runAppT env
         . runM -- Embed Federator
-        . Polysemy.runReader (view (tls . caStore) env) -- Reader CertificateStore
+        . Polysemy.runReader (view tls env) -- Reader TLSSettings
         . Polysemy.runReader (view runSettings env) -- Reader RunSettings
         . embedToMonadIO @Federator -- Embed IO
         . absorbServerError
diff --git a/services/federator/src/Federator/Remote.hs b/services/federator/src/Federator/Remote.hs
index 3339f10317..1f5eeb4d06 100644
--- a/services/federator/src/Federator/Remote.hs
+++ b/services/federator/src/Federator/Remote.hs
@@ -17,29 +17,37 @@
 -- You should have received a copy of the GNU Affero General Public License along
 -- with this program. If not, see <https://www.gnu.org/licenses/>.
 
-module Federator.Remote where
+module Federator.Remote
+  ( Remote,
+    RemoteError (..),
+    discoverAndCall,
+    interpretRemote,
+    mkGrpcClient,
+  )
+where
 
+import Control.Lens ((^.))
 import Data.Default (def)
 import Data.Domain (Domain, domainText)
 import Data.String.Conversions (cs)
 import qualified Data.X509 as X509
-import Data.X509.CertificateStore
 import qualified Data.X509.Validation as X509
 import Federator.Discovery (DiscoverFederator, LookupError, discoverFederator)
+import Federator.Env (TLSSettings, caStore)
 import Federator.Options
 import Imports
 import Mu.GRpc.Client.Optics (GRpcReply)
 import Mu.GRpc.Client.Record (GRpcMessageProtocol (MsgProtoBuf))
 import Mu.GRpc.Client.TyApps (gRpcCall)
 import Network.GRPC.Client.Helpers
-import Network.TLS
-import qualified Network.TLS as TLS
+import Network.TLS as TLS
 import qualified Network.TLS.Extra.Cipher as TLS
 import Polysemy
 import qualified Polysemy.Error as Polysemy
 import qualified Polysemy.Reader as Polysemy
 import Polysemy.TinyLog (TinyLog)
 import qualified Polysemy.TinyLog as Log
+import System.IO (hPutStrLn)
 import qualified System.Logger.Message as Log
 import Wire.API.Federation.GRPC.Client
 import Wire.API.Federation.GRPC.Types
@@ -57,7 +65,7 @@ data Remote m a where
 makeSem ''Remote
 
 interpretRemote ::
-  (Members [Embed IO, DiscoverFederator, TinyLog, Polysemy.Reader RunSettings, Polysemy.Reader CertificateStore] r) =>
+  (Members [Embed IO, DiscoverFederator, TinyLog, Polysemy.Reader RunSettings, Polysemy.Reader TLSSettings] r) =>
   Sem (Remote ': r) a ->
   Sem r a
 interpretRemote = interpret $ \case
@@ -84,11 +92,10 @@ callInward client request =
 -- FUTUREWORK(federation): Consider using HsOpenSSL instead of tls for better
 -- security and to avoid having to depend on cryptonite and override validation
 -- hooks. This might involve forking http2-client: https://github.com/lucasdicioccio/http2-client/issues/76
--- FUTUREWORK(federation): Allow a configurable trust store to be used in TLS certificate validation
 --   See also https://github.com/lucasdicioccio/http2-client/issues/76
 -- FUTUREWORK(federation): Cache this client and use it for many requests
 mkGrpcClient ::
-  Members '[Embed IO, TinyLog, Polysemy.Reader CertificateStore] r =>
+  Members '[Embed IO, TinyLog, Polysemy.Reader TLSSettings] r =>
   SrvTarget ->
   Sem r (Either RemoteError GrpcClient)
 mkGrpcClient target@(SrvTarget host port) = logAndReturn target $ do
@@ -115,7 +122,7 @@ mkGrpcClient target@(SrvTarget host port) = logAndReturn target $ do
           TLS.cipher_ECDHE_RSA_CHACHA20POLY1305_SHA256
         ]
 
-  caStore <- Polysemy.ask
+  settings <- Polysemy.ask
 
   -- validate the hostname without a trailing dot as the certificate is not
   -- expected to have the trailing dot.
@@ -139,10 +146,12 @@ mkGrpcClient target@(SrvTarget host port) = logAndReturn target $ do
                     X509.validate
                       X509.HashSHA256
                       (X509.defaultHooks {TLS.hookValidateName = validateName})
-                      X509.defaultChecks
+                      X509.defaultChecks,
+                  TLS.onCertificateRequest = \_ -> do
+                    hPutStrLn stderr "***** CERTIFICATE REQUEST *****"
+                    pure Nothing
                 },
-            -- FUTUREWORK: use onCertificateRequest to provide client certificates
-            TLS.clientShared = def {TLS.sharedCAStore = caStore}
+            TLS.clientShared = def {TLS.sharedCAStore = settings ^. caStore}
           }
   let cfg' = cfg {_grpcClientConfigTLS = Just tlsConfig}
   Polysemy.mapError (RemoteErrorClientFailure target)
diff --git a/services/federator/src/Federator/Run.hs b/services/federator/src/Federator/Run.hs
index 4ac5b99bfb..15b1f42287 100644
--- a/services/federator/src/Federator/Run.hs
+++ b/services/federator/src/Federator/Run.hs
@@ -26,7 +26,7 @@ module Federator.Run
 
     -- * App Environment
     newEnv,
-    mkCAStore,
+    mkTLSSettings,
     closeEnv,
   )
 where
@@ -101,9 +101,7 @@ newEnv o _dnsResolver = do
   let _service Brig = mkEndpoint (Opt.brig o)
       _service Galley = mkEndpoint (Opt.galley o)
   _httpManager <- initHttpManager
-  _caStore <- mkCAStore _runSettings
-  _creds <- mkCreds _runSettings
-  let _tls = TLSSettings {..}
+  _tls <- mkTLSSettings _runSettings
   return Env {..}
   where
     mkEndpoint s = RPC.host (encodeUtf8 (s ^. epHost)) . RPC.port (s ^. epPort) $ RPC.empty
@@ -127,6 +125,12 @@ mkCreds settings = runMaybeT $ do
     Left e -> lift (throw (InvalidClientCertificate e))
     Right x -> pure x
 
+mkTLSSettings :: RunSettings -> IO TLSSettings
+mkTLSSettings settings =
+  TLSSettings
+    <$> mkCAStore settings
+    <*> mkCreds settings
+
 closeEnv :: Env -> IO ()
 closeEnv e = do
   Log.flush $ e ^. applog
diff --git a/services/federator/test/integration/Test/Federator/IngressSpec.hs b/services/federator/test/integration/Test/Federator/IngressSpec.hs
index 07491211a4..c11a2067e9 100644
--- a/services/federator/test/integration/Test/Federator/IngressSpec.hs
+++ b/services/federator/test/integration/Test/Federator/IngressSpec.hs
@@ -59,8 +59,8 @@ inwardBrigCallViaIngress requestPath payload = do
   Endpoint ingressHost ingressPort <- cfgNginxIngress . view teTstOpts <$> ask
   let target = SrvTarget (cs ingressHost) ingressPort
   runSettings <- optSettings . view teOpts <$> ask
-  caStore <- view teCAStore <$> ask
-  c <- liftIO . Polysemy.runM . discardLogs . Polysemy.runReader caStore . Polysemy.runReader runSettings $ mkGrpcClient target
+  tlsSettings <- view teTLSSettings
+  c <- liftIO . Polysemy.runM . discardLogs . Polysemy.runReader tlsSettings . Polysemy.runReader runSettings $ mkGrpcClient target
   client <- case c of
     Left clientErr -> liftIO $ assertFailure (show clientErr)
     Right cli -> pure cli
diff --git a/services/federator/test/integration/Test/Federator/Util.hs b/services/federator/test/integration/Test/Federator/Util.hs
index ddd2e1e3c3..7c95f0faa2 100644
--- a/services/federator/test/integration/Test/Federator/Util.hs
+++ b/services/federator/test/integration/Test/Federator/Util.hs
@@ -42,10 +42,10 @@ import Data.String.Conversions
 import qualified Data.Text as Text
 import qualified Data.UUID as UUID
 import qualified Data.UUID.V4 as UUID
-import Data.X509.CertificateStore
 import qualified Data.Yaml as Yaml
+import Federator.Env (TLSSettings (..))
 import Federator.Options
-import Federator.Run (mkCAStore)
+import Federator.Run (mkTLSSettings)
 import Imports
 import Mu.GRpc.Client.TyApps
 import qualified Options.Applicative as OPA
@@ -86,7 +86,7 @@ runTestFederator env = flip runReaderT env . unwrapTestFederator
 -- | See 'mkEnv' about what's in here.
 data TestEnv = TestEnv
   { _teMgr :: Manager,
-    _teCAStore :: CertificateStore,
+    _teTLSSettings :: TLSSettings,
     _teBrig :: BrigReq,
     -- | federator config
     _teOpts :: Opts,
@@ -143,7 +143,7 @@ mkEnv :: HasCallStack => IntegrationConfig -> Opts -> IO TestEnv
 mkEnv _teTstOpts _teOpts = do
   _teMgr :: Manager <- newManager defaultManagerSettings
   let _teBrig = endpointToReq (cfgBrig _teTstOpts)
-  _teCAStore <- mkCAStore (optSettings _teOpts)
+  _teTLSSettings <- mkTLSSettings (optSettings _teOpts)
   pure TestEnv {..}
 
 destroyEnv :: HasCallStack => TestEnv -> IO ()
diff --git a/services/federator/test/unit/Test/Federator/Remote.hs b/services/federator/test/unit/Test/Federator/Remote.hs
index ba8d4c5439..7168acbe5a 100644
--- a/services/federator/test/unit/Test/Federator/Remote.hs
+++ b/services/federator/test/unit/Test/Federator/Remote.hs
@@ -5,7 +5,7 @@ module Test.Federator.Remote where
 import Data.Streaming.Network (bindRandomPortTCP)
 import Federator.Options
 import Federator.Remote
-import Federator.Run (mkCAStore)
+import Federator.Run (mkTLSSettings)
 import Imports
 import Network.HTTP.Types (status200)
 import Network.Wai
@@ -37,39 +37,39 @@ testValidatesCertificateSuccess =
     "can get response with valid certificate"
     [ testCase "when hostname=localhost and certificate-for=localhost" $ do
         bracket (startMockServer certForLocalhost) (\(serverThread, _) -> Async.cancel serverThread) $ \(_, port) -> do
-          caStore <-
-            mkCAStore $
+          tlsSettings <-
+            mkTLSSettings $
               defRunSettings
                 { useSystemCAStore = False,
                   remoteCAStore = Just "test/resources/unit/unit-ca.pem"
                 }
-          eitherClient <- Polysemy.runM . TinyLog.discardLogs . Polysemy.runReader caStore $ mkGrpcClient (SrvTarget "localhost" (fromIntegral port))
+          eitherClient <- Polysemy.runM . TinyLog.discardLogs . Polysemy.runReader tlsSettings $ mkGrpcClient (SrvTarget "localhost" (fromIntegral port))
           case eitherClient of
             Left err -> assertFailure $ "Unexpected error: " <> show err
             Right _ -> pure (),
       testCase "when hostname=localhost. and certificate-for=localhost" $ do
         bracket (startMockServer certForLocalhost) (\(serverThread, _) -> Async.cancel serverThread) $ \(_, port) -> do
-          caStore <-
-            mkCAStore $
+          tlsSettings <-
+            mkTLSSettings $
               defRunSettings
                 { useSystemCAStore = False,
                   remoteCAStore = Just "test/resources/unit/unit-ca.pem"
                 }
-          eitherClient <- Polysemy.runM . TinyLog.discardLogs . Polysemy.runReader caStore $ mkGrpcClient (SrvTarget "localhost." (fromIntegral port))
+          eitherClient <- Polysemy.runM . TinyLog.discardLogs . Polysemy.runReader tlsSettings $ mkGrpcClient (SrvTarget "localhost." (fromIntegral port))
           case eitherClient of
             Left err -> assertFailure $ "Unexpected error: " <> show err
             Right _ -> pure (),
       -- This is a limitation of the TLS library, this test just exists to document that.
       testCase "when hostname=localhost. and certificate-for=localhost." $ do
         bracket (startMockServer certForLocalhostDot) (\(serverThread, _) -> Async.cancel serverThread) $ \(_, port) -> do
-          caStore <-
-            mkCAStore $
+          tlsSettings <-
+            mkTLSSettings $
               defRunSettings
                 { useSystemCAStore = False,
                   remoteCAStore = Just "test/resources/unit/unit-ca.pem"
                 }
           eitherClient <-
-            Polysemy.runM . TinyLog.discardLogs . Polysemy.runReader caStore $
+            Polysemy.runM . TinyLog.discardLogs . Polysemy.runReader tlsSettings $
               mkGrpcClient (SrvTarget "localhost." (fromIntegral port))
           case eitherClient of
             Left _ -> pure ()
@@ -82,14 +82,14 @@ testValidatesCertificateWrongHostname =
     "refuses to connect with server"
     [ testCase "when the server's certificate doesn't match the hostname" $
         bracket (startMockServer certForWrongDomain) (Async.cancel . fst) $ \(_, port) -> do
-          caStore <-
-            mkCAStore $
+          tlsSettings <-
+            mkTLSSettings $
               defRunSettings
                 { useSystemCAStore = False,
                   remoteCAStore = Just "test/resources/unit/unit-ca.pem"
                 }
           eitherClient <-
-            Polysemy.runM . TinyLog.discardLogs . Polysemy.runReader caStore $
+            Polysemy.runM . TinyLog.discardLogs . Polysemy.runReader tlsSettings $
               mkGrpcClient (SrvTarget "localhost." (fromIntegral port))
           case eitherClient of
             Left (RemoteErrorTLSException _ _) -> pure ()

From 64f14032a39ff89664bccb93005b9f1b19564924 Mon Sep 17 00:00:00 2001
From: Paolo Capriotti <paolo@capriotti.io>
Date: Mon, 26 Jul 2021 09:04:04 +0200
Subject: [PATCH 05/41] Use leaf as client cert in integration tests

---
 services/federator/federator.cabal            | 6 +++++-
 services/federator/federator.integration.yaml | 3 +++
 services/federator/src/Federator/Remote.hs    | 7 ++-----
 services/federator/src/Federator/Run.hs       | 7 +++++--
 4 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/services/federator/federator.cabal b/services/federator/federator.cabal
index 5f5836d0f9..e7575bdc1d 100644
--- a/services/federator/federator.cabal
+++ b/services/federator/federator.cabal
@@ -4,7 +4,7 @@ cabal-version: 1.12
 --
 -- see: https://github.com/sol/hpack
 --
--- hash: 7ae942605e2cd7ddc1fc423b068b429970064332a02e90994295385a0722d1b9
+-- hash: cdb6771f196b12f78bb882147d96fd428eee25077be318520b83679ed05d5db9
 
 name:           federator
 version:        1.0.0
@@ -82,6 +82,7 @@ library
     , text
     , tinylog
     , tls
+    , transformers
     , types-common
     , unliftio
     , uri-bytestring
@@ -140,6 +141,7 @@ executable federator
     , text
     , tinylog
     , tls
+    , transformers
     , types-common
     , unliftio
     , uri-bytestring
@@ -208,6 +210,7 @@ executable federator-integration
     , text
     , tinylog
     , tls
+    , transformers
     , types-common
     , unliftio
     , uri-bytestring
@@ -277,6 +280,7 @@ test-suite federator-tests
     , text
     , tinylog
     , tls
+    , transformers
     , types-common
     , unliftio
     , uri-bytestring
diff --git a/services/federator/federator.integration.yaml b/services/federator/federator.integration.yaml
index 9afe9b3857..ececd8c3b9 100644
--- a/services/federator/federator.integration.yaml
+++ b/services/federator/federator.integration.yaml
@@ -32,3 +32,6 @@ optSettings:
   #     - example.com
 
   useSystemCAStore: true
+
+  clientCertificate: "test/resources/integration-leaf.pem"
+  clientPrivateKey: "test/resources/integration-leaf-key.pem"
diff --git a/services/federator/src/Federator/Remote.hs b/services/federator/src/Federator/Remote.hs
index 1f5eeb4d06..8bb49dbb11 100644
--- a/services/federator/src/Federator/Remote.hs
+++ b/services/federator/src/Federator/Remote.hs
@@ -33,7 +33,7 @@ import Data.String.Conversions (cs)
 import qualified Data.X509 as X509
 import qualified Data.X509.Validation as X509
 import Federator.Discovery (DiscoverFederator, LookupError, discoverFederator)
-import Federator.Env (TLSSettings, caStore)
+import Federator.Env (TLSSettings, caStore, creds)
 import Federator.Options
 import Imports
 import Mu.GRpc.Client.Optics (GRpcReply)
@@ -47,7 +47,6 @@ import qualified Polysemy.Error as Polysemy
 import qualified Polysemy.Reader as Polysemy
 import Polysemy.TinyLog (TinyLog)
 import qualified Polysemy.TinyLog as Log
-import System.IO (hPutStrLn)
 import qualified System.Logger.Message as Log
 import Wire.API.Federation.GRPC.Client
 import Wire.API.Federation.GRPC.Types
@@ -147,9 +146,7 @@ mkGrpcClient target@(SrvTarget host port) = logAndReturn target $ do
                       X509.HashSHA256
                       (X509.defaultHooks {TLS.hookValidateName = validateName})
                       X509.defaultChecks,
-                  TLS.onCertificateRequest = \_ -> do
-                    hPutStrLn stderr "***** CERTIFICATE REQUEST *****"
-                    pure Nothing
+                  TLS.onCertificateRequest = \_ -> pure (settings ^. creds)
                 },
             TLS.clientShared = def {TLS.sharedCAStore = settings ^. caStore}
           }
diff --git a/services/federator/src/Federator/Run.hs b/services/federator/src/Federator/Run.hs
index 15b1f42287..a108bb2aac 100644
--- a/services/federator/src/Federator/Run.hs
+++ b/services/federator/src/Federator/Run.hs
@@ -32,7 +32,7 @@ module Federator.Run
 where
 
 import qualified Bilge as RPC
-import Control.Exception (throw)
+import Control.Exception (handle, throw)
 import Control.Lens ((^.))
 import Control.Monad.Trans.Maybe (runMaybeT)
 import Data.Default (def)
@@ -118,12 +118,15 @@ mkCAStore settings = do
   pure (customCAStore <> systemCAStore)
 
 mkCreds :: RunSettings -> IO (Maybe TLS.Credential)
-mkCreds settings = runMaybeT $ do
+mkCreds settings = handle h . runMaybeT $ do
   cert <- maybe mzero pure (clientCertificate settings)
   key <- maybe mzero pure (clientPrivateKey settings)
   lift (TLS.credentialLoadX509 cert key) >>= \case
     Left e -> lift (throw (InvalidClientCertificate e))
     Right x -> pure x
+  where
+    h :: IOException -> IO a
+    h = throw . InvalidClientCertificate . show
 
 mkTLSSettings :: RunSettings -> IO TLSSettings
 mkTLSSettings settings =

From a768032f275b427c017c15d4c91a0ec90404880d Mon Sep 17 00:00:00 2001
From: Paolo Capriotti <paolo@capriotti.io>
Date: Mon, 26 Jul 2021 11:32:45 +0200
Subject: [PATCH 06/41] Configure client certificate in helm

---
 charts/federator/templates/configmap-ca.yaml      | 14 --------------
 charts/federator/templates/configmap.yaml         | 15 ++++++++++++++-
 charts/federator/templates/deployment.yaml        | 12 ++++++------
 charts/federator/templates/secrets.yaml           | 13 +++++++++++++
 charts/federator/values.yaml                      |  3 +++
 .../nginx-ingress-services/templates/ingress.yaml |  2 ++
 charts/nginx-ingress-services/values.yaml         |  4 ++++
 hack/bin/selfsigned-kubernetes.sh                 | 13 +++++++++++++
 8 files changed, 55 insertions(+), 21 deletions(-)
 delete mode 100644 charts/federator/templates/configmap-ca.yaml
 create mode 100644 charts/federator/templates/secrets.yaml

diff --git a/charts/federator/templates/configmap-ca.yaml b/charts/federator/templates/configmap-ca.yaml
deleted file mode 100644
index f73da24264..0000000000
--- a/charts/federator/templates/configmap-ca.yaml
+++ /dev/null
@@ -1,14 +0,0 @@
-apiVersion: v1
-kind: Secret
-metadata:
-  name: "federator-ca"
-  labels:
-    wireService: federator
-    chart: {{ .Chart.Name }}-{{ .Chart.Version | replace "+" "_" }}
-    release: {{ .Release.Name }}
-    heritage: {{ .Release.Service }}
-data:
-  # TODO: add validation and fail early during templating: either contents should be provided; or explicitly system trust store enabled
-  {{- if .Values.remoteCAContents }}
-  remote-ca.pem: {{ .Values.remoteCAContents | b64enc | quote }}
-  {{- end }}
diff --git a/charts/federator/templates/configmap.yaml b/charts/federator/templates/configmap.yaml
index 50df2444de..d097ef6908 100644
--- a/charts/federator/templates/configmap.yaml
+++ b/charts/federator/templates/configmap.yaml
@@ -43,7 +43,12 @@ data:
       # Filepath to one or more PEM-encoded server certificates to use as a trust
       # store when making grpc requests to remote backends
       {{- if $.Values.remoteCAContents }}
-      remoteCAStore: "/etc/wire/federator/ca/remote-ca.pem"
+      remoteCAStore: "/etc/wire/federator/conf/remote-ca.pem"
+      {{- end }}
+      useSystemCAStore: false
+      {{- if $.Values.clientCertificateContents }}
+      clientCertificate: "/etc/wire/federator/conf/client.pem"
+      clientPrivateKey: "/etc/wire/federator/secrets/client-key.pem"
       {{- end }}
       useSystemCAStore: {{ .useSystemCAStore }}
       federationStrategy:
@@ -61,3 +66,11 @@ data:
         {{- end}}
     {{- end }}
     {{- end }}
+
+  # TODO: add validation and fail early during templating: either contents should be provided; or explicitly system trust store enabled
+  {{- if .Values.remoteCAContents }}
+  remote-ca.pem: {{ .Values.remoteCAContents | quote }}
+  {{- end }}
+  {{- if .Values.clientCertificateContents }}
+  client.pem: {{ .Values.clientCertificateContents | quote }}
+  {{- end }}
diff --git a/charts/federator/templates/deployment.yaml b/charts/federator/templates/deployment.yaml
index 8c5bebe832..2f305b7db3 100644
--- a/charts/federator/templates/deployment.yaml
+++ b/charts/federator/templates/deployment.yaml
@@ -25,18 +25,18 @@ spec:
       annotations:
         # An annotation of the configmap checksum ensures changes to the configmap cause a redeployment upon `helm upgrade`
         checksum/configmap: {{ include (print .Template.BasePath "/configmap.yaml") . | sha256sum }}
-        checksum/configmap-ca: {{ include (print .Template.BasePath "/configmap-ca.yaml") . | sha256sum }}
+        checksum/secrets: {{ include (print .Template.BasePath "/secrets.yaml") . | sha256sum }}
         fluentbit.io/parser: json
     spec:
       volumes:
         - name: "federator-config"
           configMap:
             name: "federator"
-        # federator-ca holds CA certificates to use as a trust store
+        # federator-secrets holds the private key for the client certificate to use
         # when making requests to remote backends
-        - name: "federator-ca"
+        - name: "federator-secrets"
           secret:
-            secretName: "federator-ca"
+            secretName: "federator-secrets"
       containers:
         - name: federator
           image: "{{ .Values.image.repository }}:{{ .Values.image.tag }}"
@@ -44,8 +44,8 @@ spec:
           volumeMounts:
             - name: "federator-config"
               mountPath: "/etc/wire/federator/conf"
-            - name: "federator-ca"
-              mountPath: "/etc/wire/federator/ca"
+            - name: "federator-secrets"
+              mountPath: "/etc/wire/federator/secrets"
           ports:
             - name: internal
               containerPort: {{ .Values.service.internalFederatorPort }}
diff --git a/charts/federator/templates/secrets.yaml b/charts/federator/templates/secrets.yaml
new file mode 100644
index 0000000000..487ad8ca7e
--- /dev/null
+++ b/charts/federator/templates/secrets.yaml
@@ -0,0 +1,13 @@
+apiVersion: v1
+kind: Secret
+metadata:
+  name: "federator-secrets"
+  labels:
+    wireService: federator
+    chart: {{ .Chart.Name }}-{{ .Chart.Version | replace "+" "_" }}
+    release: {{ .Release.Name }}
+    heritage: {{ .Release.Service }}
+data:
+  {{- if .Values.clientPrivateKeyContents }}
+  client-key.pem: {{ .Values.clientPrivateKeyContents | b64enc | quote }}
+  {{- end }}
diff --git a/charts/federator/values.yaml b/charts/federator/values.yaml
index a316fda763..b6be91f04d 100644
--- a/charts/federator/values.yaml
+++ b/charts/federator/values.yaml
@@ -30,6 +30,9 @@ config:
     #
     # Using custom CA doesn't automatically disable system CA store, it should
     # be disabled explicitly by setting useSystemCAStore to false.
+    #
+    # A client certificate and corresponding private key can be specified
+    # similarly to a custom CA store.
     useSystemCAStore: true
     federationStrategy:
       allowedDomains: []
diff --git a/charts/nginx-ingress-services/templates/ingress.yaml b/charts/nginx-ingress-services/templates/ingress.yaml
index 34dbcdb35c..8a7551fa21 100644
--- a/charts/nginx-ingress-services/templates/ingress.yaml
+++ b/charts/nginx-ingress-services/templates/ingress.yaml
@@ -4,6 +4,8 @@ metadata:
   name: nginx-ingress
   annotations:
     kubernetes.io/ingress.class: "nginx"
+    # nginx.ingress.kubernetes.io/auth-tls-verify-client: "on"
+    # nginx.ingress.kubernetes.io/auth-tls-secret: "tlsClientCA"
 spec:
   # This assumes you have created the given cert (see secret.yaml)
   # https://github.com/kubernetes/ingress-nginx/blob/master/docs/examples/PREREQUISITES.md#tls-certificates
diff --git a/charts/nginx-ingress-services/values.yaml b/charts/nginx-ingress-services/values.yaml
index fc87dbebcc..6a4c1a51d4 100644
--- a/charts/nginx-ingress-services/values.yaml
+++ b/charts/nginx-ingress-services/values.yaml
@@ -75,6 +75,10 @@ service:
 #     tlsWildcardKey: |
 #         -----BEGIN PRIVATE KEY-----
 #         -----END PRIVATE KEY-----
+#     tlsClientCA: |
+#         -----BEGIN PRIVATE KEY-----
+#         -----END PRIVATE KEY-----
+#     ^ CA to use to verify client certificates.
 #
 # For Services:
 # service:
diff --git a/hack/bin/selfsigned-kubernetes.sh b/hack/bin/selfsigned-kubernetes.sh
index df2d7f5252..73b9776231 100755
--- a/hack/bin/selfsigned-kubernetes.sh
+++ b/hack/bin/selfsigned-kubernetes.sh
@@ -10,6 +10,7 @@ TEMP=${TEMP:-/tmp}
 CSR="$TEMP/csr.json"
 OUTPUTNAME_CA="integration-ca"
 OUTPUTNAME_LEAF_CERT="integration-leaf"
+OUTPUTNAME_CLIENT_CERT="integration-client"
 DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 TOP_LEVEL="$DIR/../.."
 OUTPUT_CONFIG_FEDERATOR="$TOP_LEVEL/hack/helm_vars/wire-server/certificates.yaml"
@@ -55,6 +56,9 @@ echo '{
 # generate cert and key based on CA given comma-separated hostnames as SANs
 cfssl gencert -ca "$OUTPUTNAME_CA.pem" -ca-key "$OUTPUTNAME_CA-key.pem" -hostname="*.$FEDERATION_DOMAIN_BASE" "$CSR" | cfssljson -bare "$OUTPUTNAME_LEAF_CERT"
 
+# generate client certificate and key
+cfssl gencert -ca "$OUTPUTNAME_CA.pem" -ca-key "$OUTPUTNAME_CA-key.pem" -hostname="*.$FEDERATION_DOMAIN_BASE" "$CSR" | cfssljson -bare "$OUTPUTNAME_CLIENT_CERT"
+
 # the following yaml override file is needed as an override to
 # nginx-ingress-services helm chart
 # for domain A, ingress@A needs cert+key for A
@@ -64,6 +68,8 @@ cfssl gencert -ca "$OUTPUTNAME_CA.pem" -ca-key "$OUTPUTNAME_CA-key.pem" -hostnam
     sed -e 's/^/    /' $OUTPUTNAME_LEAF_CERT.pem
     echo "  tlsWildcardKey: |"
     sed -e 's/^/    /' $OUTPUTNAME_LEAF_CERT-key.pem
+    echo "  tlsClientCA: |"
+    sed -e 's/^/    /' $OUTPUTNAME_CA.pem
 } | tee "$OUTPUT_CONFIG_INGRESS"
 
 # the following yaml override file is needed as an override to
@@ -75,10 +81,17 @@ cfssl gencert -ca "$OUTPUTNAME_CA.pem" -ca-key "$OUTPUTNAME_CA-key.pem" -hostnam
     echo "federator:"
     echo "  remoteCAContents: |"
     sed -e 's/^/    /' $OUTPUTNAME_CA.pem
+    echo "  clientCertificateContents: |"
+    sed -e 's/^/    /' $OUTPUTNAME_CLIENT_CERT.pem
+    echo "  clientPrivateKeyContents: |"
+    sed -e 's/^/    /' $OUTPUTNAME_CLIENT_CERT-key.pem
 } | tee "$OUTPUT_CONFIG_FEDERATOR"
 
 # cleanup unneeded files
 rm "$OUTPUTNAME_LEAF_CERT.csr"
 rm "$OUTPUTNAME_LEAF_CERT.pem"
 rm "$OUTPUTNAME_LEAF_CERT-key.pem"
+rm "$OUTPUTNAME_CLIENT_CERT.csr"
+rm "$OUTPUTNAME_CLIENT_CERT.pem"
+rm "$OUTPUTNAME_CLIENT_CERT-key.pem"
 rm "$CSR"

From eda1e5b92e8b6728f5ae99774135e6435098d5cc Mon Sep 17 00:00:00 2001
From: Paolo Capriotti <paolo@capriotti.io>
Date: Mon, 26 Jul 2021 17:04:07 +0200
Subject: [PATCH 07/41] Enable client certificates in federator ingress

Also add client certificate CA to TLS secret
---
 charts/nginx-ingress-services/templates/ingress.yaml        | 2 --
 .../nginx-ingress-services/templates/ingress_federator.yaml | 2 ++
 charts/nginx-ingress-services/templates/secret.yaml         | 6 ++++--
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/charts/nginx-ingress-services/templates/ingress.yaml b/charts/nginx-ingress-services/templates/ingress.yaml
index 8a7551fa21..34dbcdb35c 100644
--- a/charts/nginx-ingress-services/templates/ingress.yaml
+++ b/charts/nginx-ingress-services/templates/ingress.yaml
@@ -4,8 +4,6 @@ metadata:
   name: nginx-ingress
   annotations:
     kubernetes.io/ingress.class: "nginx"
-    # nginx.ingress.kubernetes.io/auth-tls-verify-client: "on"
-    # nginx.ingress.kubernetes.io/auth-tls-secret: "tlsClientCA"
 spec:
   # This assumes you have created the given cert (see secret.yaml)
   # https://github.com/kubernetes/ingress-nginx/blob/master/docs/examples/PREREQUISITES.md#tls-certificates
diff --git a/charts/nginx-ingress-services/templates/ingress_federator.yaml b/charts/nginx-ingress-services/templates/ingress_federator.yaml
index ea375b0ec4..9b951308a5 100644
--- a/charts/nginx-ingress-services/templates/ingress_federator.yaml
+++ b/charts/nginx-ingress-services/templates/ingress_federator.yaml
@@ -12,6 +12,8 @@ metadata:
     kubernetes.io/ingress.class: "nginx"
     nginx.ingress.kubernetes.io/ssl-redirect: "true"
     nginx.ingress.kubernetes.io/backend-protocol: "GRPC"
+    nginx.ingress.kubernetes.io/auth-tls-verify-client: "on"
+    nginx.ingress.kubernetes.io/auth-tls-secret: "{{ .Release.Namespace }}/{{ include "nginx-ingress-services.getCertificateSecretName" . }}"
 spec:
   tls:
   - hosts:
diff --git a/charts/nginx-ingress-services/templates/secret.yaml b/charts/nginx-ingress-services/templates/secret.yaml
index e0472b0fb4..236fc3769b 100644
--- a/charts/nginx-ingress-services/templates/secret.yaml
+++ b/charts/nginx-ingress-services/templates/secret.yaml
@@ -7,14 +7,16 @@ metadata:
     release: "{{ .Release.Name }}"
     heritage: "{{ .Release.Service }}"
 type: kubernetes.io/tls
+data:
+{{- if (and .Values.federator.enabled .Values.secrets.tlsClientCA) }}
+  ca.crt: {{ .Values.secrets.tlsClientCA | b64enc | quote }}
+{{- end }}
 {{ if .Values.tls.useCertManager -}}
 {{- /* NOTE: providing `data` (and empty strings) allows to manage this secret resource with Helm if cert-manager is used */ -}}
-data:
   tls.crt: ""
   tls.key: ""
 {{- end -}}
 {{- if (not .Values.tls.useCertManager) -}}
-data:
   {{- /* for_helm_linting is necessary only since the 'with' block below does not throw an error upon an empty .Values.secrets */}}
   for_helm_linting: {{ required "No .secrets found in configuration. Did you forget to helm <command> -f path/to/secrets.yaml ?" .Values.secrets | quote | b64enc | quote }}
 

From 32ca0469587361f4cde944e72b7d0cd3654c6b3a Mon Sep 17 00:00:00 2001
From: Paolo Capriotti <paolo@capriotti.io>
Date: Tue, 27 Jul 2021 08:40:24 +0200
Subject: [PATCH 08/41] Validate credentials and add tests

It is now an error to specify a client certificate without a private key
or vice versa. We also fail in case the certificate cannot be parsed,
instead of returning an empty certificate chain.
---
 services/federator/federator.cabal            |   7 +-
 services/federator/package.yaml               |   1 -
 services/federator/src/Federator/Options.hs   |   2 +-
 services/federator/src/Federator/Run.hs       |  33 +++-
 .../federator/test/resources/unit/invalid.pem |   1 +
 .../test/unit/Test/Federator/Options.hs       | 143 +++++++++++++++++-
 6 files changed, 172 insertions(+), 15 deletions(-)
 create mode 100644 services/federator/test/resources/unit/invalid.pem

diff --git a/services/federator/federator.cabal b/services/federator/federator.cabal
index e7575bdc1d..d4371fd60c 100644
--- a/services/federator/federator.cabal
+++ b/services/federator/federator.cabal
@@ -4,7 +4,7 @@ cabal-version: 1.12
 --
 -- see: https://github.com/sol/hpack
 --
--- hash: cdb6771f196b12f78bb882147d96fd428eee25077be318520b83679ed05d5db9
+-- hash: 2683ce1fbc5506e8987313d9d20b3b5aecf2e1edfcc3d5b0165a391a0f5a1c7c
 
 name:           federator
 version:        1.0.0
@@ -18,6 +18,7 @@ build-type:     Simple
 extra-source-files:
     test/resources/integration-ca.pem
     test/resources/unit/gen-certs.sh
+    test/resources/unit/invalid.pem
     test/resources/unit/localhost-dot-key.pem
     test/resources/unit/localhost-dot.pem
     test/resources/unit/localhost-key.pem
@@ -82,7 +83,6 @@ library
     , text
     , tinylog
     , tls
-    , transformers
     , types-common
     , unliftio
     , uri-bytestring
@@ -141,7 +141,6 @@ executable federator
     , text
     , tinylog
     , tls
-    , transformers
     , types-common
     , unliftio
     , uri-bytestring
@@ -210,7 +209,6 @@ executable federator-integration
     , text
     , tinylog
     , tls
-    , transformers
     , types-common
     , unliftio
     , uri-bytestring
@@ -280,7 +278,6 @@ test-suite federator-tests
     , text
     , tinylog
     , tls
-    , transformers
     , types-common
     , unliftio
     , uri-bytestring
diff --git a/services/federator/package.yaml b/services/federator/package.yaml
index bd38ce129d..081e2a9766 100644
--- a/services/federator/package.yaml
+++ b/services/federator/package.yaml
@@ -45,7 +45,6 @@ dependencies:
 - text
 - tinylog
 - tls
-- transformers
 - types-common
 - unliftio
 - uri-bytestring
diff --git a/services/federator/src/Federator/Options.hs b/services/federator/src/Federator/Options.hs
index d489777331..f08d9d1d98 100644
--- a/services/federator/src/Federator/Options.hs
+++ b/services/federator/src/Federator/Options.hs
@@ -66,7 +66,7 @@ data RunSettings = RunSettings
     clientCertificate :: Maybe FilePath,
     clientPrivateKey :: Maybe FilePath
   }
-  deriving (Show, Generic)
+  deriving (Eq, Show, Generic)
 
 defRunSettings :: RunSettings
 defRunSettings =
diff --git a/services/federator/src/Federator/Run.hs b/services/federator/src/Federator/Run.hs
index a108bb2aac..6456ebf8ea 100644
--- a/services/federator/src/Federator/Run.hs
+++ b/services/federator/src/Federator/Run.hs
@@ -27,6 +27,7 @@ module Federator.Run
     -- * App Environment
     newEnv,
     mkTLSSettings,
+    FederationSetupError (..),
     closeEnv,
   )
 where
@@ -34,10 +35,10 @@ where
 import qualified Bilge as RPC
 import Control.Exception (handle, throw)
 import Control.Lens ((^.))
-import Control.Monad.Trans.Maybe (runMaybeT)
 import Data.Default (def)
 import qualified Data.Metrics.Middleware as Metrics
 import Data.Text.Encoding (encodeUtf8)
+import qualified Data.X509 as X509
 import Data.X509.CertificateStore
 import Federator.Env
 import Federator.ExternalServer (serveInward)
@@ -117,13 +118,31 @@ mkCAStore settings = do
       else pure mempty
   pure (customCAStore <> systemCAStore)
 
+getClientCredentials :: RunSettings -> Either String (Maybe (FilePath, FilePath))
+getClientCredentials settings = case clientCertificate settings of
+  Nothing -> noCreds1 $> Nothing
+  Just cert -> Just . (cert,) <$> getCreds1
+  where
+    noCreds1 :: Either String ()
+    noCreds1
+      | isNothing (clientPrivateKey settings) = pure ()
+      | otherwise = Left "invalid client credentials: no certificate"
+
+    getCreds1 :: Either String FilePath
+    getCreds1 =
+      maybe (Left "invalid client credentials: no private key") pure $
+        clientPrivateKey settings
+
 mkCreds :: RunSettings -> IO (Maybe TLS.Credential)
-mkCreds settings = handle h . runMaybeT $ do
-  cert <- maybe mzero pure (clientCertificate settings)
-  key <- maybe mzero pure (clientPrivateKey settings)
-  lift (TLS.credentialLoadX509 cert key) >>= \case
-    Left e -> lift (throw (InvalidClientCertificate e))
-    Right x -> pure x
+mkCreds settings = handle h $ case getClientCredentials settings of
+  Left e -> throw (InvalidClientCertificate e)
+  Right Nothing -> pure Nothing
+  Right (Just (cert, key)) ->
+    TLS.credentialLoadX509 cert key >>= \case
+      Left e -> throw (InvalidClientCertificate e)
+      Right (X509.CertificateChain [], _) ->
+        throw (InvalidClientCertificate "could not read client certificate")
+      Right x -> pure (Just x)
   where
     h :: IOException -> IO a
     h = throw . InvalidClientCertificate . show
diff --git a/services/federator/test/resources/unit/invalid.pem b/services/federator/test/resources/unit/invalid.pem
new file mode 100644
index 0000000000..2716bf650c
--- /dev/null
+++ b/services/federator/test/resources/unit/invalid.pem
@@ -0,0 +1 @@
+not a certificate
diff --git a/services/federator/test/unit/Test/Federator/Options.hs b/services/federator/test/unit/Test/Federator/Options.hs
index d05a62d10c..1eba51b2e1 100644
--- a/services/federator/test/unit/Test/Federator/Options.hs
+++ b/services/federator/test/unit/Test/Federator/Options.hs
@@ -21,12 +21,16 @@
 
 module Test.Federator.Options where
 
+import Control.Exception (try)
+import Control.Lens
 import Data.Aeson (FromJSON)
 import qualified Data.Aeson as Aeson
 import Data.ByteString.Lazy (toStrict)
 import Data.Domain (mkDomain)
 import qualified Data.Yaml as Yaml
+import Federator.Env
 import Federator.Options
+import Federator.Run
 import Imports
 import Test.Tasty
 import Test.Tasty.HUnit
@@ -35,7 +39,8 @@ tests :: TestTree
 tests =
   testGroup
     "Options"
-    [ parseFederationStrategy
+    [ parseFederationStrategy,
+      testTLSSettings
     ]
 
 parseFederationStrategy :: TestTree
@@ -63,6 +68,142 @@ parseFederationStrategy =
     withAllowList =
       AllowList . AllowedDomains . map (either error id . mkDomain)
 
+testTLSSettings :: TestTree
+testTLSSettings =
+  testGroup
+    "TLS settings"
+    [ testCase "succefully read client credentials" $ do
+        let settings =
+              defRunSettings
+                { clientCertificate = Just "test/resources/unit/localhost.pem",
+                  clientPrivateKey = Just "test/resources/unit/localhost-key.pem"
+                }
+        assertParsesAs settings $
+          "useSystemCAStore: true\n\
+          \federationStrategy:\n\
+          \  allowAll: null\n\
+          \clientCertificate: test/resources/unit/localhost.pem\n\
+          \clientPrivateKey: test/resources/unit/localhost-key.pem\n"
+        tlsSettings <- mkTLSSettings settings
+        assertBool "expected TLS client credentials" $
+          notNullOf (creds . _Just) tlsSettings,
+      testCase "parse missing client credentials" $ do
+        let settings = defRunSettings
+        assertParsesAs settings $
+          "useSystemCAStore: true\n\
+          \federationStrategy:\n\
+          \  allowAll: null\n"
+        tlsSettings <- mkTLSSettings settings
+        assertBool "unexpected TLS client credentials" $
+          nullOf (creds . _Just) tlsSettings,
+      testCase "fail on missing client private key" $ do
+        let settings =
+              defRunSettings
+                { clientCertificate = Just "test/resources/unit/localhost.pem"
+                }
+        assertParsesAs settings $
+          "useSystemCAStore: true\n\
+          \federationStrategy:\n\
+          \  allowAll: null\n\
+          \clientCertificate: test/resources/unit/localhost.pem\n"
+        try @FederationSetupError (mkTLSSettings settings) >>= \case
+          Left (InvalidClientCertificate _) -> pure ()
+          Left e ->
+            assertFailure $
+              "expected invalid client certificate exception, got: "
+                <> show e
+          Right tlsSettings ->
+            assertFailure $
+              "expected failure for partial client credentials, got: "
+                <> show (tlsSettings ^. creds),
+      testCase "fail on missing certificate" $ do
+        let settings =
+              defRunSettings
+                { clientPrivateKey = Just "test/resources/unit/localhost-key.pem"
+                }
+        assertParsesAs settings $
+          "useSystemCAStore: true\n\
+          \federationStrategy:\n\
+          \  allowAll: null\n\
+          \clientPrivateKey: test/resources/unit/localhost-key.pem\n"
+        try @FederationSetupError (mkTLSSettings settings) >>= \case
+          Left (InvalidClientCertificate _) -> pure ()
+          Left e ->
+            assertFailure $
+              "expected invalid client certificate exception, got: "
+                <> show e
+          Right tlsSettings ->
+            assertFailure $
+              "expected failure for partial client credentials, got: "
+                <> show (tlsSettings ^. creds),
+      testCase "fail on non-existent certificate" $ do
+        let settings =
+              defRunSettings
+                { clientCertificate = Just "non-existent",
+                  clientPrivateKey = Just "non-existent"
+                }
+        assertParsesAs settings $
+          "useSystemCAStore: true\n\
+          \federationStrategy:\n\
+          \  allowAll: null\n\
+          \clientCertificate: non-existent\n\
+          \clientPrivateKey: non-existent"
+        try @FederationSetupError (mkTLSSettings settings) >>= \case
+          Left (InvalidClientCertificate _) -> pure ()
+          Left e ->
+            assertFailure $
+              "expected invalid client certificate exception, got: "
+                <> show e
+          Right tlsSettings ->
+            assertFailure $
+              "expected failure for non-existing client certificate, got: "
+                <> show (tlsSettings ^. creds),
+      testCase "fail on invalid certificate" $ do
+        let settings =
+              defRunSettings
+                { clientCertificate = Just "test/resources/unit/invalid.pem",
+                  clientPrivateKey = Just "test/resources/unit/localhost-key.pem"
+                }
+        assertParsesAs settings $
+          "useSystemCAStore: true\n\
+          \federationStrategy:\n\
+          \  allowAll: null\n\
+          \clientCertificate: test/resources/unit/invalid.pem\n\
+          \clientPrivateKey: test/resources/unit/localhost-key.pem"
+        try @FederationSetupError (mkTLSSettings settings) >>= \case
+          Left (InvalidClientCertificate _) -> pure ()
+          Left e ->
+            assertFailure $
+              "expected invalid client certificate exception, got: "
+                <> show e
+          Right tlsSettings ->
+            assertFailure $
+              "expected failure for invalid client certificate, got: "
+                <> show (tlsSettings ^. creds),
+      testCase "fail on invalid private key" $ do
+        let settings =
+              defRunSettings
+                { clientCertificate = Just "test/resources/unit/localhost.pem",
+                  clientPrivateKey = Just "test/resources/unit/invalid.pem"
+                }
+        assertParsesAs settings $
+          "useSystemCAStore: true\n\
+          \federationStrategy:\n\
+          \  allowAll: null\n\
+          \clientCertificate: test/resources/unit/localhost.pem\n\
+          \clientPrivateKey: test/resources/unit/invalid.pem"
+        try @FederationSetupError (mkTLSSettings settings) >>= \case
+          Left (InvalidClientCertificate _) -> pure ()
+          Left e ->
+            assertFailure $
+              "expected invalid client certificate exception, got: "
+                <> show e
+          Right tlsSettings ->
+            assertFailure $
+              "expected failure for invalid private key, got: "
+                <> show (tlsSettings ^. creds)
+    ]
+
 assertParsesAs :: (HasCallStack, Eq a, FromJSON a, Show a) => a -> ByteString -> Assertion
 assertParsesAs v bs =
   assertEqual "YAML parsing" (Right v) $

From 6c9973d26b67b3563ea25174edd90f18105d64c8 Mon Sep 17 00:00:00 2001
From: Paolo Capriotti <paolo@capriotti.io>
Date: Tue, 27 Jul 2021 09:02:31 +0200
Subject: [PATCH 09/41] Document client certificate options

Also add examples of federator configuration
---
 docs/reference/config-options.md              | 47 ++++++++++++++++---
 .../test/unit/Test/Federator/Options.hs       | 36 +++++++++++---
 2 files changed, 71 insertions(+), 12 deletions(-)

diff --git a/docs/reference/config-options.md b/docs/reference/config-options.md
index 8b34ab4829..03bbec884f 100644
--- a/docs/reference/config-options.md
+++ b/docs/reference/config-options.md
@@ -224,14 +224,49 @@ federator:
 ### Federation TLS Config
 
 When a federator connects with another federator, it does so over HTTPS. There
-are two options to configure the CA for this:
+are a few options to configure the CA for this:
 1. `useSystemCAStore`: Boolean. If set to `True` it will use the system CA.
-1. `remoteCAStore`: Maybe Filepath. This config option can be used to specify
+2. `remoteCAStore`: Maybe Filepath. This config option can be used to specify
    multiple certificates from either a single file (multiple PEM formatted
    certificates concatenated) or directory (one certificate per file, file names
    are hashes from certificate).
+3. `clientCertificate`: Maybe Filepath. A client certificate to use when
+   connecting to remote federators. If this option is omitted, no client
+   certificate is used. If it is provided, then the `clientPrivateKey` option
+   (see below) must be provided as well.
+4. `clientPrivateKey`: Maybe Filepath. The private key corresponding to the
+   `clientCertificate` option above. It is an error to provide only a private key
+   without the corresponding certificate.
 
-Both of these options can be specified, in this case the stores are concatenated
-and used for verifying certificates. When `useSystemCAStore` is `False` and
-`remoteCAStore` is not set, then all outbound connections will fail with TLS
-error as there will be no CA to verify.
+Both the `useSystemCAStore` and `remoteCAStore` options can be specified, in
+which case the stores are concatenated and used for verifying certificates.
+When `useSystemCAStore` is set to `false` and `remoteCAStore` is not provided,
+all outbound connections will fail with a TLS error as there will be no CA for
+verifying the server certificate.
+
+#### Examples
+
+Federate with anyone, no client certificates, use system CA store to verify
+server certificates:
+
+```yaml
+federator:
+  optSettings:
+    federationStrategy:
+      allowAll:
+    useSystemCAStore: true
+```
+
+Federate only with `server2.example.com`, use a client certificate and a
+specific CA:
+
+```yaml
+federator:
+  optSettings:
+    federationStrategy:
+      allowedDomains:
+        - server2.example.com
+    useSystemCAStore: false
+    clientCertificate: client.pem
+    clientPrivateKey: client-key.pem
+```
diff --git a/services/federator/test/unit/Test/Federator/Options.hs b/services/federator/test/unit/Test/Federator/Options.hs
index 1eba51b2e1..c1dc834a2d 100644
--- a/services/federator/test/unit/Test/Federator/Options.hs
+++ b/services/federator/test/unit/Test/Federator/Options.hs
@@ -26,7 +26,7 @@ import Control.Lens
 import Data.Aeson (FromJSON)
 import qualified Data.Aeson as Aeson
 import Data.ByteString.Lazy (toStrict)
-import Data.Domain (mkDomain)
+import Data.Domain (Domain (..), mkDomain)
 import qualified Data.Yaml as Yaml
 import Federator.Env
 import Federator.Options
@@ -40,7 +40,7 @@ tests =
   testGroup
     "Options"
     [ parseFederationStrategy,
-      testTLSSettings
+      testSettings
     ]
 
 parseFederationStrategy :: TestTree
@@ -68,11 +68,35 @@ parseFederationStrategy =
     withAllowList =
       AllowList . AllowedDomains . map (either error id . mkDomain)
 
-testTLSSettings :: TestTree
-testTLSSettings =
+testSettings :: TestTree
+testSettings =
   testGroup
-    "TLS settings"
-    [ testCase "succefully read client credentials" $ do
+    "settings"
+    [ testCase "parse configuration example (open federation)" $ do
+        assertParsesAs
+          defRunSettings
+          "federationStrategy:\n\
+          \  allowAll:\n\
+          \useSystemCAStore: true",
+      testCase "parse configuration example (closed federation)" $ do
+        let settings =
+              defRunSettings
+                { federationStrategy =
+                    AllowList
+                      ( AllowedDomains [Domain "server2.example.com"]
+                      ),
+                  useSystemCAStore = False,
+                  clientCertificate = Just "client.pem",
+                  clientPrivateKey = Just "client-key.pem"
+                }
+        assertParsesAs settings $
+          "federationStrategy:\n\
+          \  allowedDomains:\n\
+          \    - server2.example.com\n\
+          \useSystemCAStore: false\n\
+          \clientCertificate: client.pem\n\
+          \clientPrivateKey: client-key.pem",
+      testCase "succefully read client credentials" $ do
         let settings =
               defRunSettings
                 { clientCertificate = Just "test/resources/unit/localhost.pem",

From a4bec3fe62c6aef2a8581eff68afc4f6e7b1e2a3 Mon Sep 17 00:00:00 2001
From: Paolo Capriotti <paolo@capriotti.io>
Date: Tue, 27 Jul 2021 09:28:32 +0200
Subject: [PATCH 10/41] Update CHANGELOG

---
 CHANGELOG.md | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 35e8d14978..4d1ddc9015 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -22,7 +22,7 @@
 
 # [unreleased]
 
-[please put all changes that only affect federation into this section to unclutter the rest of the release notes.]
+[please put all changes that only affect federation into the "Federation changes" section to unclutter the rest of the release notes.]
 [if something is both an API change and a feature, please mention it twice (you can abbreviate the second mention and add "see above").]
 
 ## Release Notes
@@ -42,6 +42,10 @@
 * Replaced uses of `UVerb` and `EmptyResult` with `MultiVerb` (#1693)
 * Added a mechanism to derive `AsUnion` instances automatically (#1693)
 
+## Federation changes
+
+* Added client certificate support for server to server authentication (#1682)
+
 # [2021-08-02]
 
 ## Release Notes
@@ -93,7 +97,6 @@ Upgrade nginz (#1658)
 * Renamed `DomainHeader` type to `OriginDomainHeader` (#1689)
 * Added golden tests for protobuf serialisation / deserialisation (#1644).
 
-
 # [2021-07-09]
 
 ## Release Notes

From 748103254f8d40b74ae49fa78d76eaf5d569dc80 Mon Sep 17 00:00:00 2001
From: Paolo Capriotti <paolo@capriotti.io>
Date: Wed, 28 Jul 2021 09:27:12 +0200
Subject: [PATCH 11/41] Fix federator-integration chart

---
 .../templates/tests/federator-integration.yaml         | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/charts/federator/templates/tests/federator-integration.yaml b/charts/federator/templates/tests/federator-integration.yaml
index 6891e7dbc5..f2ae63671b 100644
--- a/charts/federator/templates/tests/federator-integration.yaml
+++ b/charts/federator/templates/tests/federator-integration.yaml
@@ -13,10 +13,10 @@ spec:
     - name: "federator-config"
       configMap:
         name: "federator"
-    # integration tests need access to the CA
-    - name: "federator-ca"
+    # integration tests need access to the client certificate private key
+    - name: "federator-secrets"
       secret:
-        secretName: "federator-ca"
+        secretName: "federator-secrets"
   containers:
   - name: integration
     command: ["federator-integration"]
@@ -26,6 +26,6 @@ spec:
       mountPath: "/etc/wire/integration"
     - name: "federator-config"
       mountPath: "/etc/wire/federator/conf"
-    - name: "federator-ca"
-      mountPath: "/etc/wire/federator/ca"
+    - name: "federator-secrets"
+      mountPath: "/etc/wire/federator/secrets"
   restartPolicy: Never

From e642d0ea0cb141a2c34dd95d5955659bf3bbc587 Mon Sep 17 00:00:00 2001
From: jschaul <jschaul@users.noreply.github.com>
Date: Wed, 28 Jul 2021 14:33:06 +0200
Subject: [PATCH 12/41] try out helm templating also when using certificates

---
 hack/bin/helm-template.sh | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hack/bin/helm-template.sh b/hack/bin/helm-template.sh
index 0cd1306f56..d72684ce9b 100755
--- a/hack/bin/helm-template.sh
+++ b/hack/bin/helm-template.sh
@@ -15,10 +15,14 @@ TOP_LEVEL="$DIR/../.."
 CHARTS_DIR="${TOP_LEVEL}/.local/charts"
 
 valuesfile="${DIR}/../helm_vars/${chart}/values.yaml"
+certificatesfile="${DIR}/../helm_vars/${chart}/certificates.yaml"
 declare -a options=()
 if [ -f "$valuesfile" ]; then
     options+=(-f "$valuesfile")
 fi
+if [ -f "$certificatesfile" ]; then
+    options+=(-f "$certificatesfile")
+fi
 
 "$DIR/update.sh" "$CHARTS_DIR/$chart"
 helm template $"chart" "$CHARTS_DIR/$chart" ${options[*]}

From 90af59ce30a146dfa209c8ecd0d4102fdefe9660 Mon Sep 17 00:00:00 2001
From: Paolo Capriotti <paolo@capriotti.io>
Date: Thu, 29 Jul 2021 18:02:04 +0200
Subject: [PATCH 13/41] Add leaf certificates for integration tests

---
 services/federator/test/resources/integration-leaf-key.pem | 1 +
 services/federator/test/resources/integration-leaf.pem     | 1 +
 2 files changed, 2 insertions(+)
 create mode 120000 services/federator/test/resources/integration-leaf-key.pem
 create mode 120000 services/federator/test/resources/integration-leaf.pem

diff --git a/services/federator/test/resources/integration-leaf-key.pem b/services/federator/test/resources/integration-leaf-key.pem
new file mode 120000
index 0000000000..f5d4e842e4
--- /dev/null
+++ b/services/federator/test/resources/integration-leaf-key.pem
@@ -0,0 +1 @@
+../../../../deploy/services-demo/conf/nginz/integration-leaf-key.pem
\ No newline at end of file
diff --git a/services/federator/test/resources/integration-leaf.pem b/services/federator/test/resources/integration-leaf.pem
new file mode 120000
index 0000000000..8e5558292c
--- /dev/null
+++ b/services/federator/test/resources/integration-leaf.pem
@@ -0,0 +1 @@
+../../../../deploy/services-demo/conf/nginz/integration-leaf.pem
\ No newline at end of file

From dc701986b883f0a2dbf2dbeb72e75fc9f9ca1ed5 Mon Sep 17 00:00:00 2001
From: Paolo Capriotti <paolo@capriotti.io>
Date: Fri, 30 Jul 2021 06:50:41 +0200
Subject: [PATCH 14/41] Remove default value for useSystemCAStore from template

Co-authored-by: jschaul <jschaul@users.noreply.github.com>
---
 charts/federator/templates/configmap.yaml | 1 -
 1 file changed, 1 deletion(-)

diff --git a/charts/federator/templates/configmap.yaml b/charts/federator/templates/configmap.yaml
index d097ef6908..11f947233b 100644
--- a/charts/federator/templates/configmap.yaml
+++ b/charts/federator/templates/configmap.yaml
@@ -45,7 +45,6 @@ data:
       {{- if $.Values.remoteCAContents }}
       remoteCAStore: "/etc/wire/federator/conf/remote-ca.pem"
       {{- end }}
-      useSystemCAStore: false
       {{- if $.Values.clientCertificateContents }}
       clientCertificate: "/etc/wire/federator/conf/client.pem"
       clientPrivateKey: "/etc/wire/federator/secrets/client-key.pem"

From 68579e0dee26e9ef8f5fd2facba09f6254d8e9f1 Mon Sep 17 00:00:00 2001
From: Paolo Capriotti <paolo@capriotti.io>
Date: Fri, 30 Jul 2021 07:18:17 +0200
Subject: [PATCH 15/41] Move defRunSettings to a test module

---
 services/federator/federator.cabal                     |  4 +++-
 services/federator/src/Federator/Options.hs            | 10 ----------
 .../test/unit/Test/Federator/ExternalServer.hs         |  2 +-
 .../test/unit/Test/Federator/InternalServer.hs         |  3 ++-
 services/federator/test/unit/Test/Federator/Options.hs | 10 ++++++++++
 services/federator/test/unit/Test/Federator/Remote.hs  |  1 +
 .../federator/test/unit/Test/Federator/Validation.hs   |  1 +
 7 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/services/federator/federator.cabal b/services/federator/federator.cabal
index d4371fd60c..dbf05f0824 100644
--- a/services/federator/federator.cabal
+++ b/services/federator/federator.cabal
@@ -4,7 +4,7 @@ cabal-version: 1.12
 --
 -- see: https://github.com/sol/hpack
 --
--- hash: 2683ce1fbc5506e8987313d9d20b3b5aecf2e1edfcc3d5b0165a391a0f5a1c7c
+-- hash: 451ccf76c662e1aed949e16099a7c771478e2563768df87b145adf53f4c0f4b8
 
 name:           federator
 version:        1.0.0
@@ -17,6 +17,8 @@ license:        AGPL-3
 build-type:     Simple
 extra-source-files:
     test/resources/integration-ca.pem
+    test/resources/integration-leaf-key.pem
+    test/resources/integration-leaf.pem
     test/resources/unit/gen-certs.sh
     test/resources/unit/invalid.pem
     test/resources/unit/localhost-dot-key.pem
diff --git a/services/federator/src/Federator/Options.hs b/services/federator/src/Federator/Options.hs
index f08d9d1d98..e8a8c651ed 100644
--- a/services/federator/src/Federator/Options.hs
+++ b/services/federator/src/Federator/Options.hs
@@ -68,16 +68,6 @@ data RunSettings = RunSettings
   }
   deriving (Eq, Show, Generic)
 
-defRunSettings :: RunSettings
-defRunSettings =
-  RunSettings
-    { federationStrategy = AllowAll,
-      useSystemCAStore = True,
-      remoteCAStore = Nothing,
-      clientCertificate = Nothing,
-      clientPrivateKey = Nothing
-    }
-
 instance FromJSON RunSettings
 
 data Opts = Opts
diff --git a/services/federator/test/unit/Test/Federator/ExternalServer.hs b/services/federator/test/unit/Test/Federator/ExternalServer.hs
index 854f7bc331..f06140f839 100644
--- a/services/federator/test/unit/Test/Federator/ExternalServer.hs
+++ b/services/federator/test/unit/Test/Federator/ExternalServer.hs
@@ -22,13 +22,13 @@ module Test.Federator.ExternalServer where
 import Data.Domain (Domain (..))
 import Data.String.Conversions (cs)
 import Federator.ExternalServer (callLocal)
-import Federator.Options (defRunSettings)
 import Federator.Service (Service)
 import Imports
 import qualified Network.HTTP.Types as HTTP
 import Polysemy (embed, runM)
 import qualified Polysemy.Reader as Polysemy
 import qualified Polysemy.TinyLog as TinyLog
+import Test.Federator.Options (defRunSettings)
 import Test.Polysemy.Mock (Mock (mock), evalMock)
 import Test.Polysemy.Mock.TH (genMock)
 import Test.Tasty (TestTree, testGroup)
diff --git a/services/federator/test/unit/Test/Federator/InternalServer.hs b/services/federator/test/unit/Test/Federator/InternalServer.hs
index 691b573c33..192817a76a 100644
--- a/services/federator/test/unit/Test/Federator/InternalServer.hs
+++ b/services/federator/test/unit/Test/Federator/InternalServer.hs
@@ -22,13 +22,14 @@ module Test.Federator.InternalServer (tests) where
 import Data.Domain (Domain (Domain))
 import Federator.Discovery (LookupError (LookupErrorDNSError, LookupErrorSrvNotAvailable))
 import Federator.InternalServer (callOutward)
-import Federator.Options (AllowedDomains (..), FederationStrategy (..), RunSettings (..), defRunSettings)
+import Federator.Options (AllowedDomains (..), FederationStrategy (..), RunSettings (..))
 import Federator.Remote (Remote, RemoteError (RemoteErrorDiscoveryFailure))
 import Imports
 import Mu.GRpc.Client.Record
 import Network.HTTP2.Client (TooMuchConcurrency (TooMuchConcurrency))
 import Polysemy (embed, runM)
 import qualified Polysemy.Reader as Polysemy
+import Test.Federator.Options (defRunSettings)
 import Test.Polysemy.Mock (Mock (mock), evalMock)
 import Test.Polysemy.Mock.TH (genMock)
 import Test.Tasty (TestTree, testGroup)
diff --git a/services/federator/test/unit/Test/Federator/Options.hs b/services/federator/test/unit/Test/Federator/Options.hs
index c1dc834a2d..f58c0a2a3e 100644
--- a/services/federator/test/unit/Test/Federator/Options.hs
+++ b/services/federator/test/unit/Test/Federator/Options.hs
@@ -35,6 +35,16 @@ import Imports
 import Test.Tasty
 import Test.Tasty.HUnit
 
+defRunSettings :: RunSettings
+defRunSettings =
+  RunSettings
+    { federationStrategy = AllowAll,
+      useSystemCAStore = True,
+      remoteCAStore = Nothing,
+      clientCertificate = Nothing,
+      clientPrivateKey = Nothing
+    }
+
 tests :: TestTree
 tests =
   testGroup
diff --git a/services/federator/test/unit/Test/Federator/Remote.hs b/services/federator/test/unit/Test/Federator/Remote.hs
index 7168acbe5a..6ab6a5b077 100644
--- a/services/federator/test/unit/Test/Federator/Remote.hs
+++ b/services/federator/test/unit/Test/Federator/Remote.hs
@@ -14,6 +14,7 @@ import qualified Network.Wai.Handler.WarpTLS as WarpTLS
 import qualified Polysemy
 import qualified Polysemy.Reader as Polysemy
 import qualified Polysemy.TinyLog as TinyLog
+import Test.Federator.Options (defRunSettings)
 import Test.Tasty
 import Test.Tasty.HUnit
 import UnliftIO (bracket, timeout)
diff --git a/services/federator/test/unit/Test/Federator/Validation.hs b/services/federator/test/unit/Test/Federator/Validation.hs
index 112fe89cef..89d5b84818 100644
--- a/services/federator/test/unit/Test/Federator/Validation.hs
+++ b/services/federator/test/unit/Test/Federator/Validation.hs
@@ -32,6 +32,7 @@ import Polysemy.Embed
 import qualified Polysemy.Error as Polysemy
 import qualified Polysemy.Reader as Polysemy
 import Test.Federator.InternalServer ()
+import Test.Federator.Options (defRunSettings)
 import Test.Polysemy.Mock (evalMock)
 import Test.Tasty
 import Test.Tasty.HUnit

From bdb94902f3fb3dccd0656966e7c517ccd0ace77a Mon Sep 17 00:00:00 2001
From: Paolo Capriotti <paolo@capriotti.io>
Date: Fri, 30 Jul 2021 07:39:52 +0200
Subject: [PATCH 16/41] Use interpolate QQ for configuration tests

---
 services/federator/federator.cabal            |   3 +-
 services/federator/package.yaml               |   1 +
 .../test/unit/Test/Federator/Options.hs       | 125 ++++++++++--------
 3 files changed, 73 insertions(+), 56 deletions(-)

diff --git a/services/federator/federator.cabal b/services/federator/federator.cabal
index dbf05f0824..3fa1acfcab 100644
--- a/services/federator/federator.cabal
+++ b/services/federator/federator.cabal
@@ -4,7 +4,7 @@ cabal-version: 1.12
 --
 -- see: https://github.com/sol/hpack
 --
--- hash: 451ccf76c662e1aed949e16099a7c771478e2563768df87b145adf53f4c0f4b8
+-- hash: e2fb4c3ece84a865c43c0a7b1afb561dbe57b3d45bf71269a45cffe320a1ad67
 
 name:           federator
 version:        1.0.0
@@ -259,6 +259,7 @@ test-suite federator-tests
     , http2-client
     , http2-client-grpc
     , imports
+    , interpolate
     , lens
     , metrics-core
     , metrics-wai
diff --git a/services/federator/package.yaml b/services/federator/package.yaml
index 081e2a9766..6fbd5c2701 100644
--- a/services/federator/package.yaml
+++ b/services/federator/package.yaml
@@ -98,6 +98,7 @@ tests:
     dependencies:
     - bytestring
     - federator
+    - interpolate
     - polysemy-mocks
     - streaming-commons
     - tasty
diff --git a/services/federator/test/unit/Test/Federator/Options.hs b/services/federator/test/unit/Test/Federator/Options.hs
index f58c0a2a3e..ce797f8ea6 100644
--- a/services/federator/test/unit/Test/Federator/Options.hs
+++ b/services/federator/test/unit/Test/Federator/Options.hs
@@ -25,8 +25,10 @@ import Control.Exception (try)
 import Control.Lens
 import Data.Aeson (FromJSON)
 import qualified Data.Aeson as Aeson
+import qualified Data.ByteString.Char8 as B8
 import Data.ByteString.Lazy (toStrict)
 import Data.Domain (Domain (..), mkDomain)
+import Data.String.Interpolate as QQ
 import qualified Data.Yaml as Yaml
 import Federator.Env
 import Federator.Options
@@ -60,13 +62,15 @@ parseFederationStrategy =
       "allowAll: null"
     assertParsesAs (withAllowList []) $
       "allowedDomains: []"
-    assertParsesAs (withAllowList ["test.org"]) $
-      "allowedDomains:\n\
-      \  - test.org"
-    assertParsesAs (withAllowList ["example.com", "wire.com"]) $
-      "allowedDomains:\n\
-      \  - example.com\n\
-      \  - wire.com"
+    assertParsesAs (withAllowList ["test.org"]) . B8.pack $
+      [QQ.i|
+      allowedDomains:
+        - test.org|]
+    assertParsesAs (withAllowList ["example.com", "wire.com"]) . B8.pack $
+      [QQ.i|
+      allowedDomains:
+        - example.com
+        - wire.com|]
     -- manual roundtrip example AllowAll
     let allowA = toStrict $ Aeson.encode AllowAll
     assertParsesAs AllowAll $ allowA
@@ -85,9 +89,12 @@ testSettings =
     [ testCase "parse configuration example (open federation)" $ do
         assertParsesAs
           defRunSettings
-          "federationStrategy:\n\
-          \  allowAll:\n\
-          \useSystemCAStore: true",
+          ( B8.pack
+              [QQ.i|
+                federationStrategy:
+                  allowAll:
+                useSystemCAStore: true|]
+          ),
       testCase "parse configuration example (closed federation)" $ do
         let settings =
               defRunSettings
@@ -99,34 +106,37 @@ testSettings =
                   clientCertificate = Just "client.pem",
                   clientPrivateKey = Just "client-key.pem"
                 }
-        assertParsesAs settings $
-          "federationStrategy:\n\
-          \  allowedDomains:\n\
-          \    - server2.example.com\n\
-          \useSystemCAStore: false\n\
-          \clientCertificate: client.pem\n\
-          \clientPrivateKey: client-key.pem",
+        assertParsesAs settings . B8.pack $
+          [QQ.i|
+          federationStrategy:
+            allowedDomains:
+              - server2.example.com
+          useSystemCAStore: false
+          clientCertificate: client.pem
+          clientPrivateKey: client-key.pem|],
       testCase "succefully read client credentials" $ do
         let settings =
               defRunSettings
                 { clientCertificate = Just "test/resources/unit/localhost.pem",
                   clientPrivateKey = Just "test/resources/unit/localhost-key.pem"
                 }
-        assertParsesAs settings $
-          "useSystemCAStore: true\n\
-          \federationStrategy:\n\
-          \  allowAll: null\n\
-          \clientCertificate: test/resources/unit/localhost.pem\n\
-          \clientPrivateKey: test/resources/unit/localhost-key.pem\n"
+        assertParsesAs settings . B8.pack $
+          [QQ.i|
+          useSystemCAStore: true
+          federationStrategy:
+            allowAll: null
+          clientCertificate: test/resources/unit/localhost.pem
+          clientPrivateKey: test/resources/unit/localhost-key.pem|]
         tlsSettings <- mkTLSSettings settings
         assertBool "expected TLS client credentials" $
           notNullOf (creds . _Just) tlsSettings,
       testCase "parse missing client credentials" $ do
         let settings = defRunSettings
-        assertParsesAs settings $
-          "useSystemCAStore: true\n\
-          \federationStrategy:\n\
-          \  allowAll: null\n"
+        assertParsesAs settings . B8.pack $
+          [QQ.i|
+          useSystemCAStore: true
+          federationStrategy:
+            allowAll: null|]
         tlsSettings <- mkTLSSettings settings
         assertBool "unexpected TLS client credentials" $
           nullOf (creds . _Just) tlsSettings,
@@ -135,11 +145,12 @@ testSettings =
               defRunSettings
                 { clientCertificate = Just "test/resources/unit/localhost.pem"
                 }
-        assertParsesAs settings $
-          "useSystemCAStore: true\n\
-          \federationStrategy:\n\
-          \  allowAll: null\n\
-          \clientCertificate: test/resources/unit/localhost.pem\n"
+        assertParsesAs settings . B8.pack $
+          [QQ.i|
+          useSystemCAStore: true
+          federationStrategy:
+            allowAll: null
+          clientCertificate: test/resources/unit/localhost.pem|]
         try @FederationSetupError (mkTLSSettings settings) >>= \case
           Left (InvalidClientCertificate _) -> pure ()
           Left e ->
@@ -155,11 +166,12 @@ testSettings =
               defRunSettings
                 { clientPrivateKey = Just "test/resources/unit/localhost-key.pem"
                 }
-        assertParsesAs settings $
-          "useSystemCAStore: true\n\
-          \federationStrategy:\n\
-          \  allowAll: null\n\
-          \clientPrivateKey: test/resources/unit/localhost-key.pem\n"
+        assertParsesAs settings . B8.pack $
+          [QQ.i|
+          useSystemCAStore: true
+          federationStrategy:
+            allowAll: null
+          clientPrivateKey: test/resources/unit/localhost-key.pem|]
         try @FederationSetupError (mkTLSSettings settings) >>= \case
           Left (InvalidClientCertificate _) -> pure ()
           Left e ->
@@ -176,12 +188,13 @@ testSettings =
                 { clientCertificate = Just "non-existent",
                   clientPrivateKey = Just "non-existent"
                 }
-        assertParsesAs settings $
-          "useSystemCAStore: true\n\
-          \federationStrategy:\n\
-          \  allowAll: null\n\
-          \clientCertificate: non-existent\n\
-          \clientPrivateKey: non-existent"
+        assertParsesAs settings . B8.pack $
+          [QQ.i|
+          useSystemCAStore: true
+          federationStrategy:
+            allowAll: null
+          clientCertificate: non-existent
+          clientPrivateKey: non-existent|]
         try @FederationSetupError (mkTLSSettings settings) >>= \case
           Left (InvalidClientCertificate _) -> pure ()
           Left e ->
@@ -198,12 +211,13 @@ testSettings =
                 { clientCertificate = Just "test/resources/unit/invalid.pem",
                   clientPrivateKey = Just "test/resources/unit/localhost-key.pem"
                 }
-        assertParsesAs settings $
-          "useSystemCAStore: true\n\
-          \federationStrategy:\n\
-          \  allowAll: null\n\
-          \clientCertificate: test/resources/unit/invalid.pem\n\
-          \clientPrivateKey: test/resources/unit/localhost-key.pem"
+        assertParsesAs settings . B8.pack $
+          [QQ.i|
+          useSystemCAStore: true
+          federationStrategy:
+            allowAll: null
+          clientCertificate: test/resources/unit/invalid.pem
+          clientPrivateKey: test/resources/unit/localhost-key.pem|]
         try @FederationSetupError (mkTLSSettings settings) >>= \case
           Left (InvalidClientCertificate _) -> pure ()
           Left e ->
@@ -220,12 +234,13 @@ testSettings =
                 { clientCertificate = Just "test/resources/unit/localhost.pem",
                   clientPrivateKey = Just "test/resources/unit/invalid.pem"
                 }
-        assertParsesAs settings $
-          "useSystemCAStore: true\n\
-          \federationStrategy:\n\
-          \  allowAll: null\n\
-          \clientCertificate: test/resources/unit/localhost.pem\n\
-          \clientPrivateKey: test/resources/unit/invalid.pem"
+        assertParsesAs settings . B8.pack $
+          [QQ.i|
+          useSystemCAStore: true
+          federationStrategy:
+            allowAll: null
+          clientCertificate: test/resources/unit/localhost.pem
+          clientPrivateKey: test/resources/unit/invalid.pem|]
         try @FederationSetupError (mkTLSSettings settings) >>= \case
           Left (InvalidClientCertificate _) -> pure ()
           Left e ->

From 6fc85ba36cf85ad529faabcc8e98524956f3376b Mon Sep 17 00:00:00 2001
From: Paolo Capriotti <paolo@capriotti.io>
Date: Fri, 30 Jul 2021 14:58:21 +0200
Subject: [PATCH 17/41] Make federator-secret of type tls

This makes federator-secret look exactly like the secret used by
nginx-ingress-services, which makes it possible to use a single secret
for both.
---
 charts/federator/templates/configmap.yaml     | 13 +++---------
 charts/federator/templates/deployment.yaml    |  8 ++++---
 charts/federator/templates/secret.yaml        | 21 +++++++++++++++++++
 charts/federator/templates/secrets.yaml       | 13 ------------
 .../templates/secret.yaml                     | 13 ++++++------
 5 files changed, 35 insertions(+), 33 deletions(-)
 create mode 100644 charts/federator/templates/secret.yaml
 delete mode 100644 charts/federator/templates/secrets.yaml

diff --git a/charts/federator/templates/configmap.yaml b/charts/federator/templates/configmap.yaml
index 11f947233b..6783c06e1b 100644
--- a/charts/federator/templates/configmap.yaml
+++ b/charts/federator/templates/configmap.yaml
@@ -43,11 +43,11 @@ data:
       # Filepath to one or more PEM-encoded server certificates to use as a trust
       # store when making grpc requests to remote backends
       {{- if $.Values.remoteCAContents }}
-      remoteCAStore: "/etc/wire/federator/conf/remote-ca.pem"
+      remoteCAStore: "/etc/wire/federator/secrets/ca.crt"
       {{- end }}
       {{- if $.Values.clientCertificateContents }}
-      clientCertificate: "/etc/wire/federator/conf/client.pem"
-      clientPrivateKey: "/etc/wire/federator/secrets/client-key.pem"
+      clientCertificate: "/etc/wire/federator/secrets/tls.crt"
+      clientPrivateKey: "/etc/wire/federator/secrets/tls.key"
       {{- end }}
       useSystemCAStore: {{ .useSystemCAStore }}
       federationStrategy:
@@ -66,10 +66,3 @@ data:
     {{- end }}
     {{- end }}
 
-  # TODO: add validation and fail early during templating: either contents should be provided; or explicitly system trust store enabled
-  {{- if .Values.remoteCAContents }}
-  remote-ca.pem: {{ .Values.remoteCAContents | quote }}
-  {{- end }}
-  {{- if .Values.clientCertificateContents }}
-  client.pem: {{ .Values.clientCertificateContents | quote }}
-  {{- end }}
diff --git a/charts/federator/templates/deployment.yaml b/charts/federator/templates/deployment.yaml
index 2f305b7db3..92bb6fdee0 100644
--- a/charts/federator/templates/deployment.yaml
+++ b/charts/federator/templates/deployment.yaml
@@ -32,9 +32,11 @@ spec:
         - name: "federator-config"
           configMap:
             name: "federator"
-        # federator-secrets holds the private key for the client certificate to use
-        # when making requests to remote backends
-        - name: "federator-secrets"
+
+        # federator-secrets contains the client certificate and the
+        # corresponding private key to use # when making requests to remote
+        # backends, as well as the CA used to verify the server certificate
+        - name: "federator-secret"
           secret:
             secretName: "federator-secrets"
       containers:
diff --git a/charts/federator/templates/secret.yaml b/charts/federator/templates/secret.yaml
new file mode 100644
index 0000000000..cccd6a1575
--- /dev/null
+++ b/charts/federator/templates/secret.yaml
@@ -0,0 +1,21 @@
+apiVersion: v1
+kind: Secret
+metadata:
+  name: "federator-secret"
+  labels:
+    wireService: federator
+    chart: {{ .Chart.Name }}-{{ .Chart.Version | replace "+" "_" }}
+    release: {{ .Release.Name }}
+    heritage: {{ .Release.Service }}
+type: kubernetes.io/tls
+data:
+{{- if .Values.remoteCAContents }}
+  ca.crt: {{ .Values.remoteCAContents | b64enc | quote }}
+{{- end -}}
+  # TODO: add validation and fail early during templating: either contents should be provided; or explicitly system trust store enabled
+  {{- if .Values.clientPrivateKeyContents }}
+  tls.key: {{ .Values.clientPrivateKeyContents | b64enc | quote }}
+  {{- end -}}
+  {{- if .Values.clientCertificateContents }}
+  tls.crt: {{ .Values.clientCertificateContents | b64enc | quote }}
+  {{- end -}}
diff --git a/charts/federator/templates/secrets.yaml b/charts/federator/templates/secrets.yaml
deleted file mode 100644
index 487ad8ca7e..0000000000
--- a/charts/federator/templates/secrets.yaml
+++ /dev/null
@@ -1,13 +0,0 @@
-apiVersion: v1
-kind: Secret
-metadata:
-  name: "federator-secrets"
-  labels:
-    wireService: federator
-    chart: {{ .Chart.Name }}-{{ .Chart.Version | replace "+" "_" }}
-    release: {{ .Release.Name }}
-    heritage: {{ .Release.Service }}
-data:
-  {{- if .Values.clientPrivateKeyContents }}
-  client-key.pem: {{ .Values.clientPrivateKeyContents | b64enc | quote }}
-  {{- end }}
diff --git a/charts/nginx-ingress-services/templates/secret.yaml b/charts/nginx-ingress-services/templates/secret.yaml
index 236fc3769b..0b93364525 100644
--- a/charts/nginx-ingress-services/templates/secret.yaml
+++ b/charts/nginx-ingress-services/templates/secret.yaml
@@ -10,18 +10,17 @@ type: kubernetes.io/tls
 data:
 {{- if (and .Values.federator.enabled .Values.secrets.tlsClientCA) }}
   ca.crt: {{ .Values.secrets.tlsClientCA | b64enc | quote }}
-{{- end }}
-{{ if .Values.tls.useCertManager -}}
-{{- /* NOTE: providing `data` (and empty strings) allows to manage this secret resource with Helm if cert-manager is used */ -}}
+{{- end -}}
+{{- if .Values.tls.useCertManager }}
+  {{/* NOTE: providing `data` (and empty strings) allows to manage this secret resource with Helm if cert-manager is used */}}
   tls.crt: ""
   tls.key: ""
-{{- end -}}
-{{- if (not .Values.tls.useCertManager) -}}
-  {{- /* for_helm_linting is necessary only since the 'with' block below does not throw an error upon an empty .Values.secrets */}}
+{{- else }}
+  {{/* for_helm_linting is necessary only since the 'with' block below does not throw an error upon an empty .Values.secrets */}}
   for_helm_linting: {{ required "No .secrets found in configuration. Did you forget to helm <command> -f path/to/secrets.yaml ?" .Values.secrets | quote | b64enc | quote }}
 
   {{- with .Values.secrets }}
   tls.crt: {{ .tlsWildcardCert | b64enc | quote }}
   tls.key: {{ .tlsWildcardKey | b64enc | quote }}
-  {{- end }}
+  {{- end -}}
 {{- end -}}

From 629792ec4d6c23108c1f5840d53d3acf8dc8911f Mon Sep 17 00:00:00 2001
From: Paolo Capriotti <paolo@capriotti.io>
Date: Fri, 30 Jul 2021 15:52:58 +0200
Subject: [PATCH 18/41] Use a different certificate for federator

This makes it less awkward to use the server certificate for the
federator ingress also as a client certificate for requests to other
federators. Otherwise, the client certificate would contain extraneous
domains.

This also uses Let's Encrypt intemediate CA (from their staging
environment) as a default value for the CA in the secret.
---
 .../templates/certificate.yaml                |  3 -
 .../templates/certificate_federator.yaml      | 30 ++++++++++
 .../templates/ingress_federator.yaml          |  4 +-
 .../templates/secret.yaml                     |  3 -
 .../templates/secret_federator.yaml           | 55 +++++++++++++++++++
 .../templates/staging_issuer.yaml             | 22 ++++++++
 6 files changed, 109 insertions(+), 8 deletions(-)
 create mode 100644 charts/nginx-ingress-services/templates/certificate_federator.yaml
 create mode 100644 charts/nginx-ingress-services/templates/secret_federator.yaml
 create mode 100644 charts/nginx-ingress-services/templates/staging_issuer.yaml

diff --git a/charts/nginx-ingress-services/templates/certificate.yaml b/charts/nginx-ingress-services/templates/certificate.yaml
index bf2561d9d9..21975e93c7 100644
--- a/charts/nginx-ingress-services/templates/certificate.yaml
+++ b/charts/nginx-ingress-services/templates/certificate.yaml
@@ -36,7 +36,4 @@ spec:
     {{- if .Values.accountPages.enabled }}
     - {{ .Values.config.dns.accountPages }}
     {{- end }}
-    {{- if .Values.federator.enabled }}
-    - {{ .Values.config.dns.federator }}
-    {{- end }}
 {{- end -}}
diff --git a/charts/nginx-ingress-services/templates/certificate_federator.yaml b/charts/nginx-ingress-services/templates/certificate_federator.yaml
new file mode 100644
index 0000000000..a2e3e506fc
--- /dev/null
+++ b/charts/nginx-ingress-services/templates/certificate_federator.yaml
@@ -0,0 +1,30 @@
+{{- if and .Values.tls.enabled .Values.tls.useCertManager .Values.federator.enabled }}
+apiVersion: cert-manager.io/v1alpha2
+kind: Certificate
+metadata:
+  name: "federator-{{ include "nginx-ingress-services.zone" . | replace "." "-" }}-csr"
+  namespace: {{ .Release.Namespace }}
+  labels:
+    chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
+    release: "{{ .Release.Name }}"
+    heritage: "{{ .Release.Service }}"
+spec:
+  issuerRef:
+    name: letsencrypt-staging
+    kind: Issuer
+  usages:
+    - server auth
+  duration: 2160h     # 90d, Letsencrypt default; NOTE: changes are ignored by Letsencrypt
+  renewBefore: 360h   # 15d
+  isCA: false
+  keyAlgorithm: ecdsa
+  keySize: 384        # 521 is not supported by Letsencrypt
+  keyEncoding: pkcs1
+  secretName: federator-certificate-secret
+  # NOTE: disabled due to https://github.com/jetstack/cert-manager/issues/2978
+  # TODO: enable when fixed (probably when cert-manager:v0.16 released)
+  #privateKey:
+  #  rotationPolicy: Always
+  dnsNames:
+    - {{ .Values.config.dns.federator }}
+{{- end -}}
diff --git a/charts/nginx-ingress-services/templates/ingress_federator.yaml b/charts/nginx-ingress-services/templates/ingress_federator.yaml
index 9b951308a5..20ce8fac9a 100644
--- a/charts/nginx-ingress-services/templates/ingress_federator.yaml
+++ b/charts/nginx-ingress-services/templates/ingress_federator.yaml
@@ -13,12 +13,12 @@ metadata:
     nginx.ingress.kubernetes.io/ssl-redirect: "true"
     nginx.ingress.kubernetes.io/backend-protocol: "GRPC"
     nginx.ingress.kubernetes.io/auth-tls-verify-client: "on"
-    nginx.ingress.kubernetes.io/auth-tls-secret: "{{ .Release.Namespace }}/{{ include "nginx-ingress-services.getCertificateSecretName" . }}"
+    nginx.ingress.kubernetes.io/auth-tls-secret: "{{ .Release.Namespace }}/federator-certificate-secret"
 spec:
   tls:
   - hosts:
       - {{ .Values.config.dns.federator }}
-    secretName: {{ include "nginx-ingress-services.getCertificateSecretName" . | quote }}
+    secretName: "federator-certificate-secret"
   rules:
     - host: {{ .Values.config.dns.federator }}
       http:
diff --git a/charts/nginx-ingress-services/templates/secret.yaml b/charts/nginx-ingress-services/templates/secret.yaml
index 0b93364525..aa9e3e9627 100644
--- a/charts/nginx-ingress-services/templates/secret.yaml
+++ b/charts/nginx-ingress-services/templates/secret.yaml
@@ -8,9 +8,6 @@ metadata:
     heritage: "{{ .Release.Service }}"
 type: kubernetes.io/tls
 data:
-{{- if (and .Values.federator.enabled .Values.secrets.tlsClientCA) }}
-  ca.crt: {{ .Values.secrets.tlsClientCA | b64enc | quote }}
-{{- end -}}
 {{- if .Values.tls.useCertManager }}
   {{/* NOTE: providing `data` (and empty strings) allows to manage this secret resource with Helm if cert-manager is used */}}
   tls.crt: ""
diff --git a/charts/nginx-ingress-services/templates/secret_federator.yaml b/charts/nginx-ingress-services/templates/secret_federator.yaml
new file mode 100644
index 0000000000..f249e2587c
--- /dev/null
+++ b/charts/nginx-ingress-services/templates/secret_federator.yaml
@@ -0,0 +1,55 @@
+{{- if .Values.federator.enabled -}}
+{{- default .Chart.Name .Values.nameOverride | trunc 63 | trimSuffix "-" -}}
+{{- end -}}
+apiVersion: v1
+kind: Secret
+metadata:
+  name: federator-certificate-secret
+  labels:
+    chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
+    release: "{{ .Release.Name }}"
+    heritage: "{{ .Release.Service }}"
+type: kubernetes.io/tls
+data:
+{{- if .Values.tls.useCertManager }}
+  # Unless an explicit CA certificate is set, use Let's Encrypt ECDSA staging
+  # intermediate certificate. This is useful when the certificates obtained
+  # from Let's Encrypt are also used as client certificates to connect to other
+  # federators.
+  ca.crt: {{ if .Values.secrets.tlsClientCA -}}
+    {{- .Values.secrets.tlsClientCA | quote -}}
+  {{- else -}} |
+    -----BEGIN CERTIFICATE-----
+    MIIDCzCCApGgAwIBAgIRALRY4992FVxZJKOJ3bpffWIwCgYIKoZIzj0EAwMwaDEL
+    MAkGA1UEBhMCVVMxMzAxBgNVBAoTKihTVEFHSU5HKSBJbnRlcm5ldCBTZWN1cml0
+    eSBSZXNlYXJjaCBHcm91cDEkMCIGA1UEAxMbKFNUQUdJTkcpIEJvZ3VzIEJyb2Nj
+    b2xpIFgyMB4XDTIwMDkwNDAwMDAwMFoXDTI1MDkxNTE2MDAwMFowVTELMAkGA1UE
+    BhMCVVMxIDAeBgNVBAoTFyhTVEFHSU5HKSBMZXQncyBFbmNyeXB0MSQwIgYDVQQD
+    ExsoU1RBR0lORykgRXJzYXR6IEVkYW1hbWUgRTEwdjAQBgcqhkjOPQIBBgUrgQQA
+    IgNiAAT9v/PJUtHOTk28nXCXrpP665vI4Z094h8o7R+5E6yNajZa0UubqjpZFoGq
+    u785/vGXj6mdfIzc9boITGusZCSWeMj5ySMZGZkS+VSvf8VQqj+3YdEu4PLZEjBA
+    ivRFpEejggEQMIIBDDAOBgNVHQ8BAf8EBAMCAYYwHQYDVR0lBBYwFAYIKwYBBQUH
+    AwIGCCsGAQUFBwMBMBIGA1UdEwEB/wQIMAYBAf8CAQAwHQYDVR0OBBYEFOv5JcKA
+    KGbibQiSMvPC4a3D/zVFMB8GA1UdIwQYMBaAFN7Ro1lkDsGaNqNG7rAQdu+ul5Vm
+    MDYGCCsGAQUFBwEBBCowKDAmBggrBgEFBQcwAoYaaHR0cDovL3N0Zy14Mi5pLmxl
+    bmNyLm9yZy8wKwYDVR0fBCQwIjAgoB6gHIYaaHR0cDovL3N0Zy14Mi5jLmxlbmNy
+    Lm9yZy8wIgYDVR0gBBswGTAIBgZngQwBAgEwDQYLKwYBBAGC3xMBAQEwCgYIKoZI
+    zj0EAwMDaAAwZQIwXcZbdgxcGH9rTErfSTkXfBKKygU0yO7OpbuNeY1id0FZ/hRY
+    N5fdLOGuc+aHfCsMAjEA0P/xwKr6NQ9MN7vrfGAzO397PApdqfM7VdFK18aEu1xm
+    3HMFKzIR8eEPsMx4smMl
+    -----END CERTIFICATE-----
+  {{- end }}
+  {{/* NOTE: providing empty strings here allows to manage this secret resource with Helm if cert-manager is used */}}
+  tls.crt: ""
+  tls.key: ""
+{{- else }}
+  {{/* for_helm_linting is necessary only since the 'with' block below does not throw an error upon an empty .Values.secrets */}}
+  for_helm_linting: {{ required "No .secrets found in configuration. Did you forget to helm <command> -f path/to/secrets.yaml ?" .Values.secrets | quote | b64enc | quote }}
+
+  {{- with .Values.secrets }}
+  ca.crt: {{ .tlsClientCA | b64enc | quote }}
+  tls.crt: {{ .tlsWildcardCert | b64enc | quote }}
+  tls.key: {{ .tlsWildcardKey | b64enc | quote }}
+  {{- end -}}
+{{- end -}}
+{{- end -}}
diff --git a/charts/nginx-ingress-services/templates/staging_issuer.yaml b/charts/nginx-ingress-services/templates/staging_issuer.yaml
new file mode 100644
index 0000000000..12c76c310c
--- /dev/null
+++ b/charts/nginx-ingress-services/templates/staging_issuer.yaml
@@ -0,0 +1,22 @@
+{{- if and .Values.tls.enabled .Values.tls.useCertManager -}}
+apiVersion: cert-manager.io/v1alpha2
+kind: Issuer
+metadata:
+  name: letsencrypt-staging
+  namespace: {{ .Release.Namespace }}
+  labels:
+    chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
+    release: "{{ .Release.Name }}"
+    heritage: "{{ .Release.Service }}"
+spec:
+  acme:
+    server: "https://acme-staging-v02.api.letsencrypt.org/directory"
+    email: {{ required "Missing value: certmasterEmail" .Values.certManager.certmasterEmail | quote }}
+    # NOTE: this secret doesnt need to be created, it only gets a name with this
+    privateKeySecretRef:
+      name: letsencrypt-staging-account-key
+    solvers:
+      - http01:
+          ingress:
+            class: nginx
+{{- end -}}

From 2fd0234cc146ab5968fcab7ab0846984a9c48c4d Mon Sep 17 00:00:00 2001
From: Paolo Capriotti <paolo@capriotti.io>
Date: Fri, 30 Jul 2021 15:54:35 +0200
Subject: [PATCH 19/41] Enable sharing federator and ingress secrets

---
 charts/federator/templates/deployment.yaml | 8 +++++++-
 charts/federator/templates/secret.yaml     | 2 ++
 charts/federator/values.yaml               | 5 +++++
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/charts/federator/templates/deployment.yaml b/charts/federator/templates/deployment.yaml
index 92bb6fdee0..d24ad40018 100644
--- a/charts/federator/templates/deployment.yaml
+++ b/charts/federator/templates/deployment.yaml
@@ -36,9 +36,15 @@ spec:
         # federator-secrets contains the client certificate and the
         # corresponding private key to use # when making requests to remote
         # backends, as well as the CA used to verify the server certificate
+        # NOTE: if tls.shareFederatorSecret is set, we use the same secret as
+        # the one for the federator ingress
         - name: "federator-secret"
           secret:
-            secretName: "federator-secrets"
+            secretName: {{ if .Values.tls.useCertManager -}}
+              "federator-certificate-secret"
+              {{- else -}}
+              "federator-secret"
+              {{- end }}
       containers:
         - name: federator
           image: "{{ .Values.image.repository }}:{{ .Values.image.tag }}"
diff --git a/charts/federator/templates/secret.yaml b/charts/federator/templates/secret.yaml
index cccd6a1575..6d7fc6cd58 100644
--- a/charts/federator/templates/secret.yaml
+++ b/charts/federator/templates/secret.yaml
@@ -1,3 +1,4 @@
+{{- if not .Values.tls.shareFederatorSecret -}}
 apiVersion: v1
 kind: Secret
 metadata:
@@ -19,3 +20,4 @@ data:
   {{- if .Values.clientCertificateContents }}
   tls.crt: {{ .Values.clientCertificateContents | b64enc | quote }}
   {{- end -}}
+{{- end -}}
diff --git a/charts/federator/values.yaml b/charts/federator/values.yaml
index b6be91f04d..8a2df8f149 100644
--- a/charts/federator/values.yaml
+++ b/charts/federator/values.yaml
@@ -8,6 +8,11 @@ service:
   internalFederatorPort: 8080
   externalFederatorPort: 8081
 
+tls:
+  # if enabled, federator will get its client certificate and private key from
+  # the secret used by the federator ingress
+  shareFederatorSecret: false
+
 resources:
   # FUTUREWORK: come up with numbers which didn't appear out of thin air
   requests:

From d94dee4024e7af9dc95c46d0d54a8224e26633f1 Mon Sep 17 00:00:00 2001
From: Paolo Capriotti <paolo@capriotti.io>
Date: Fri, 30 Jul 2021 17:22:00 +0200
Subject: [PATCH 20/41] Fix naming of federator secret

---
 charts/federator/templates/deployment.yaml                  | 6 ++++--
 charts/federator/templates/tests/federator-integration.yaml | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/charts/federator/templates/deployment.yaml b/charts/federator/templates/deployment.yaml
index d24ad40018..fa4b91a7b8 100644
--- a/charts/federator/templates/deployment.yaml
+++ b/charts/federator/templates/deployment.yaml
@@ -25,7 +25,9 @@ spec:
       annotations:
         # An annotation of the configmap checksum ensures changes to the configmap cause a redeployment upon `helm upgrade`
         checksum/configmap: {{ include (print .Template.BasePath "/configmap.yaml") . | sha256sum }}
-        checksum/secrets: {{ include (print .Template.BasePath "/secrets.yaml") . | sha256sum }}
+        {{- if not .Values.tls.shareFederatorSecret }}
+        checksum/secret: {{ include (print .Template.BasePath "/secret.yaml") . | sha256sum }}
+        {{- end }}
         fluentbit.io/parser: json
     spec:
       volumes:
@@ -38,7 +40,7 @@ spec:
         # backends, as well as the CA used to verify the server certificate
         # NOTE: if tls.shareFederatorSecret is set, we use the same secret as
         # the one for the federator ingress
-        - name: "federator-secret"
+        - name: "federator-secrets"
           secret:
             secretName: {{ if .Values.tls.useCertManager -}}
               "federator-certificate-secret"
diff --git a/charts/federator/templates/tests/federator-integration.yaml b/charts/federator/templates/tests/federator-integration.yaml
index f2ae63671b..99b649f7bf 100644
--- a/charts/federator/templates/tests/federator-integration.yaml
+++ b/charts/federator/templates/tests/federator-integration.yaml
@@ -16,7 +16,7 @@ spec:
     # integration tests need access to the client certificate private key
     - name: "federator-secrets"
       secret:
-        secretName: "federator-secrets"
+        secretName: "federator-secret"
   containers:
   - name: integration
     command: ["federator-integration"]

From 8af4fd2e1f6073a9e8d7517c2a50c1ef14350561 Mon Sep 17 00:00:00 2001
From: Paolo Capriotti <paolo@capriotti.io>
Date: Mon, 2 Aug 2021 08:13:25 +0200
Subject: [PATCH 21/41] Fix federator certificate secret chart

---
 charts/nginx-ingress-services/templates/secret_federator.yaml | 2 --
 1 file changed, 2 deletions(-)

diff --git a/charts/nginx-ingress-services/templates/secret_federator.yaml b/charts/nginx-ingress-services/templates/secret_federator.yaml
index f249e2587c..77a6bf3030 100644
--- a/charts/nginx-ingress-services/templates/secret_federator.yaml
+++ b/charts/nginx-ingress-services/templates/secret_federator.yaml
@@ -1,6 +1,4 @@
 {{- if .Values.federator.enabled -}}
-{{- default .Chart.Name .Values.nameOverride | trunc 63 | trimSuffix "-" -}}
-{{- end -}}
 apiVersion: v1
 kind: Secret
 metadata:

From f1df55ca495be74bf8c255b4d3a3b17e2f940b51 Mon Sep 17 00:00:00 2001
From: Paolo Capriotti <paolo@capriotti.io>
Date: Tue, 3 Aug 2021 13:11:13 +0200
Subject: [PATCH 22/41] Remove staging issuer

Use the same issuer for both certificates. Whether we are using Let's
Encrypt staging or production is determined by `certManager.inTestMode`
so we don't need a separate issuer to enable staging.
---
 .../templates/certificate_federator.yaml      |  2 +-
 .../templates/secret_federator.yaml           | 29 +------------------
 .../templates/staging_issuer.yaml             | 22 --------------
 3 files changed, 2 insertions(+), 51 deletions(-)
 delete mode 100644 charts/nginx-ingress-services/templates/staging_issuer.yaml

diff --git a/charts/nginx-ingress-services/templates/certificate_federator.yaml b/charts/nginx-ingress-services/templates/certificate_federator.yaml
index a2e3e506fc..ac068434b2 100644
--- a/charts/nginx-ingress-services/templates/certificate_federator.yaml
+++ b/charts/nginx-ingress-services/templates/certificate_federator.yaml
@@ -10,7 +10,7 @@ metadata:
     heritage: "{{ .Release.Service }}"
 spec:
   issuerRef:
-    name: letsencrypt-staging
+    name: letsencrypt-http01
     kind: Issuer
   usages:
     - server auth
diff --git a/charts/nginx-ingress-services/templates/secret_federator.yaml b/charts/nginx-ingress-services/templates/secret_federator.yaml
index 77a6bf3030..4de3d682af 100644
--- a/charts/nginx-ingress-services/templates/secret_federator.yaml
+++ b/charts/nginx-ingress-services/templates/secret_federator.yaml
@@ -9,34 +9,8 @@ metadata:
     heritage: "{{ .Release.Service }}"
 type: kubernetes.io/tls
 data:
+  ca.crt: {{ .Values.secrets.tlsClientCA | b64enc | quote }}
 {{- if .Values.tls.useCertManager }}
-  # Unless an explicit CA certificate is set, use Let's Encrypt ECDSA staging
-  # intermediate certificate. This is useful when the certificates obtained
-  # from Let's Encrypt are also used as client certificates to connect to other
-  # federators.
-  ca.crt: {{ if .Values.secrets.tlsClientCA -}}
-    {{- .Values.secrets.tlsClientCA | quote -}}
-  {{- else -}} |
-    -----BEGIN CERTIFICATE-----
-    MIIDCzCCApGgAwIBAgIRALRY4992FVxZJKOJ3bpffWIwCgYIKoZIzj0EAwMwaDEL
-    MAkGA1UEBhMCVVMxMzAxBgNVBAoTKihTVEFHSU5HKSBJbnRlcm5ldCBTZWN1cml0
-    eSBSZXNlYXJjaCBHcm91cDEkMCIGA1UEAxMbKFNUQUdJTkcpIEJvZ3VzIEJyb2Nj
-    b2xpIFgyMB4XDTIwMDkwNDAwMDAwMFoXDTI1MDkxNTE2MDAwMFowVTELMAkGA1UE
-    BhMCVVMxIDAeBgNVBAoTFyhTVEFHSU5HKSBMZXQncyBFbmNyeXB0MSQwIgYDVQQD
-    ExsoU1RBR0lORykgRXJzYXR6IEVkYW1hbWUgRTEwdjAQBgcqhkjOPQIBBgUrgQQA
-    IgNiAAT9v/PJUtHOTk28nXCXrpP665vI4Z094h8o7R+5E6yNajZa0UubqjpZFoGq
-    u785/vGXj6mdfIzc9boITGusZCSWeMj5ySMZGZkS+VSvf8VQqj+3YdEu4PLZEjBA
-    ivRFpEejggEQMIIBDDAOBgNVHQ8BAf8EBAMCAYYwHQYDVR0lBBYwFAYIKwYBBQUH
-    AwIGCCsGAQUFBwMBMBIGA1UdEwEB/wQIMAYBAf8CAQAwHQYDVR0OBBYEFOv5JcKA
-    KGbibQiSMvPC4a3D/zVFMB8GA1UdIwQYMBaAFN7Ro1lkDsGaNqNG7rAQdu+ul5Vm
-    MDYGCCsGAQUFBwEBBCowKDAmBggrBgEFBQcwAoYaaHR0cDovL3N0Zy14Mi5pLmxl
-    bmNyLm9yZy8wKwYDVR0fBCQwIjAgoB6gHIYaaHR0cDovL3N0Zy14Mi5jLmxlbmNy
-    Lm9yZy8wIgYDVR0gBBswGTAIBgZngQwBAgEwDQYLKwYBBAGC3xMBAQEwCgYIKoZI
-    zj0EAwMDaAAwZQIwXcZbdgxcGH9rTErfSTkXfBKKygU0yO7OpbuNeY1id0FZ/hRY
-    N5fdLOGuc+aHfCsMAjEA0P/xwKr6NQ9MN7vrfGAzO397PApdqfM7VdFK18aEu1xm
-    3HMFKzIR8eEPsMx4smMl
-    -----END CERTIFICATE-----
-  {{- end }}
   {{/* NOTE: providing empty strings here allows to manage this secret resource with Helm if cert-manager is used */}}
   tls.crt: ""
   tls.key: ""
@@ -45,7 +19,6 @@ data:
   for_helm_linting: {{ required "No .secrets found in configuration. Did you forget to helm <command> -f path/to/secrets.yaml ?" .Values.secrets | quote | b64enc | quote }}
 
   {{- with .Values.secrets }}
-  ca.crt: {{ .tlsClientCA | b64enc | quote }}
   tls.crt: {{ .tlsWildcardCert | b64enc | quote }}
   tls.key: {{ .tlsWildcardKey | b64enc | quote }}
   {{- end -}}
diff --git a/charts/nginx-ingress-services/templates/staging_issuer.yaml b/charts/nginx-ingress-services/templates/staging_issuer.yaml
deleted file mode 100644
index 12c76c310c..0000000000
--- a/charts/nginx-ingress-services/templates/staging_issuer.yaml
+++ /dev/null
@@ -1,22 +0,0 @@
-{{- if and .Values.tls.enabled .Values.tls.useCertManager -}}
-apiVersion: cert-manager.io/v1alpha2
-kind: Issuer
-metadata:
-  name: letsencrypt-staging
-  namespace: {{ .Release.Namespace }}
-  labels:
-    chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
-    release: "{{ .Release.Name }}"
-    heritage: "{{ .Release.Service }}"
-spec:
-  acme:
-    server: "https://acme-staging-v02.api.letsencrypt.org/directory"
-    email: {{ required "Missing value: certmasterEmail" .Values.certManager.certmasterEmail | quote }}
-    # NOTE: this secret doesnt need to be created, it only gets a name with this
-    privateKeySecretRef:
-      name: letsencrypt-staging-account-key
-    solvers:
-      - http01:
-          ingress:
-            class: nginx
-{{- end -}}

From 06e6ff732d7e7056949341bb514097ac803541c0 Mon Sep 17 00:00:00 2001
From: Paolo Capriotti <paolo@capriotti.io>
Date: Tue, 3 Aug 2021 14:59:28 +0200
Subject: [PATCH 23/41] Hi CI


From 19ad1951964a0a77fd8a17a64ee1b839b306623a Mon Sep 17 00:00:00 2001
From: Paolo Capriotti <paolo@capriotti.io>
Date: Thu, 5 Aug 2021 14:58:41 +0200
Subject: [PATCH 24/41] Fix whitespace in template

---
 charts/federator/templates/secret.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/charts/federator/templates/secret.yaml b/charts/federator/templates/secret.yaml
index 6d7fc6cd58..a9d1468cd3 100644
--- a/charts/federator/templates/secret.yaml
+++ b/charts/federator/templates/secret.yaml
@@ -12,7 +12,7 @@ type: kubernetes.io/tls
 data:
 {{- if .Values.remoteCAContents }}
   ca.crt: {{ .Values.remoteCAContents | b64enc | quote }}
-{{- end -}}
+{{- end }}
   # TODO: add validation and fail early during templating: either contents should be provided; or explicitly system trust store enabled
   {{- if .Values.clientPrivateKeyContents }}
   tls.key: {{ .Values.clientPrivateKeyContents | b64enc | quote }}

From 7d99903c3096225f309b069266e16ccd4bdebc6d Mon Sep 17 00:00:00 2001
From: Paolo Capriotti <paolo@capriotti.io>
Date: Fri, 6 Aug 2021 08:22:03 +0200
Subject: [PATCH 25/41] Add script to serve helm charts locally

---
 Makefile                 |  4 ++++
 hack/bin/serve-charts.sh | 14 ++++++++++++++
 2 files changed, 18 insertions(+)
 create mode 100755 hack/bin/serve-charts.sh

diff --git a/Makefile b/Makefile
index dec6ef464a..3dc1142568 100644
--- a/Makefile
+++ b/Makefile
@@ -334,6 +334,10 @@ chart-%:
 .PHONY: charts-integration
 charts-integration: $(foreach chartName,$(CHARTS_INTEGRATION),chart-$(chartName))
 
+.PHONY: charts-serve
+charts-serve: charts-integration
+	./hack/bin/serve-charts.sh $(CHARTS_INTEGRATION)
+
 # Usecase for this make target:
 # 1. for releases of helm charts
 # 2. for testing helm charts more generally
diff --git a/hack/bin/serve-charts.sh b/hack/bin/serve-charts.sh
new file mode 100755
index 0000000000..eb69bf2af1
--- /dev/null
+++ b/hack/bin/serve-charts.sh
@@ -0,0 +1,14 @@
+#!/bin/bash -e
+
+: ${HELM_SERVER_PORT:=4001}
+
+# get rid of all helm repositories
+helm repo list -o json | jq -r '.[] | .name' | xargs -I% helm repo remove %
+
+cd "$(dirname "$BASH_SOURCE[0]")/../../.local/charts"
+for chart in $@; do
+  ../../hack/bin/update.sh "$chart"
+  helm package "$chart"
+done
+helm repo index .
+python -m http.server $HELM_SERVER_PORT

From 0dfadf2d7a2b3cb99329f9492af40aab0094808f Mon Sep 17 00:00:00 2001
From: Paolo Capriotti <paolo@capriotti.io>
Date: Fri, 6 Aug 2021 08:22:14 +0200
Subject: [PATCH 26/41] Fix secret name when sharing federator secrets

---
 charts/federator/templates/deployment.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/charts/federator/templates/deployment.yaml b/charts/federator/templates/deployment.yaml
index fa4b91a7b8..bcbfee6373 100644
--- a/charts/federator/templates/deployment.yaml
+++ b/charts/federator/templates/deployment.yaml
@@ -42,7 +42,7 @@ spec:
         # the one for the federator ingress
         - name: "federator-secrets"
           secret:
-            secretName: {{ if .Values.tls.useCertManager -}}
+            secretName: {{ if .Values.tls.shareFederatorSecret -}}
               "federator-certificate-secret"
               {{- else -}}
               "federator-secret"

From cad8c1dd10d0829d565e04f8d9a18c8c8071378e Mon Sep 17 00:00:00 2001
From: Paolo Capriotti <paolo@capriotti.io>
Date: Fri, 6 Aug 2021 10:12:59 +0200
Subject: [PATCH 27/41] Separate ca secret from tls secret

It seems cert-manager takes control of the `ca.crt` key in the secret
corresponding to a certificate, so it does not seem possible to set it
manually when cert-manager is enabled. Unfortunately, it still needs to
be a secret, since the ingress requires it.

This commit moves the CA to a separate secret. Therefore, it needs to be
set explicitly in the chart values for both federator and the
nginx-ingress-services, even if federator secret sharing is enabled.
---
 charts/federator/templates/ca.yaml            | 15 +++++++++++++++
 charts/federator/templates/configmap.yaml     |  3 +--
 charts/federator/templates/deployment.yaml    | 10 ++++++++--
 charts/federator/templates/secret.yaml        |  3 ---
 .../templates/ca_federator.yaml               | 19 +++++++++++++++++++
 .../templates/ingress_federator.yaml          |  2 +-
 .../templates/secret_federator.yaml           |  1 -
 .../nginx-ingress-services/values.yaml        |  4 ++--
 8 files changed, 46 insertions(+), 11 deletions(-)
 create mode 100644 charts/federator/templates/ca.yaml
 create mode 100644 charts/nginx-ingress-services/templates/ca_federator.yaml

diff --git a/charts/federator/templates/ca.yaml b/charts/federator/templates/ca.yaml
new file mode 100644
index 0000000000..2567bedef7
--- /dev/null
+++ b/charts/federator/templates/ca.yaml
@@ -0,0 +1,15 @@
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: "federator-ca"
+  labels:
+    wireService: federator
+    chart: {{ .Chart.Name }}-{{ .Chart.Version | replace "+" "_" }}
+    release: {{ .Release.Name }}
+    heritage: {{ .Release.Service }}
+data:
+{{- if .Values.remoteCAContents }}
+  ca.crt: {{ .Values.remoteCAContents | quote }}
+{{- else -}}
+  {}
+{{- end }}
diff --git a/charts/federator/templates/configmap.yaml b/charts/federator/templates/configmap.yaml
index 6783c06e1b..b4900d9293 100644
--- a/charts/federator/templates/configmap.yaml
+++ b/charts/federator/templates/configmap.yaml
@@ -43,7 +43,7 @@ data:
       # Filepath to one or more PEM-encoded server certificates to use as a trust
       # store when making grpc requests to remote backends
       {{- if $.Values.remoteCAContents }}
-      remoteCAStore: "/etc/wire/federator/secrets/ca.crt"
+      remoteCAStore: "/etc/wire/federator/ca/ca.crt"
       {{- end }}
       {{- if $.Values.clientCertificateContents }}
       clientCertificate: "/etc/wire/federator/secrets/tls.crt"
@@ -65,4 +65,3 @@ data:
         {{- end}}
     {{- end }}
     {{- end }}
-
diff --git a/charts/federator/templates/deployment.yaml b/charts/federator/templates/deployment.yaml
index bcbfee6373..7a4a5d100c 100644
--- a/charts/federator/templates/deployment.yaml
+++ b/charts/federator/templates/deployment.yaml
@@ -36,8 +36,8 @@ spec:
             name: "federator"
 
         # federator-secrets contains the client certificate and the
-        # corresponding private key to use # when making requests to remote
-        # backends, as well as the CA used to verify the server certificate
+        # corresponding private key to use when making requests to remote
+        # backends.
         # NOTE: if tls.shareFederatorSecret is set, we use the same secret as
         # the one for the federator ingress
         - name: "federator-secrets"
@@ -47,6 +47,10 @@ spec:
               {{- else -}}
               "federator-secret"
               {{- end }}
+
+        - name: "federator-ca"
+          configMap:
+            name: "federator-ca"
       containers:
         - name: federator
           image: "{{ .Values.image.repository }}:{{ .Values.image.tag }}"
@@ -56,6 +60,8 @@ spec:
               mountPath: "/etc/wire/federator/conf"
             - name: "federator-secrets"
               mountPath: "/etc/wire/federator/secrets"
+            - name: "federator-ca"
+              mountPath: "/etc/wire/federator/ca"
           ports:
             - name: internal
               containerPort: {{ .Values.service.internalFederatorPort }}
diff --git a/charts/federator/templates/secret.yaml b/charts/federator/templates/secret.yaml
index a9d1468cd3..012bd776cf 100644
--- a/charts/federator/templates/secret.yaml
+++ b/charts/federator/templates/secret.yaml
@@ -10,9 +10,6 @@ metadata:
     heritage: {{ .Release.Service }}
 type: kubernetes.io/tls
 data:
-{{- if .Values.remoteCAContents }}
-  ca.crt: {{ .Values.remoteCAContents | b64enc | quote }}
-{{- end }}
   # TODO: add validation and fail early during templating: either contents should be provided; or explicitly system trust store enabled
   {{- if .Values.clientPrivateKeyContents }}
   tls.key: {{ .Values.clientPrivateKeyContents | b64enc | quote }}
diff --git a/charts/nginx-ingress-services/templates/ca_federator.yaml b/charts/nginx-ingress-services/templates/ca_federator.yaml
new file mode 100644
index 0000000000..471a7ac8b3
--- /dev/null
+++ b/charts/nginx-ingress-services/templates/ca_federator.yaml
@@ -0,0 +1,19 @@
+{{- /* This is the CA used by the federator ingress to verify client
+certificates. This does not need to be a secret in principle, but the ingress
+controller requires it to be. Also, this could in principle be bundled with the
+corresponding certificate (in secret_federator.yaml), but it is a separate
+secret because cert-manager interferes with the ca.crt field when setting the
+certificate in a secret. */ -}}
+
+{{- if .Values.federator.enabled -}}
+apiVersion: v1
+kind: Secret
+metadata:
+  name: federator-ca-secret
+  labels:
+    chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
+    release: "{{ .Release.Name }}"
+    heritage: "{{ .Release.Service }}"
+data:
+  ca.crt: {{ .Values.secrets.tlsClientCA | b64enc | quote }}
+{{- end -}}
diff --git a/charts/nginx-ingress-services/templates/ingress_federator.yaml b/charts/nginx-ingress-services/templates/ingress_federator.yaml
index 20ce8fac9a..f6ac61ea31 100644
--- a/charts/nginx-ingress-services/templates/ingress_federator.yaml
+++ b/charts/nginx-ingress-services/templates/ingress_federator.yaml
@@ -13,7 +13,7 @@ metadata:
     nginx.ingress.kubernetes.io/ssl-redirect: "true"
     nginx.ingress.kubernetes.io/backend-protocol: "GRPC"
     nginx.ingress.kubernetes.io/auth-tls-verify-client: "on"
-    nginx.ingress.kubernetes.io/auth-tls-secret: "{{ .Release.Namespace }}/federator-certificate-secret"
+    nginx.ingress.kubernetes.io/auth-tls-secret: "{{ .Release.Namespace }}/federator-ca-secret"
 spec:
   tls:
   - hosts:
diff --git a/charts/nginx-ingress-services/templates/secret_federator.yaml b/charts/nginx-ingress-services/templates/secret_federator.yaml
index 4de3d682af..3eebcbba06 100644
--- a/charts/nginx-ingress-services/templates/secret_federator.yaml
+++ b/charts/nginx-ingress-services/templates/secret_federator.yaml
@@ -9,7 +9,6 @@ metadata:
     heritage: "{{ .Release.Service }}"
 type: kubernetes.io/tls
 data:
-  ca.crt: {{ .Values.secrets.tlsClientCA | b64enc | quote }}
 {{- if .Values.tls.useCertManager }}
   {{/* NOTE: providing empty strings here allows to manage this secret resource with Helm if cert-manager is used */}}
   tls.crt: ""
diff --git a/hack/helm_vars/nginx-ingress-services/values.yaml b/hack/helm_vars/nginx-ingress-services/values.yaml
index 34208c0de1..76aa0657e8 100644
--- a/hack/helm_vars/nginx-ingress-services/values.yaml
+++ b/hack/helm_vars/nginx-ingress-services/values.yaml
@@ -18,5 +18,5 @@ config:
     accountPages: account.integration.example.com
     # federator: dynamically set by hack/bin/integration-setup.sh
 
-# the secrets/tlsWildcardCert and secrets/tlsWildcardKey are
-# dynamically provided from hack/bin/selfsigned-kubernetes.sh
+# secrets/tlsWildcardCert, secrets/tlsWildcardKey and secrets/tlsClientCA
+# are dynamically generated by hack/bin/selfsigned-kubernetes.sh

From 4fd5bfc33a52b10b26c0b0db80172dad56a9aa95 Mon Sep 17 00:00:00 2001
From: Paolo Capriotti <paolo@capriotti.io>
Date: Mon, 9 Aug 2021 10:06:05 +0200
Subject: [PATCH 28/41] Add ca volume to federator integration pod

---
 charts/federator/templates/ca.yaml                          | 2 +-
 charts/federator/templates/tests/federator-integration.yaml | 6 ++++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/charts/federator/templates/ca.yaml b/charts/federator/templates/ca.yaml
index 2567bedef7..8363507e1b 100644
--- a/charts/federator/templates/ca.yaml
+++ b/charts/federator/templates/ca.yaml
@@ -10,6 +10,6 @@ metadata:
 data:
 {{- if .Values.remoteCAContents }}
   ca.crt: {{ .Values.remoteCAContents | quote }}
-{{- else -}}
+{{- else }}
   {}
 {{- end }}
diff --git a/charts/federator/templates/tests/federator-integration.yaml b/charts/federator/templates/tests/federator-integration.yaml
index 99b649f7bf..88fa8efbc2 100644
--- a/charts/federator/templates/tests/federator-integration.yaml
+++ b/charts/federator/templates/tests/federator-integration.yaml
@@ -17,6 +17,10 @@ spec:
     - name: "federator-secrets"
       secret:
         secretName: "federator-secret"
+    # integration tests need access to the CA
+    - name: "federator-ca"
+      secret:
+        secretName: "federator-ca"
   containers:
   - name: integration
     command: ["federator-integration"]
@@ -28,4 +32,6 @@ spec:
       mountPath: "/etc/wire/federator/conf"
     - name: "federator-secrets"
       mountPath: "/etc/wire/federator/secrets"
+    - name: "federator-ca"
+      mountPath: "/etc/wire/federator/ca"
   restartPolicy: Never

From 24aa1911473c427f529a00de492bdc38ea9a78b9 Mon Sep 17 00:00:00 2001
From: Paolo Capriotti <paolo@capriotti.io>
Date: Mon, 9 Aug 2021 11:01:07 +0200
Subject: [PATCH 29/41] Fix one more issue in federator integration chart

federator-ca is a configmap, not a secret
---
 charts/federator/templates/tests/federator-integration.yaml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/charts/federator/templates/tests/federator-integration.yaml b/charts/federator/templates/tests/federator-integration.yaml
index 88fa8efbc2..32e6eef09e 100644
--- a/charts/federator/templates/tests/federator-integration.yaml
+++ b/charts/federator/templates/tests/federator-integration.yaml
@@ -19,8 +19,8 @@ spec:
         secretName: "federator-secret"
     # integration tests need access to the CA
     - name: "federator-ca"
-      secret:
-        secretName: "federator-ca"
+      configMap:
+        name: "federator-ca"
   containers:
   - name: integration
     command: ["federator-integration"]

From ae4fa09439ce2754e14c6f02fde6ef51d3c6178b Mon Sep 17 00:00:00 2001
From: Paolo Capriotti <paolo@capriotti.io>
Date: Wed, 11 Aug 2021 10:11:47 +0200
Subject: [PATCH 30/41] Client certificate config with secret sharing

---
 charts/federator/templates/configmap.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/charts/federator/templates/configmap.yaml b/charts/federator/templates/configmap.yaml
index b4900d9293..2fc6b64529 100644
--- a/charts/federator/templates/configmap.yaml
+++ b/charts/federator/templates/configmap.yaml
@@ -45,7 +45,7 @@ data:
       {{- if $.Values.remoteCAContents }}
       remoteCAStore: "/etc/wire/federator/ca/ca.crt"
       {{- end }}
-      {{- if $.Values.clientCertificateContents }}
+      {{- if $.Values.clientCertificateContents or $.Values.tls.shareFederatorSecret }}
       clientCertificate: "/etc/wire/federator/secrets/tls.crt"
       clientPrivateKey: "/etc/wire/federator/secrets/tls.key"
       {{- end }}

From 803fce7d35375bf34abf6592933a932694fe530b Mon Sep 17 00:00:00 2001
From: Paolo Capriotti <paolo@capriotti.io>
Date: Wed, 11 Aug 2021 10:25:52 +0200
Subject: [PATCH 31/41] Change ECDSA certificate to use p256 curve

---
 .../nginx-ingress-services/templates/certificate_federator.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/charts/nginx-ingress-services/templates/certificate_federator.yaml b/charts/nginx-ingress-services/templates/certificate_federator.yaml
index ac068434b2..07dd2056ae 100644
--- a/charts/nginx-ingress-services/templates/certificate_federator.yaml
+++ b/charts/nginx-ingress-services/templates/certificate_federator.yaml
@@ -18,7 +18,7 @@ spec:
   renewBefore: 360h   # 15d
   isCA: false
   keyAlgorithm: ecdsa
-  keySize: 384        # 521 is not supported by Letsencrypt
+  keySize: 256        # hs-tls only supports p256
   keyEncoding: pkcs1
   secretName: federator-certificate-secret
   # NOTE: disabled due to https://github.com/jetstack/cert-manager/issues/2978

From c7d717a817fc33016c5ad952a2f79ecc5a89f164 Mon Sep 17 00:00:00 2001
From: Paolo Capriotti <paolo@capriotti.io>
Date: Wed, 11 Aug 2021 10:28:48 +0200
Subject: [PATCH 32/41] Rename shareFederatorSecret

Renamed to `useSharedFederatorSecret`.

Co-authored-by: Akshay Mankar <akshay@wire.com>
---
 charts/federator/templates/configmap.yaml  | 2 +-
 charts/federator/templates/deployment.yaml | 6 +++---
 charts/federator/templates/secret.yaml     | 2 +-
 charts/federator/values.yaml               | 2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/charts/federator/templates/configmap.yaml b/charts/federator/templates/configmap.yaml
index 2fc6b64529..48be920d86 100644
--- a/charts/federator/templates/configmap.yaml
+++ b/charts/federator/templates/configmap.yaml
@@ -45,7 +45,7 @@ data:
       {{- if $.Values.remoteCAContents }}
       remoteCAStore: "/etc/wire/federator/ca/ca.crt"
       {{- end }}
-      {{- if $.Values.clientCertificateContents or $.Values.tls.shareFederatorSecret }}
+      {{- if $.Values.clientCertificateContents or $.Values.tls.useSharedFederatorSecret }}
       clientCertificate: "/etc/wire/federator/secrets/tls.crt"
       clientPrivateKey: "/etc/wire/federator/secrets/tls.key"
       {{- end }}
diff --git a/charts/federator/templates/deployment.yaml b/charts/federator/templates/deployment.yaml
index 7a4a5d100c..6133871b58 100644
--- a/charts/federator/templates/deployment.yaml
+++ b/charts/federator/templates/deployment.yaml
@@ -38,11 +38,11 @@ spec:
         # federator-secrets contains the client certificate and the
         # corresponding private key to use when making requests to remote
         # backends.
-        # NOTE: if tls.shareFederatorSecret is set, we use the same secret as
-        # the one for the federator ingress
+        # NOTE: if tls.useSharedFederatorSecret is set, we use the same secret
+        # as the one for the federator ingress
         - name: "federator-secrets"
           secret:
-            secretName: {{ if .Values.tls.shareFederatorSecret -}}
+            secretName: {{ if .Values.tls.useSharedFederatorSecret -}}
               "federator-certificate-secret"
               {{- else -}}
               "federator-secret"
diff --git a/charts/federator/templates/secret.yaml b/charts/federator/templates/secret.yaml
index 012bd776cf..551dd96619 100644
--- a/charts/federator/templates/secret.yaml
+++ b/charts/federator/templates/secret.yaml
@@ -1,4 +1,4 @@
-{{- if not .Values.tls.shareFederatorSecret -}}
+{{- if not .Values.tls.useSharedFederatorSecret -}}
 apiVersion: v1
 kind: Secret
 metadata:
diff --git a/charts/federator/values.yaml b/charts/federator/values.yaml
index 8a2df8f149..5a29356f21 100644
--- a/charts/federator/values.yaml
+++ b/charts/federator/values.yaml
@@ -11,7 +11,7 @@ service:
 tls:
   # if enabled, federator will get its client certificate and private key from
   # the secret used by the federator ingress
-  shareFederatorSecret: false
+  useSharedFederatorSecret: false
 
 resources:
   # FUTUREWORK: come up with numbers which didn't appear out of thin air

From 3322865b5282b39a5cef2a836d4d8eedac6e0fd9 Mon Sep 17 00:00:00 2001
From: Paolo Capriotti <paolo@capriotti.io>
Date: Wed, 11 Aug 2021 12:04:57 +0200
Subject: [PATCH 33/41] Make client certificates required

---
 services/federator/src/Federator/Env.hs       |  3 +-
 services/federator/src/Federator/Options.hs   |  4 +-
 services/federator/src/Federator/Remote.hs    |  2 +-
 services/federator/src/Federator/Run.hs       | 34 ++-----
 .../unit/Test/Federator/ExternalServer.hs     |  8 +-
 .../unit/Test/Federator/InternalServer.hs     | 18 ++--
 .../test/unit/Test/Federator/Options.hs       | 97 ++++++-------------
 .../test/unit/Test/Federator/Remote.hs        | 42 +++-----
 .../test/unit/Test/Federator/Validation.hs    |  4 +-
 9 files changed, 77 insertions(+), 135 deletions(-)

diff --git a/services/federator/src/Federator/Env.hs b/services/federator/src/Federator/Env.hs
index 0a6e68a896..71d1f17817 100644
--- a/services/federator/src/Federator/Env.hs
+++ b/services/federator/src/Federator/Env.hs
@@ -26,7 +26,6 @@ import Control.Lens (makeLenses)
 import Data.Metrics (Metrics)
 import Data.X509.CertificateStore
 import Federator.Options (RunSettings)
-import Imports
 import Network.DNS.Resolver (Resolver)
 import qualified Network.HTTP.Client as HTTP
 import qualified Network.TLS as TLS
@@ -35,7 +34,7 @@ import Wire.API.Federation.GRPC.Types
 
 data TLSSettings = TLSSettings
   { _caStore :: CertificateStore,
-    _creds :: Maybe TLS.Credential
+    _creds :: TLS.Credential
   }
 
 data Env = Env
diff --git a/services/federator/src/Federator/Options.hs b/services/federator/src/Federator/Options.hs
index e8a8c651ed..33c4ee6ff3 100644
--- a/services/federator/src/Federator/Options.hs
+++ b/services/federator/src/Federator/Options.hs
@@ -63,8 +63,8 @@ data RunSettings = RunSettings
     federationStrategy :: FederationStrategy,
     useSystemCAStore :: Bool,
     remoteCAStore :: Maybe FilePath,
-    clientCertificate :: Maybe FilePath,
-    clientPrivateKey :: Maybe FilePath
+    clientCertificate :: FilePath,
+    clientPrivateKey :: FilePath
   }
   deriving (Eq, Show, Generic)
 
diff --git a/services/federator/src/Federator/Remote.hs b/services/federator/src/Federator/Remote.hs
index 8bb49dbb11..ec4ed7e4e4 100644
--- a/services/federator/src/Federator/Remote.hs
+++ b/services/federator/src/Federator/Remote.hs
@@ -146,7 +146,7 @@ mkGrpcClient target@(SrvTarget host port) = logAndReturn target $ do
                       X509.HashSHA256
                       (X509.defaultHooks {TLS.hookValidateName = validateName})
                       X509.defaultChecks,
-                  TLS.onCertificateRequest = \_ -> pure (settings ^. creds)
+                  TLS.onCertificateRequest = \_ -> pure (Just (settings ^. creds))
                 },
             TLS.clientShared = def {TLS.sharedCAStore = settings ^. caStore}
           }
diff --git a/services/federator/src/Federator/Run.hs b/services/federator/src/Federator/Run.hs
index 6456ebf8ea..bfa89954b2 100644
--- a/services/federator/src/Federator/Run.hs
+++ b/services/federator/src/Federator/Run.hs
@@ -118,31 +118,15 @@ mkCAStore settings = do
       else pure mempty
   pure (customCAStore <> systemCAStore)
 
-getClientCredentials :: RunSettings -> Either String (Maybe (FilePath, FilePath))
-getClientCredentials settings = case clientCertificate settings of
-  Nothing -> noCreds1 $> Nothing
-  Just cert -> Just . (cert,) <$> getCreds1
-  where
-    noCreds1 :: Either String ()
-    noCreds1
-      | isNothing (clientPrivateKey settings) = pure ()
-      | otherwise = Left "invalid client credentials: no certificate"
-
-    getCreds1 :: Either String FilePath
-    getCreds1 =
-      maybe (Left "invalid client credentials: no private key") pure $
-        clientPrivateKey settings
-
-mkCreds :: RunSettings -> IO (Maybe TLS.Credential)
-mkCreds settings = handle h $ case getClientCredentials settings of
-  Left e -> throw (InvalidClientCertificate e)
-  Right Nothing -> pure Nothing
-  Right (Just (cert, key)) ->
-    TLS.credentialLoadX509 cert key >>= \case
-      Left e -> throw (InvalidClientCertificate e)
-      Right (X509.CertificateChain [], _) ->
-        throw (InvalidClientCertificate "could not read client certificate")
-      Right x -> pure (Just x)
+mkCreds :: RunSettings -> IO TLS.Credential
+mkCreds settings =
+  handle h $
+    TLS.credentialLoadX509 (clientCertificate settings) (clientPrivateKey settings)
+      >>= \case
+        Left e -> throw (InvalidClientCertificate e)
+        Right (X509.CertificateChain [], _) ->
+          throw (InvalidClientCertificate "could not read client certificate")
+        Right x -> pure x
   where
     h :: IOException -> IO a
     h = throw . InvalidClientCertificate . show
diff --git a/services/federator/test/unit/Test/Federator/ExternalServer.hs b/services/federator/test/unit/Test/Federator/ExternalServer.hs
index f06140f839..0f41489a84 100644
--- a/services/federator/test/unit/Test/Federator/ExternalServer.hs
+++ b/services/federator/test/unit/Test/Federator/ExternalServer.hs
@@ -28,7 +28,7 @@ import qualified Network.HTTP.Types as HTTP
 import Polysemy (embed, runM)
 import qualified Polysemy.Reader as Polysemy
 import qualified Polysemy.TinyLog as TinyLog
-import Test.Federator.Options (defRunSettings)
+import Test.Federator.Options (noClientCertSettings)
 import Test.Polysemy.Mock (Mock (mock), evalMock)
 import Test.Polysemy.Mock.TH (genMock)
 import Test.Tasty (TestTree, testGroup)
@@ -53,7 +53,7 @@ requestBrigSuccess =
       mockServiceCallReturns @IO (\_ _ _ _ -> pure (HTTP.ok200, Just "response body"))
       let request = Request Brig "/federation/get-user-by-handle" "\"foo\"" exampleDomain
 
-      res :: InwardResponse <- mock @Service @IO . TinyLog.discardLogs . Polysemy.runReader defRunSettings $ callLocal request
+      res :: InwardResponse <- mock @Service @IO . TinyLog.discardLogs . Polysemy.runReader noClientCertSettings $ callLocal request
       actualCalls <- mockServiceCallCalls @IO
       let expectedCall = (Brig, "federation/get-user-by-handle", "\"foo\"", aValidDomain)
       embed $ assertEqual "one call to brig should be made" [expectedCall] actualCalls
@@ -66,7 +66,7 @@ requestBrigFailure =
       mockServiceCallReturns @IO (\_ _ _ _ -> pure (HTTP.notFound404, Just "response body"))
       let request = Request Brig "/federation/get-user-by-handle" "\"foo\"" exampleDomain
 
-      res <- mock @Service @IO . TinyLog.discardLogs . Polysemy.runReader defRunSettings $ callLocal request
+      res <- mock @Service @IO . TinyLog.discardLogs . Polysemy.runReader noClientCertSettings $ callLocal request
 
       actualCalls <- mockServiceCallCalls @IO
       let expectedCall = (Brig, "federation/get-user-by-handle", "\"foo\"", aValidDomain)
@@ -82,7 +82,7 @@ requestGalleySuccess =
       mockServiceCallReturns @IO (\_ _ _ _ -> pure (HTTP.ok200, Just "response body"))
       let request = Request Galley "federation/get-conversations" "{}" exampleDomain
 
-      res :: InwardResponse <- mock @Service @IO . TinyLog.discardLogs . Polysemy.runReader defRunSettings $ callLocal request
+      res :: InwardResponse <- mock @Service @IO . TinyLog.discardLogs . Polysemy.runReader noClientCertSettings $ callLocal request
       actualCalls <- mockServiceCallCalls @IO
       let expectedCall = (Galley, "federation/get-conversations", "{}", aValidDomain)
       embed $ assertEqual "one call to brig should be made" [expectedCall] actualCalls
diff --git a/services/federator/test/unit/Test/Federator/InternalServer.hs b/services/federator/test/unit/Test/Federator/InternalServer.hs
index 192817a76a..6b691de7c5 100644
--- a/services/federator/test/unit/Test/Federator/InternalServer.hs
+++ b/services/federator/test/unit/Test/Federator/InternalServer.hs
@@ -29,7 +29,7 @@ import Mu.GRpc.Client.Record
 import Network.HTTP2.Client (TooMuchConcurrency (TooMuchConcurrency))
 import Polysemy (embed, runM)
 import qualified Polysemy.Reader as Polysemy
-import Test.Federator.Options (defRunSettings)
+import Test.Federator.Options (noClientCertSettings)
 import Test.Polysemy.Mock (Mock (mock), evalMock)
 import Test.Polysemy.Mock.TH (genMock)
 import Test.Tasty (TestTree, testGroup)
@@ -54,7 +54,7 @@ tests =
 
 settingsWithAllowList :: [Domain] -> RunSettings
 settingsWithAllowList domains =
-  defRunSettings {federationStrategy = AllowList (AllowedDomains domains)}
+  noClientCertSettings {federationStrategy = AllowList (AllowedDomains domains)}
 
 federatedRequestSuccess :: TestTree
 federatedRequestSuccess =
@@ -63,7 +63,9 @@ federatedRequestSuccess =
       mockDiscoverAndCallReturns @IO (const $ pure (Right (GRpcOk (InwardResponseBody "success!"))))
       let federatedRequest = FederatedRequest validDomainText (Just validLocalPart)
 
-      res <- mock @Remote @IO . Polysemy.runReader defRunSettings $ callOutward federatedRequest
+      res <-
+        mock @Remote @IO . Polysemy.runReader noClientCertSettings $
+          callOutward federatedRequest
 
       actualCalls <- mockDiscoverAndCallCalls @IO
       let expectedCall = ValidatedFederatedRequest (Domain validDomainText) validLocalPart
@@ -79,7 +81,7 @@ federatedRequestFailureTMC =
       mockDiscoverAndCallReturns @IO (const $ pure (Right (GRpcTooMuchConcurrency (TooMuchConcurrency 2))))
       let federatedRequest = FederatedRequest validDomainText (Just validLocalPart)
 
-      res <- mock @Remote @IO . Polysemy.runReader defRunSettings $ callOutward federatedRequest
+      res <- mock @Remote @IO . Polysemy.runReader noClientCertSettings $ callOutward federatedRequest
 
       actualCalls <- mockDiscoverAndCallCalls @IO
       let expectedCall = ValidatedFederatedRequest (Domain validDomainText) validLocalPart
@@ -94,7 +96,7 @@ federatedRequestFailureErrCode =
       mockDiscoverAndCallReturns @IO (const $ pure (Right (GRpcErrorCode 77))) -- TODO: Maybe use some legit HTTP2 error code?
       let federatedRequest = FederatedRequest validDomainText (Just validLocalPart)
 
-      res <- mock @Remote @IO . Polysemy.runReader defRunSettings $ callOutward federatedRequest
+      res <- mock @Remote @IO . Polysemy.runReader noClientCertSettings $ callOutward federatedRequest
 
       actualCalls <- mockDiscoverAndCallCalls @IO
       let expectedCall = ValidatedFederatedRequest (Domain validDomainText) validLocalPart
@@ -109,7 +111,7 @@ federatedRequestFailureErrStr =
       mockDiscoverAndCallReturns @IO (const $ pure (Right (GRpcErrorString "some grpc error")))
       let federatedRequest = FederatedRequest validDomainText (Just validLocalPart)
 
-      res <- mock @Remote @IO . Polysemy.runReader defRunSettings $ callOutward federatedRequest
+      res <- mock @Remote @IO . Polysemy.runReader noClientCertSettings $ callOutward federatedRequest
 
       actualCalls <- mockDiscoverAndCallCalls @IO
       let expectedCall = ValidatedFederatedRequest (Domain validDomainText) validLocalPart
@@ -124,7 +126,7 @@ federatedRequestFailureNoRemote =
       mockDiscoverAndCallReturns @IO (const $ pure (Left $ RemoteErrorDiscoveryFailure (Domain "example.com") (LookupErrorSrvNotAvailable "_something._tcp.example.com")))
       let federatedRequest = FederatedRequest validDomainText (Just validLocalPart)
 
-      res <- mock @Remote @IO . Polysemy.runReader defRunSettings $ callOutward federatedRequest
+      res <- mock @Remote @IO . Polysemy.runReader noClientCertSettings $ callOutward federatedRequest
 
       actualCalls <- mockDiscoverAndCallCalls @IO
       let expectedCall = ValidatedFederatedRequest (Domain validDomainText) validLocalPart
@@ -139,7 +141,7 @@ federatedRequestFailureDNS =
       mockDiscoverAndCallReturns @IO (const $ pure (Left $ RemoteErrorDiscoveryFailure (Domain "example.com") (LookupErrorDNSError "No route to 1.1.1.1")))
       let federatedRequest = FederatedRequest validDomainText (Just validLocalPart)
 
-      res <- mock @Remote @IO . Polysemy.runReader defRunSettings $ callOutward federatedRequest
+      res <- mock @Remote @IO . Polysemy.runReader noClientCertSettings $ callOutward federatedRequest
 
       actualCalls <- mockDiscoverAndCallCalls @IO
       let expectedCall = ValidatedFederatedRequest (Domain validDomainText) validLocalPart
diff --git a/services/federator/test/unit/Test/Federator/Options.hs b/services/federator/test/unit/Test/Federator/Options.hs
index ce797f8ea6..0b7b796a5f 100644
--- a/services/federator/test/unit/Test/Federator/Options.hs
+++ b/services/federator/test/unit/Test/Federator/Options.hs
@@ -37,16 +37,19 @@ import Imports
 import Test.Tasty
 import Test.Tasty.HUnit
 
-defRunSettings :: RunSettings
-defRunSettings =
+defRunSettings :: FilePath -> FilePath -> RunSettings
+defRunSettings client key =
   RunSettings
     { federationStrategy = AllowAll,
       useSystemCAStore = True,
       remoteCAStore = Nothing,
-      clientCertificate = Nothing,
-      clientPrivateKey = Nothing
+      clientCertificate = client,
+      clientPrivateKey = key
     }
 
+noClientCertSettings :: RunSettings
+noClientCertSettings = defRunSettings "invalid-cert" "invalid-private-key"
+
 tests :: TestTree
 tests =
   testGroup
@@ -88,23 +91,23 @@ testSettings =
     "settings"
     [ testCase "parse configuration example (open federation)" $ do
         assertParsesAs
-          defRunSettings
+          (defRunSettings "client.pem" "client-key.pem")
           ( B8.pack
               [QQ.i|
                 federationStrategy:
                   allowAll:
+                clientCertificate: client.pem
+                clientPrivateKey: client-key.pem
                 useSystemCAStore: true|]
           ),
       testCase "parse configuration example (closed federation)" $ do
         let settings =
-              defRunSettings
+              (defRunSettings "client.pem" "client-key.pem")
                 { federationStrategy =
                     AllowList
                       ( AllowedDomains [Domain "server2.example.com"]
                       ),
-                  useSystemCAStore = False,
-                  clientCertificate = Just "client.pem",
-                  clientPrivateKey = Just "client-key.pem"
+                  useSystemCAStore = False
                 }
         assertParsesAs settings . B8.pack $
           [QQ.i|
@@ -117,9 +120,8 @@ testSettings =
       testCase "succefully read client credentials" $ do
         let settings =
               defRunSettings
-                { clientCertificate = Just "test/resources/unit/localhost.pem",
-                  clientPrivateKey = Just "test/resources/unit/localhost-key.pem"
-                }
+                "test/resources/unit/localhost.pem"
+                "test/resources/unit/localhost-key.pem"
         assertParsesAs settings . B8.pack $
           [QQ.i|
           useSystemCAStore: true
@@ -127,67 +129,29 @@ testSettings =
             allowAll: null
           clientCertificate: test/resources/unit/localhost.pem
           clientPrivateKey: test/resources/unit/localhost-key.pem|]
-        tlsSettings <- mkTLSSettings settings
-        assertBool "expected TLS client credentials" $
-          notNullOf (creds . _Just) tlsSettings,
-      testCase "parse missing client credentials" $ do
-        let settings = defRunSettings
-        assertParsesAs settings . B8.pack $
+        void (mkTLSSettings settings),
+      testCase "fail on missing client credentials" $
+        assertParseFailure @RunSettings . B8.pack $
           [QQ.i|
           useSystemCAStore: true
           federationStrategy:
-            allowAll: null|]
-        tlsSettings <- mkTLSSettings settings
-        assertBool "unexpected TLS client credentials" $
-          nullOf (creds . _Just) tlsSettings,
+            allowAll: null|],
       testCase "fail on missing client private key" $ do
-        let settings =
-              defRunSettings
-                { clientCertificate = Just "test/resources/unit/localhost.pem"
-                }
-        assertParsesAs settings . B8.pack $
+        assertParseFailure @RunSettings . B8.pack $
           [QQ.i|
           useSystemCAStore: true
           federationStrategy:
             allowAll: null
-          clientCertificate: test/resources/unit/localhost.pem|]
-        try @FederationSetupError (mkTLSSettings settings) >>= \case
-          Left (InvalidClientCertificate _) -> pure ()
-          Left e ->
-            assertFailure $
-              "expected invalid client certificate exception, got: "
-                <> show e
-          Right tlsSettings ->
-            assertFailure $
-              "expected failure for partial client credentials, got: "
-                <> show (tlsSettings ^. creds),
+          clientCertificate: test/resources/unit/localhost.pem|],
       testCase "fail on missing certificate" $ do
-        let settings =
-              defRunSettings
-                { clientPrivateKey = Just "test/resources/unit/localhost-key.pem"
-                }
-        assertParsesAs settings . B8.pack $
+        assertParseFailure @RunSettings . B8.pack $
           [QQ.i|
           useSystemCAStore: true
           federationStrategy:
             allowAll: null
-          clientPrivateKey: test/resources/unit/localhost-key.pem|]
-        try @FederationSetupError (mkTLSSettings settings) >>= \case
-          Left (InvalidClientCertificate _) -> pure ()
-          Left e ->
-            assertFailure $
-              "expected invalid client certificate exception, got: "
-                <> show e
-          Right tlsSettings ->
-            assertFailure $
-              "expected failure for partial client credentials, got: "
-                <> show (tlsSettings ^. creds),
+          clientPrivateKey: test/resources/unit/localhost-key.pem|],
       testCase "fail on non-existent certificate" $ do
-        let settings =
-              defRunSettings
-                { clientCertificate = Just "non-existent",
-                  clientPrivateKey = Just "non-existent"
-                }
+        let settings = defRunSettings "non-existent" "non-existent"
         assertParsesAs settings . B8.pack $
           [QQ.i|
           useSystemCAStore: true
@@ -208,9 +172,8 @@ testSettings =
       testCase "fail on invalid certificate" $ do
         let settings =
               defRunSettings
-                { clientCertificate = Just "test/resources/unit/invalid.pem",
-                  clientPrivateKey = Just "test/resources/unit/localhost-key.pem"
-                }
+                "test/resources/unit/invalid.pem"
+                "test/resources/unit/localhost-key.pem"
         assertParsesAs settings . B8.pack $
           [QQ.i|
           useSystemCAStore: true
@@ -231,9 +194,8 @@ testSettings =
       testCase "fail on invalid private key" $ do
         let settings =
               defRunSettings
-                { clientCertificate = Just "test/resources/unit/localhost.pem",
-                  clientPrivateKey = Just "test/resources/unit/invalid.pem"
-                }
+                "test/resources/unit/localhost.pem"
+                "test/resources/unit/invalid.pem"
         assertParsesAs settings . B8.pack $
           [QQ.i|
           useSystemCAStore: true
@@ -257,3 +219,8 @@ assertParsesAs :: (HasCallStack, Eq a, FromJSON a, Show a) => a -> ByteString ->
 assertParsesAs v bs =
   assertEqual "YAML parsing" (Right v) $
     either (Left . show) Right (Yaml.decodeEither' bs)
+
+assertParseFailure :: forall a. (FromJSON a, Show a) => ByteString -> Assertion
+assertParseFailure bs = case Yaml.decodeEither' bs of
+  Left _ -> pure ()
+  Right (x :: a) -> assertFailure $ "expected YAML parsing failure, got: " <> show x
diff --git a/services/federator/test/unit/Test/Federator/Remote.hs b/services/federator/test/unit/Test/Federator/Remote.hs
index 6ab6a5b077..51392236e2 100644
--- a/services/federator/test/unit/Test/Federator/Remote.hs
+++ b/services/federator/test/unit/Test/Federator/Remote.hs
@@ -32,30 +32,30 @@ tests =
         ]
     ]
 
+settings :: RunSettings
+settings =
+  ( defRunSettings
+      "test/resources/unit/localhost.pem"
+      "test/resources/unit/localhost-key.pem"
+  )
+    { useSystemCAStore = False,
+      remoteCAStore = Just "test/resources/unit/unit-ca.pem"
+    }
+
 testValidatesCertificateSuccess :: TestTree
 testValidatesCertificateSuccess =
   testGroup
     "can get response with valid certificate"
     [ testCase "when hostname=localhost and certificate-for=localhost" $ do
         bracket (startMockServer certForLocalhost) (\(serverThread, _) -> Async.cancel serverThread) $ \(_, port) -> do
-          tlsSettings <-
-            mkTLSSettings $
-              defRunSettings
-                { useSystemCAStore = False,
-                  remoteCAStore = Just "test/resources/unit/unit-ca.pem"
-                }
+          tlsSettings <- mkTLSSettings settings
           eitherClient <- Polysemy.runM . TinyLog.discardLogs . Polysemy.runReader tlsSettings $ mkGrpcClient (SrvTarget "localhost" (fromIntegral port))
           case eitherClient of
             Left err -> assertFailure $ "Unexpected error: " <> show err
             Right _ -> pure (),
       testCase "when hostname=localhost. and certificate-for=localhost" $ do
         bracket (startMockServer certForLocalhost) (\(serverThread, _) -> Async.cancel serverThread) $ \(_, port) -> do
-          tlsSettings <-
-            mkTLSSettings $
-              defRunSettings
-                { useSystemCAStore = False,
-                  remoteCAStore = Just "test/resources/unit/unit-ca.pem"
-                }
+          tlsSettings <- mkTLSSettings settings
           eitherClient <- Polysemy.runM . TinyLog.discardLogs . Polysemy.runReader tlsSettings $ mkGrpcClient (SrvTarget "localhost." (fromIntegral port))
           case eitherClient of
             Left err -> assertFailure $ "Unexpected error: " <> show err
@@ -63,12 +63,7 @@ testValidatesCertificateSuccess =
       -- This is a limitation of the TLS library, this test just exists to document that.
       testCase "when hostname=localhost. and certificate-for=localhost." $ do
         bracket (startMockServer certForLocalhostDot) (\(serverThread, _) -> Async.cancel serverThread) $ \(_, port) -> do
-          tlsSettings <-
-            mkTLSSettings $
-              defRunSettings
-                { useSystemCAStore = False,
-                  remoteCAStore = Just "test/resources/unit/unit-ca.pem"
-                }
+          tlsSettings <- mkTLSSettings settings
           eitherClient <-
             Polysemy.runM . TinyLog.discardLogs . Polysemy.runReader tlsSettings $
               mkGrpcClient (SrvTarget "localhost." (fromIntegral port))
@@ -83,12 +78,7 @@ testValidatesCertificateWrongHostname =
     "refuses to connect with server"
     [ testCase "when the server's certificate doesn't match the hostname" $
         bracket (startMockServer certForWrongDomain) (Async.cancel . fst) $ \(_, port) -> do
-          tlsSettings <-
-            mkTLSSettings $
-              defRunSettings
-                { useSystemCAStore = False,
-                  remoteCAStore = Just "test/resources/unit/unit-ca.pem"
-                }
+          tlsSettings <- mkTLSSettings settings
           eitherClient <-
             Polysemy.runM . TinyLog.discardLogs . Polysemy.runReader tlsSettings $
               mkGrpcClient (SrvTarget "localhost." (fromIntegral port))
@@ -111,14 +101,14 @@ startMockServer :: MonadIO m => WarpTLS.TLSSettings -> m (Async.Async (), Warp.P
 startMockServer tlsSettings = liftIO $ do
   (port, sock) <- bindRandomPortTCP "*6"
   serverStarted <- newEmptyMVar
-  let settings =
+  let wsettings =
         Warp.defaultSettings
           & Warp.setPort port
           & Warp.setGracefulCloseTimeout2 0 -- Defaults to 2 seconds, causes server stop to take very long
           & Warp.setBeforeMainLoop (putMVar serverStarted ())
       app _req respond = respond $ responseLBS status200 [] "dragons be here"
 
-  serverThread <- Async.async $ WarpTLS.runTLSSocket tlsSettings settings sock app
+  serverThread <- Async.async $ WarpTLS.runTLSSocket tlsSettings wsettings sock app
   serverStartedSignal <- timeout 10_000_000 (takeMVar serverStarted)
   case serverStartedSignal of
     Nothing -> do
diff --git a/services/federator/test/unit/Test/Federator/Validation.hs b/services/federator/test/unit/Test/Federator/Validation.hs
index 89d5b84818..0c0f67a3dd 100644
--- a/services/federator/test/unit/Test/Federator/Validation.hs
+++ b/services/federator/test/unit/Test/Federator/Validation.hs
@@ -32,7 +32,7 @@ import Polysemy.Embed
 import qualified Polysemy.Error as Polysemy
 import qualified Polysemy.Reader as Polysemy
 import Test.Federator.InternalServer ()
-import Test.Federator.Options (defRunSettings)
+import Test.Federator.Options (noClientCertSettings)
 import Test.Polysemy.Mock (evalMock)
 import Test.Tasty
 import Test.Tasty.HUnit
@@ -178,4 +178,4 @@ expectErr expectedType (Left err) =
 
 settingsWithAllowList :: [Domain] -> RunSettings
 settingsWithAllowList domains =
-  defRunSettings {federationStrategy = AllowList (AllowedDomains domains)}
+  noClientCertSettings {federationStrategy = AllowList (AllowedDomains domains)}

From 5054ada31c0a89d79b9fd22e9ef7fe46f66a4f2f Mon Sep 17 00:00:00 2001
From: Paolo Capriotti <paolo@capriotti.io>
Date: Wed, 11 Aug 2021 12:06:00 +0200
Subject: [PATCH 34/41] Update FUTUREWORK comment

Co-authored-by: Akshay Mankar <akshay@wire.com>
---
 services/federator/src/Federator/Remote.hs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/services/federator/src/Federator/Remote.hs b/services/federator/src/Federator/Remote.hs
index ec4ed7e4e4..d3b27f5d68 100644
--- a/services/federator/src/Federator/Remote.hs
+++ b/services/federator/src/Federator/Remote.hs
@@ -91,6 +91,7 @@ callInward client request =
 -- FUTUREWORK(federation): Consider using HsOpenSSL instead of tls for better
 -- security and to avoid having to depend on cryptonite and override validation
 -- hooks. This might involve forking http2-client: https://github.com/lucasdicioccio/http2-client/issues/76
+-- FUTUREWORK(federation): Use openssl
 --   See also https://github.com/lucasdicioccio/http2-client/issues/76
 -- FUTUREWORK(federation): Cache this client and use it for many requests
 mkGrpcClient ::

From c31d88b2480323bd49172a6aacad9b195a0b466f Mon Sep 17 00:00:00 2001
From: Paolo Capriotti <paolo@capriotti.io>
Date: Wed, 11 Aug 2021 12:11:35 +0200
Subject: [PATCH 35/41] Fix helm template syntax

---
 charts/federator/templates/configmap.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/charts/federator/templates/configmap.yaml b/charts/federator/templates/configmap.yaml
index 48be920d86..b19b1bc272 100644
--- a/charts/federator/templates/configmap.yaml
+++ b/charts/federator/templates/configmap.yaml
@@ -45,7 +45,7 @@ data:
       {{- if $.Values.remoteCAContents }}
       remoteCAStore: "/etc/wire/federator/ca/ca.crt"
       {{- end }}
-      {{- if $.Values.clientCertificateContents or $.Values.tls.useSharedFederatorSecret }}
+      {{- if or $.Values.clientCertificateContents $.Values.tls.useSharedFederatorSecret }}
       clientCertificate: "/etc/wire/federator/secrets/tls.crt"
       clientPrivateKey: "/etc/wire/federator/secrets/tls.key"
       {{- end }}

From 78b97fc5e952d8ed2a5be162194daa152a8a432f Mon Sep 17 00:00:00 2001
From: Paolo Capriotti <paolo@capriotti.io>
Date: Thu, 12 Aug 2021 15:15:18 +0200
Subject: [PATCH 36/41] Upgrade to tls 1.5.5

---
 stack.yaml      |  2 ++
 stack.yaml.lock | 14 ++++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/stack.yaml b/stack.yaml
index 51be210158..9c2b6a95c8 100644
--- a/stack.yaml
+++ b/stack.yaml
@@ -174,6 +174,8 @@ extra-deps:
 - splitmix-0.0.4 # needed for QuickCheck
 - servant-mock-0.8.7
 - servant-swagger-ui-0.3.4.3.36.1
+- tls-1.5.5
+- cryptonite-0.28
 
 # For changes from #128 and #135, not released to hackage yet
 - git: https://github.com/haskell-servant/servant-swagger
diff --git a/stack.yaml.lock b/stack.yaml.lock
index e412ea3e03..7fec0d171f 100644
--- a/stack.yaml.lock
+++ b/stack.yaml.lock
@@ -519,6 +519,20 @@ packages:
       sha256: 137680a15ee0147cd19634d8296d90c2150ca4fd62ed3d56f7e07ccd8823810c
   original:
     hackage: servant-swagger-ui-0.3.4.3.36.1
+- completed:
+    hackage: tls-1.5.5@sha256:f6681d6624071211edd509a8f56e0c96b4f003bb349b7dc706d4333775a373c5,6996
+    pantry-tree:
+      size: 4897
+      sha256: 6400901a8f8ddd0c7d2fb30c95857753a654a60a63fcec38aab4a08b23e0984f
+  original:
+    hackage: tls-1.5.5
+- completed:
+    hackage: cryptonite-0.28@sha256:b6c75e62b4c655d4cb1bcbb80d01430d136aac32bd6962c86c84738935cc8f9d,18195
+    pantry-tree:
+      size: 23132
+      sha256: d80d7be9b1d0799a8e401ca5d4f4f424e0d8c42d4a30cc37bf6f82970232bfcf
+  original:
+    hackage: cryptonite-0.28
 - completed:
     name: servant-swagger
     version: 1.1.11

From 11ca67de1ec6a4bccebb26c3d6245b2e70706a89 Mon Sep 17 00:00:00 2001
From: Paolo Capriotti <paolo@capriotti.io>
Date: Fri, 13 Aug 2021 13:40:05 +0200
Subject: [PATCH 37/41] Hi CI


From e80417d21818b47378547558cf10d1f5f7e9ed8e Mon Sep 17 00:00:00 2001
From: Paolo Capriotti <paolo@capriotti.io>
Date: Fri, 13 Aug 2021 14:38:05 +0200
Subject: [PATCH 38/41] Ensure that tls is enabled when federation is

Co-authored-by: Akshay Mankar <akshay@wire.com>
---
 .../templates/certificate_federator.yaml                     | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/charts/nginx-ingress-services/templates/certificate_federator.yaml b/charts/nginx-ingress-services/templates/certificate_federator.yaml
index 07dd2056ae..8324875e0c 100644
--- a/charts/nginx-ingress-services/templates/certificate_federator.yaml
+++ b/charts/nginx-ingress-services/templates/certificate_federator.yaml
@@ -1,4 +1,7 @@
-{{- if and .Values.tls.enabled .Values.tls.useCertManager .Values.federator.enabled }}
+{{- if and .Values.federator.enabled (not .Values.tls.enabled) }}
+{{- fail "TLS is required by federator. Either disable federation or enable tls." }}
+{{- end }}
+{{- if and .Values.tls.enabled .Values.tls.useCertManager }}
 apiVersion: cert-manager.io/v1alpha2
 kind: Certificate
 metadata:

From 476e3209f84ac5b1776116cb402636c9ac0ca7fd Mon Sep 17 00:00:00 2001
From: Akshay Mankar <akshay@wire.com>
Date: Mon, 16 Aug 2021 10:48:11 +0200
Subject: [PATCH 39/41] serve-charts.sh: Make compatible with nixos

---
 hack/bin/serve-charts.sh | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/hack/bin/serve-charts.sh b/hack/bin/serve-charts.sh
index eb69bf2af1..09172c43f1 100755
--- a/hack/bin/serve-charts.sh
+++ b/hack/bin/serve-charts.sh
@@ -1,9 +1,12 @@
-#!/bin/bash -e
+#!/usr/bin/env bash
+
+set -euo pipefail
 
 : ${HELM_SERVER_PORT:=4001}
 
 # get rid of all helm repositories
-helm repo list -o json | jq -r '.[] | .name' | xargs -I% helm repo remove %
+# We need to deal with helm repo list failing because of https://github.com/helm/helm/issues/10028
+(helm repo list -o json || echo '[]') | jq -r '.[] | .name' | xargs -I% helm repo remove %
 
 cd "$(dirname "$BASH_SOURCE[0]")/../../.local/charts"
 for chart in $@; do

From c5758586aa59c2d53b57ff89d9b10aa329d603a6 Mon Sep 17 00:00:00 2001
From: Akshay Mankar <akshay@wire.com>
Date: Mon, 16 Aug 2021 12:07:14 +0200
Subject: [PATCH 40/41] charts/federator: Ensure client certs are provided

---
 charts/federator/templates/configmap.yaml  | 2 --
 charts/federator/templates/deployment.yaml | 4 +++-
 charts/federator/templates/secret.yaml     | 1 -
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/charts/federator/templates/configmap.yaml b/charts/federator/templates/configmap.yaml
index b19b1bc272..da4623a738 100644
--- a/charts/federator/templates/configmap.yaml
+++ b/charts/federator/templates/configmap.yaml
@@ -45,10 +45,8 @@ data:
       {{- if $.Values.remoteCAContents }}
       remoteCAStore: "/etc/wire/federator/ca/ca.crt"
       {{- end }}
-      {{- if or $.Values.clientCertificateContents $.Values.tls.useSharedFederatorSecret }}
       clientCertificate: "/etc/wire/federator/secrets/tls.crt"
       clientPrivateKey: "/etc/wire/federator/secrets/tls.key"
-      {{- end }}
       useSystemCAStore: {{ .useSystemCAStore }}
       federationStrategy:
         {{- if .federationStrategy.allowAll }}
diff --git a/charts/federator/templates/deployment.yaml b/charts/federator/templates/deployment.yaml
index 6133871b58..c09a239710 100644
--- a/charts/federator/templates/deployment.yaml
+++ b/charts/federator/templates/deployment.yaml
@@ -44,8 +44,10 @@ spec:
           secret:
             secretName: {{ if .Values.tls.useSharedFederatorSecret -}}
               "federator-certificate-secret"
-              {{- else -}}
+              {{- else if .Values.clientCertificateContents -}}
               "federator-secret"
+              {{- else }}
+              {{ fail "must set .Values.tls.useSharedFederatorSecret to true or specify .Values.clientCertificateContents" }}
               {{- end }}
 
         - name: "federator-ca"
diff --git a/charts/federator/templates/secret.yaml b/charts/federator/templates/secret.yaml
index 551dd96619..f1337b952d 100644
--- a/charts/federator/templates/secret.yaml
+++ b/charts/federator/templates/secret.yaml
@@ -10,7 +10,6 @@ metadata:
     heritage: {{ .Release.Service }}
 type: kubernetes.io/tls
 data:
-  # TODO: add validation and fail early during templating: either contents should be provided; or explicitly system trust store enabled
   {{- if .Values.clientPrivateKeyContents }}
   tls.key: {{ .Values.clientPrivateKeyContents | b64enc | quote }}
   {{- end -}}

From fa2698b6c2e74e958f9ee5bc685a6e2c9ba4548a Mon Sep 17 00:00:00 2001
From: Akshay Mankar <akshay@wire.com>
Date: Mon, 16 Aug 2021 13:46:59 +0200
Subject: [PATCH 41/41] Fix CHANGELOG

---
 CHANGELOG.md | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index acead2b5be..8d48ed22d2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -37,6 +37,9 @@
 
 ## Internal changes
 
+## Federation changes
+
+* Added client certificate support for server to server authentication (#1682)
 
 # [2021-08-16]
 
@@ -72,11 +75,6 @@ This is a routine release requiring only the routine upgrade steps.
 * Added a mechanism to derive `AsUnion` instances automatically (#1693)
 * Integration test coverage (#1696, #1704)
 
-
-## Federation changes
-
-* Added client certificate support for server to server authentication (#1682)
-
 # [2021-08-02]
 
 ## Release Notes