12:16 a.m.
I'm assume there will be a couple of bz's on these...
Patch 1 fixes a problem where the vol-dumpxml would not list the <secret>
element from a volume in a pool after the initial creation and a pool
refresh (or libvirtd restart). This issue was missed due to rewriting
the LUKS support to not use it's own "new" secret type (key), but rather
use the existing "volume" secret type.
Patch 2 fixes a problem where QCOW2 (or QCOW1) encrypted volume would
lose the <encryption> and <secret> after a volume refresh. Details are
in the patch. The issue is rooted in proper detection of the volume type
during the virStorageFileGetMetadataFromBuf call.
John Ferlan (2):
storage: Need to refresh secret for luks volume after volume refresh
storage: Need to properly read the crypt offset value
src/storage/storage_backend_fs.c | 16 ++++++++++------
src/util/virstoragefile.c | 7 +++++--
2 files changed, 15 insertions(+), 8 deletions(-)
--
2.7.4
12:16 a.m.
New subject: [libvirt] [PATCH 1/2] storage: Need to refresh secret for luks volume after volume refresh
A LUKS volume uses the volume secret type just like the QCOW2 secret, so
adjust the loading of the default secrets to handle any volume that the
virStorageFileGetMetadataFromBuf code has deemed to be an encrypted volume
to search for the volume's secret. This lookup is done by volume usage
where the usage is expected to be the path to volume.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/storage/storage_backend_fs.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index ac6abbb..6c8bae2 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -1270,8 +1270,8 @@ virStorageBackendFileSystemVolDelete(virConnectPtr conn
ATTRIBUTE_UNUSED,
* @conn: Connection pointer to fetch secret
* @vol: volume being refreshed
*
- * If the volume had a QCOW secret generated, we need to regenerate the
- * secret
+ * If the volume had a secret generated, we need to regenerate the
+ * encryption secret information
*
* Returns 0 if no secret or secret setup was successful,
* -1 on failures w/ error message set
@@ -1283,12 +1283,16 @@ virStorageBackendFileSystemLoadDefaultSecrets(virConnectPtr conn,
virSecretPtr sec;
virStorageEncryptionSecretPtr encsec = NULL;
- /* Only necessary for qcow format */
- if (!vol->target.encryption ||
- vol->target.encryption->format != VIR_STORAGE_ENCRYPTION_FORMAT_QCOW ||
- vol->target.encryption->nsecrets != 0)
+ if (!vol->target.encryption || vol->target.encryption->nsecrets != 0)
return 0;
+ /* The encryption secret for qcow2 and luks volumes use the path
+ * to the volume, so look for a secret with the path. If not found,
+ * then we cannot generate the secret after a refresh (or restart).
+ * This may be the case if someone didn't follow instructions and created
+ * a usage string that although matched with the secret usage string,
+ * didn't contain the path to the volume. We won't error in that case,
+ * but we also cannot find the secret. */
if (!(sec = virSecretLookupByUsage(conn, VIR_SECRET_USAGE_TYPE_VOLUME,
vol->target.path)))
return 0;
--
2.7.4
12:16 a.m.
New subject: [libvirt] [PATCH 2/2] storage: Need to properly read the crypt offset value
Commit id 'a48c7141' altered how to determine if a volume was encrypted
by adding a peek at an offset into the file at a specific buffer location.
Unfortunately, all that was compared was the first "char" of the buffer
against the expect "int" value.
Restore the virReadBufInt32BE to get the complete field in order to
compare against the expected value from the qcow2EncryptionInfo or
qcow1EncryptionInfo "modeValue" field.
This restores the capability to create a volume with encryption, then
refresh the pool, and still find the encryption for the volume.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/util/virstoragefile.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 41827f0..272db67 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -888,7 +888,7 @@ virStorageFileHasEncryptionFormat(const struct FileEncryptionInfo
*info,
size_t len)
{
if (!info->magic && info->modeOffset == -1)
- return 0; /* Shouldn't happen - expect at least one */
+ return false; /* Shouldn't happen - expect at least one */
if (info->magic) {
if (!virStorageFileMatchesMagic(info->magicOffset,
@@ -906,10 +906,13 @@ virStorageFileHasEncryptionFormat(const struct FileEncryptionInfo
*info,
return true;
} else if (info->modeOffset != -1) {
+ int crypt_format;
+
if (info->modeOffset >= len)
return false;
- if (buf[info->modeOffset] != info->modeValue)
+ crypt_format = virReadBufInt32BE(buf + info->modeOffset);
+ if (crypt_format != info->modeValue)
return false;
return true;
--
2.7.4
3 p.m.
On 06.09.2016 23:16, John Ferlan wrote:
I'm assume there will be a couple of bz's on these...
Patch 1 fixes a problem where the vol-dumpxml would not list the <secret>
element from a volume in a pool after the initial creation and a pool
refresh (or libvirtd restart). This issue was missed due to rewriting
the LUKS support to not use it's own "new" secret type (key), but rather
use the existing "volume" secret type.
Patch 2 fixes a problem where QCOW2 (or QCOW1) encrypted volume would
lose the <encryption> and <secret> after a volume refresh. Details are
in the patch. The issue is rooted in proper detection of the volume type
during the virStorageFileGetMetadataFromBuf call.
John Ferlan (2):
storage: Need to refresh secret for luks volume after volume refresh
storage: Need to properly read the crypt offset value
src/storage/storage_backend_fs.c | 16 ++++++++++------
src/util/virstoragefile.c | 7 +++++--
2 files changed, 15 insertions(+), 8 deletions(-)
ACK to both.
Michal
3068
days inactive
3074
days old
3 comments
2 participants
participants (2)
-
John Ferlan -
Michal Privoznik