Skip to content

Commit

Permalink
CP-51527: Add --force option to pool-uninstall-ca-certificate
Browse files Browse the repository at this point in the history
This allows the CA certificate to be removed from the DB even if the
certificate file does not exist.

Signed-off-by: Steven Woods <[email protected]>
  • Loading branch information
snwoods committed Oct 21, 2024
1 parent 264558d commit ed90086
Show file tree
Hide file tree
Showing 11 changed files with 111 additions and 36 deletions.
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 =
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
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 =
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

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

0 comments on commit ed90086

Please sign in to comment.