-
Notifications
You must be signed in to change notification settings - Fork 296
Improve handling of trusted certificates #6826
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
base: feature/trusted-certs
Are you sure you want to change the base?
Improve handling of trusted certificates #6826
Conversation
Signed-off-by: Ming Lu <[email protected]>
Signed-off-by: Ming Lu <[email protected]>
Signed-off-by: Ming Lu <[email protected]>
Signed-off-by: Ming Lu <[email protected]>
Signed-off-by: Ming Lu <[email protected]>
Signed-off-by: Ming Lu <[email protected]>
Signed-off-by: Ming Lu <[email protected]>
Signed-off-by: Ming Lu <[email protected]>
Signed-off-by: Ming Lu <[email protected]>
Signed-off-by: Ming Lu <[email protected]>
Signed-off-by: Ming Lu <[email protected]>
f490581 to
ac35717
Compare
| let certificate_purpose = | ||
| Enum | ||
| ( "certificate_purpose" | ||
| , [("licensing", "Trusted certificates that are for licensing purpose.")] |
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.
"that are for" could be simplified to "Trusted certificates for licensing"
| let install_trusted_certificate = | ||
| call ~flags:[`Session] ~pool_internal:true ~hide_from_docs:true | ||
| ~name:"install_trusted_certificate" | ||
| ~doc:"Install a TLS trusted certificate on this host." |
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.
It's not clear how TLS and trusted are related: is it a TLS-trusted certificate or a trusted TLS certificate?
|
|
||
| let install_trusted_certificate = | ||
| call ~name:"install_trusted_certificate" | ||
| ~doc:"Install a TLS trusted certificate, pool-wide." |
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.
See above for the ambiguity about how trust and TLS are related.
| Xapi_stdext_threads.Threadext.Mutex.execute m (fun () -> | ||
| let (_ : string), (_ : string) = | ||
| Forkhelpers.execute_command_get_output | ||
| "/opt/xensource/bin/update-ca-bundle.sh" [cert_dir; bundle_path] |
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.
This script so far does not take arguments and I don't see it being changed but maybe I missed the change. How is this supposed to work and does this mean other invocations of this script need to be updated?
A draft to collect initial comments. Only basic manual tests are done. More tests are in progress.
The design is https://github.com/xapi-project/xen-api/blob/master/doc/content/design/trusted-certificates.md
(It's a little bit out-of-date. Will update it later. And remaining changes will be raised in following PRs)