Skip to content
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

CA-398341: Populate fingerprints of CA certificates on startup #6006

Merged
merged 2 commits into from
Oct 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ocaml/idl/datamodel_common.ml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ open Datamodel_roles
to leave a gap for potential hotfixes needing to increment the schema version.*)
let schema_major_vsn = 5

let schema_minor_vsn = 782
let schema_minor_vsn = 783

(* Historical schema versions just in case this is useful later *)
let rio_schema_major_vsn = 5
Expand Down
34 changes: 31 additions & 3 deletions ocaml/idl/datamodel_host.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1479,12 +1479,40 @@ let install_ca_certificate =
let uninstall_ca_certificate =
call ~pool_internal:true ~hide_from_docs:true ~name:"uninstall_ca_certificate"
~doc:"Remove a TLS CA certificate from this host."
~params:
~versioned_params:
[
(Ref _host, "host", "The host"); (String, "name", "The certificate name")
{
param_type= Ref _host
; param_name= "host"
; param_doc= "The host"
; param_release= numbered_release "1.290.0"
; param_default= None
}
; {
param_type= String
; param_name= "name"
; param_doc= "The certificate name"
; param_release= numbered_release "1.290.0"
; param_default= None
}
; {
param_type= Bool
; param_name= "force"
; param_doc= "Remove the DB entry even if the file is non-existent"
; param_release= numbered_release "24.35.0"
; param_default= Some (VBool false)
}
]
~allowed_roles:_R_LOCAL_ROOT_ONLY
~lifecycle:[(Published, "1.290.0", "Uninstall TLS CA certificate")]
~lifecycle:
[
(Published, "1.290.0", "Uninstall TLS CA certificate")
; ( Changed
, "24.35.0"
, "Added --force option to allow DB entries to be removed for \
non-existent files"
)
]
()

let certificate_list =
Expand Down
36 changes: 34 additions & 2 deletions ocaml/idl/datamodel_pool.ml
Original file line number Diff line number Diff line change
Expand Up @@ -851,9 +851,41 @@ let certificate_uninstall =
let uninstall_ca_certificate =
call ~name:"uninstall_ca_certificate"
~doc:"Remove a pool-wide TLS CA certificate."
~params:[(String, "name", "The certificate name")]
~params:
[
(String, "name", "The certificate name")
; ( Bool
, "force"
, "If true, remove the DB entry even if the file is non-existent"
)
]
~versioned_params:
[
{
param_type= String
; param_name= "name"
; param_doc= "The certificate name"
; param_release= numbered_release "1.290.0"
; param_default= None
}
; {
param_type= Bool
; param_name= "force"
; param_doc= "Remove the DB entry even if the file is non-existent"
; param_release= numbered_release "24.35.0"
; param_default= Some (VBool false)
}
]
~allowed_roles:(_R_POOL_OP ++ _R_CLIENT_CERT)
~lifecycle:[(Published, "1.290.0", "Uninstall TLS CA certificate")]
~lifecycle:
[
(Published, "1.290.0", "Uninstall TLS CA certificate")
; ( Changed
, "24.35.0"
, "Added --force option to allow DB entries to be removed for \
non-existent files"
)
]
()

