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

Fix empty fingerprints (upstream upgrade bug) #76

Merged
merged 1 commit into from
Sep 3, 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
104 changes: 104 additions & 0 deletions SOURCES/xapi-24.19.2-more-fingerprint-field-updates-fixes.XCP-ng.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
From 4c80156c69dc57e7df9c696eceffb331d0686a6f Mon Sep 17 00:00:00 2001
From: Pau Ruiz Safont <[email protected]>
Date: Thu, 29 Aug 2024 13:28:27 +0100
Subject: [PATCH] CA-398341: Populate fingerprints of CA certificates on
startup

SHA256 and SHA1 certificates' fingerprints do not get populated when the
database is upgraded, so empty values need to be detected and amended on
startup.

Signed-off-by: Pau Ruiz Safont <[email protected]>
---
ocaml/xapi/certificates.ml | 43 +++++++++++++++++++++++++++++++++++++
ocaml/xapi/certificates.mli | 2 ++
ocaml/xapi/xapi.ml | 4 ++++
3 files changed, 49 insertions(+)

diff --git a/ocaml/xapi/certificates.ml b/ocaml/xapi/certificates.ml
index fe66194cb0e..29e555f54bb 100644
--- a/ocaml/xapi/certificates.ml
+++ b/ocaml/xapi/certificates.ml
@@ -173,6 +173,8 @@ module Db_util : sig
* of type [type'] belonging to [host] (the term 'host' is overloaded here) *)

val get_ca_certs : __context:Context.t -> API.ref_Certificate list
+
+ val upgrade_ca_fingerprints : __context:Context.t -> unit
end = struct
module Date = Xapi_stdext_date.Date

@@ -256,6 +258,47 @@ end = struct
Eq (Field "type", Literal "ca")
in
Db.Certificate.get_refs_where ~__context ~expr
+
+ let upgrade_ca_fingerprints ~__context =
+ let __FUN = __FUNCTION__ in
+ 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
+ let* certificate =
+ Xapi_stdext_unix.Unixext.string_of_file filename
+ |> Cstruct.of_string
+ |> X509.Certificate.decode_pem
+ in
+ let sha1 = pp_fingerprint ~hash_type:`SHA1 certificate in
+ let sha256 = pp_fingerprint ~hash_type:`SHA256 certificate in
+ Ok (sha1, sha256)
+ 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" __FUN
+ record.API.certificate_name msg
+ )
+ empty
end

let local_list kind =
diff --git a/ocaml/xapi/certificates.mli b/ocaml/xapi/certificates.mli
index 486ada825e2..1a514ce4a91 100644
--- a/ocaml/xapi/certificates.mli
+++ b/ocaml/xapi/certificates.mli
@@ -83,4 +83,6 @@ module Db_util : sig
-> API.ref_Certificate list

val get_ca_certs : __context:Context.t -> API.ref_Certificate list
+
+ val upgrade_ca_fingerprints : __context:Context.t -> unit
end
diff --git a/ocaml/xapi/xapi.ml b/ocaml/xapi/xapi.ml
index b702001ef2e..5c002e1534a 100644
--- a/ocaml/xapi/xapi.ml
+++ b/ocaml/xapi/xapi.ml
@@ -1147,6 +1147,10 @@ let server_init () =
, []
, fun () -> report_tls_verification ~__context
)
+ ; ( "Update shared certificate's metadata"
+ , [Startup.OnlyMaster]
+ , fun () -> Certificates.Db_util.upgrade_ca_fingerprints ~__context
+ )
; ( "Remote requests"
, [Startup.OnThread]
, Remote_requests.handle_requests
Original file line number Diff line number Diff line change
@@ -0,0 +1,192 @@
From d8fa301fab466d1a15fea69f9f2ff1d522c594a4 Mon Sep 17 00:00:00 2001
From: Steven Woods <[email protected]>
Date: Tue, 9 Jul 2024 14:48:10 +0100
Subject: [PATCH] CP-50193: Update new fingerprint fields on DB upgrade

The new fingerprint_sha256 and fingerprint_sha1 fields will be empty
when upgrading from a version without the fields. This commit checks for
this and fills them in, stopping the certificate from being needlessly
reinstalled.

Signed-off-by: Steven Woods <[email protected]>
---
ocaml/idl/datamodel_certificate.ml | 4 +--
ocaml/idl/datamodel_common.ml | 2 +-
ocaml/idl/datamodel_lifecycle.ml | 4 +++
ocaml/idl/schematest.ml | 2 +-
ocaml/xapi/certificates.ml | 11 ++++----
ocaml/xapi/certificates.mli | 3 +++
ocaml/xapi/certificates_sync.ml | 43 +++++++++++++++++++++---------
7 files changed, 47 insertions(+), 22 deletions(-)

diff --git a/ocaml/idl/datamodel_certificate.ml b/ocaml/idl/datamodel_certificate.ml
index 409d35e8233..bfbdd2b60b5 100644
--- a/ocaml/idl/datamodel_certificate.ml
+++ b/ocaml/idl/datamodel_certificate.ml
@@ -69,10 +69,10 @@ let t =
[(Published, rel_stockholm, ""); (Deprecated, "24.19.0", "")]
~ty:String "fingerprint" ~default_value:(Some (VString ""))
"Use fingerprint_sha256 instead"
- ; field ~qualifier:StaticRO ~lifecycle ~ty:String "fingerprint_sha256"
+ ; field ~qualifier:StaticRO ~lifecycle:[] ~ty:String "fingerprint_sha256"
~default_value:(Some (VString ""))
"The certificate's SHA256 fingerprint / hash"
- ; field ~qualifier:StaticRO ~lifecycle ~ty:String "fingerprint_sha1"
+ ; field ~qualifier:StaticRO ~lifecycle:[] ~ty:String "fingerprint_sha1"
~default_value:(Some (VString ""))
"The certificate's SHA1 fingerprint / hash"
]
diff --git a/ocaml/idl/datamodel_common.ml b/ocaml/idl/datamodel_common.ml
index de22cf2e5ad..9afd7bd37c0 100644
--- a/ocaml/idl/datamodel_common.ml
+++ b/ocaml/idl/datamodel_common.ml
@@ -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 = 779
+let schema_minor_vsn = 780

