Merge branch 'pcapriotti/client-certificates' into pcapriotti/refacto…

…r-remote-v2

CHANGELOG.md

@@ -19,7 +19,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
@@ -45,7 +45,6 @@ Upgrade nginz (#1658)
 * Extend feature config API (#1658)
 * `fileSharing` feature config (#1652, #1654, #1655)
 * Add user_id to csv export (#1663)
-* Validate server TLS certificate between federators (#1662)
 
 ## Bug fixes and other updates
 
@@ -71,6 +70,8 @@ Upgrade nginz (#1658)
 
 ## Federation changes (alpha feature, do not use yet)
 
+* Validate server TLS certificate between federators (#1662)
+* Added client certificate support for server to server authentication (#1682)
 
 # [2021-07-09]
 

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 }}

charts/federator/templates/deployment.yaml

@@ -25,27 +25,27 @@ 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 }}"
           imagePullPolicy: {{ default "" .Values.imagePullPolicy | quote }}
           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 }}

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 }}

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: []

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:

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 }}
 

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:

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;

docs/reference/config-options.md

@@ -197,14 +197,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
+```

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"

services/federator/federator.cabal

@@ -4,7 +4,7 @@ cabal-version: 1.12
 --
 -- see: https://github.com/sol/hpack
 --
--- hash: b6af0b547d17e02b65a3d3cdef9ca81eb86c1292957b7f9ae6b6e3e108cb983f
+-- hash: 2683ce1fbc5506e8987313d9d20b3b5aecf2e1edfcc3d5b0165a391a0f5a1c7c
 
 name:           federator
 version:        1.0.0
@@ -17,9 +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
     test/resources/unit/localhost-dot.pem
     test/resources/unit/localhost-key.pem

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"

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,26 @@ 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
 - 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:

services/federator/src/Federator/Env.hs

@@ -26,11 +26,18 @@ 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,
+    _creds :: Maybe TLS.Credential
+  }
+
 data Env = Env
   { _metrics :: Metrics,
     _applog :: LC.Logger,
@@ -39,7 +46,8 @@ data Env = Env
     _runSettings :: RunSettings,
     _service :: Component -> RPC.Request,
     _httpManager :: HTTP.Manager,
-    _caStore :: CertificateStore
+    _tls :: TLSSettings
   }
 
+makeLenses ''TLSSettings
 makeLenses ''Env