[libvirt] [PATCH] storage: Support different wiping algorithms
Currently, we support only filling a volume with zeroes on wiping. However, it is not enough as data might still be readable by experienced and equipped attacker. Many technical papers have been written, therefore we should support other wiping algorithms. --- Okay, this is not as complete as I'd like it. Wiping could take ages, therefore we *want* it to be asynchronous. But that would mean: 1) Create new API to answer: "Are we done yet?" 2) Create event emitting system - like we have for domains 3) Combination of previous two In fact, I've started writing events, but it turned out to be huge and I am not even in a half yet. I need to find a way of re-using event code we already have. But thats another day and another patch set. For those interested, take a look at Section 4.1.11 of DSS_PCI requirements at [1]. 1: https://www.pcisecuritystandards.org/documents/Virtualization_InfoSupp_v2.pd... configure.ac | 26 ++++++++++- include/libvirt/libvirt.h.in | 24 ++++++++++ src/libvirt.c | 6 ++- src/storage/storage_driver.c | 103 ++++++++++++++++++++++++++++++++++------- tools/virsh.c | 41 +++++++++++++++- tools/virsh.pod | 25 ++++++++++- 6 files changed, 201 insertions(+), 24 deletions(-) diff --git a/configure.ac b/configure.ac index 46a9129..fb5f669 100644 --- a/configure.ac +++ b/configure.ac @@ -2500,7 +2500,31 @@ AM_CONDITIONAL([HAVE_LIBNL], [test "$have_libnl" = "yes"]) AC_SUBST([LIBNL_CFLAGS]) AC_SUBST([LIBNL_LIBS]) - +dnl scrub program for different volume wiping algorithms + +AC_ARG_WITH([scrub], + AC_HELP_STRING([--with-scrub], [enable different volume wiping algorithms + @<:@default=check@:>@]), + []. + [with_scrub=check]) + +if test "$with_scrub" != "no"; then + AC_PATH_PROG([SCRUB], [scrub]) + if test -z "$SCRUB" ; then + if test "$with_scrub" = "check"; then + with_scrub=no + else + AC_MSG_ERROR([You must install the 'scrub' binary to enable + different volume wiping algorithms]) + fi + else + with_scrub=yes + fi + if test "$with_scrub" = "yes"; then + AC_DEFINE_UNQUOTED([SCRUB], ["$SCRUB"], + [Location of the scrub program]) + fi +fi # Only COPYING.LIB is under version control, yet COPYING diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index ad6fcce..caa9127 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2118,6 +2118,30 @@ typedef enum { VIR_STORAGE_VOL_DELETE_ZEROED = 1, /* Clear all data to zeros (slow) */ } virStorageVolDeleteFlags; +typedef enum { + /* Keep this place for async flag. Currently, + * it is not implemented because of lack of + * events support within storage driver. + * VIR_STORAGE_VOL_WIPE_ASYNC = (1 << 0), + */ + VIR_STORAGE_VOL_WIPE_ALG_NNSA = (1 << 1), /* 4-pass NNSA Policy Letter + NAP-14.1-C (XVI-8) */ + VIR_STORAGE_VOL_WIPE_ALG_DOD = (1 << 2), /* 4-pass DoD 5220.22-M section + 8-306 procedure */ + VIR_STORAGE_VOL_WIPE_ALG_BSI = (1 << 3), /* 9-pass method recommended by the + German Center of Security in + Information Technologies */ + VIR_STORAGE_VOL_WIPE_ALG_GUTMANN = (1 << 4), /* The canonical 35-pass sequence */ + VIR_STORAGE_VOL_WIPE_ALG_SCHNEIER = (1 << 5), /* 7-pass method described by + Bruce Schneier in "Applied + Cryptography" (1996) */ + VIR_STORAGE_VOL_WIPE_ALG_PFITZNER7 = (1 << 6), /* 7-pass random */ + + VIR_STORAGE_VOL_WIPE_ALG_PFITZNER33 = (1 << 7), /* 33-pass random */ + + VIR_STORAGE_VOL_WIPE_ALG_RANDOM = (1 << 8), /* 1-pass random */ +} virStorageVolWipeFlags; + typedef struct _virStorageVolInfo virStorageVolInfo; struct _virStorageVolInfo { diff --git a/src/libvirt.c b/src/libvirt.c index feb3ca6..c3b3665 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -12557,7 +12557,11 @@ error: * @vol: pointer to storage volume * @flags: future flags, use 0 for now * - * Ensure data previously on a volume is not accessible to future reads + * Ensure data previously on a volume is not accessible to future reads. + * When specifying wipe algorithm only one algorithm flag is allowed. + * Therefore if one wants to wipe a volume with say NNSA and DOD algorithms, + * he/she needs two subsequent calls. First with _ALG_NNSA flag, second + * with _ALG_DOD. * * Returns 0 on success, or -1 on error */ diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 8c2d6e1..f3a084a 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1801,14 +1801,16 @@ out: static int -storageVolumeWipeInternal(virStorageVolDefPtr def) +storageVolumeWipeInternal(virStorageVolDefPtr def, + unsigned int flags) { int ret = -1, fd = -1; struct stat st; char *writebuf = NULL; size_t bytes_wiped = 0; + virCommandPtr cmd = NULL; - VIR_DEBUG("Wiping volume with path '%s'", def->target.path); + VIR_DEBUG("Wiping volume with path '%s' flags %x", def->target.path, flags); fd = open(def->target.path, O_RDWR); if (fd == -1) { @@ -1825,29 +1827,62 @@ storageVolumeWipeInternal(virStorageVolDefPtr def) goto out; } - if (S_ISREG(st.st_mode) && st.st_blocks < (st.st_size / DEV_BSIZE)) { - ret = storageVolumeZeroSparseFile(def, st.st_size, fd); - } else { - - if (VIR_ALLOC_N(writebuf, st.st_blksize) != 0) { - virReportOOMError(); + if (flags) { + cmd = virCommandNew(SCRUB); + virCommandAddArg(cmd, "-f"); + if (flags &VIR_STORAGE_VOL_WIPE_ALG_NNSA) { + virCommandAddArgList(cmd, "-p", "nnsa", NULL); + } else if (flags & VIR_STORAGE_VOL_WIPE_ALG_DOD) { + virCommandAddArgList(cmd, "-p", "dod", NULL); + } else if (flags & VIR_STORAGE_VOL_WIPE_ALG_BSI) { + virCommandAddArgList(cmd, "-p", "bsi", NULL); + } else if (flags & VIR_STORAGE_VOL_WIPE_ALG_GUTMANN) { + virCommandAddArgList(cmd, "-p", "gutmann", NULL); + } else if (flags & VIR_STORAGE_VOL_WIPE_ALG_SCHNEIER) { + virCommandAddArgList(cmd, "-p", "schneier", NULL); + } else if (flags & VIR_STORAGE_VOL_WIPE_ALG_PFITZNER7) { + virCommandAddArgList(cmd, "-p", "pfitzner7", NULL); + } else if (flags & VIR_STORAGE_VOL_WIPE_ALG_PFITZNER33) { + virCommandAddArgList(cmd, "-p", "pfitzner33", NULL); + } else if (flags & VIR_STORAGE_VOL_WIPE_ALG_RANDOM) { + virCommandAddArgList(cmd, "-p", "random", NULL); + } else { + virStorageReportError(VIR_ERR_INTERNAL_ERROR, + _("Unsupported flags %x"), + flags); goto out; } - ret = storageWipeExtent(def, - fd, - 0, - def->allocation, - writebuf, - st.st_blksize, - &bytes_wiped); + virCommandAddArg(cmd, def->target.path); + + if (virCommandRun(cmd, NULL) < 0) + goto out; + + ret = 0; + } else { + if (S_ISREG(st.st_mode) && st.st_blocks < (st.st_size / DEV_BSIZE)) { + ret = storageVolumeZeroSparseFile(def, st.st_size, fd); + } else { + + if (VIR_ALLOC_N(writebuf, st.st_blksize) != 0) { + virReportOOMError(); + goto out; + } + + ret = storageWipeExtent(def, + fd, + 0, + def->allocation, + writebuf, + st.st_blksize, + &bytes_wiped); + } } out: + virCommandFree(cmd); VIR_FREE(writebuf); - VIR_FORCE_CLOSE(fd); - return ret; } @@ -1859,9 +1894,41 @@ storageVolumeWipe(virStorageVolPtr obj, virStorageDriverStatePtr driver = obj->conn->storagePrivateData; virStoragePoolObjPtr pool = NULL; virStorageVolDefPtr vol = NULL; + unsigned int alg = 0; + unsigned int supported_algs = 0; + unsigned int count = 0; int ret = -1; + supported_algs = VIR_STORAGE_VOL_WIPE_ALG_NNSA | + VIR_STORAGE_VOL_WIPE_ALG_DOD | + VIR_STORAGE_VOL_WIPE_ALG_BSI | + VIR_STORAGE_VOL_WIPE_ALG_GUTMANN | + VIR_STORAGE_VOL_WIPE_ALG_SCHNEIER | + VIR_STORAGE_VOL_WIPE_ALG_PFITZNER7 | + VIR_STORAGE_VOL_WIPE_ALG_PFITZNER33 | + VIR_STORAGE_VOL_WIPE_ALG_RANDOM; + +#ifdef SCRUB + virCheckFlags(supported_algs, -1); +#else virCheckFlags(0, -1); +#endif + + alg = flags & supported_algs; + /* As we support only one algorithm in @flags, + * check if somebody didn't pass two or more + */ + while (alg) { + count += alg & 1L; + alg >>= 1; + } + + if (count > 1) { + virStorageReportError(VIR_ERR_INVALID_ARG, "%s", + _("only one wiping algorithm " + "per call is allowed")); + return -1; + } storageDriverLock(driver); pool = virStoragePoolObjFindByName(&driver->pools, obj->pool); @@ -1895,7 +1962,7 @@ storageVolumeWipe(virStorageVolPtr obj, goto out; } - if (storageVolumeWipeInternal(vol) == -1) { + if (storageVolumeWipeInternal(vol, flags) == -1) { goto out; } diff --git a/tools/virsh.c b/tools/virsh.c index 0bc0519..3cb4109 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -10961,6 +10961,7 @@ static const vshCmdInfo info_vol_wipe[] = { static const vshCmdOptDef opts_vol_wipe[] = { {"vol", VSH_OT_DATA, VSH_OFLAG_REQ, N_("vol name, key or path")}, {"pool", VSH_OT_STRING, 0, N_("pool name or uuid")}, + {"algorithm", VSH_OT_STRING, 0, N_("perform selected wiping algorithm")}, {NULL, 0, 0, NULL} }; @@ -10968,8 +10969,10 @@ static bool cmdVolWipe(vshControl *ctl, const vshCmd *cmd) { virStorageVolPtr vol; - bool ret = true; + bool ret = false; const char *name; + const char *algorithm = NULL; + unsigned int flags = 0; if (!vshConnectionUsability(ctl, ctl->conn)) return false; @@ -10978,13 +10981,45 @@ cmdVolWipe(vshControl *ctl, const vshCmd *cmd) return false; } - if (virStorageVolWipe(vol, 0) == 0) { + if (vshCommandOptString(cmd, "algorithm", &algorithm) < 0) { + vshError(ctl, "%s", _("missing argument")); + goto out; + } + + if (algorithm) { + if (STREQ(algorithm, "nnsa")) + flags |= VIR_STORAGE_VOL_WIPE_ALG_NNSA; + else if (STREQ(algorithm, "dod")) + flags |= VIR_STORAGE_VOL_WIPE_ALG_DOD; + else if (STREQ(algorithm, "bsi")) + flags |= VIR_STORAGE_VOL_WIPE_ALG_BSI; + else if (STREQ(algorithm, "gutmann")) + flags |= VIR_STORAGE_VOL_WIPE_ALG_GUTMANN; + else if (STREQ(algorithm, "schneier")) + flags |= VIR_STORAGE_VOL_WIPE_ALG_SCHNEIER; + else if (STREQ(algorithm, "pfitzner7")) + flags |= VIR_STORAGE_VOL_WIPE_ALG_PFITZNER7; + else if (STREQ(algorithm, "pfitzner33")) + flags |= VIR_STORAGE_VOL_WIPE_ALG_PFITZNER33; + else if (STREQ(algorithm, "random")) + flags |= VIR_STORAGE_VOL_WIPE_ALG_RANDOM; + else { + vshError(ctl, _("Unsupported algorithm '%s'"), + algorithm); + goto out; + } + } + + if (virStorageVolWipe(vol, flags) == 0) { vshPrint(ctl, _("Vol %s wiped\n"), name); } else { vshError(ctl, _("Failed to wipe vol %s"), name); - ret = false; + goto out; } + ret = true; + +out: virStorageVolFree(vol); return ret; } diff --git a/tools/virsh.pod b/tools/virsh.pod index 138f886..d96e8c8 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1912,12 +1912,35 @@ I<vol-name-or-key-or-path> is the name or key or path of the volume to wipe. I<--offset> is the position in the storage volume at which to start reading the data. I<--length> is an upper bound of the amount of data to be downloaded. -=item B<vol-wipe> [I<--pool> I<pool-or-uuid>] I<vol-name-or-key-or-path> +=item B<vol-wipe> [I<--pool> I<pool-or-uuid>] [I<--algorithm> I<algorithm>] +I<vol-name-or-key-or-path> Wipe a volume, ensure data previously on the volume is not accessible to future reads. I<--pool> I<pool-or-uuid> is the name or UUID of the storage pool the volume is in. I<vol-name-or-key-or-path> is the name or key or path of the volume to wipe. +It is possible to choose different wiping algorithms instead of re-writing +volume with zeroes. This can be done via I<--algorithm> switch. + +B<Supported algorithms> + nnsa - 4-pass NNSA Policy Letter NAP-14.1-C (XVI-8) for + sanitizing removable and non-removable hard disks: + random(x2), 0x00, verify. + dod - 4-pass DoD 5220.22-M section 8-306 procedure for + sanitizing removeable and non-removeable rigid + disks: random, 0x00, 0xff, verify. + bsi - 9-pass method recommended by the German Center of + Security in Information Technologies + (http://www.bsi.bund.de): 0xff, 0xfe, 0xfd, 0xfb, + 0xf7, 0xef, 0xdf, 0xbf, 0x7f. + gutmann - The canonical 35-pass sequence described in + Gutmann's paper. + schneier - 7-pass method described by Bruce Schneier in + "Applied Cryptography" (1996): 0x00, 0xff, + random(x5). + pfitzner7 - Roy Pfitzner's 7-random-pass method: random(x7). + pfitzner33 - Roy Pfitzner's 33-random-pass method: random(x33). + random - 1-pass pattern: random(x1). =item B<vol-dumpxml> [I<--pool> I<pool-or-uuid>] I<vol-name-or-key-or-path> -- 1.7.3.4
On Tue, Jan 03, 2012 at 05:12:47PM +0100, Michal Privoznik wrote:
Currently, we support only filling a volume with zeroes on wiping. However, it is not enough as data might still be readable by experienced and equipped attacker. Many technical papers have been written, therefore we should support other wiping algorithms. --- Okay, this is not as complete as I'd like it. Wiping could take ages, therefore we *want* it to be asynchronous. But that would mean:
1) Create new API to answer: "Are we done yet?"
IMHO, we basically want to duplicate the async job system from virDomainPtr, for virStorageVolPtr. eg, virStorageVolAbortJob(virStorageVolPtr vol); virStorageVolGetJobInfo(virStorageVolPtr vol, virStorageVolJobInfoPtr info); this would be useful for several other storage vol related APIs. We might also want similar APIs against virStoragePoolPtr
2) Create event emitting system - like we have for domains
3) Combination of previous two
In fact, I've started writing events, but it turned out to be huge and I am not even in a half yet. I need to find a way of re-using event code we already have. But thats another day and another patch set.
Yes, the abilty to support async events for other objects like virStoragePoolPtr, virStorageVolPtr, virNetworkPtr, etc is a well overdue todo item. I think it would be acceptable todo option 1 as the first impl, and then add option 2 as a follow on patchset, since events are merely an optimization. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 03.01.2012 17:39, Daniel P. Berrange wrote:
On Tue, Jan 03, 2012 at 05:12:47PM +0100, Michal Privoznik wrote:
Currently, we support only filling a volume with zeroes on wiping. However, it is not enough as data might still be readable by experienced and equipped attacker. Many technical papers have been written, therefore we should support other wiping algorithms. --- Okay, this is not as complete as I'd like it. Wiping could take ages, therefore we *want* it to be asynchronous. But that would mean:
1) Create new API to answer: "Are we done yet?"
IMHO, we basically want to duplicate the async job system from virDomainPtr, for virStorageVolPtr.
eg, virStorageVolAbortJob(virStorageVolPtr vol); virStorageVolGetJobInfo(virStorageVolPtr vol, virStorageVolJobInfoPtr info);
this would be useful for several other storage vol related APIs. We might also want similar APIs against virStoragePoolPtr
2) Create event emitting system - like we have for domains
3) Combination of previous two
In fact, I've started writing events, but it turned out to be huge and I am not even in a half yet. I need to find a way of re-using event code we already have. But thats another day and another patch set.
Yes, the abilty to support async events for other objects like virStoragePoolPtr, virStorageVolPtr, virNetworkPtr, etc is a well overdue todo item.
I think it would be acceptable todo option 1 as the first impl, and then add option 2 as a follow on patchset, since events are merely an optimization.
Daniel
One thing I am wondering about is - can we separate these steps? I mean, take this part in and do the rest in follow up patches. Certainly, writing JobInfo for storage is rather huge code to be written and thus worth its own patch set. Moreover, this functionality can be desired by many products based on libvirt that want to be certified (e.g. PCI DSS) and thus need other wiping algorithms than just zeroing. From this POV is JobInfo just a "nice to have" feature. So if we get consensus on this, please review. Michal
On Mon, Jan 09, 2012 at 11:05:28AM +0100, Michal Privoznik wrote:
On 03.01.2012 17:39, Daniel P. Berrange wrote:
On Tue, Jan 03, 2012 at 05:12:47PM +0100, Michal Privoznik wrote:
Currently, we support only filling a volume with zeroes on wiping. However, it is not enough as data might still be readable by experienced and equipped attacker. Many technical papers have been written, therefore we should support other wiping algorithms. --- Okay, this is not as complete as I'd like it. Wiping could take ages, therefore we *want* it to be asynchronous. But that would mean:
1) Create new API to answer: "Are we done yet?"
IMHO, we basically want to duplicate the async job system from virDomainPtr, for virStorageVolPtr.
eg, virStorageVolAbortJob(virStorageVolPtr vol); virStorageVolGetJobInfo(virStorageVolPtr vol, virStorageVolJobInfoPtr info);
this would be useful for several other storage vol related APIs. We might also want similar APIs against virStoragePoolPtr
2) Create event emitting system - like we have for domains
3) Combination of previous two
In fact, I've started writing events, but it turned out to be huge and I am not even in a half yet. I need to find a way of re-using event code we already have. But thats another day and another patch set.
Yes, the abilty to support async events for other objects like virStoragePoolPtr, virStorageVolPtr, virNetworkPtr, etc is a well overdue todo item.
I think it would be acceptable todo option 1 as the first impl, and then add option 2 as a follow on patchset, since events are merely an optimization.
Daniel
One thing I am wondering about is - can we separate these steps? I mean, take this part in and do the rest in follow up patches. Certainly, writing JobInfo for storage is rather huge code to be written and thus worth its own patch set. Moreover, this functionality can be desired by many products based on libvirt that want to be certified (e.g. PCI DSS) and thus need other wiping algorithms than just zeroing. From this POV is JobInfo just a "nice to have" feature.
Sure, the storage wiping code is already slow, so there's no reason to reject your current patch on the basis of speed. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Tue, Jan 03, 2012 at 05:12:47PM +0100, Michal Privoznik wrote:
Currently, we support only filling a volume with zeroes on wiping. However, it is not enough as data might still be readable by experienced and equipped attacker. Many technical papers have been written, therefore we should support other wiping algorithms. --- Okay, this is not as complete as I'd like it. Wiping could take ages, therefore we *want* it to be asynchronous. But that would mean:
1) Create new API to answer: "Are we done yet?"
2) Create event emitting system - like we have for domains
3) Combination of previous two
In fact, I've started writing events, but it turned out to be huge and I am not even in a half yet. I need to find a way of re-using event code we already have. But thats another day and another patch set.
For those interested, take a look at Section 4.1.11 of DSS_PCI requirements at [1].
1: https://www.pcisecuritystandards.org/documents/Virtualization_InfoSupp_v2.pd...
configure.ac | 26 ++++++++++- include/libvirt/libvirt.h.in | 24 ++++++++++ src/libvirt.c | 6 ++- src/storage/storage_driver.c | 103 ++++++++++++++++++++++++++++++++++------- tools/virsh.c | 41 +++++++++++++++- tools/virsh.pod | 25 ++++++++++- 6 files changed, 201 insertions(+), 24 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index ad6fcce..caa9127 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2118,6 +2118,30 @@ typedef enum { VIR_STORAGE_VOL_DELETE_ZEROED = 1, /* Clear all data to zeros (slow) */ } virStorageVolDeleteFlags;
+typedef enum { + /* Keep this place for async flag. Currently, + * it is not implemented because of lack of + * events support within storage driver. + * VIR_STORAGE_VOL_WIPE_ASYNC = (1 << 0), + */
Just remove this reservation entirely - if we ever need it, there's no reason why we can't put it in as (1 << 9), or whatever other flag is next available.
+ VIR_STORAGE_VOL_WIPE_ALG_NNSA = (1 << 1), /* 4-pass NNSA Policy Letter + NAP-14.1-C (XVI-8) */ + VIR_STORAGE_VOL_WIPE_ALG_DOD = (1 << 2), /* 4-pass DoD 5220.22-M section + 8-306 procedure */ + VIR_STORAGE_VOL_WIPE_ALG_BSI = (1 << 3), /* 9-pass method recommended by the + German Center of Security in + Information Technologies */ + VIR_STORAGE_VOL_WIPE_ALG_GUTMANN = (1 << 4), /* The canonical 35-pass sequence */ + VIR_STORAGE_VOL_WIPE_ALG_SCHNEIER = (1 << 5), /* 7-pass method described by + Bruce Schneier in "Applied + Cryptography" (1996) */ + VIR_STORAGE_VOL_WIPE_ALG_PFITZNER7 = (1 << 6), /* 7-pass random */ + + VIR_STORAGE_VOL_WIPE_ALG_PFITZNER33 = (1 << 7), /* 33-pass random */ + + VIR_STORAGE_VOL_WIPE_ALG_RANDOM = (1 << 8), /* 1-pass random */ +} virStorageVolWipeFlags; +
Hmm, I always feel slightly bad, when we use bit flags to encode options that are to be treated as all mutually exclusive. ie, this scheme allows us to pass in multiple algorithms at once, but we only actually want 1 algorithm. Should we instead create a new API for this int virStorageVolWipePattern(virStorageVolPtr vol, unsigned int algorithm, unsigned int flags); And define a normal enum typedef enum { VIR_STORAGE_VOL_WIPE_ALG_ZEROS = 1, VIR_STORAGE_VOL_WIPE_ALG_NNSA = 2, } virStorageVolWipeAlgorithm;
typedef struct _virStorageVolInfo virStorageVolInfo;
struct _virStorageVolInfo { diff --git a/src/libvirt.c b/src/libvirt.c index feb3ca6..c3b3665 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -12557,7 +12557,11 @@ error: * @vol: pointer to storage volume * @flags: future flags, use 0 for now * - * Ensure data previously on a volume is not accessible to future reads + * Ensure data previously on a volume is not accessible to future reads. + * When specifying wipe algorithm only one algorithm flag is allowed. + * Therefore if one wants to wipe a volume with say NNSA and DOD algorithms, + * he/she needs two subsequent calls. First with _ALG_NNSA flag, second + * with _ALG_DOD.
eg, this comment would not be needed if we encoded the algorithm as a separate enum parameter.
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 8c2d6e1..f3a084a 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1801,14 +1801,16 @@ out:
static int -storageVolumeWipeInternal(virStorageVolDefPtr def) +storageVolumeWipeInternal(virStorageVolDefPtr def, + unsigned int flags) { int ret = -1, fd = -1; struct stat st; char *writebuf = NULL; size_t bytes_wiped = 0; + virCommandPtr cmd = NULL;
- VIR_DEBUG("Wiping volume with path '%s'", def->target.path); + VIR_DEBUG("Wiping volume with path '%s' flags %x", def->target.path, flags);
fd = open(def->target.path, O_RDWR); if (fd == -1) { @@ -1825,29 +1827,62 @@ storageVolumeWipeInternal(virStorageVolDefPtr def) goto out; }
- if (S_ISREG(st.st_mode) && st.st_blocks < (st.st_size / DEV_BSIZE)) { - ret = storageVolumeZeroSparseFile(def, st.st_size, fd); - } else { - - if (VIR_ALLOC_N(writebuf, st.st_blksize) != 0) { - virReportOOMError(); + if (flags) { + cmd = virCommandNew(SCRUB); + virCommandAddArg(cmd, "-f"); + if (flags &VIR_STORAGE_VOL_WIPE_ALG_NNSA) { + virCommandAddArgList(cmd, "-p", "nnsa", NULL); + } else if (flags & VIR_STORAGE_VOL_WIPE_ALG_DOD) { + virCommandAddArgList(cmd, "-p", "dod", NULL); + } else if (flags & VIR_STORAGE_VOL_WIPE_ALG_BSI) { + virCommandAddArgList(cmd, "-p", "bsi", NULL); + } else if (flags & VIR_STORAGE_VOL_WIPE_ALG_GUTMANN) { + virCommandAddArgList(cmd, "-p", "gutmann", NULL); + } else if (flags & VIR_STORAGE_VOL_WIPE_ALG_SCHNEIER) { + virCommandAddArgList(cmd, "-p", "schneier", NULL); + } else if (flags & VIR_STORAGE_VOL_WIPE_ALG_PFITZNER7) { + virCommandAddArgList(cmd, "-p", "pfitzner7", NULL); + } else if (flags & VIR_STORAGE_VOL_WIPE_ALG_PFITZNER33) { + virCommandAddArgList(cmd, "-p", "pfitzner33", NULL); + } else if (flags & VIR_STORAGE_VOL_WIPE_ALG_RANDOM) { + virCommandAddArgList(cmd, "-p", "random", NULL);
Then, we could just use a VIR_ENUM_DECL/IMPL lookup to avoid this big if/elseif' block. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Currently, we support only filling a volume with zeroes on wiping. However, it is not enough as data might still be readable by experienced and equipped attacker. Many technical papers have been written, therefore we should support other wiping algorithms. --- diff to v1: -Daniel's suggestions taken in (notably, moved to new API) configure.ac | 27 ++++++++++- include/libvirt/libvirt.h.in | 30 ++++++++++++ src/driver.h | 5 ++ src/libvirt.c | 49 +++++++++++++++++++ src/libvirt_public.syms | 5 ++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 9 +++- src/remote_protocol-structs | 6 ++ src/storage/storage_driver.c | 105 ++++++++++++++++++++++++++++++++++-------- tools/virsh.c | 37 +++++++++++++-- tools/virsh.pod | 26 ++++++++++- 11 files changed, 271 insertions(+), 29 deletions(-) diff --git a/configure.ac b/configure.ac index b12e9fb..19e2681 100644 --- a/configure.ac +++ b/configure.ac @@ -2496,8 +2496,31 @@ AM_CONDITIONAL([HAVE_LIBNL], [test "$have_libnl" = "yes"]) AC_SUBST([LIBNL_CFLAGS]) AC_SUBST([LIBNL_LIBS]) - - +dnl scrub program for different volume wiping algorithms + +AC_ARG_WITH([scrub], + AC_HELP_STRING([--with-scrub], [enable different volume wiping algorithms + @<:@default=check@:>@]), + [with_macvtap=${withval}], + [with_scrub=check]) + +if test "$with_scrub" != "no"; then + AC_PATH_PROG([SCRUB], [scrub]) + if test -z "$SCRUB" ; then + if test "$with_scrub" = "check"; then + with_scrub=no + else + AC_MSG_ERROR([You must install the 'scrub' binary to enable + different volume wiping algorithms]) + fi + else + with_scrub=yes + fi + if test "$with_scrub" = "yes"; then + AC_DEFINE_UNQUOTED([SCRUB], ["$SCRUB"], + [Location of the scrub program]) + fi +fi # Only COPYING.LIB is under version control, yet COPYING # is included as part of the distribution tarball. diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index ad6fcce..15ba928 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2118,6 +2118,33 @@ typedef enum { VIR_STORAGE_VOL_DELETE_ZEROED = 1, /* Clear all data to zeros (slow) */ } virStorageVolDeleteFlags; +typedef enum { + VIR_STORAGE_VOL_WIPE_ALG_ZERO = 0, /* 1-pass, all zeroes */ + VIR_STORAGE_VOL_WIPE_ALG_NNSA = 1, /* 4-pass NNSA Policy Letter + NAP-14.1-C (XVI-8) */ + VIR_STORAGE_VOL_WIPE_ALG_DOD = 2, /* 4-pass DoD 5220.22-M section + 8-306 procedure */ + VIR_STORAGE_VOL_WIPE_ALG_BSI = 3, /* 9-pass method recommended by the + German Center of Security in + Information Technologies */ + VIR_STORAGE_VOL_WIPE_ALG_GUTMANN = 4, /* The canonical 35-pass sequence */ + VIR_STORAGE_VOL_WIPE_ALG_SCHNEIER = 5, /* 7-pass method described by + Bruce Schneier in "Applied + Cryptography" (1996) */ + VIR_STORAGE_VOL_WIPE_ALG_PFITZNER7 = 6, /* 7-pass random */ + + VIR_STORAGE_VOL_WIPE_ALG_PFITZNER33 = 7, /* 33-pass random */ + + VIR_STORAGE_VOL_WIPE_ALG_RANDOM = 8, /* 1-pass random */ + + /* + * NB: this enum value will increase over time as new algorithms are + * added to the libvirt API. It reflects the last algorithm supported + * by this version of the libvirt API. + */ + VIR_STORAGE_VOL_WIPE_ALG_LAST +} virStorageVolWipeAlgorithm; + typedef struct _virStorageVolInfo virStorageVolInfo; struct _virStorageVolInfo { @@ -2255,6 +2282,9 @@ int virStorageVolDelete (virStorageVolPtr vol, unsigned int flags); int virStorageVolWipe (virStorageVolPtr vol, unsigned int flags); +int virStorageVolWipePattern (virStorageVolPtr vol, + unsigned int algorithm, + unsigned int flags); int virStorageVolRef (virStorageVolPtr vol); int virStorageVolFree (virStorageVolPtr vol); diff --git a/src/driver.h b/src/driver.h index ec4abf3..22026d4 100644 --- a/src/driver.h +++ b/src/driver.h @@ -1224,6 +1224,10 @@ typedef int typedef int (*virDrvStorageVolWipe) (virStorageVolPtr vol, unsigned int flags); +typedef int + (*virDrvStorageVolWipePattern) (virStorageVolPtr vol, + unsigned int algorithm, + unsigned int flags); typedef int (*virDrvStorageVolGetInfo) (virStorageVolPtr vol, @@ -1309,6 +1313,7 @@ struct _virStorageDriver { virDrvStorageVolUpload volUpload; virDrvStorageVolDelete volDelete; virDrvStorageVolWipe volWipe; + virDrvStorageVolWipePattern volWipePattern; virDrvStorageVolGetInfo volGetInfo; virDrvStorageVolGetXMLDesc volGetXMLDesc; virDrvStorageVolGetPath volGetPath; diff --git a/src/libvirt.c b/src/libvirt.c index c288f00..14e2da9 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -12600,6 +12600,55 @@ error: /** + * virStorageVolWipePattern: + * @vol: pointer to storage volume + * @algorithm: one of virStorageVolWipeAlgorithm + * @flags: future flags, use 0 for now + * + * Similar to virStorageVolWipe, but one can choose + * between different wiping algorithms. + * + * Returns 0 on success, or -1 on error. + */ +int +virStorageVolWipePattern(virStorageVolPtr vol, + unsigned int algorithm, + unsigned int flags) +{ + virConnectPtr conn; + VIR_DEBUG("vol=%p, algorithm=%d, flags=%x", vol, algorithm, flags); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_STORAGE_VOL(vol)) { + virLibStorageVolError(VIR_ERR_INVALID_STORAGE_VOL, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + conn = vol->conn; + if (conn->flags & VIR_CONNECT_RO) { + virLibStorageVolError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + + if (conn->storageDriver && conn->storageDriver->volWipePattern) { + int ret; + ret = conn->storageDriver->volWipePattern(vol, algorithm, flags); + if (ret < 0) { + goto error; + } + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(vol->conn); + return -1; +} + +/** * virStorageVolFree: * @vol: pointer to storage volume * diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 4ca7216..09dd17c 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -516,4 +516,9 @@ LIBVIRT_0.9.9 { virDomainSetNumaParameters; } LIBVIRT_0.9.8; +LIBVIRT_0.9.10 { + global: + virStorageVolWipePattern; +} LIBVIRT_0.9.9; + # .... define new API here using predicted next version number .... diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 7580477..7f89247 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -4821,6 +4821,7 @@ static virStorageDriver storage_driver = { .volUpload = remoteStorageVolUpload, /* 0.9.0 */ .volDelete = remoteStorageVolDelete, /* 0.4.1 */ .volWipe = remoteStorageVolWipe, /* 0.8.0 */ + .volWipePattern = remoteStorageVolWipePattern, /* 0.9.10 */ .volGetInfo = remoteStorageVolGetInfo, /* 0.4.1 */ .volGetXMLDesc = remoteStorageVolGetXMLDesc, /* 0.4.1 */ .volGetPath = remoteStorageVolGetPath, /* 0.4.1 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index ca739ff..7fffeb0 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -1642,6 +1642,12 @@ struct remote_storage_vol_wipe_args { unsigned int flags; }; +struct remote_storage_vol_wipe_pattern_args { + remote_nonnull_storage_vol vol; + unsigned int algorithm; + unsigned int flags; +}; + struct remote_storage_vol_get_xml_desc_args { remote_nonnull_storage_vol vol; unsigned int flags; @@ -2653,7 +2659,8 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_SET_NUMA_PARAMETERS = 254, /* autogen autogen */ REMOTE_PROC_DOMAIN_GET_NUMA_PARAMETERS = 255, /* skipgen skipgen */ REMOTE_PROC_DOMAIN_SET_INTERFACE_PARAMETERS = 256, /* autogen autogen */ - REMOTE_PROC_DOMAIN_GET_INTERFACE_PARAMETERS = 257 /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_GET_INTERFACE_PARAMETERS = 257, /* skipgen skipgen */ + REMOTE_PROC_STORAGE_VOL_WIPE_PATTERN = 258 /* autogen autogen */ /* * Notice how the entries are grouped in sets of 10 ? diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 2758315..7181def 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -1234,6 +1234,11 @@ struct remote_storage_vol_wipe_args { remote_nonnull_storage_vol vol; u_int flags; }; +struct remote_storage_vol_wipe_pattern_args { + remote_nonnull_storage_vol vol; + u_int algorithm; + u_int flags; +}; struct remote_storage_vol_get_xml_desc_args { remote_nonnull_storage_vol vol; u_int flags; @@ -2090,4 +2095,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_GET_NUMA_PARAMETERS = 255, REMOTE_PROC_DOMAIN_SET_INTERFACE_PARAMETERS = 256, REMOTE_PROC_DOMAIN_GET_INTERFACE_PARAMETERS = 257, + REMOTE_PROC_STORAGE_VOL_WIPE_PATTERN = 258, }; diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 8c2d6e1..bbaf22f 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1801,14 +1801,17 @@ out: static int -storageVolumeWipeInternal(virStorageVolDefPtr def) +storageVolumeWipeInternal(virStorageVolDefPtr def, + unsigned int algorithm) { int ret = -1, fd = -1; struct stat st; char *writebuf = NULL; size_t bytes_wiped = 0; + virCommandPtr cmd = NULL; - VIR_DEBUG("Wiping volume with path '%s'", def->target.path); + VIR_DEBUG("Wiping volume with path '%s' and algorithm %d", + def->target.path, algorithm); fd = open(def->target.path, O_RDWR); if (fd == -1) { @@ -1825,36 +1828,83 @@ storageVolumeWipeInternal(virStorageVolDefPtr def) goto out; } - if (S_ISREG(st.st_mode) && st.st_blocks < (st.st_size / DEV_BSIZE)) { - ret = storageVolumeZeroSparseFile(def, st.st_size, fd); - } else { + if (algorithm != VIR_STORAGE_VOL_WIPE_ALG_ZERO) { + const char *alg_char ATTRIBUTE_UNUSED = NULL; + switch (algorithm) { +#ifdef SCRUB + case VIR_STORAGE_VOL_WIPE_ALG_NNSA: + alg_char = "nnsa"; + break; + case VIR_STORAGE_VOL_WIPE_ALG_DOD: + alg_char = "dod"; + break; + case VIR_STORAGE_VOL_WIPE_ALG_BSI: + alg_char = "bsi"; + break; + case VIR_STORAGE_VOL_WIPE_ALG_GUTMANN: + alg_char = "gutmann"; + break; + case VIR_STORAGE_VOL_WIPE_ALG_SCHNEIER: + alg_char = "shneier"; + break; + case VIR_STORAGE_VOL_WIPE_ALG_PFITZNER7: + alg_char = "pfitzner7"; + break; + case VIR_STORAGE_VOL_WIPE_ALG_PFITZNER33: + alg_char = " pfitzner33"; + break; + case VIR_STORAGE_VOL_WIPE_ALG_RANDOM: + alg_char = "random"; + break; +#endif + default: + virStorageReportError(VIR_ERR_INVALID_ARG, + _("unsupported algorithm %d"), + algorithm); + } +#ifdef SCRUB + cmd = virCommandNew(SCRUB); + virCommandAddArgList(cmd, "-f", "-p", alg_char, + def->target.path, NULL); - if (VIR_ALLOC_N(writebuf, st.st_blksize) != 0) { - virReportOOMError(); + if (virCommandRun(cmd, NULL) < 0) goto out; - } - ret = storageWipeExtent(def, - fd, - 0, - def->allocation, - writebuf, - st.st_blksize, - &bytes_wiped); + ret = 0; +#endif + goto out; + } else { + if (S_ISREG(st.st_mode) && st.st_blocks < (st.st_size / DEV_BSIZE)) { + ret = storageVolumeZeroSparseFile(def, st.st_size, fd); + } else { + + if (VIR_ALLOC_N(writebuf, st.st_blksize) != 0) { + virReportOOMError(); + goto out; + } + + ret = storageWipeExtent(def, + fd, + 0, + def->allocation, + writebuf, + st.st_blksize, + &bytes_wiped); + } } out: + virCommandFree(cmd); VIR_FREE(writebuf); - VIR_FORCE_CLOSE(fd); - return ret; } static int -storageVolumeWipe(virStorageVolPtr obj, - unsigned int flags) +storageVolumeWipePattern(virStorageVolPtr obj, + unsigned int algorithm, + unsigned int flags) { virStorageDriverStatePtr driver = obj->conn->storagePrivateData; virStoragePoolObjPtr pool = NULL; @@ -1863,6 +1913,13 @@ storageVolumeWipe(virStorageVolPtr obj, virCheckFlags(0, -1); + if (algorithm >= VIR_STORAGE_VOL_WIPE_ALG_LAST) { + virStorageReportError(VIR_ERR_INVALID_ARG, + _("wiping algorithm %d not supported"), + algorithm); + return -1; + } + storageDriverLock(driver); pool = virStoragePoolObjFindByName(&driver->pools, obj->pool); storageDriverUnlock(driver); @@ -1895,7 +1952,7 @@ storageVolumeWipe(virStorageVolPtr obj, goto out; } - if (storageVolumeWipeInternal(vol) == -1) { + if (storageVolumeWipeInternal(vol, algorithm) == -1) { goto out; } @@ -1911,6 +1968,13 @@ out: } static int +storageVolumeWipe(virStorageVolPtr obj, + unsigned int flags) +{ + return storageVolumeWipePattern(obj, VIR_STORAGE_VOL_WIPE_ALG_ZERO, flags); +} + +static int storageVolumeDelete(virStorageVolPtr obj, unsigned int flags) { virStorageDriverStatePtr driver = obj->conn->storagePrivateData; @@ -2175,6 +2239,7 @@ static virStorageDriver storageDriver = { .volUpload = storageVolumeUpload, /* 0.9.0 */ .volDelete = storageVolumeDelete, /* 0.4.0 */ .volWipe = storageVolumeWipe, /* 0.8.0 */ + .volWipePattern = storageVolumeWipePattern, /* 0.9.10 */ .volGetInfo = storageVolumeGetInfo, /* 0.4.0 */ .volGetXMLDesc = storageVolumeGetXMLDesc, /* 0.4.0 */ .volGetPath = storageVolumeGetPath, /* 0.4.0 */ diff --git a/tools/virsh.c b/tools/virsh.c index e4b812e..fb9db8f 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -10961,15 +10961,24 @@ static const vshCmdInfo info_vol_wipe[] = { static const vshCmdOptDef opts_vol_wipe[] = { {"vol", VSH_OT_DATA, VSH_OFLAG_REQ, N_("vol name, key or path")}, {"pool", VSH_OT_STRING, 0, N_("pool name or uuid")}, + {"algorithm", VSH_OT_STRING, 0, N_("perform selected wiping algorithm")}, {NULL, 0, 0, NULL} }; +VIR_ENUM_DECL(virStorageVolWipeAlgorithm) +VIR_ENUM_IMPL(virStorageVolWipeAlgorithm, VIR_STORAGE_VOL_WIPE_ALG_LAST, + "zero", "nnsa", "dod", "bsi", "gutmann", "schneier", + "pfitzner7", "pfitzner33", "random"); + static bool cmdVolWipe(vshControl *ctl, const vshCmd *cmd) { virStorageVolPtr vol; - bool ret = true; + bool ret = false; const char *name; + const char *algorithm_str = NULL; + int algorithm = VIR_STORAGE_VOL_WIPE_ALG_ZERO; + int funcRet; if (!vshConnectionUsability(ctl, ctl->conn)) return false; @@ -10978,13 +10987,31 @@ cmdVolWipe(vshControl *ctl, const vshCmd *cmd) return false; } - if (virStorageVolWipe(vol, 0) == 0) { - vshPrint(ctl, _("Vol %s wiped\n"), name); - } else { + if (vshCommandOptString(cmd, "algorithm", &algorithm_str) < 0) { + vshError(ctl, "%s", _("missing argument")); + goto out; + } + + if (algorithm_str && + (algorithm = virStorageVolWipeAlgorithmTypeFromString(algorithm_str)) < 0) { + vshError(ctl, _("Unsupported algorithm '%s'"), algorithm_str); + goto out; + } + + if ((funcRet = virStorageVolWipePattern(vol, algorithm, 0)) < 0) { + if (last_error->code == VIR_ERR_NO_SUPPORT && + algorithm == VIR_STORAGE_VOL_WIPE_ALG_ZERO) + funcRet = virStorageVolWipe(vol, 0); + } + + if (funcRet < 0) { vshError(ctl, _("Failed to wipe vol %s"), name); - ret = false; + goto out; } + vshPrint(ctl, _("Vol %s wiped\n"), name); + ret = true; +out: virStorageVolFree(vol); return ret; } diff --git a/tools/virsh.pod b/tools/virsh.pod index 138f886..aeb863b 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1912,12 +1912,36 @@ I<vol-name-or-key-or-path> is the name or key or path of the volume to wipe. I<--offset> is the position in the storage volume at which to start reading the data. I<--length> is an upper bound of the amount of data to be downloaded. -=item B<vol-wipe> [I<--pool> I<pool-or-uuid>] I<vol-name-or-key-or-path> +=item B<vol-wipe> [I<--pool> I<pool-or-uuid>] [I<--algorithm> I<algorithm>] +I<vol-name-or-key-or-path> Wipe a volume, ensure data previously on the volume is not accessible to future reads. I<--pool> I<pool-or-uuid> is the name or UUID of the storage pool the volume is in. I<vol-name-or-key-or-path> is the name or key or path of the volume to wipe. +It is possible to choose different wiping algorithms instead of re-writing +volume with zeroes. This can be done via I<--algorithm> switch. + +B<Supported algorithms> + zero - 1-pass all zeroes + nnsa - 4-pass NNSA Policy Letter NAP-14.1-C (XVI-8) for + sanitizing removable and non-removable hard disks: + random x2, 0x00, verify. + dod - 4-pass DoD 5220.22-M section 8-306 procedure for + sanitizing removeable and non-removeable rigid + disks: random, 0x00, 0xff, verify. + bsi - 9-pass method recommended by the German Center of + Security in Information Technologies + (http://www.bsi.bund.de): 0xff, 0xfe, 0xfd, 0xfb, + 0xf7, 0xef, 0xdf, 0xbf, 0x7f. + gutmann - The canonical 35-pass sequence described in + Gutmann's paper. + schneier - 7-pass method described by Bruce Schneier in + "Applied Cryptography" (1996): 0x00, 0xff, + random x5. + pfitzner7 - Roy Pfitzner's 7-random-pass method: random x7. + pfitzner33 - Roy Pfitzner's 33-random-pass method: random x33. + random - 1-pass pattern: random. =item B<vol-dumpxml> [I<--pool> I<pool-or-uuid>] I<vol-name-or-key-or-path> -- 1.7.3.4
On Mon, Jan 09, 2012 at 05:56:19PM +0100, Michal Privoznik wrote:
Currently, we support only filling a volume with zeroes on wiping. However, it is not enough as data might still be readable by experienced and equipped attacker. Many technical papers have been written, therefore we should support other wiping algorithms. --- diff to v1: -Daniel's suggestions taken in (notably, moved to new API) configure.ac | 27 ++++++++++- include/libvirt/libvirt.h.in | 30 ++++++++++++ src/driver.h | 5 ++ src/libvirt.c | 49 +++++++++++++++++++ src/libvirt_public.syms | 5 ++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 9 +++- src/remote_protocol-structs | 6 ++ src/storage/storage_driver.c | 105 ++++++++++++++++++++++++++++++++++-------- tools/virsh.c | 37 +++++++++++++-- tools/virsh.pod | 26 ++++++++++- 11 files changed, 271 insertions(+), 29 deletions(-)
Sorry I missed this before - it is better to start a new top level thread, and include "v2" in the subject line to make it stand out, otherwise it gets threaded in with old archived mail.
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index ad6fcce..15ba928 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2118,6 +2118,33 @@ typedef enum { VIR_STORAGE_VOL_DELETE_ZEROED = 1, /* Clear all data to zeros (slow) */ } virStorageVolDeleteFlags;
+typedef enum { + VIR_STORAGE_VOL_WIPE_ALG_ZERO = 0, /* 1-pass, all zeroes */ q> + VIR_STORAGE_VOL_WIPE_ALG_NNSA = 1, /* 4-pass NNSA Policy Letter + NAP-14.1-C (XVI-8) */ + VIR_STORAGE_VOL_WIPE_ALG_DOD = 2, /* 4-pass DoD 5220.22-M section + 8-306 procedure */ + VIR_STORAGE_VOL_WIPE_ALG_BSI = 3, /* 9-pass method recommended by the + German Center of Security in + Information Technologies */ + VIR_STORAGE_VOL_WIPE_ALG_GUTMANN = 4, /* The canonical 35-pass sequence */ + VIR_STORAGE_VOL_WIPE_ALG_SCHNEIER = 5, /* 7-pass method described by + Bruce Schneier in "Applied + Cryptography" (1996) */ + VIR_STORAGE_VOL_WIPE_ALG_PFITZNER7 = 6, /* 7-pass random */ + + VIR_STORAGE_VOL_WIPE_ALG_PFITZNER33 = 7, /* 33-pass random */ + + VIR_STORAGE_VOL_WIPE_ALG_RANDOM = 8, /* 1-pass random */ +
With eric's recent change you can add #ifdef VIR_ENUM_SENTINELS
+ /* + * NB: this enum value will increase over time as new algorithms are + * added to the libvirt API. It reflects the last algorithm supported + * by this version of the libvirt API. + */ + VIR_STORAGE_VOL_WIPE_ALG_LAST
#endif
+} virStorageVolWipeAlgorithm; +
/** + * virStorageVolWipePattern: + * @vol: pointer to storage volume + * @algorithm: one of virStorageVolWipeAlgorithm + * @flags: future flags, use 0 for now + * + * Similar to virStorageVolWipe, but one can choose + * between different wiping algorithms. + * + * Returns 0 on success, or -1 on error. + */ +int +virStorageVolWipePattern(virStorageVolPtr vol, + unsigned int algorithm, + unsigned int flags) +{ + virConnectPtr conn; + VIR_DEBUG("vol=%p, algorithm=%d, flags=%x", vol, algorithm, flags);
%u for algorithm since it is unsigned now
+ + virResetLastError(); + + if (!VIR_IS_CONNECTED_STORAGE_VOL(vol)) { + virLibStorageVolError(VIR_ERR_INVALID_STORAGE_VOL, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + conn = vol->conn; + if (conn->flags & VIR_CONNECT_RO) { + virLibStorageVolError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + + if (conn->storageDriver && conn->storageDriver->volWipePattern) { + int ret; + ret = conn->storageDriver->volWipePattern(vol, algorithm, flags); + if (ret < 0) { + goto error; + } + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(vol->conn); + return -1; +} + +/** * virStorageVolFree: * @vol: pointer to storage volume * diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 4ca7216..09dd17c 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -516,4 +516,9 @@ LIBVIRT_0.9.9 { virDomainSetNumaParameters; } LIBVIRT_0.9.8;
+LIBVIRT_0.9.10 { + global: + virStorageVolWipePattern; +} LIBVIRT_0.9.9;
Trivial rebase to avoid conflict
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 8c2d6e1..bbaf22f 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1801,14 +1801,17 @@ out:
static int -storageVolumeWipeInternal(virStorageVolDefPtr def) +storageVolumeWipeInternal(virStorageVolDefPtr def, + unsigned int algorithm) { int ret = -1, fd = -1; struct stat st; char *writebuf = NULL; size_t bytes_wiped = 0; + virCommandPtr cmd = NULL;
- VIR_DEBUG("Wiping volume with path '%s'", def->target.path); + VIR_DEBUG("Wiping volume with path '%s' and algorithm %d", + def->target.path, algorithm);
%u here too ACK if those minor fixes are done + obvious rebase conflict resolution Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 26.01.2012 12:47, Daniel P. Berrange wrote:
On Mon, Jan 09, 2012 at 05:56:19PM +0100, Michal Privoznik wrote:
Currently, we support only filling a volume with zeroes on wiping. However, it is not enough as data might still be readable by experienced and equipped attacker. Many technical papers have been written, therefore we should support other wiping algorithms. --- diff to v1: -Daniel's suggestions taken in (notably, moved to new API) configure.ac | 27 ++++++++++- include/libvirt/libvirt.h.in | 30 ++++++++++++ src/driver.h | 5 ++ src/libvirt.c | 49 +++++++++++++++++++ src/libvirt_public.syms | 5 ++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 9 +++- src/remote_protocol-structs | 6 ++ src/storage/storage_driver.c | 105 ++++++++++++++++++++++++++++++++++-------- tools/virsh.c | 37 +++++++++++++-- tools/virsh.pod | 26 ++++++++++- 11 files changed, 271 insertions(+), 29 deletions(-)
Sorry I missed this before - it is better to start a new top level thread, and include "v2" in the subject line to make it stand out, otherwise it gets threaded in with old archived mail.
Yeah, I've noticed after I sent the e-mail. Sorry for that.
ACK if those minor fixes are done + obvious rebase conflict resolution
Fixed, rebased and pushed. Thanks. Michal
participants (2)
-
Daniel P. Berrange -
Michal Privoznik