(* Historical schema versions just in case this is useful later *)
let rio_schema_major_vsn = 5
diff --git a/ocaml/idl/datamodel_lifecycle.ml b/ocaml/idl/datamodel_lifecycle.ml
index 92316d8ee26..763b3944caa 100644
--- a/ocaml/idl/datamodel_lifecycle.ml
+++ b/ocaml/idl/datamodel_lifecycle.ml
@@ -27,6 +27,10 @@ let prototyped_of_field = function
Some "23.14.0"
| "Repository", "gpgkey_path" ->
Some "22.12.0"
+ | "Certificate", "fingerprint_sha1" ->
+ Some "24.19.1-next"
+ | "Certificate", "fingerprint_sha256" ->
+ Some "24.19.1-next"
| "Cluster_host", "last_update_live" ->
Some "24.3.0"
| "Cluster_host", "live" ->
diff --git a/ocaml/idl/schematest.ml b/ocaml/idl/schematest.ml
index f2ee8fe4be2..4ba16fbfe1c 100644
--- a/ocaml/idl/schematest.ml
+++ b/ocaml/idl/schematest.ml
@@ -3,7 +3,7 @@ let hash x = Digest.string x |> Digest.to_hex
(* BEWARE: if this changes, check that schema has been bumped accordingly in
ocaml/idl/datamodel_common.ml, usually schema_minor_vsn *)

-let last_known_schema_hash = "efdb1c7e536362523741ccdb7f33f797"
+let last_known_schema_hash = "7885f7b085e4a5e32977a4b222030412"

let current_schema_hash : string =
let open Datamodel_types in
diff --git a/ocaml/xapi/certificates.ml b/ocaml/xapi/certificates.ml
index 4f6747762ea..effb154877e 100644
--- a/ocaml/xapi/certificates.ml
+++ b/ocaml/xapi/certificates.ml
@@ -80,6 +80,9 @@ let pp_hash hash =
in
String.init length value_of

+let pp_fingerprint ~hash_type cert =
+ X509.Certificate.fingerprint hash_type cert |> pp_hash
+
let safe_char c =
match c with
| 'A' .. 'Z' | 'a' .. 'z' | '0' .. '9' | '.' | '_' | '-' ->
@@ -218,12 +221,8 @@ end = struct
let not_before, not_after =
dates_of_ptimes (X509.Certificate.validity certificate)
in
- let fingerprint_sha256 =
- X509.Certificate.fingerprint `SHA256 certificate |> pp_hash
- in
- let fingerprint_sha1 =
- X509.Certificate.fingerprint `SHA1 certificate |> pp_hash
- in
+ let fingerprint_sha256 = pp_fingerprint ~hash_type:`SHA256 certificate in
+ let fingerprint_sha1 = pp_fingerprint ~hash_type:`SHA1 certificate in
let uuid = Uuidx.(to_string (make ())) in
let ref' = Ref.make () in
Db.Certificate.create ~__context ~ref:ref' ~uuid ~host ~not_before
diff --git a/ocaml/xapi/certificates.mli b/ocaml/xapi/certificates.mli
index ddb2677df1c..486ada825e2 100644
--- a/ocaml/xapi/certificates.mli
+++ b/ocaml/xapi/certificates.mli
@@ -20,6 +20,9 @@ val pem_of_string : string -> X509.Certificate.t

val pp_hash : Cstruct.t -> string

+val pp_fingerprint :
+ hash_type:Mirage_crypto.Hash.hash -> X509.Certificate.t -> string
+
val validate_name : t_trusted -> string -> unit