let certificate_list =
Expand Down
7 changes: 5 additions & 2 deletions ocaml/xapi-cli-server/cli_frontend.ml
Original file line number Diff line number Diff line change
Expand Up @@ -396,8 +396,11 @@ let rec cmdtable_data : (string * cmd_spec) list =
; ( "pool-uninstall-ca-certificate"
, {
reqd= ["name"]
; optn= []
; help= "Uninstall a pool-wide TLS CA certificate."
; optn= ["force"]
; help=
"Uninstall a pool-wide TLS CA certificate. The optional parameter \
'--force' will remove the DB entry even if the certificate file is \
non-existent"
; implementation= No_fd Cli_operations.pool_uninstall_ca_certificate
; flags= []
}
Expand Down
3 changes: 2 additions & 1 deletion ocaml/xapi-cli-server/cli_operations.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1770,7 +1770,8 @@ let pool_install_ca_certificate fd _printer rpc session_id params =

let pool_uninstall_ca_certificate _printer rpc session_id params =
let name = List.assoc "name" params in
Client.Pool.uninstall_ca_certificate ~rpc ~session_id ~name
let force = get_bool_param params "force" in
Client.Pool.uninstall_ca_certificate ~rpc ~session_id ~name ~force

let pool_certificate_list printer rpc session_id _params =
printer (Cli_printer.PList (Client.Pool.certificate_list ~rpc ~session_id))
Expand Down
30 changes: 18 additions & 12 deletions ocaml/xapi/certificates.ml
Original file line number Diff line number Diff line change
Expand Up @@ -304,17 +304,21 @@ let host_install kind ~name ~cert =
(ExnHelper.string_of_exn e) ;
raise_library_corrupt ()

let host_uninstall kind ~name =
let host_uninstall kind ~name ~force =
validate_name kind name ;
let filename = library_filename kind name in
if not (Sys.file_exists filename) then
raise_does_not_exist kind name ;
debug "Uninstalling %s %s" (to_string kind) name ;
try Sys.remove filename ; update_ca_bundle ()
with e ->
warn "Exception uninstalling %s %s: %s" (to_string kind) name
(ExnHelper.string_of_exn e) ;
raise_corrupt kind name
if Sys.file_exists filename then (
debug "Uninstalling %s %s" (to_string kind) name ;
try Sys.remove filename ; update_ca_bundle ()
with e ->
warn "Exception uninstalling %s %s: %s" (to_string kind) name
(ExnHelper.string_of_exn e) ;
raise_corrupt kind name
) else if force then
info "Certificate file %s is non-existent but ignoring this due to force."
name
else
raise_does_not_exist kind name

let get_cert kind name =
validate_name kind name ;
Expand Down Expand Up @@ -367,6 +371,7 @@ let sync_certs kind ~__context master_certs host =
)
(fun rpc session_id host name ->
Client.Host.uninstall_ca_certificate ~rpc ~session_id ~host ~name
~force:false
)
~__context master_certs host
| CRL ->
Expand Down Expand Up @@ -403,15 +408,16 @@ let pool_install kind ~__context ~name ~cert =
host_install kind ~name ~cert ;
try pool_sync ~__context
with exn ->
( try host_uninstall kind ~name
( try host_uninstall kind ~name ~force:false
with e ->
warn "Exception unwinding install of %s %s: %s" (to_string kind) name
(ExnHelper.string_of_exn e)
) ;
raise exn

let pool_uninstall kind ~__context ~name =
host_uninstall kind ~name ; pool_sync ~__context
let pool_uninstall kind ~__context ~name ~force =
host_uninstall kind ~name ~force ;
pool_sync ~__context

(* Extracts the server certificate from the server certificate pem file.
It strips the private key as well as the rest of the certificate chain. *)
Expand Down
5 changes: 3 additions & 2 deletions ocaml/xapi/certificates.mli
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,13 @@ val install_server_certificate :

val host_install : t_trusted -> name:string -> cert:string -> unit

val host_uninstall : t_trusted -> name:string -> unit
val host_uninstall : t_trusted -> name:string -> force:bool -> unit

val pool_install :
t_trusted -> __context:Context.t -> name:string -> cert:string -> unit

val pool_uninstall : t_trusted -> __context:Context.t -> name:string -> unit
val pool_uninstall :
t_trusted -> __context:Context.t -> name:string -> force:bool -> unit

(* Database manipulation *)

Expand Down
13 changes: 8 additions & 5 deletions ocaml/xapi/message_forwarding.ml
Original file line number Diff line number Diff line change
Expand Up @@ -3745,19 +3745,22 @@ functor
~cert
)

let uninstall_ca_certificate ~__context ~host ~name =
info "Host.uninstall_ca_certificate: host = '%s'; name = '%s'"
let uninstall_ca_certificate ~__context ~host ~name ~force =
psafont marked this conversation as resolved.
Show resolved Hide resolved
info
"Host.uninstall_ca_certificate: host = '%s'; name = '%s'; force = \
'%b'"
(host_uuid ~__context host)
name ;
let local_fn = Local.Host.uninstall_ca_certificate ~host ~name in
name force ;
let local_fn = Local.Host.uninstall_ca_certificate ~host ~name ~force in
do_op_on ~local_fn ~__context ~host (fun session_id rpc ->
Client.Host.uninstall_ca_certificate ~rpc ~session_id ~host ~name
~force
)

(* legacy names *)
let certificate_install = install_ca_certificate

let certificate_uninstall = uninstall_ca_certificate
let certificate_uninstall = uninstall_ca_certificate ~force:false

let certificate_list ~__context ~host =
info "Host.certificate_list: host = '%s'" (host_uuid ~__context host) ;
Expand Down
62 changes: 62 additions & 0 deletions ocaml/xapi/xapi_db_upgrade.ml
Original file line number Diff line number Diff line change
Expand Up @@ -904,6 +904,67 @@ let upgrade_update_guidance =
)
}

let upgrade_ca_fingerprints =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is dead code until you add the new function to the rules list below.

