Libguestfs - May 2020 - [PATCH 0/4] sysprep: add FreeIPA offline unenrollment (RHBZ#1789592)

This patch series adds a new virt-sysprep operation to offline unenroll
a guest from FreeIPA. It does so by removing some configuration files
and certificates.

It requires a change in libguestfs-common before the series is applied.

Pino Toscano (4):
  customize: port do_run to run_in_guest_command
  sysprep: add a update_system_ca_store side effect
  sysprep: ca-certificates: request system CA store update
  sysprep: add FreeIPA offline unenrollment (RHBZ#1789592)

 customize/customize_run.ml                    | 48 ++------------
 sysprep/Makefile.am                           |  1 +
 sysprep/main.ml                               |  5 ++
 sysprep/sysprep_operation.ml                  |  3 +
 sysprep/sysprep_operation.mli                 |  2 +
 sysprep/sysprep_operation_ca_certificates.ml  |  8 ++-
 sysprep/sysprep_operation_unenroll_freeipa.ml | 65 +++++++++++++++++++
 sysprep/utils.ml                              | 32 +++++++++
 sysprep/utils.mli                             |  4 ++
 9 files changed, 126 insertions(+), 42 deletions(-)
 create mode 100644 sysprep/sysprep_operation_unenroll_freeipa.ml

-- 
2.25.4

Make use of the new helper function in Tools_utils to run commands in
the guest.
---
 customize/customize_run.ml | 48 ++++++--------------------------------
 1 file changed, 7 insertions(+), 41 deletions(-)

diff --git a/customize/customize_run.ml b/customize/customize_run.ml
index 3eacdaca0..f2ee20413 100644
--- a/customize/customize_run.ml
+++ b/customize/customize_run.ml
@@ -30,12 +30,6 @@ open Append_line
 module G = Guestfs
 
 let run (g : G.guestfs) root (ops : ops) -  (* Is the host_cpu compatible with
the guest arch?  ie. Can we
-   * run commands in this guest?
-   *)
-  let guest_arch = g#inspect_get_arch root in
-  let guest_arch_compatible = guest_arch_compatible guest_arch in
-
   (* Based on the guest type, choose a log file location. *)
   let logfile      match g#inspect_get_type root with
@@ -54,42 +48,14 @@ let run (g : G.guestfs) root (ops : ops)  
   (* Useful wrapper for scripts. *)
   let do_run ~display ?(warn_failed_no_network = false) cmd -    if not
guest_arch_compatible then
+    let incompatible_fn () +      let guest_arch = g#inspect_get_arch root in
       error (f_"host cpu (%s) and guest arch (%s) are not compatible, so
you cannot use command line options that involve running commands in the guest. 
Use --firstboot scripts instead.")
-            Guestfs_config.host_cpu guest_arch;
-
-    (* Add a prologue to the scripts:
-     * - Pass environment variables through from the host.
-     * - Send stdout and stderr to a log file so we capture all output
-     *   in error messages.
-     * - Use setarch when running x86_64 host + i686 guest.
-     * Also catch errors and dump the log file completely on error.
-     *)
-    let env_vars -      List.filter_map (
-        fun name ->
-          try Some (sprintf "export %s=%s" name (quote (Sys.getenv
name)))
-          with Not_found -> None
-      ) [ "http_proxy"; "https_proxy";
"ftp_proxy"; "no_proxy" ] in
-    let env_vars = String.concat "\n" env_vars ^ "\n" in
-
-    let cmd -      match Guestfs_config.host_cpu, guest_arch with
-      | "x86_64",
("i386"|"i486"|"i586"|"i686") ->
-        sprintf "setarch i686 <<\"__EOCMD\"
-%s
-__EOCMD
-" cmd
-      | _ -> cmd in
-
-    let cmd = sprintf "\
-exec >>%s 2>&1
-%s
-%s
-" (quote logfile) env_vars cmd in
-
-    debug "running command:\n%s" cmd;
-    try ignore (g#sh cmd)
+            Guestfs_config.host_cpu guest_arch
+    in
+
+    try
+      run_in_guest_command g root ~logfile ~incompatible_fn cmd
     with
       G.Error msg ->
         debug_logfile ();
-- 
2.25.4

Add a simple side effect to make operation flag that a regeneration of
the system CA store is needed. In case it is flagged, regenerate the
system CA store directly, or using a firstboot script in case of
incompatible architectures.

This change is almost a no-op, since no operation requires the
regeneration of the system CA store yet.
---
 sysprep/main.ml               |  5 +++++
 sysprep/sysprep_operation.ml  |  3 +++
 sysprep/sysprep_operation.mli |  2 ++
 sysprep/utils.ml              | 32 ++++++++++++++++++++++++++++++++
 sysprep/utils.mli             |  4 ++++
 5 files changed, 46 insertions(+)

diff --git a/sysprep/main.ml b/sysprep/main.ml
index db2388cfa..087d1a17f 100644
--- a/sysprep/main.ml
+++ b/sysprep/main.ml
@@ -25,6 +25,7 @@ open Common_gettext.Gettext
 open Getopt.OptionName
 
 open Sysprep_operation
+open Utils
 
 module G = Guestfs
 
@@ -236,6 +237,10 @@ read the man page virt-sysprep(1).
         Sysprep_operation.perform_operations_on_filesystems
           ?operations g root side_effects;
 
+        (* Do we need to update the system CA store? *)
+        if side_effects#get_update_system_ca_store then
+          update_system_ca_store g root;
+
         (* Unmount everything in this guest. *)
         g#umount_all ();
 
diff --git a/sysprep/sysprep_operation.ml b/sysprep/sysprep_operation.ml
index 0013ff504..53f042402 100644
--- a/sysprep/sysprep_operation.ml
+++ b/sysprep/sysprep_operation.ml
@@ -27,10 +27,13 @@ class filesystem_side_effects  object
   val mutable m_created_file = false
   val mutable m_changed_file = false
+  val mutable m_update_system_ca_store = false
   method created_file () = m_created_file <- true
   method get_created_file = m_created_file
   method changed_file () = m_changed_file <- true
   method get_changed_file = m_changed_file
+  method update_system_ca_store () = m_update_system_ca_store <- true
+  method get_update_system_ca_store = m_update_system_ca_store
 end
 
 class device_side_effects = object end
diff --git a/sysprep/sysprep_operation.mli b/sysprep/sysprep_operation.mli
index 3d9f12550..2a02d5e79 100644
--- a/sysprep/sysprep_operation.mli
+++ b/sysprep/sysprep_operation.mli
@@ -23,6 +23,8 @@ class filesystem_side_effects : object
   method get_created_file : bool
   method changed_file : unit -> unit
   method get_changed_file : bool
+  method update_system_ca_store : unit -> unit
+  method get_update_system_ca_store : bool
 end
 (** The callback should indicate if it has side effects by calling
     methods in this class. *)
diff --git a/sysprep/utils.ml b/sysprep/utils.ml
index 4c45d42de..29460b3c0 100644
--- a/sysprep/utils.ml
+++ b/sysprep/utils.ml
@@ -20,6 +20,9 @@
 
 open Printf
 
+open Tools_utils
+open Common_gettext.Gettext
+
 let rec pod_of_list ?(style = `Dot) xs    match style with
   | `Verbatim -> String.concat "\n" (List.map ((^) " ")
xs)
@@ -31,3 +34,32 @@ and _pod_of_list delim xs    "=over 4\n\n" ^
   String.concat "" (List.map (sprintf "=item %s\n\n%s\n\n"
delim) xs) ^
   "=back"
+
+let rec update_system_ca_store g root +  let cmd =
update_system_ca_store_command g root in
+  match cmd with
+  | None -> ()
+  | Some cmd ->
+    (* Try to run the command directly if possible, adding it as
+     * firstboot script in case of incompatible architectures.
+     *)
+    let cmd = String.concat " " cmd in
+    let incompatible_fn () +      Firstboot.add_firstboot_script g root cmd cmd
+    in
+
+    run_in_guest_command g root ~incompatible_fn cmd
+
+and update_system_ca_store_command g root +  let typ = g#inspect_get_type root
in
+  let distro = g#inspect_get_distro root in
+  match typ, distro with
+  | "linux",
("fedora"|"rhel"|"centos"|"scientificlinux"|"oraclelinux"|"redhat-based")
->
+    Some [ "update-ca-trust"; "extract" ]
+
+  | "linux",
("debian"|"ubuntu"|"kalilinux") ->
+    Some [ "update-ca-certificates" ]
+
+  | _, _ ->
+    warning (f_"updating the system CA store on this guest %s/%s is not
supported") typ distro;
+    None
diff --git a/sysprep/utils.mli b/sysprep/utils.mli
index a57a0d876..82779620e 100644
--- a/sysprep/utils.mli
+++ b/sysprep/utils.mli
@@ -26,3 +26,7 @@ val pod_of_list : ?style:[`Verbatim|`Star|`Dash|`Dot] ->
string list -> string
     use a space-indented (verbatim) block.  [`Star], [`Dash] or [`Dot]
     meaning use a real list with [*], [-] or [·].  The default
     style is [·] ([`Dot]). *)
+
+val update_system_ca_store : Guestfs.guestfs -> string -> unit
+(** Update the system CA store on the guest for the specified root
+    (which is fully mounted). *)
-- 
2.25.4

In case any certificate is removed from the guest, regenerate the system
CA store.
---
 sysprep/sysprep_operation_ca_certificates.ml | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/sysprep/sysprep_operation_ca_certificates.ml
b/sysprep/sysprep_operation_ca_certificates.ml
index e481cebf8..a2b7986c1 100644
--- a/sysprep/sysprep_operation_ca_certificates.ml
+++ b/sysprep/sysprep_operation_ca_certificates.ml
@@ -39,7 +39,11 @@ let ca_certificates_perform (g : Guestfs.guestfs) root
side_effects      let set = StringSet.diff set excepts in
     StringSet.iter (
       fun filename ->
-        try g#rm filename with G.Error _ -> ()
+        try
+          g#rm filename;
+          side_effects#update_system_ca_store ()
+        with
+          G.Error _ -> ()
     ) set
   )
 
@@ -48,6 +52,8 @@ let op = {
     name = "ca-certificates";
     enabled_by_default = false;
     heading = s_"Remove CA certificates in the guest";
+    pod_description = Some (s_"\
+In case any certificate is removed, the system CA store is updated.");
     perform_on_filesystems = Some ca_certificates_perform;
 }
 
-- 
2.25.4

This new operation unenrolls the guest from a FreeIPA server offline, by
removing the configuration files and certificates.

Thanks to Christian Heimes for the hints.
---
 sysprep/Makefile.am                           |  1 +
 sysprep/sysprep_operation_unenroll_freeipa.ml | 65 +++++++++++++++++++
 2 files changed, 66 insertions(+)
 create mode 100644 sysprep/sysprep_operation_unenroll_freeipa.ml

diff --git a/sysprep/Makefile.am b/sysprep/Makefile.am
index 451a3478f..30254c717 100644
--- a/sysprep/Makefile.am
+++ b/sysprep/Makefile.am
@@ -66,6 +66,7 @@ operations = \
 	sssd_db_log \
 	tmp_files \
 	udev_persistent_net \
+	unenroll_freeipa \
 	user_account \
 	utmp yum_uuid
 
diff --git a/sysprep/sysprep_operation_unenroll_freeipa.ml
b/sysprep/sysprep_operation_unenroll_freeipa.ml
new file mode 100644
index 000000000..5dd2bcc61
--- /dev/null
+++ b/sysprep/sysprep_operation_unenroll_freeipa.ml
@@ -0,0 +1,65 @@
+(* virt-sysprep
+ * Copyright (C) 2020 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *)
+
+open Sysprep_operation
+open Common_gettext.Gettext
+
+module G = Guestfs
+
+let unenroll_freeipa_perform (g : Guestfs.guestfs) root side_effects +  let typ
= g#inspect_get_type root in
+  if typ = "linux" then (
+    (* Simple paths with no side effects. *)
+    let paths = [ "/etc/ipa/ca.crt";
+                  "/etc/ipa/default.conf";
+                  "/var/lib/ipa-client/sysrestore/*";
+                  "/var/lib/ipa-client/pki/*" ] in
+    let paths = List.concat (List.map Array.to_list (List.map g#glob_expand
paths)) in
+    List.iter (
+      fun filename ->
+        try g#rm filename with G.Error _ -> ()
+    ) paths;
+
+    (* Certificates in the system CA store. *)
+    let certs = [ "/etc/pki/ca-trust/source/anchors/ipa-ca.crt";
+                  "/usr/local/share/ca-certificates/ipa-ca.crt";
+                  "/etc/pki/ca-trust/source/ipa.p11-kit" ] in
+    List.iter (
+      fun filename ->
+        try
+          g#rm filename;
+          side_effects#update_system_ca_store ()
+        with
+          G.Error _ -> ()
+    ) certs
+  )
+
+let op = {
+  defaults with
+    name = "unenroll-freeipa";
+    enabled_by_default = true;
+    heading = s_"Offline unenroll the guest from FreeIPA";
+    pod_description = Some (s_"\
+Unenroll the guest from FreeIPA, reverting and cleaning up the local
+server settings only, without interacting with the FreeIPA server.
+
+This operation does not run C<ipa-client>.");
+    perform_on_filesystems = Some unenroll_freeipa_perform;
+}
+
+let () = register_operation op
-- 
2.25.4

On Monday, 4 May 2020 15:22:10 CEST Pino Toscano wrote:
> It requires a change in libguestfs-common before the series is applied.
https://www.redhat.com/archives/libguestfs/2020-May/msg00016.html
which I failed to group in the same thread...

-- 
Pino Toscano

So with the proviso that I don't know much about freeipa, this code
still all looks sensible to me, and therefore:

ACK

Thanks,

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html

On Monday, 4 May 2020 15:22:14 CEST Pino Toscano wrote:
> This new operation unenrolls the guest from a FreeIPA server offline, by
> removing the configuration files and certificates.
> 
> Thanks to Christian Heimes for the hints.
> ---
>  sysprep/Makefile.am                           |  1 +
>  sysprep/sysprep_operation_unenroll_freeipa.ml | 65 +++++++++++++++++++
>  2 files changed, 66 insertions(+)
>  create mode 100644 sysprep/sysprep_operation_unenroll_freeipa.ml
Following the feedback of Christian and François, this operation was
renamed to "ipa-client", and pushed.

-- 
Pino Toscano