-
Notifications
You must be signed in to change notification settings - Fork 334
Validate server TLS certificate between federators #1662
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
1e37e83
a42976f
349d312
f15d8cb
f336ca6
906e455
fef7873
81c03e8
0e39191
c550172
e5c2e5d
bf8270d
388e658
0d71244
7304697
79dfd9e
c1a6b1d
2ac39b0
8c0890c
5649ce5
0d0bc1f
216dae1
ffb9041
7f71f7d
3a63b96
9c457d5
1accf60
1e1769e
9087125
6b7274d
b0361bc
3ab9971
b7a38cf
07eb851
7b8e3ea
25dfd18
b915368
9f221ed
2e57c77
b5239e9
0ae9d6f
824ed2b
3368a6f
7976ba5
2f72d39
1a449f3
d973ef5
3e40ee4
befabef
b756d62
5330d8d
4dbb314
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -99,4 +99,4 @@ i.yaml | |
| b.yaml | ||
| telepresence.log | ||
|
|
||
| /.ghci | ||
| /.ghci | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| 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 }} | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,7 @@ kind: ConfigMap | |
| metadata: | ||
| name: "federator" | ||
| labels: | ||
| wireService: fedrator | ||
| wireService: federator | ||
| chart: {{ .Chart.Name }}-{{ .Chart.Version | replace "+" "_" }} | ||
| release: {{ .Release.Name }} | ||
| heritage: {{ .Release.Service }} | ||
|
|
@@ -40,12 +40,18 @@ data: | |
|
|
||
| {{- with .optSettings }} | ||
| optSettings: | ||
| setFederationStrategy: | ||
| {{- if .setFederationStrategy.allowAll }} | ||
| # 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" | ||
| {{- end }} | ||
| useSystemCAStore: {{ .useSystemCAStore }} | ||
| federationStrategy: | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note: Remember to update the values in the federation anta/bella/chala environments when this PR gets merged.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This part should be fine in anta/bella/chala, as by default we use the system CA, and those certificates are supposed to be Let's Encrypt. However, it seems the Let's Encrypt certificates are not actually being used, so I guess there is a problem somewhere in the ingress configuration. |
||
| {{- if .federationStrategy.allowAll }} | ||
| allowAll: | ||
| {{- else if .setFederationStrategy.allowedDomains }} | ||
| {{- else if .federationStrategy.allowedDomains }} | ||
| allowedDomains: | ||
| {{- range $domain := .setFederationStrategy.allowedDomains }} | ||
| {{- range $domain := .federationStrategy.allowedDomains }} | ||
| - {{ $domain | quote }} | ||
| {{- end }} | ||
| {{- else }} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -153,12 +153,12 @@ optSettings: | |
|
|
||
| ### Federation allow list | ||
|
|
||
| As of 2021-02, federation (whatever is implemented by the time you read this) is turned off by default by means of having an empty allow list: | ||
| As of 2021-07, federation (whatever is implemented by the time you read this) is turned off by default by means of having an empty allow list: | ||
|
|
||
| ```yaml | ||
| # federator.yaml | ||
| optSettings: | ||
| setFederationStrategy: | ||
| federationStrategy: | ||
| allowedDomains: [] | ||
| ``` | ||
|
|
||
|
|
@@ -168,7 +168,7 @@ You can choose to federate with a specific list of allowed servers: | |
| ```yaml | ||
| # federator.yaml | ||
| optSettings: | ||
| setFederationStrategy: | ||
| federationStrategy: | ||
| allowedDomains: | ||
| - server1.example.com | ||
| - server2.example.com | ||
|
|
@@ -179,14 +179,29 @@ or, you can federate with everyone: | |
| ```yaml | ||
| # federator.yaml | ||
| optSettings: | ||
| setFederationStrategy: | ||
| federationStrategy: | ||
| # note the 'empty' value after 'allowAll' | ||
| allowAll: | ||
|
|
||
| # when configuring helm charts, this becomes (note 'true' after 'allowAll') | ||
| # inside helm_vars/wire-server: | ||
| federator: | ||
| optSettings: | ||
| setFederationStrategy: | ||
| federationStrategy: | ||
| allowAll: true | ||
| ``` | ||
|
|
||
| ### 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: | ||
| 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 | ||
| multiple certificates from either a single file (multiple PEM formatted | ||
| certificates concatenated) or directory (one certificate per file, file names | ||
| are hashes from certificate). | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be good to provide an example yaml documentation block with the exact syntax and spelling. And ti would be good to list the default.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've added some examples in the followup PR #1682 (commit 011e83a2fa55e10554a825b70e2f9325487b57c2), since that also contains new options. |
||
|
|
||
| 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. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| #!/usr/bin/env bash | ||
|
|
||
| # Create a self-signed x509 certificate in the hack/helm_vars directories (as helm yaml config). | ||
| # Requires 'cfssl' to be on your PATH (see https://github.com/cloudflare/cfssl) | ||
| # These certificates are only meant for integration tests. | ||
| # (The CA certificates are assumed to be re-used across the domains A and B for end2end integration tests.) | ||
|
|
||
| set -ex | ||
| TEMP=${TEMP:-/tmp} | ||
| CSR="$TEMP/csr.json" | ||
| OUTPUTNAME_CA="integration-ca" | ||
| OUTPUTNAME_LEAF_CERT="integration-leaf" | ||
| DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | ||
| TOP_LEVEL="$DIR/../.." | ||
| OUTPUT_CONFIG_FEDERATOR="$TOP_LEVEL/hack/helm_vars/wire-server/certificates.yaml" | ||
| OUTPUT_CONFIG_INGRESS="$TOP_LEVEL/hack/helm_vars/nginx-ingress-services/certificates.yaml" | ||
|
|
||
| command -v cfssl >/dev/null 2>&1 || { | ||
| echo >&2 "cfssl is not installed, aborting. See https://github.com/cloudflare/cfssl" | ||
| exit 1 | ||
| } | ||
| command -v cfssljson >/dev/null 2>&1 || { | ||
| echo >&2 "cfssljson is not installed, aborting. See https://github.com/cloudflare/cfssl" | ||
| exit 1 | ||
| } | ||
|
|
||
| FEDERATION_DOMAIN_BASE=${FEDERATION_DOMAIN_BASE:?"you must provide a FEDERATION_DOMAIN_BASE env variable"} | ||
|
|
||
| # generate CA key and cert | ||
| if [ ! -f "$OUTPUTNAME_CA.pem" ]; then | ||
| echo "CA file not found, generating CA..." | ||
| echo '{ | ||
| "CN": "ca.example.com", | ||
| "key": { | ||
| "algo": "rsa", | ||
| "size": 2048 | ||
| } | ||
| }' >"$CSR" | ||
| cfssl gencert -initca "$CSR" | cfssljson -bare "$OUTPUTNAME_CA" | ||
| rm "$OUTPUTNAME_CA.csr" | ||
| else | ||
| echo "Re-using previous CA" | ||
| fi | ||
|
|
||
| # For federation end2end tests, only the | ||
| # 'federation-test-helper.$FEDERATION_DOMAIN_BASE' is necessary for | ||
| # ingress->federator traffic. For other potential traffic in the integration | ||
| # tests of the future, we use a wildcard certificate here. | ||
| echo '{ | ||
| "key": { | ||
| "algo": "rsa", | ||
| "size": 2048 | ||
| } | ||
| }' >"$CSR" | ||
| # 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" | ||
|
|
||
| # 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 | ||
| { | ||
| echo "secrets:" | ||
| echo " tlsWildcardCert: |" | ||
| sed -e 's/^/ /' $OUTPUTNAME_LEAF_CERT.pem | ||
| echo " tlsWildcardKey: |" | ||
| sed -e 's/^/ /' $OUTPUTNAME_LEAF_CERT-key.pem | ||
| } | tee "$OUTPUT_CONFIG_INGRESS" | ||
|
|
||
| # the following yaml override file is needed as an override to | ||
| # the wire-server (federator) helm chart | ||
| # e.g. for installing on domain A, federator@A needs the CA for B | ||
| # As a "shortcut" for integration tests, we re-use the same CA for both domains | ||
| # A and B. | ||
| { | ||
| echo "federator:" | ||
| echo " remoteCAContents: |" | ||
| sed -e 's/^/ /' $OUTPUTNAME_CA.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 "$CSR" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| certificates.yaml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jschaul Why does this have to be a secret? It is the public key anyway. I think we can merge it into the configmap
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did this refactoring the followup PR #1682