{
description= "Upgrade the fingerprint fields for ca certificates"
; version= (fun x -> x < (5, 783))
; (* the version where we started updating missing fingerprint_sha256
and fingerprint_sha1 fields for ca certs *)
fn=
(fun ~__context ->
let expr =
let open Xapi_database.Db_filter_types in
And
( Or
( Eq (Field "fingerprint_sha256", Literal "")
, Eq (Field "fingerprint_sha1", Literal "")
)
, Eq (Field "type", Literal "ca")
)
in
let empty = Db.Certificate.get_records_where ~__context ~expr in
List.iter
(fun (self, record) ->
let read_fingerprints filename =
let ( let* ) = Result.bind in
try
let* certificate =
Xapi_stdext_unix.Unixext.string_of_file filename
|> Cstruct.of_string
|> X509.Certificate.decode_pem
in
let sha1 =
Certificates.pp_fingerprint ~hash_type:`SHA1 certificate
in
let sha256 =
Certificates.pp_fingerprint ~hash_type:`SHA256 certificate
in
Ok (sha1, sha256)
with
| Unix.Unix_error (Unix.ENOENT, _, _) ->
Error
(`Msg (Printf.sprintf "filename %s does not exist" filename))
| exn ->
Error (`Msg (Printexc.to_string exn))
in
let filename =
Filename.concat
!Xapi_globs.trusted_certs_dir
record.API.certificate_name
in
match read_fingerprints filename with
| Ok (sha1, sha256) ->
Db.Certificate.set_fingerprint_sha1 ~__context ~self ~value:sha1 ;
Db.Certificate.set_fingerprint_sha256 ~__context ~self
~value:sha256
| Error (`Msg msg) ->
D.info "%s: ignoring error when reading CA certificate %s: %s"
__FUNCTION__ record.API.certificate_name msg
)
empty
)
}

let rules =
[
upgrade_domain_type
Expand Down Expand Up @@ -933,6 +994,7 @@ let rules =
; remove_legacy_ssl_support
; empty_pool_uefi_certificates
; upgrade_update_guidance
; upgrade_ca_fingerprints
]

(* Maybe upgrade most recent db *)
Expand Down
6 changes: 3 additions & 3 deletions ocaml/xapi/xapi_host.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1548,9 +1548,9 @@ let install_ca_certificate ~__context ~host:_ ~name ~cert =
(* don't modify db - Pool.install_ca_certificate will handle that *)
Certificates.(host_install CA_Certificate ~name ~cert)

let uninstall_ca_certificate ~__context ~host:_ ~name =
let uninstall_ca_certificate ~__context ~host:_ ~name ~force =
(* don't modify db - Pool.uninstall_ca_certificate will handle that *)
Certificates.(host_uninstall CA_Certificate ~name)
Certificates.(host_uninstall CA_Certificate ~name ~force)

let certificate_list ~__context ~host:_ =
Certificates.(local_list CA_Certificate)
Expand All @@ -1559,7 +1559,7 @@ let crl_install ~__context ~host:_ ~name ~crl =
Certificates.(host_install CRL ~name ~cert:crl)

let crl_uninstall ~__context ~host:_ ~name =
Certificates.(host_uninstall CRL ~name)
Certificates.(host_uninstall CRL ~name ~force:false)

let crl_list ~__context ~host:_ = Certificates.(local_list CRL)

Expand Down
2 changes: 1 addition & 1 deletion ocaml/xapi/xapi_host.mli
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ val install_ca_certificate :
__context:Context.t -> host:API.ref_host -> name:string -> cert:string -> unit

val uninstall_ca_certificate :
__context:Context.t -> host:API.ref_host -> name:string -> unit
__context:Context.t -> host:API.ref_host -> name:string -> force:bool -> unit

val certificate_list : __context:'a -> host:'b -> string list

Expand Down
8 changes: 4 additions & 4 deletions ocaml/xapi/xapi_pool.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1432,12 +1432,12 @@ let certificate_install ~__context ~name ~cert =

let install_ca_certificate = certificate_install

let certificate_uninstall ~__context ~name =
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trying to understand how the legacy names work. Why is the legacy name "certificate_uninstall" still needed (here and in message_forwarding.ml and datamodel_pool.ml if the deprecated xe command doesn't even use it? It calls pool_install_ca_certificate which calls uninstall_ca_certificate, not certificate_uninstall. What is it providing compatibility with, is this for upgrades from much older xapis?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

API clients might still use this function, this is why it needs to be kept.

let uninstall_ca_certificate ~__context ~name ~force =
let open Certificates in
pool_uninstall CA_Certificate ~__context ~name ;
pool_uninstall CA_Certificate ~__context ~name ~force ;
Db_util.remove_ca_cert_by_name ~__context name

let uninstall_ca_certificate = certificate_uninstall
let certificate_uninstall = uninstall_ca_certificate ~force:false
psafont marked this conversation as resolved.
Show resolved Hide resolved

let certificate_list ~__context =
let open Certificates in
Expand All @@ -1446,7 +1446,7 @@ let certificate_list ~__context =

let crl_install = Certificates.(pool_install CRL)

let crl_uninstall = Certificates.(pool_uninstall CRL)
let crl_uninstall = Certificates.(pool_uninstall CRL ~force:false)

let crl_list ~__context = Certificates.(local_list CRL)

Expand Down
3 changes: 2 additions & 1 deletion ocaml/xapi/xapi_pool.mli
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,8 @@ val install_ca_certificate :

val certificate_uninstall : __context:Context.t -> name:string -> unit

val uninstall_ca_certificate : __context:Context.t -> name:string -> unit
val uninstall_ca_certificate :
__context:Context.t -> name:string -> force:bool -> unit

val certificate_list : __context:Context.t -> string list

Expand Down
Loading