[libvirt] [PATCH 0/2] A couple of volume encryption patches
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
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@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
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@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
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
participants (2)
-
John Ferlan -
Michal Privoznik