certificate checking for clusterd #4628
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lindig
force-pushed
the
private/christianlin/tls-v2
branch
2 times, most recently
from
54803df
to
782eb93
Compare
edwintorok
reviewed
Feb 21, 2022
| @@ -1008,8 +1008,6 @@ functor | |||
| debug "Pool.enable_tls_verification sleeping on fistpoint" ; | |||
| Thread.delay 5.0 | |||
| done ; | |||
| Db.Cluster.get_all ~__context | |||
| |> List.iter (Xapi_cluster_helpers.Pem.maybe_write_new ~__context) ; | |||
| all_hosts | |||
| |> List.iter (fun host -> | |||
| do_op_on ~local_fn ~__context ~host (fun session_id rpc -> | |||
There was a problem hiding this comment.
Since you loop over all hosts then this should be Client.Host.enable_tls_verification, not Client.Pool.enable_tls_verification (unrelated to clusterd changes, perhaps fix it in a separate commit).
If it was really a pool level operation then it should've taken a pool as an argument and do the looping internally (but you'd still need a host-level API to call).
There was a problem hiding this comment.
This code pings all hosts in a pool before enabling TLS verification on the local host. These functions are not available on the host level - so moving the code is not trivial.
edwintorok
approved these changes
Feb 21, 2022
psafont
reviewed
Feb 21, 2022
psafont
approved these changes
Feb 21, 2022
lindig
force-pushed
the
private/christianlin/tls-v2
branch
2 times, most recently
from
fbe11d3
to
5a7056a
Compare
Certificate checking for clusterd is implemented by relying on the certificates xapi uses for intra-pool communication. This implies that only hosts part of a xapi pool can form a cluster. Each host in the pool has a certficate (public and private key) and knows the public keys of all hosts in the pool. These are kept in PEM bundles. We remove code from a previous design where clusterd used a single certificate generated by xapi which was passed to clusterd. A tls_config, sent to clusterd, contains the essential information: * The common name (CN) * Path the server certificate * Optional path to bundle with trusted certificate or None if clusterd should not perform certficate checking. The tls_config is global per cluster and hence the CN has to be a global value and can't be specific per host. For now, use a simple string to avoid confusion. Signed-off-by: Christian Lindig <christian.lindig@citrix.com>
Signed-off-by: Christian Lindig <christian.lindig@citrix.com>
Signed-off-by: Christian Lindig <christian.lindig@citrix.com>
Certificates for xapi's API clients and internal pool communication are generated by gencert.service, which invokes gencert for this. The certificate for the API clients requires the IP of the management interface (obtained by gencert), which might not be yet available and causes this to fail. The certificate for the pool communication does not suffer from this dependency. So generate it first to have it available. If the second call then fails, the systemd will run it again. But gencert is idempotent such that the already created certificate for pool communication won't be overwritten. Signed-off-by: Christian Lindig <christian.lindig@citrix.com>
Signed-off-by: Christian Lindig <christian.lindig@citrix.com>
lindig
force-pushed
the
private/christianlin/tls-v2
branch
from
5a7056a
to
de6ee60
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Just to collect some comments - changes in xapi to enable certificate checking in clusterd. Clusterd uses the certificates used also by xapi.
The protocol/IDL between xapi/clusterd and clusterd/clusterd changes to include a TLS configuration which holds the paths to the files containing the certificates. This simplifies the previous design which involved using single certificate for the cluster.