Add pool.download_trusted_certificates API#7056
Add pool.download_trusted_certificates API#7056minglumlu wants to merge 3 commits intoxapi-project:feature/ldapsfrom
Conversation
Signed-off-by: Ming Lu <ming.lu@cloud.com>
Signed-off-by: Ming Lu <ming.lu@cloud.com>
Signed-off-by: Ming Lu <ming.lu@cloud.com>
| 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.
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.
Yes. You are right. It will not fail the pool.join. The install_trusted_certificate_ignore_dup can ignore the error Api_errors.trusted_certificate_already_exists. So actually the ldaps certificates will not be exchanged at all during pool.join. And once Toolstack on the joiner restarts after pool.join, as a pool member, its' trusted certificates will be synced as the coordinator.
But if the certificate has two purposes (i.e., licensing and ldaps), the exchange on the pool.join will not set the purpose licensing on the joiner during joining. This is the point the exchange in pool.join.
There was a problem hiding this comment.
(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)
Could we add a certificate exchange function for ldaps certificates only just before the AD checks? This would mean if there's an error it would be easy to roll them back. And the rest of the code would not have to be changed
There was a problem hiding this comment.
(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)
Could we add a certificate exchange function for ldaps certificates only just before the AD checks? This would mean if there's an error it would be easy to roll them back. And the rest of the code would not have to be changed
It may not easily, as it it not in one single API.
- XC join domain, pool pre-check failed
- XC fix up by joining AD (ask user and password)
- XC retry pool join.
If xapi sync ldaps certs before AD prechecks, xapi does not know whether AD fix up/join can be succeed and roolback.
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.
Add a new API pool.download_trusted_certificates to facilitate the secure LDAP feature in pool.join case. It is also a general API to download trusted certificates.