Add pool.sync_trusted_certificates_from API#7056
Conversation
| Note over client,coor: sync all ldaps certs | ||
| client->>coor: pool.download_trusted_certificate | ||
| client->>coor: pool.download_trusted_certificates | ||
| coor-->>client: | ||
| client->>join: pool.install_trusted_certificate | ||
| join-->>client: |
There was a problem hiding this comment.
Why does the client need to be involved in the certificate exchange between the hosts? Can't the joiner join the domain as part of the pool join, after the certificate exchange is done there?
Regarding the API call, would it be possible to request certificates for a singular purpose, instead of a list of IDs? It's less flexible this way, but harder to mis-use, IMO
There was a problem hiding this comment.
This is needed when joining a host (does not enabled AD) to a pool with AD enabled (and the pool has ldaps enabled). In this case XC will ask user to join AD and continue the pool join.
Here XC will call the download and upload certs API to sync ldaps certs between the joining host and the pool.
I'm also not convinced by this implementation, the client does not have access to the purpose for each certificate with the current API. This is needed to be able to install the certificates for the right uses
Does the list API contain the purpose details?
My question is, do we prefer download_trusted_certificates other than download_trusted_certificate, in later case, user can freely download a cert and save into a file. (and does not need to manage the certs order)
There was a problem hiding this comment.
The client can get the purpose (a set) from the certificate record separately. An API like this is flexible for the programming with SDK which doesn't need to save into a file at all. A client can download one with this API as well.
Downloading all certificates for a singular purpose is possible but it would cause error when the exchanging happens during pool.join. The exchanging will fail if the purpose sets don't match.
Alternatively, we could return the purpose set also. But this would make the return data structure complicated.
Why does the client need to be involved in the certificate exchange between the hosts? Can't the joiner join the domain as part of the pool join, after the certificate exchange is done there?
This was discussed in the design stage. The trusted certificates are necessary to ensure the secure LDAP work well before pool.join, although they would be exchanged automatically during pool.join.
There was a problem hiding this comment.
Downloading all certificates for a singular purpose is possible but it would cause error when the exchanging happens during pool.join. The exchanging will fail if the purpose sets don't match.
I'm not sure I understand this point, I'm imagining the actions of the client like this:
client-->coor: pool.download_trusted_certificates ~purpose:"ldaps"
for certificate in certificates:
client-->join: pool.install_trusted_certificate ... ~purpose:"ldaps"
After this, the pool join will find that the certificates for the purpose have to be exchanged (the UUIDs) do not match, but I don't see how it would make the join to fail.
This also means that the call that's being propose wouldn't deduplicate certificates, and it's another why the certificate exchange and LDAP joining should happen at pool join, where all these things can be coordinated, while on the client they cannot.
There was a problem hiding this comment.
The major reason why client is involved here is because the certificate sync is relatively late, https://github.com/xapi-project/xen-api/blob/master/ocaml/xapi/xapi_pool.ml#L1929
later than various pre_join_checks, specially assert_external_auth_matches.
The certs needs to be synced (at least for the purpose of ldaps) to fix this check to ensure pool homogeneous.
If the backend sync the certs, doesn't it make xapi too complicated and a little strange?
(Or we move the AD check right after xapi certificate sync? The side effect is the certificate is synced even pool join failed due to AD check. If this can be accepted, I am happy to update design to simplify XC)
For the duplicate certificates, can xapi filter the duplicates certs by fingerprint during certificate sync?
There was a problem hiding this comment.
About the new function I was thinking of adding it to ocaml/xapi/xapi_pool.ml using functions in
ocaml/xapi/cert_distrib.mli, similar to how exchange_trusted_certificates works, but only with ldaps certificates, and only with the download from the joined pool to the joiner host, withou without uploading certs to the joined pool.
There was a problem hiding this comment.
I see. You prefer to get the client involved as little as possible. So ideally a single API would import ldaps certificates from the pool into the joining host and complete the check (and would rollback the importing if the checking failed).
While the current PR, in reverse, is offloading more logic to clients.
Actually I think your idea of "pool.download_trusted_certificates ~purpose:"ldaps" is fine if we could accept the certificates are absent for other purpose during pool.join. As I think this would work in most cases. And users can still resolve the issue if the certificates for other purpose are necessary during pool.join by installing the certificates with multiple purposes.
There was a problem hiding this comment.
Hi @psafont
May I please know more about your thoughts on this?
The current proposal is pool.download_trusted_certificates ~purpose:"ldaps". And the client needs to install the dowloaded certs into the joining host. This would ignore the multi-purpose certificates case. In other words, the client needs to handle it in other way, like install the certificates with multiple purposes before pool.join.
Another point to be clear is that I think it would be better to keep the downloading and installing separated two APIs. They are for different pools. Combining them into one would have to pass one's credential to another.
There was a problem hiding this comment.
The current proposal is
pool.download_trusted_certificates ~purpose:"ldaps". And the client needs to install the dowloaded certs into the joining host. This would ignore the multi-purpose certificates case. In other words, the client needs to handle it in other way, like install the certificates with multiple purposes before pool.join.
If the call with a purpose parameter is not working, the one you proposed for several certificates at the same time is fine.
I would like to nudge you again towards a solution where the client is not active participant in the transfer. The joining host controlling the transfer and installation of LDAPS certificates means that it should be able to retain all the metadata without much issues.
There was a problem hiding this comment.
I force pushed two commits to introduce a new pool.sync_trusted_certificates_from to let the joining host do the job which is basically similar to what the pool.exchange_trusted_certificates does except that it is an uni-direction sync, from pool to joining host only.
I changed the name from download to sync to get it more accurate. And beside the remote pool's hostname or address, the new API pool.sync_trusted_certificates_from requires a remote_session to authenticate with the remote pool, and the pool coordinator's host certificate for verifying the identity of the remote pool.
So from the change of the document, now the client is not active participant of the transferring and installing. Most of the work are offloaded to the new API on the joining host.
Signed-off-by: Ming Lu <ming.lu@cloud.com>
Signed-off-by: Ming Lu <ming.lu@cloud.com>
df52c2d to
91596ab
Compare
| ) | ||
| ; (Bool, "ca", "true for 'ca' or false for 'pinned'") | ||
| ] | ||
| ~allowed_roles:(_R_POOL_OP ++ _R_CLIENT_CERT) |
There was a problem hiding this comment.
I think the call should return the certificates (references) that have been added the the system. This allows the client to delete them if the pool join fails, which the design document requires.
Signed-off-by: Ming Lu <ming.lu@cloud.com>
|
Add a new API pool.sync_trusted_certificates_from to facilitate the secure LDAP feature in pool.join case. It is also a general API to download trusted certificates.