val hostnames_of_pem_cert :
diff --git a/ocaml/xapi/certificates_sync.ml b/ocaml/xapi/certificates_sync.ml
index 735b1a9c936..e1bf42630a0 100644
--- a/ocaml/xapi/certificates_sync.ml
+++ b/ocaml/xapi/certificates_sync.ml
@@ -29,16 +29,26 @@ let install ~__context ~host:_ ~type' cert =
error "certificates_sync.install exception: %s" (Printexc.to_string e) ;
Error (`Msg ("installation of host certificate failed", []))

+type to_update = Certificate | Hashes of {sha256: string; sha1: string}
+
(** determine if the database is up to date by comparing the fingerprint
of xapi-ssl.pem with the entry in the database *)
-let is_unchanged ~__context cert_ref cert =
+let to_update ~__context cert_ref cert =
let ref_hash =
Db.Certificate.get_fingerprint_sha256 ~__context ~self:cert_ref
in
- let cert_hash =
- X509.Certificate.fingerprint `SHA256 cert |> Certificates.pp_hash
- in
- cert_hash = ref_hash
+ let sha256 = Certificates.pp_fingerprint ~hash_type:`SHA256 cert in
+ if ref_hash = "" then
+ (* We must be upgrading from a version predating fingerprint_sha256, so check fingerprint instead *)
+ if sha256 = Db.Certificate.get_fingerprint ~__context ~self:cert_ref then
+ let sha1 = Certificates.pp_fingerprint ~hash_type:`SHA1 cert in
+ Some (Hashes {sha256; sha1})
+ else
+ Some Certificate
+ else if sha256 = ref_hash then
+ None
+ else
+ Some Certificate

(** [get_server_cert] loads [path] from the file system and
returns it decoded *)
@@ -76,17 +86,26 @@ let sync ~__context ~type' =
| [] ->
info "Host %s has no active server certificate" host_uuid ;
install ~__context ~host ~type' cert
- | [cert_ref] ->
- let unchanged = is_unchanged ~__context cert_ref cert in
- if unchanged then (
- info "Active server certificate for host %s is unchanged" host_uuid ;
- Ok ()
- ) else (
+ | [cert_ref] -> (
+ match to_update ~__context cert_ref cert with
+ | Some Certificate ->
info "Server certificate for host %s changed - updating" host_uuid ;
let* () = install ~__context ~host ~type' cert in
uninstall ~__context cert_ref ;
Ok ()
- )
+ | Some (Hashes {sha256; sha1}) ->
+ info "Active server certificate for host %s is unchanged" host_uuid ;
+ Db.Certificate.set_fingerprint_sha256 ~__context ~self:cert_ref
+ ~value:sha256 ;
+ Db.Certificate.set_fingerprint_sha1 ~__context ~self:cert_ref
+ ~value:sha1 ;
+ info "Populated new fingerprint fields: sha256= %s; sha1= %s" sha256
+ sha1 ;
+ Ok ()
+ | None ->
+ info "Active server certificate for host %s is unchanged" host_uuid ;
+ Ok ()
+ )
| cert_refs ->
warn "The host has more than one certificate: %s"
(String.concat ", " (List.map Ref.string_of cert_refs)) ;
12 changes: 11 additions & 1 deletion SPECS/xapi.spec
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
Summary: xapi - xen toolstack for XCP
Name: xapi
Version: 24.19.2
Release: 1.2%{?xsrel}%{?dist}
Release: 1.3%{?xsrel}%{?dist}
Group: System/Hypervisor
License: LGPL-2.1-or-later WITH OCaml-LGPL-linking-exception
URL: http://www.xen.org
Expand Down Expand Up @@ -71,6 +71,10 @@ Patch1003: xapi-24.11.0-update-db-tunnel-protocol-from-other_config.XCP-ng.patch
# Upstream PR: https://github.com/xapi-project/xen-api/pull/5918
Patch1004: xapi-24.16.0-openvswitch-config-update-fix-python2ism-in-python3.patch
Patch1005: xapi-24.19.2-fix-ipv6-import.XCP-ng.patch
# Backport from 24.20
Patch1006: xapi-24.19.2-update-new-fingerprint-fields-on-DB-upgrade.backport.patch
# Fix fingerprints for CA certificates too
Patch1007: xapi-24.19.2-more-fingerprint-field-updates-fixes.XCP-ng.patch

%{?_cov_buildrequires}
BuildRequires: ocaml-ocamldoc
Expand Down Expand Up @@ -1402,6 +1406,12 @@ Coverage files from unit tests
%{?_cov_results_package}

%changelog
* Wed Aug 28 2024 Samuel Verschelde <[email protected]> - 24.19.2-1.3
- Add xapi-24.19.2-update-new-fingerprint-fields-on-DB-upgrade.backport.patch, backported from XAPI project
- Add xapi-24.19.2-more-fingerprint-field-updates-fixes.XCP-ng.patch to complement the fix
- Fixes an issue where new fingerprint fields are not populated, which under
some circumstances makes pool join fail.

* Wed Aug 14 2024 Benjamin Reis <[email protected]> - 24.19.2-1.2
- Add xapi-24.19.2-fix-ipv6-import.XCP-ng.patch

Expand Down