Libguestfs - May 2023 - [libguestfs-common PATCH 1/2] options/keys: key_store_import_key(): un-constify "key" parameter

https://bugzilla.redhat.com/show_bug.cgi?id=2168506

The commit message of the second patch explains the idea.

The test suites in libguestfs, guestfs-tools and virt-v2v will have to
be extended so they cover this feature; I'm soon going to post the
virt-v2v update. I plan to cover the libguestfs and guestfs-tools test
suites later.

Thanks for reviewing,
Laszlo

Laszlo Ersek (2):
  options/keys: key_store_import_key(): un-constify "key" parameter
  options/keys: introduce unescape_device_mapper_lvm()

 options/keys.c    | 88 +++++++++++++++++++-
 options/options.h |  3 +-
 2 files changed, 89 insertions(+), 2 deletions(-)

The key_store_import_key() function is called from both C-language
utilities -- via key_store_add_from_selector() -- and OCaml-language ones
-- via guestfs_int_mllib_inspect_decrypt().  We currently declare the
function's second parameter as

  const struct key_store_key *key

That is however a superficial, if not outright false, promise: in
key_store_import_key(), we take ownership of all three string fields
within "key", as evidenced by free_key_store(), where we free() all
three
strings. With the same effort, we might as well modify the contents of
those strings at once. Drop "const". (None of the callers care, but
let's
be honest.)

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2168506
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 options/options.h | 3 ++-
 options/keys.c    | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/options/options.h b/options/options.h
index 94573ee063bb..94e8b9eef43e 100644
--- a/options/options.h
+++ b/options/options.h
@@ -169,7 +169,8 @@ extern struct matching_key *get_keys (struct key_store *ks,
const char *device,
                                       const char *uuid, size_t *nr_matches);
 extern void free_keys (struct matching_key *keys, size_t nr_matches);
 extern struct key_store *key_store_add_from_selector (struct key_store *ks,
const char *selector);
-extern struct key_store *key_store_import_key (struct key_store *ks, const
struct key_store_key *key);
+extern struct key_store *key_store_import_key (struct key_store *ks,
+                                               struct key_store_key *key);
 extern bool key_store_requires_network (const struct key_store *ks);
 extern void free_key_store (struct key_store *ks);
 
diff --git a/options/keys.c b/options/keys.c
index 48f1bc7c7c47..bc7459749276 100644
--- a/options/keys.c
+++ b/options/keys.c
@@ -261,7 +261,7 @@ key_store_add_from_selector (struct key_store *ks, const
char *selector)
 }
 
 struct key_store *
-key_store_import_key (struct key_store *ks, const struct key_store_key *key)
+key_store_import_key (struct key_store *ks, struct key_store_key *key)
 {
   struct key_store_key *new_keys;

Introduce unescape_device_mapper_lvm() for turning

  /dev/mapper/VG-LV

key IDs into

  /dev/VG/LV

ones, unescaping doubled hyphens to single hyphens in both VG and LV in
the process. Libguestfs uses the /dev/VG/LV format internally, for
identifying devices, but the user might want to specify the
/dev/mapper/VG-LV ID format with the "--key ID:..." options.

Call unescape_device_mapper_lvm() from key_store_import_key(). That is,
translate the ID as soon as the "--key" option is processed -- let the
keystore only know about the usual /dev/VG/LV format, for later matching.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2168506
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---

Notes:
    regcomp() must definitely allocate memory dynamically, and we
    (intentionally) never free that -- we never call regfree(). I assume
    valgrind will catch this as a leak, so we might have to extend
    "valgrind-suppressions" in each dependent superproject. However,
I'm
    unable to run "make check-valgrind" successfully in e.g. virt-v2v
even
    before these patches; see also
    <https://listman.redhat.com/archives/libguestfs/2023-May/031496.html>.

 options/keys.c | 86 ++++++++++++++++++++
 1 file changed, 86 insertions(+)

diff --git a/options/keys.c b/options/keys.c
index bc7459749276..f0164a6ed987 100644
--- a/options/keys.c
+++ b/options/keys.c
@@ -27,6 +27,7 @@
 #include <errno.h>
 #include <error.h>
 #include <assert.h>
+#include <regex.h>
 
 #include "guestfs.h"
 
@@ -260,6 +261,90 @@ key_store_add_from_selector (struct key_store *ks, const
char *selector)
   return key_store_import_key (ks, &key);
 }
 
+/* A /dev/mapper/ escaped character is:
+ * - either any character except a slash or a hyphen,
+ * - or a double hyphen.
+ *
+ * Note that parens are deliberately not included here -- they will be included
+ * in the full pattern, for making them more easily countable.
+ *
+ * Also note that the bracket expression below does not contain a range
+ * expression, therefore it should not be locale-sensitive.
+ */
+#define ESC_CH "[^-/]|--"
+
+/* Turn /dev/mapper/VG-LV into /dev/VG/LV. */
+static void
+unescape_device_mapper_lvm (char *id)
+{
+  static bool compiled;
+  int rc;
+  static regex_t regex;
+  regmatch_t match[5];
+  char *output;
+  const char *vg_start, *vg_end, *lv_start, *lv_end;
+  const char *input;
+
+  if (!compiled) {
+    /* Recognize /dev/mapper/VG-LV pathnames, where VG and LV don't contain
+     * slashes, plus any original hyphens in them are doubled for escaping.
+     */
+    static const char pattern[] = "^/dev/("
+                                    "mapper/"
+                                    "((" ESC_CH ")+)"
+                                    "-"
+                                    "((" ESC_CH ")+)"
+                                  ")$";
+
+    rc = regcomp (&regex, pattern, REG_EXTENDED);
+    if (rc != 0) {
+      char errbuf[256];
+
+      /* When regcomp() fails, the contents of "regex" are undefined,
so we
+       * cannot pass "regex" to regerror().
+       */
+      (void)regerror (rc, NULL, errbuf, sizeof errbuf);
+      error (EXIT_FAILURE, 0,
+             _("%s: Failed to compile pattern (%s).  Please report a bug
for "
+               "libguestfs -- refer to guestfs(3) section BUGS."),
+             __func__, errbuf);
+    }
+    compiled = true;
+  }
+
+  rc = regexec (&regex, id, 5, match, 0);
+
+  /* If there's no match, just leave "id" alone. */
+  if (rc != 0)
+    return;
+
+  /* Start outputting after "/dev/". */
+  output   = id + match[1].rm_so;
+  vg_start = id + match[2].rm_so;
+  vg_end   = id + match[2].rm_eo;
+  lv_start = id + match[4].rm_so;
+  lv_end   = id + match[4].rm_eo;
+
+  /* Each of the following two loops produces at most as many bytes as it
+   * consumes, so we unescape "id" in-place.
+   */
+  input = vg_start;
+  while (input < vg_end)
+    if ((*output++ = *input++) == '-')
+      input++;
+
+  *output++ = '/';
+
+  input = lv_start;
+  while (input < lv_end)
+    if ((*output++ = *input++) == '-')
+      input++;
+
+  *output++ = '\0';
+}
+
+#undef ESC_CH
+
 struct key_store *
 key_store_import_key (struct key_store *ks, struct key_store_key *key)
 {
@@ -278,6 +363,7 @@ key_store_import_key (struct key_store *ks, struct
key_store_key *key)
     error (EXIT_FAILURE, errno, "realloc");
 
   ks->keys = new_keys;
+  unescape_device_mapper_lvm (key->id);
   ks->keys[ks->nr_keys] = *key;
   ++ks->nr_keys;