Libguestfs - May 2023 - [libguestfs-common PATCH v2 0/2] recognize "/dev/mapper/VG-LV" with "--key"

Bugzilla:

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

v1:

  [libguestfs-common PATCH 0/2] recognize "/dev/mapper/VG-LV" with
"--key"
  Message-Id: <20230515174924.290409-1-lersek at redhat.com>
  https://listman.redhat.com/archives/libguestfs/2023-May/031497.html

See the Notes section on each patch, for the updates in the current
version.

The virt-v2v test suite update is already on the list, and has been
reviewed by Rich (thanks!):

  [v2v PATCH 0/2] test "/dev/mapper/VG-LV" with "--key"
  Message-Id: <20230515175529.290724-1-lersek at redhat.com>
  https://listman.redhat.com/archives/libguestfs/2023-May/031501.html

Once I merge both series, I'll extend the libguestfs and guestfs-tools
test suites similarly.

Thanks!
Laszlo

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

 options/keys.c    | 102 +++++++++++++++++++-
 options/options.h |   3 +-
 2 files changed, 103 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>
Reviewed-by: Richard W.M. Jones <rjones at redhat.com>
---

Notes:
    v2:
    
    - pick up Rich's R-b

 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;

Libguestfs uses the

  /dev/VG/LV

format internally, for identifying LVM logical volumes, but the user might
want to specify the

  /dev/mapper/VG-LV ID

format with the "--key ID:..." options.

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.

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:
    v2:
    
    - rewrite without regular expressions [Rich, Laszlo]
    
    - restructure the commit message
    
    v1:
    
    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 | 100 ++++++++++++++++++++
 1 file changed, 100 insertions(+)

diff --git a/options/keys.c b/options/keys.c
index bc7459749276..52b27369016a 100644
--- a/options/keys.c
+++ b/options/keys.c
@@ -260,6 +260,105 @@ key_store_add_from_selector (struct key_store *ks, const
char *selector)
   return key_store_import_key (ks, &key);
 }
 
+/* Turn /dev/mapper/VG-LV into /dev/VG/LV, in-place. */
+static void
+unescape_device_mapper_lvm (char *id)
+{
+  static const char dev[] = "/dev/", dev_mapper[] =
"/dev/mapper/";
+  const char *input_start;
+  char *output;
+  enum { M_SCAN, M_FILL, M_DONE } mode;
+
+  if (!STRPREFIX (id, dev_mapper))
+    return;
+
+  /* Start parsing "VG-LV" from "id" after
"/dev/mapper/". */
+  input_start = id + (sizeof dev_mapper - 1);
+
+  /* Start writing the unescaped "VG/LV" output after
"/dev/". */
+  output = id + (sizeof dev - 1);
+
+  for (mode = M_SCAN; mode < M_DONE; ++mode) {
+    char c;
+    const char *input = input_start;
+    const char *hyphen_buffered = NULL;
+    bool single_hyphen_seen = false;
+
+    do {
+      c = *input;
+
+      switch (c) {
+      case '-':
+        if (hyphen_buffered == NULL)
+          /* This hyphen may start an escaped hyphen, or it could be the
+           * separator in VG-LV.
+           */
+          hyphen_buffered = input;
+        else {
+          /* This hyphen completes an escaped hyphen; unescape it. */
+          if (mode == M_FILL)
+            *output++ = '-';
+          hyphen_buffered = NULL;
+        }
+        break;
+
+      case '/':
+        /* Slash characters are forbidden in VG-LV anywhere. If there's
any,
+         * we'll find it in the first (i.e., scanning) phase, before we
output
+         * anything back to "id".
+         */
+        assert (mode == M_SCAN);
+        return;
+
+      default:
+        /* Encountered a non-slash, non-hyphen character -- which also may be
+         * the terminating NUL.
+         */
+        if (hyphen_buffered != NULL) {
+          /* The non-hyphen character comes after a buffered hyphen, so the
+           * buffered hyphen is supposed to be the single hyphen that separates
+           * VG from LV in VG-LV. There are three requirements for this
+           * separator: (a) it must be unique (we must not have seen another
+           * such separator earlier), (b) it must not be at the start of VG-LV
+           * (because VG would be empty that way), (c) it must not be at the
end
+           * of VG-LV (because LV would be empty that way). Should any of these
+           * be violated, we'll catch that during the first (i.e.,
scanning)
+           * phase, before modifying "id".
+           */
+          if (single_hyphen_seen || hyphen_buffered == input_start ||
+              c == '\0') {
+            assert (mode == M_SCAN);
+            return;
+          }
+
+          /* Translate the separator hyphen to a slash character. */
+          if (mode == M_FILL)
+            *output++ = '/';
+          hyphen_buffered = NULL;
+          single_hyphen_seen = true;
+        }
+
+        /* Output the non-hyphen character (including the terminating NUL)
+         * regardless of whether there was a buffered hyphen separator (which,
+         * by now, we'll have attempted to translate and flush).
+         */
+        if (mode == M_FILL)
+          *output++ = c;
+      }
+
+      ++input;
+    } while (c != '\0');
+
+    /* We must have seen the VG-LV separator. If that's not the case,
we'll
+     * catch it before modifying "id".
+     */
+    if (!single_hyphen_seen) {
+      assert (mode == M_SCAN);
+      return;
+    }
+  }
+}
+
 struct key_store *
 key_store_import_key (struct key_store *ks, struct key_store_key *key)
 {
@@ -278,6 +377,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;