[libvirt] [PATCH v2 11/15] storage: Add support to create a luks volume

[John Ferlan]

On 06/24/2016 10:48 AM, Peter Krempa wrote:
> On Thu, Jun 23, 2016 at 13:29:07 -0400, John Ferlan wrote:
>> Partially resolves:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1301021
>>
>> If the volume xml was looking to create a luks volume take the necessary
>> steps in order to make that happen.
>>
>> The processing will be:
>>  1. create a temporary file in the storage driver state dir path
>>    1a. the file name will include the pool name, the volume name, secret,
>>        and XXXXXX for usage in the mkostemp call.
>>    1b. mkostemp the file, initially setting mode to 0600 with current
>>        effective uid:gid as owner
>>    1c. fetch the secret into a buffer and write that into the file
>>    1d. change file protections to 0400
>>
>>  2. create a secret object
>>    2a. use an alias combinding the volume name and "_luks0"
>>    2b. add the file to the object
>>
>>  3. create/add luks options to the commandline
>>    3a. at the very least a "key-secret" using the secret object alias
>>    3b. if found in the XML the various "cipher" and "ivgen" options
>>
>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>> ---
>>  src/libvirt_private.syms       |   1 +
>>  src/storage/storage_backend.c  | 260 ++++++++++++++++++++++++++++++++++++++---
>>  src/storage/storage_backend.h  |   3 +-
>>  src/util/virqemu.c             |  23 ++++
>>  src/util/virqemu.h             |   6 +
>>  tests/storagevolxml2argvtest.c |   3 +-
>>  6 files changed, 275 insertions(+), 21 deletions(-)
>>
> 
> [...]
> 
>> @@ -1240,6 +1366,84 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn,
>>      return cmd;
>>  }
>>  
>> +
>> +static char *
>> +virStorageBackendCreateQemuImgSecretPath(virConnectPtr conn,
>> +                                         virStoragePoolObjPtr pool,
>> +                                         virStorageVolDefPtr vol)
>> +{
>> +    virStorageEncryptionPtr enc = vol->target.encryption;
>> +    char *secretPath = NULL;
>> +    int fd = -1;
>> +    uint8_t *secret = NULL;
>> +    size_t secretlen = 0;
>> +
>> +    if (!enc) {
>> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +                       _("missing encryption description"));
>> +        return NULL;
>> +    }
>> +
>> +    if (!conn || !conn->secretDriver ||
>> +        !conn->secretDriver->secretLookupByUUID ||
>> +        !conn->secretDriver->secretLookupByUsage ||
>> +        !conn->secretDriver->secretGetValue) {
>> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +                       _("unable to look up encryption secret"));
>> +        return NULL;
>> +    }
>> +
>> +    /* Since we don't have a file, just go to cleanup using NULL secretPath */
>> +    if (!(secretPath = virStoragePoolObjBuildTempFilePath(pool, vol)))
>> +        goto cleanup;
>> +
>> +    if ((fd = mkostemp(secretPath, O_CLOEXEC)) < 0) {
>> +        virReportSystemError(errno, "%s",
>> +                             _("failed to open luks secret file for write"));
>> +        goto error;
>> +    }
>> +
>> +    if (virSecretGetSecretString(conn, &enc->secrets[0]->seclookupdef,
>> +                                 VIR_SECRET_USAGE_TYPE_PASSPHRASE,
>> +                                 &secret, &secretlen) < 0)
>> +        goto error;
>> +
>> +    if (safewrite(fd, secret, secretlen) < 0) {
>> +        virReportSystemError(errno, "%s",
>> +                             _("failed to write luks secret file"));
>> +        goto error;
>> +    }
>> +    VIR_FORCE_CLOSE(fd);
>> +
>> +    if ((vol->target.perms->uid != (uid_t) -1) &&
>> +        (vol->target.perms->gid != (gid_t) -1)) {
>> +        if (chown(secretPath, vol->target.perms->uid,
> 
> [1]
> 
>> +                  vol->target.perms->gid) < 0) {
>> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +                           _("failed to chown luks secret file"));
>> +            goto error;
>> +        }
>> +    }
>> +
>> +    if (chmod(secretPath, 0400) < 0) {
> 
> I still don't understand this step. Owner [1] of the file is able to
> add the write permission back anyways.
> 
>> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +                       _("failed to chown luks secret file"));
>> +        goto error;
>> +    }
>> +
>> + cleanup:
>> +    VIR_DISPOSE_N(secret, secretlen);
>> +    VIR_FORCE_CLOSE(fd);
>> +
>> +    return secretPath;
>> +
>> + error:
>> +    unlink(secretPath);
>> +    VIR_FREE(secretPath);
>> +    goto cleanup;
>> +}
>> +
>> +
>>  int
>>  virStorageBackendCreateQemuImg(virConnectPtr conn,
>>                                 virStoragePoolObjPtr pool,
> 
> ACK with either a very good explanation or the chmod gone.
> 

chmod is gone.

Again, the set protections is a reaction to a comment Dan made in an
earlier review:

http://www.redhat.com/archives/libvir-list/2016-June/msg00021.html


John