Pino Toscano
2014-Feb-20 10:53 UTC
[Libguestfs] [PATCH 1/2] mllib: add an hook to cleanup directories on exit
Much similar to unlink_on_exit, but recursively cleaning directories.
---
mllib/common_utils.ml | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/mllib/common_utils.ml b/mllib/common_utils.ml
index 3943417..f49ede6 100644
--- a/mllib/common_utils.ml
+++ b/mllib/common_utils.ml
@@ -386,6 +386,35 @@ let unlink_on_exit registered_handlers := true
)
+(* Remove a temporary directory on exit. *)
+let rmdir_on_exit + let dirs = ref [] in
+ let registered_handlers = ref false in
+
+ let rec unlink_dirs () + let rec recursive_rmdir fn + if
Sys.is_directory fn then (
+ let names = Array.map (fun d -> fn // d) (Sys.readdir fn) in
+ Array.iter recursive_rmdir names;
+ Unix.rmdir fn
+ ) else
+ Unix.unlink fn
+ in
+ List.iter (
+ fun dir -> try recursive_rmdir dir with _ -> ()
+ ) !dirs
+ and register_handlers () + (* Remove on exit. *)
+ at_exit unlink_dirs
+ in
+
+ fun dir ->
+ dirs := dir :: !dirs;
+ if not !registered_handlers then (
+ register_handlers ();
+ registered_handlers := true
+ )
+
(* Using the libguestfs API, recursively remove only files from the
* given directory. Useful for cleaning /var/cache etc in sysprep
* without removing the actual directory structure. Also if 'dir' is
--
1.8.3.1
Pino Toscano
2014-Feb-20 10:53 UTC
[Libguestfs] [PATCH 2/2] builder: use a disposable GPG keyring for every Sigchecker
Create a temporary directory and tell gpg to use it as homedir, so
imported keys do not get into the user's keyring. This also avoid
importing the default key when a different one is needed to check the
signature.
The only exception is when a non-default fingerprint is used: in this
case, that key is read from the user's keyring, since it is where it is.
---
builder/Makefile.am | 5 +++++
builder/mkdtemp-c.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++
builder/mkdtemp.ml | 19 ++++++++++++++++
builder/mkdtemp.mli | 20 +++++++++++++++++
builder/sigchecker.ml | 61 ++++++++++++++++++++++++++++++++++++++++-----------
po/POTFILES | 1 +
po/POTFILES-ml | 1 +
7 files changed, 150 insertions(+), 13 deletions(-)
create mode 100644 builder/mkdtemp-c.c
create mode 100644 builder/mkdtemp.ml
create mode 100644 builder/mkdtemp.mli
diff --git a/builder/Makefile.am b/builder/Makefile.am
index 23336c0..d668377 100644
--- a/builder/Makefile.am
+++ b/builder/Makefile.am
@@ -48,6 +48,9 @@ SOURCES = \
index-parser-c.c \
list_entries.mli \
list_entries.ml \
+ mkdtemp.mli \
+ mkdtemp.ml \
+ mkdtemp-c.c \
paths.ml \
pxzcat.ml \
pxzcat.mli \
@@ -92,6 +95,8 @@ OBJECTS = \
pxzcat.cmx \
setlocale-c.o \
setlocale.cmx \
+ mkdtemp-c.o \
+ mkdtemp.cmx \
paths.cmx \
get_kernel.cmx \
downloader.cmx \
diff --git a/builder/mkdtemp-c.c b/builder/mkdtemp-c.c
new file mode 100644
index 0000000..2284e3a
--- /dev/null
+++ b/builder/mkdtemp-c.c
@@ -0,0 +1,56 @@
+/* virt-builder
+ * Copyright (C) 2014 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.
+ */
+
+#include <config.h>
+
+#include <stdlib.h>
+#include <errno.h>
+#include <string.h>
+
+#include <caml/alloc.h>
+#include <caml/fail.h>
+#include <caml/memory.h>
+#include <caml/mlvalues.h>
+
+#ifdef HAVE_CAML_UNIXSUPPORT_H
+#include <caml/unixsupport.h>
+#else
+#define Nothing ((value) 0)
+extern void unix_error (int errcode, char * cmdname, value arg) Noreturn;
+#endif
+
+value
+virt_builder_mkdtemp (value val_pattern)
+{
+ CAMLparam1 (val_pattern);
+ CAMLlocal1 (rv);
+ char *pattern, *ret;
+
+ pattern = strdup (String_val (val_pattern));
+ if (pattern == NULL)
+ unix_error (errno, (char *) "strdup", val_pattern);
+
+ ret = mkdtemp (pattern);
+ if (ret == NULL)
+ unix_error (errno, (char *) "mkdtemp", val_pattern);
+
+ rv = caml_copy_string (ret);
+ free (pattern);
+
+ CAMLreturn (rv);
+}
diff --git a/builder/mkdtemp.ml b/builder/mkdtemp.ml
new file mode 100644
index 0000000..2e64862
--- /dev/null
+++ b/builder/mkdtemp.ml
@@ -0,0 +1,19 @@
+(* virt-builder
+ * Copyright (C) 2014 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.
+ *)
+
+external mkdtemp : string -> string = "virt_builder_mkdtemp"
diff --git a/builder/mkdtemp.mli b/builder/mkdtemp.mli
new file mode 100644
index 0000000..674e6f2
--- /dev/null
+++ b/builder/mkdtemp.mli
@@ -0,0 +1,20 @@
+(* virt-builder
+ * Copyright (C) 2014 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.
+ *)
+
+val mkdtemp : string -> string
+(** [mkdtemp pattern] Tiny wrapper to the C [mkdtemp]. *)
diff --git a/builder/sigchecker.ml b/builder/sigchecker.ml
index b20a186..10e342e 100644
--- a/builder/sigchecker.ml
+++ b/builder/sigchecker.ml
@@ -95,21 +95,38 @@ ZvXkQ3FVJwZoLmHw47vvlVpLD/4gi1SuHWieRvZ+UdDq00E348pm
=neBW
-----END PGP PUBLIC KEY BLOCK-----
"
-let key_imported = ref false
type t = {
debug : bool;
gpg : string;
fingerprint : string;
check_signature : bool;
+ gpghome : string;
+ mutable key_imported : bool;
}
let create ~debug ~gpg ~fingerprint ~check_signature + (* Create a temporary
directory for gnupg. *)
+ let tmpdir = Mkdtemp.mkdtemp (Filename.temp_dir_name //
"vb.gpghome.XXXXXX") in
+ rmdir_on_exit tmpdir;
+ (* Run gpg once, so it can setup its own home directory, failing
+ * if it cannot.
+ *)
+ let cmd = sprintf "%s --homedir %s --list-keys%s"
+ gpg tmpdir (if debug then "" else " >/dev/null
2>&1") in
+ if debug then eprintf "%s\n%!" cmd;
+ let r = Sys.command cmd in
+ if r <> 0 then (
+ eprintf (f_"virt-builder: error: GPG failure: could not run GPG the
first time\nUse the '-v' option and look for earlier error
messages.\n");
+ exit 1
+ );
{
debug = debug;
gpg = gpg;
fingerprint = fingerprint;
check_signature = check_signature;
+ gpghome = tmpdir;
+ key_imported = false;
}
(* Compare two strings of hex digits ignoring whitespace and case. *)
@@ -159,8 +176,9 @@ and do_verify t args let status_file = Filename.temp_file
"vbstat" ".txt" in
unlink_on_exit status_file;
let cmd - sprintf "%s --verify%s --status-file %s %s"
- t.gpg (if t.debug then "" else " -q --logger-file
/dev/null")
+ sprintf "%s --homedir %s --verify%s --status-file %s %s"
+ t.gpg t.gpghome
+ (if t.debug then "" else " -q --logger-file
/dev/null")
(quote status_file) args in
if t.debug then eprintf "%s\n%!" cmd;
let r = Sys.command cmd in
@@ -188,23 +206,40 @@ and do_verify t args exit 1
)
-(* Import the default public key. *)
+(* Import the requested public key. *)
and import_key t - if not !key_imported then (
- let filename, chan = Filename.open_temp_file "vbpubkey"
".asc" in
- unlink_on_exit filename;
- output_string chan default_pubkey;
- close_out chan;
-
- let cmd = sprintf "%s --import %s%s"
- t.gpg (quote filename)
+ if not t.key_imported then (
+ let keyfile = ref "" in
+ if equal_fingerprints default_fingerprint t.fingerprint then (
+ let filename, chan = Filename.open_temp_file "vbpubkey"
".asc" in
+ unlink_on_exit filename;
+ output_string chan default_pubkey;
+ close_out chan;
+ keyfile := filename
+ ) else (
+ let filename = Filename.temp_file "vbpubkey" ".asc"
in
+ unlink_on_exit filename;
+ let cmd = sprintf "%s --yes --armor --output %s --export %s%s"
+ t.gpg (quote filename) (quote t.fingerprint)
+ (if t.debug then "" else " >/dev/null
2>&1") in
+ if t.debug then eprintf "%s\n%!" cmd;
+ let r = Sys.command cmd in
+ if r <> 0 then (
+ eprintf (f_"virt-builder: error: could not export public key\nUse
the '-v' option and look for earlier error messages.\n");
+ exit 1
+ );
+ keyfile := filename
+ );
+
+ let cmd = sprintf "%s --homedir %s --import %s%s"
+ t.gpg t.gpghome (quote !keyfile)
(if t.debug then "" else " >/dev/null
2>&1") in
let r = Sys.command cmd in
if r <> 0 then (
eprintf (f_"virt-builder: error: could not import public key\nUse
the '-v' option and look for earlier error messages.\n");
exit 1
);
- key_imported := true
+ t.key_imported <- true
)
type csum_t = SHA512 of string
diff --git a/po/POTFILES b/po/POTFILES
index c0d20f0..1de501e 100644
--- a/po/POTFILES
+++ b/po/POTFILES
@@ -4,6 +4,7 @@ builder/index-parser-c.c
builder/index-scan.c
builder/index-struct.c
builder/index-validate.c
+builder/mkdtemp-c.c
builder/pxzcat-c.c
builder/setlocale-c.c
cat/cat.c
diff --git a/po/POTFILES-ml b/po/POTFILES-ml
index 301fe81..56551ab 100644
--- a/po/POTFILES-ml
+++ b/po/POTFILES-ml
@@ -4,6 +4,7 @@ builder/downloader.ml
builder/get_kernel.ml
builder/index_parser.ml
builder/list_entries.ml
+builder/mkdtemp.ml
builder/paths.ml
builder/pxzcat.ml
builder/setlocale.ml
--
1.8.3.1
Richard W.M. Jones
2014-Feb-20 13:08 UTC
Re: [Libguestfs] [PATCH 1/2] mllib: add an hook to cleanup directories on exit
On Thu, Feb 20, 2014 at 11:53:16AM +0100, Pino Toscano wrote:> Much similar to unlink_on_exit, but recursively cleaning directories. > --- > mllib/common_utils.ml | 29 +++++++++++++++++++++++++++++ > 1 file changed, 29 insertions(+) > > diff --git a/mllib/common_utils.ml b/mllib/common_utils.ml > index 3943417..f49ede6 100644 > --- a/mllib/common_utils.ml > +++ b/mllib/common_utils.ml > @@ -386,6 +386,35 @@ let unlink_on_exit > registered_handlers := true > ) > > +(* Remove a temporary directory on exit. *) > +let rmdir_on_exit > + let dirs = ref [] in > + let registered_handlers = ref false in > + > + let rec unlink_dirs () > + let rec recursive_rmdir fn > + if Sys.is_directory fn then (I suspect this will follow symlinks, so that if the temporary directory contains a link like: /tmp/tmpdir/foo -> / it will proceed to wipe out other bits of your filesystem. An altogether easier, safety and faster way to do this is to do it like supermin does: https://github.com/libguestfs/supermin/blob/master/src/supermin_utils.ml#L91 Especially line 105 ff. but the rest of the function may be interesting too in the context of patch 2/2. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW
Pino Toscano
2014-Feb-20 14:03 UTC
Re: [Libguestfs] [PATCH 1/2] mllib: add an hook to cleanup directories on exit
On Thursday 20 February 2014 13:08:40 Richard W.M. Jones wrote:> On Thu, Feb 20, 2014 at 11:53:16AM +0100, Pino Toscano wrote: > > Much similar to unlink_on_exit, but recursively cleaning > > directories. > > --- > > > > mllib/common_utils.ml | 29 +++++++++++++++++++++++++++++ > > 1 file changed, 29 insertions(+) > > > > diff --git a/mllib/common_utils.ml b/mllib/common_utils.ml > > index 3943417..f49ede6 100644 > > --- a/mllib/common_utils.ml > > +++ b/mllib/common_utils.ml > > @@ -386,6 +386,35 @@ let unlink_on_exit > > > > registered_handlers := true > > > > ) > > > > +(* Remove a temporary directory on exit. *) > > +let rmdir_on_exit > > + let dirs = ref [] in > > + let registered_handlers = ref false in > > + > > + let rec unlink_dirs () > > + let rec recursive_rmdir fn > > + if Sys.is_directory fn then ( > > I suspect this will follow symlinks, so that if the temporary > directory contains a link like: > > /tmp/tmpdir/foo -> / > > it will proceed to wipe out other bits of your filesystem.Uh duh, sorry for the oversight. Fixed it checking the file type using lstat.> Especially line 105 ff. but the rest of the function may be > interesting too in the context of patch 2/2.Hm, it is kind of duplicating what mkdtemp already does, so I would rather keep using it. Pity OCaml does not offer it at all... -- Pino Toscano
Richard W.M. Jones
2014-Feb-21 11:10 UTC
Re: [Libguestfs] [PATCH 2/2] builder: use a disposable GPG keyring for every Sigchecker
On Thu, Feb 20, 2014 at 11:53:17AM +0100, Pino Toscano wrote:> Create a temporary directory and tell gpg to use it as homedir, so > imported keys do not get into the user's keyring. This also avoid > importing the default key when a different one is needed to check the > signature. > > The only exception is when a non-default fingerprint is used: in this > case, that key is read from the user's keyring, since it is where it is.The mkdtemp part is fine. You could spin that off into a separate commit, so it could be a candidate for backporting. The rest I found a bit confusing. What does it do exactly?> diff --git a/builder/sigchecker.ml b/builder/sigchecker.ml > index b20a186..10e342e 100644 > --- a/builder/sigchecker.ml > +++ b/builder/sigchecker.ml > @@ -95,21 +95,38 @@ ZvXkQ3FVJwZoLmHw47vvlVpLD/4gi1SuHWieRvZ+UdDq00E348pm > =neBW > -----END PGP PUBLIC KEY BLOCK----- > " > -let key_imported = ref false > > type t = { > debug : bool; > gpg : string; > fingerprint : string; > check_signature : bool; > + gpghome : string; > + mutable key_imported : bool; > } > > let create ~debug ~gpg ~fingerprint ~check_signature > + (* Create a temporary directory for gnupg. *) > + let tmpdir = Mkdtemp.mkdtemp (Filename.temp_dir_name // "vb.gpghome.XXXXXX") in > + rmdir_on_exit tmpdir; > + (* Run gpg once, so it can setup its own home directory, failing > + * if it cannot. > + *) > + let cmd = sprintf "%s --homedir %s --list-keys%s" > + gpg tmpdir (if debug then "" else " >/dev/null 2>&1") in > + if debug then eprintf "%s\n%!" cmd; > + let r = Sys.command cmd in > + if r <> 0 then ( > + eprintf (f_"virt-builder: error: GPG failure: could not run GPG the first time\nUse the '-v' option and look for earlier error messages.\n"); > + exit 1 > + ); > { > debug = debug; > gpg = gpg; > fingerprint = fingerprint; > check_signature = check_signature; > + gpghome = tmpdir; > + key_imported = false; > } > > (* Compare two strings of hex digits ignoring whitespace and case. *) > @@ -159,8 +176,9 @@ and do_verify t args > let status_file = Filename.temp_file "vbstat" ".txt" in > unlink_on_exit status_file; > let cmd > - sprintf "%s --verify%s --status-file %s %s" > - t.gpg (if t.debug then "" else " -q --logger-file /dev/null") > + sprintf "%s --homedir %s --verify%s --status-file %s %s" > + t.gpg t.gpghome > + (if t.debug then "" else " -q --logger-file /dev/null") > (quote status_file) args in > if t.debug then eprintf "%s\n%!" cmd; > let r = Sys.command cmd in > @@ -188,23 +206,40 @@ and do_verify t args > exit 1 > ) > > -(* Import the default public key. *) > +(* Import the requested public key. *) > and import_key t > - if not !key_imported then ( > - let filename, chan = Filename.open_temp_file "vbpubkey" ".asc" in > - unlink_on_exit filename; > - output_string chan default_pubkey; > - close_out chan; > - > - let cmd = sprintf "%s --import %s%s" > - t.gpg (quote filename) > + if not t.key_imported then ( > + let keyfile = ref "" in > + if equal_fingerprints default_fingerprint t.fingerprint then ( > + let filename, chan = Filename.open_temp_file "vbpubkey" ".asc" in > + unlink_on_exit filename; > + output_string chan default_pubkey; > + close_out chan; > + keyfile := filename > + ) else ( > + let filename = Filename.temp_file "vbpubkey" ".asc" in > + unlink_on_exit filename; > + let cmd = sprintf "%s --yes --armor --output %s --export %s%s" > + t.gpg (quote filename) (quote t.fingerprint) > + (if t.debug then "" else " >/dev/null 2>&1") in > + if t.debug then eprintf "%s\n%!" cmd; > + let r = Sys.command cmd in > + if r <> 0 then ( > + eprintf (f_"virt-builder: error: could not export public key\nUse the '-v' option and look for earlier error messages.\n"); > + exit 1 > + ); > + keyfile := filename > + ); > + > + let cmd = sprintf "%s --homedir %s --import %s%s" > + t.gpg t.gpghome (quote !keyfile) > (if t.debug then "" else " >/dev/null 2>&1") in > let r = Sys.command cmd in > if r <> 0 then ( > eprintf (f_"virt-builder: error: could not import public key\nUse the '-v' option and look for earlier error messages.\n"); > exit 1 > ); > - key_imported := true > + t.key_imported <- true > ) > > type csum_t = SHA512 of stringRich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/