diff mbox

[v2,07/17] crypto: implement the LUKS block encryption format

Message ID 1453311539-1193-8-git-send-email-berrange@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel P. Berrangé Jan. 20, 2016, 5:38 p.m. UTC
Provide a block encryption implementation that follows the
LUKS/dm-crypt specification.

This supports all combinations of hash, cipher algorithm,
cipher mode and iv generator that are implemented by the
current crypto layer.

The notable missing feature is support for the 'xts'
cipher mode, which is commonly used for disk encryption
instead of 'cbc'. This is because it is not provided by
either nettle or libgcrypt. A suitable implementation
will be identified & integrated later.

There is support for opening existing volumes formatted
by dm-crypt, and for formatting new volumes. In the latter
case it will only use key slot 0.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 crypto/Makefile.objs      |    1 +
 crypto/block-luks.c       | 1105 +++++++++++++++++++++++++++++++++++++++++++++
 crypto/block-luks.h       |   28 ++
 crypto/block.c            |    2 +
 qapi/crypto.json          |   43 +-
 tests/test-crypto-block.c |  104 +++++
 6 files changed, 1280 insertions(+), 3 deletions(-)
 create mode 100644 crypto/block-luks.c
 create mode 100644 crypto/block-luks.h

Comments

Eric Blake Feb. 5, 2016, 5:38 p.m. UTC | #1
On 01/20/2016 10:38 AM, Daniel P. Berrange wrote:
> Provide a block encryption implementation that follows the
> LUKS/dm-crypt specification.
> 
> This supports all combinations of hash, cipher algorithm,
> cipher mode and iv generator that are implemented by the
> current crypto layer.
> 
> The notable missing feature is support for the 'xts'
> cipher mode, which is commonly used for disk encryption
> instead of 'cbc'. This is because it is not provided by
> either nettle or libgcrypt. A suitable implementation
> will be identified & integrated later.
> 
> There is support for opening existing volumes formatted
> by dm-crypt, and for formatting new volumes. In the latter
> case it will only use key slot 0.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---

> +++ b/crypto/block-luks.c
> @@ -0,0 +1,1105 @@
> +/*
> + * QEMU Crypto block device encryption LUKS format
> + *

> +
> +/*
> + * Reference for format is
> + *
> + * docs/on-disk-format.pdf
> + *
> + * In 'cryptsetup' package source code
> + *

Worth calling out the document version number? I reviewed based on
version 1.2.1, dated Oct 2011.

> + */
> +
> +typedef struct QCryptoBlockLUKS QCryptoBlockLUKS;
> +typedef struct QCryptoBlockLUKSHeader QCryptoBlockLUKSHeader;
> +typedef struct QCryptoBlockLUKSKeySlot QCryptoBlockLUKSKeySlot;
> +
> +#define QCRYPTO_BLOCK_LUKS_VERSION 1
> +
> +#define QCRYPTO_BLOCK_LUKS_MAGIC_LEN 6
> +#define QCRYPTO_BLOCK_LUKS_CIPHER_NAME_LEN 32
> +#define QCRYPTO_BLOCK_LUKS_CIPHER_MODE_LEN 32
> +#define QCRYPTO_BLOCK_LUKS_HASH_SPEC_LEN 32
> +#define QCRYPTO_BLOCK_LUKS_DIGEST_LEN 20
> +#define QCRYPTO_BLOCK_LUKS_SALT_LEN 32
> +#define QCRYPTO_BLOCK_LUKS_UUID_LEN 40

Matches the spec, but seems awfully wasteful.  A binary UUID is 16
bytes; the standard ASCII representation is only 36 bytes.  But the spec
calls it out as char[] (requiring a 37th byte for a NUL terminator, then
rounding it up for aligned field access).

> +#define QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS 8
> +#define QCRYPTO_BLOCK_LUKS_STRIPES 4000
> +#define QCRYPTO_BLOCK_LUKS_MIN_SLOT_KEY_ITERS 1000
> +#define QCRYPTO_BLOCK_LUKS_MIN_MASTER_KEY_ITERS 1000
> +#define QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET 4096
> +
> +#define QCRYPTO_BLOCK_LUKS_KEY_SLOT_DISABLED 0x0000DEAD
> +#define QCRYPTO_BLOCK_LUKS_KEY_SLOT_ENABLED 0x00AC71F3

Cute l33t spelling choice by the upstream folks :)


> +
> +static const QCryptoBlockLUKSCipherSizeMap
> +qcrypto_block_luks_cipher_size_map_aes[] = {
> +    { 16, QCRYPTO_CIPHER_ALG_AES_128 },
> +    { 24, QCRYPTO_CIPHER_ALG_AES_192 },
> +    { 32, QCRYPTO_CIPHER_ALG_AES_256 },
> +    { 0, 0 },
> +};
> +
> +static const QCryptoBlockLUKSCipherNameMap
> +qcrypto_block_luks_cipher_name_map[] = {
> +    { "aes", qcrypto_block_luks_cipher_size_map_aes, },

Inconsistent on the use of trailing , within the inner {} between these
two arrays.

> +};
> +
> +
> +struct QCryptoBlockLUKSKeySlot {
> +    /* state of keyslot, enabled/disable */
> +    uint32_t active;

May be worth a comment that this struct is written to disk in
big-endian, but used in native-endian for most operations for
convenience; to make sure future developers remember to put byteswaps in
the correct locations.  But it looks like you handled endianness correctly.

> +    /* iterations for PBKDF2 */
> +    uint32_t iterations;
> +    /* salt for PBKDF2 */
> +    uint8_t salt[QCRYPTO_BLOCK_LUKS_SALT_LEN];
> +    /* start sector of key material */
> +    uint32_t key_offset;
> +    /* number of anti-forensic stripes */
> +    uint32_t stripes;
> +} QEMU_PACKED;

Packed makes sense since we are writing it straight to disk; although it
looks like natural alignment prevails and this did not need to be
packed.  Do we want some sort of BUG_ON() that we have the right size
struct?

> +
> +struct QCryptoBlockLUKSHeader {
> +    /* 'L', 'U', 'K', 'S', '0xBA', '0xBE' */
> +    char magic[QCRYPTO_BLOCK_LUKS_MAGIC_LEN];
> +
> +    /* LUKS version, currently 1 */
> +    uint16_t version;
> +
> +    /* cipher name specification (aes, etc */

Missing )

> +    char cipher_name[QCRYPTO_BLOCK_LUKS_CIPHER_NAME_LEN];
> +
> +    /* cipher mode specification () */

text in the ()?

> +    char cipher_mode[QCRYPTO_BLOCK_LUKS_CIPHER_MODE_LEN];
> +
> +    /* hash specication () */

s/specication/specification/, text in the ()?

> +    char hash_spec[QCRYPTO_BLOCK_LUKS_HASH_SPEC_LEN];
> +
> +    /* start offset of the volume data (in sectors) */
> +    uint32_t payload_offset;

Should we say "in 512-byte sectors"?  I could not see anything in the
LUKS specification that said whether SECTOR_SIZE can be larger than 512
- if you have a bare metal 4k sector disk, does a payload_offset of 2
mean 1024 or 8192 bytes into the disk?  Eww. Sounds like we should
submit a bug against the LUKS spec to have it clarified.

> +
> +    /* Number of key bytes */
> +    uint32_t key_bytes;
> +
> +    /* master key checksum after PBKDF2 */
> +    uint8_t master_key_digest[QCRYPTO_BLOCK_LUKS_DIGEST_LEN];
> +
> +    /* salt for master key PBKDF2 */
> +    uint8_t master_key_salt[QCRYPTO_BLOCK_LUKS_SALT_LEN];
> +
> +    /* iterations for master key PBKDF2 */
> +    uint32_t master_key_iterations;
> +
> +    /* UUID of the partition */

Should we say "in standard ASCII representation"?

> +    uint8_t uuid[QCRYPTO_BLOCK_LUKS_UUID_LEN];
> +
> +    /* key slots */
> +    QCryptoBlockLUKSKeySlot key_slots[QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS];
> +} QEMU_PACKED;

Again, alignment should mean that we could get away without packing; but
the packing doesn't hurt; and a BUG_ON may be nice to ensure size.


> +}
> +
> +/* XXX doesn't QEMU already have a string -> enum val convertor ? */
> +static int qcrypto_block_luks_name_lookup(const char *name,

Yes, spelled qapi_enum_parse().  Works if the enum was defined in a
.json file.


> +
> +    error_setg(errp, "%s %s not supported", type, name);
> +    return 0;
> +}
> +
> +#define qcrypto_block_luks_cipher_mode_lookup(name, errp)               \
> +    qcrypto_block_luks_name_lookup(name,                                \
> +                                   QCryptoCipherMode_lookup,            \
> +                                   QCRYPTO_CIPHER_MODE__MAX,            \
> +                                   "Cipher mode",                       \
> +                                   errp)
> +
> +#define qcrypto_block_luks_hash_name_lookup(name, errp)                 \
> +    qcrypto_block_luks_name_lookup(name,                                \
> +                                   QCryptoHashAlgorithm_lookup,         \
> +                                   QCRYPTO_HASH_ALG__MAX,               \
> +                                   "Hash algorithm",                    \
> +                                   errp)

That said, your macro wrappers tweak the error message, so that it is
perhaps more legible than the standard failure mode of
qapi_enum_parse().  So up to you if you want to keep the wrappers.

> +/*
> + * Given a key slot, and user password, this will attempt to unlock
> + * the master encryption key from the key slot
> + */
> +static int
> +qcrypto_block_luks_load_key(QCryptoBlock *block,
> +                            QCryptoBlockLUKSKeySlot *slot,
> +                            const char *password,
> +                            QCryptoCipherAlgorithm cipheralg,
> +                            QCryptoCipherMode ciphermode,
> +                            QCryptoHashAlgorithm hash,
> +                            QCryptoIVGenAlgorithm ivalg,
> +                            QCryptoHashAlgorithm ivhash,
> +                            uint8_t *masterkey,
> +                            size_t masterkeylen,
> +                            QCryptoBlockReadFunc readfunc,
> +                            void *opaque,
> +                            Error **errp)
> +{
> +    QCryptoBlockLUKS *luks = block->opaque;
> +    uint8_t *splitkey;
> +    size_t splitkeylen;
> +    uint8_t *possiblekey;
> +    int ret = -1;
> +    ssize_t rv;
> +    QCryptoCipher *cipher = NULL;
> +    uint8_t keydigest[QCRYPTO_BLOCK_LUKS_DIGEST_LEN];
> +    QCryptoIVGen *ivgen = NULL;
> +    size_t niv;
> +
> +    if (slot->active != QCRYPTO_BLOCK_LUKS_KEY_SLOT_ENABLED) {
> +        return 0;
> +    }

Would be worth documenting the convention of '0' (no key found, but no
errp set), '1' (successful key), and negative (errp set).

> +
> +    splitkeylen = masterkeylen * slot->stripes;

Should we do any validation that masterkeylen was in bounds and this
doesn't overflow?  I don't want a malicious byte stream masquerading as
a LUKS header to cause us to misbehave.

For that matter, the spec states that luks->payload_offset is a
write-once calculation at creation time, and that it could be recomputed
from key layout.  Should we validate that the offset listed makes
algorithmic sense with what we know about header and key data sizes for
the parameters listed in the header?

Side note: I'm a bit surprised the upstream LUKS spec does not include
any checksum over the header contents - it does a great job at ensuring
the master key is only going to be found by someone knowing a correct
password and that a single byte error in a key material section
invalidates that entire slot's usefulness, but it is not as careful on
what happens with certain single-byte errors in the header.

> +    splitkey = g_new0(uint8_t, splitkeylen);
> +    possiblekey = g_new0(uint8_t, masterkeylen);
> +
> +    /*
> +     * The user password is used to generate a (possible)
> +     * decryption key. This may or may not successfully
> +     * decrypt the master key - we just blindly assume
> +     * the key is correct and validate the results of
> +     * decryption later.
> +     */
> +    if (qcrypto_pbkdf2(hash,
> +                       (const uint8_t *)password, strlen(password),
> +                       slot->salt, QCRYPTO_BLOCK_LUKS_SALT_LEN,
> +                       slot->iterations,
> +                       possiblekey, masterkeylen,
> +                       errp) < 0) {
> +        goto cleanup;
> +    }
> +
> +    /*
> +     * We need to read the master key material from the
> +     * LUKS key material header. What we're reading is
> +     * not the raw master key, but rather the data after
> +     * it has been passed through AFSplit and the result
> +     * then encrypted.
> +     */
> +    rv = readfunc(block,
> +                  slot->key_offset * 512ll,

I tend to write LL rather than ll, to make it obvious I didn't typo '1'.
 Looks like we're hard-coding our sector size (not the first time); but
maybe a constant instead of a magic number will make it easier for the
poor guy down the road that tries to switch to 4k sectors.


...
> +
> +    /*
> +     * Now we're decrypted the split master key, join

s/we're/that we've/

> +     * it back together to get the actual master key.
> +     */
> +    if (qcrypto_afsplit_decode(hash,

...
> +
> +    if (memcmp(keydigest, luks->header.master_key_digest,
> +               QCRYPTO_BLOCK_LUKS_DIGEST_LEN) == 0) {
> +        /* Success, we got the right master key */
> +        ret = 1;
> +        goto cleanup;
> +    }
> +
> +    /* Fail, user's password was not valid for this key slot,
> +     * tell caller to try another slot */
> +    ret = 0;
> +

Matches the spec.  Yay!

Side observation: The LUKS spec doesn't say anything about parallelizing
the lookup loop across all 8 key slots, nor any randomization on which
slot should be attempted first.  The whole point of PBKDF2 iteration
count is to burn CPU time so that an attacker can't brute force things
easily.  That means it is VERY easy to tell by timing how many active
slots were attempted before a key was found, if slots are tried serially
and we immediately break the loop on the first successful decryption.
Is there any information leak caused by the timing observations when
serially searching among 8 active slots, when the master key is only
found in slot 7 vs. slot 0?  But I don't see it as your problem to
solve, so much as something to ask the upstream LUKS design team.

> +
> +
> +static int
> +qcrypto_block_luks_open(QCryptoBlock *block,
> +                        QCryptoBlockOpenOptions *options,
> +                        QCryptoBlockReadFunc readfunc,
> +                        void *opaque,
> +                        unsigned int flags,
> +                        Error **errp)
> +{

> +    /*
> +     * The cipher_mode header contains a string that we have
> +     * to further parse of the format

s/parse/parse,/

> +     *
> +     *    <cipher-mode>-<iv-generator>[:<iv-hash>]
> +     *
> +     * eg  cbc-essiv:sha256, cbc-plain64
> +     */
> +    ivgen_name = strchr(luks->header.cipher_mode, '-');
> +    if (!ivgen_name) {
> +        ret = -EINVAL;
> +        error_setg(errp, "Unexpected cipher mode string format %s",
> +                   luks->header.cipher_mode);
> +        goto fail;
> +    }
> +    *ivgen_name = '\0';
> +    ivgen_name++;

We're modifying luks->header in place; we'd better not write it back out
to disk in the modified form.  I guess you are okay for now - your code
only reads existing LUKS disks, and can only create a single key in slot
0 on conversion (that is, I don't see code for key management of an
existing image).  Probably things we should add later, though, at which
point we'll need to be careful that we don't overwrite too much in the
header.

> +
> +    block->payload_offset = luks->header.payload_offset;

Earlier, I argued that block->payload_offset should be in bytes.  You
are constrained that luks->header.payload_offset is in sectors, but we
need not propagate that craziness any higher.

> +
> +static int
> +qcrypto_block_luks_create(QCryptoBlock *block,
> +                          QCryptoBlockCreateOptions *options,
> +                          QCryptoBlockInitFunc initfunc,
> +                          QCryptoBlockWriteFunc writefunc,
> +                          void *opaque,
> +                          Error **errp)
> +{

> +
> +    memcpy(&luks_opts, options->u.luks, sizeof(luks_opts));
> +    if (!luks_opts.has_cipher_alg) {
> +        luks_opts.cipher_alg = QCRYPTO_CIPHER_ALG_AES_256;
> +    }
> +    if (!luks_opts.has_cipher_mode) {
> +        /* XXX switch to XTS */
> +        luks_opts.cipher_mode = QCRYPTO_CIPHER_MODE_CBC;
> +    }

Yeah, my (quick) reading made it seem like XTS is a slightly more secure
default than CBC.  But as you said, XTS implementation will come later.


> +    luks = g_new0(QCryptoBlockLUKS, 1);
> +    block->opaque = luks;
> +
> +    memcpy(luks->header.magic, qcrypto_block_luks_magic,
> +           QCRYPTO_BLOCK_LUKS_MAGIC_LEN);
> +
> +    luks->header.version = QCRYPTO_BLOCK_LUKS_VERSION;

Maybe a comment here that endianness will be addressed later, as a lot
of code appears between here and the actual write-to-disk.

> +    uuid_generate(uuid);
> +    uuid_unparse(uuid, (char *)luks->header.uuid);
> +
> +    switch (luks_opts.cipher_alg) {
> +    case QCRYPTO_CIPHER_ALG_AES_128:
> +    case QCRYPTO_CIPHER_ALG_AES_192:
> +    case QCRYPTO_CIPHER_ALG_AES_256:
> +        cipher_alg = "aes";
> +        break;
> +    default:
> +        error_setg(errp, "Cipher algorithm is not supported");
> +        goto error;

That is, we aren't supporting "twofish", "serpent", "cast5", or "cast6".
 Fine for now.

> +    }
> +
> +    cipher_mode = QCryptoCipherMode_lookup[luks_opts.cipher_mode];
> +    ivgen_alg = QCryptoIVGenAlgorithm_lookup[luks_opts.ivgen_alg];
> +    if (luks_opts.has_ivgen_hash_alg) {
> +        ivgen_hash_alg = QCryptoHashAlgorithm_lookup[luks_opts.ivgen_hash_alg];
> +        cipher_mode_spec = g_strdup_printf("%s-%s:%s", cipher_mode, ivgen_alg,
> +                                           ivgen_hash_alg);
> +    } else {
> +        cipher_mode_spec = g_strdup_printf("%s-%s", cipher_mode, ivgen_alg);
> +    }

Should we validate that the mode spec is among the set documented by the
LUKS spec ("ecb", "cbc-plain", "cbc-essiv:hash", "xts-plain64") (well,
we don't support xts yet), and reject combinations that aren't supported
("cbc-plain64" comes to mind as something the LUKS spec doesn't allow,
even though we have the pieces to logically make it happen)?

> +    hash_alg = QCryptoHashAlgorithm_lookup[luks_opts.hash_alg];

Likewise, should we validate that the hash is one of the specified names
("sha1", "sha256", "sha512", "ripemd160")?


> +
> +    memcpy(luks->header.cipher_name, cipher_alg,
> +           strlen(cipher_alg) + 1);
> +    memcpy(luks->header.cipher_mode, cipher_mode_spec,
> +           strlen(cipher_mode_spec) + 1);
> +    memcpy(luks->header.hash_spec, hash_alg,
> +           strlen(hash_alg) + 1);

Couldn't these just be strcpy()?

> +
> +    /* Determine how many iterations we need to hash the master
> +     * key with in order to have 1 second of compute time used

s/with //

> +     */
> +    luks->header.master_key_iterations =
> +        qcrypto_pbkdf2_count_iters(luks_opts.hash_alg,
> +                                   masterkey, luks->header.key_bytes,
> +                                   luks->header.master_key_salt,
> +                                   QCRYPTO_BLOCK_LUKS_SALT_LEN,
> +                                   &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        goto error;
> +    }
> +
> +    /* Why /= 8 ?  That matches cryptsetup, but there's no
> +     * explanation why they chose /= 8... */

My guess?  There's 8 key slots, and if all 8 are active, you'll spend 1
second serially decrypting each of them.  Users got ticked waiting that
long, so the /=8 reduces it to 125ms per slot, and for the typical use
case of slot0 being the user's password, much faster response at opening
the device.  But that's a guess - you're right that it could be
documented better.

> +    luks->header.master_key_iterations /= 8;
> +    luks->header.master_key_iterations = MAX(
> +        luks->header.master_key_iterations,
> +        QCRYPTO_BLOCK_LUKS_MIN_MASTER_KEY_ITERS);
> +
> +
> +    /* Hash the master key, saving the result in the LUKS
> +     * header. This hash is used when opening the encrypted
> +     * device to verify that the user password unlocked a
> +     * valid master key
> +     */
> +    if (qcrypto_pbkdf2(luks_opts.hash_alg,
> +                       masterkey, luks->header.key_bytes,
> +                       luks->header.master_key_salt,
> +                       QCRYPTO_BLOCK_LUKS_SALT_LEN,
> +                       luks->header.master_key_iterations,
> +                       luks->header.master_key_digest,
> +                       QCRYPTO_BLOCK_LUKS_DIGEST_LEN,
> +                       errp) < 0) {
> +        goto error;
> +    }
> +
> +
> +    /* Although LUKS has multiple key slots, we're just going
> +     * to use the first key slot */
> +
> +    splitkeylen = luks->header.key_bytes * QCRYPTO_BLOCK_LUKS_STRIPES;

The LUKS spec says that it is okay to allow the user to specify stripes.
 Should that be one of our options, with a sane default?  But it can be
added later as a followup, this patch is already quite big and I'm fine
with hardcoding stripes for now.

> +    for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
> +        luks->header.key_slots[i].active = i == 0 ?
> +            QCRYPTO_BLOCK_LUKS_KEY_SLOT_ENABLED :
> +            QCRYPTO_BLOCK_LUKS_KEY_SLOT_DISABLED;
> +        luks->header.key_slots[i].stripes = QCRYPTO_BLOCK_LUKS_STRIPES;
> +        luks->header.key_slots[i].key_offset =
> +            (QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET / 512) +
> +            (ROUND_UP(((splitkeylen + 511) / 512),
> +                      (QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET / 512)) * i);

Eww. The spec seems buggy, saying:

> // integer divisions, result rounded down:
> baseOffset = (size of phdr)/SECTOR SIZE + 1
> keymaterialSectors = (stripes*masterKeyLength)/SECTOR SIZE + 1

First, look at the calculation of baseOffset.  Since size of phdr is 592
bytes, and that is NOT a SECTOR SIZE in any disk I know of, that makes
sense that baseOffset will be at least 2 (512-byte) sectors into the
disk, or, if SECTOR SIZE is 4, that will result in an offset of 1
(4096-byte) sector.  However, you've defined
QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET as 4096, which means for our 512-byte
sector usage, you are setting the first key material at sector 4 no
matter what.  I guess that matches what cryptsetup does?  Maybe worth a
comment?

Now, look at the calculation of keymaterial Sectors.  The algorithm in
the spec mishandles the case where stripes happens to be an exact
multiple of sector size (that is, for a keysize of 20 bytes coupled with
512 stripes, it would reserve 21 sectors rather than the 20 actually
required).  I think your use of ROUND_UP() makes more sense, but it
would be nice to file a bug against the LUKS spec to have them fix their
math, and/or document that it is okay to use values larger than the
absolute minimum.

> +    }
> +
> +    if (qcrypto_random_bytes(luks->header.key_slots[0].salt,
> +                             QCRYPTO_BLOCK_LUKS_SALT_LEN,
> +                             errp) < 0) {
> +        goto error;
> +    }
> +
> +    /* Again we determine how many iterations are required to
> +     * hash the user password while consuming 1 second of compute
> +     * time */
> +    luks->header.key_slots[0].iterations =
> +        qcrypto_pbkdf2_count_iters(luks_opts.hash_alg,
> +                                   (uint8_t *)password, strlen(password),
> +                                   luks->header.key_slots[0].salt,
> +                                   QCRYPTO_BLOCK_LUKS_SALT_LEN,
> +                                   &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        goto error;
> +    }
> +    /* Why /= 2 ?  That matches cryptsetup, but there's no
> +     * explanation why they chose /= 2... */

Again, guessing that half a second feels nicer than 1 second for
responsiveness.

> +    luks->header.key_slots[0].iterations /= 2;
> +    luks->header.key_slots[0].iterations = MAX(
> +        luks->header.key_slots[0].iterations,
> +        QCRYPTO_BLOCK_LUKS_MIN_SLOT_KEY_ITERS);
> +

> +
> +    /* Before storing the master key, we need to vastly
> +     * increase its size, as protection against forensic
> +     * disk data recovery */
> +    splitkey = g_new0(uint8_t, splitkeylen);

If we later add the ability to do key management on an active disk, this
allocation may be large enough that it would warrant the _try variant
and reporting ENOMEM, rather than failing.


> +    }
> +
> +
> +
> +    /* The total size of the LUKS headers is the partition header + key

Three blank lines?

> +     * slot headers, rounded up to the nearest sector, combined with
> +     * the size of each master key material region, also rounded up
> +     * to the nearest sector */
> +    luks->header.payload_offset =
> +        (QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET / 512) +
> +        (ROUND_UP(((splitkeylen + 511) / 512),
> +                  (QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET / 512)) *
> +         QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS);
> +
> +    block->payload_offset = luks->header.payload_offset;

Again, I argue that block->payload_offset would be saner in bytes.

> +
> +    /* Reserve header space to match payload offset */
> +    initfunc(block, block->payload_offset * 512, &local_err, opaque);

Especially since initfunc() takes bytes.

> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        goto error;
> +    }
> +
> +    /* Everything on disk uses Big Endian, so flip header fields
> +     * before writing them */

> +
> +    /* Write out the master key material, starting at the
> +     * sector immediately following the partition header. */
> +    if (writefunc(block,
> +                  luks->header.key_slots[0].key_offset * 512,
> +                  splitkey, splitkeylen,
> +                  errp,
> +                  opaque) != splitkeylen) {
> +        goto error;
> +    }
> +
> +    memset(masterkey, 0, luks->header.key_bytes);
> +    g_free(masterkey);

Nice, trying to avoid leaking the key from a heap attack.

> +
> +static int
> +qcrypto_block_luks_decrypt(QCryptoBlock *block,
> +                           uint64_t startsector,
> +                           uint8_t *buf,
> +                           size_t len,
> +                           Error **errp)
> +{
> +    return qcrypto_block_decrypt_helper(block->cipher,
> +                                        block->niv, block->ivgen,
> +                                        startsector, buf, len, errp);
> +}

What happens when we try to read a LUKS device with 4k sectors?  Worth
worrying about, maybe just as a comment that we are hard-coded to 512
bytes at the moment?

> +++ b/qapi/crypto.json
> @@ -102,12 +102,13 @@
>  #
>  # @qcow: QCow/QCow2 built-in AES-CBC encryption. Use only
>  #        for liberating data from old images.
> +# @luks: LUKS encryption format. Recommended for new images
>  #
>  # Since: 2.6
>  ##
>  { 'enum': 'QCryptoBlockFormat',
>  #  'prefix': 'QCRYPTO_BLOCK_FORMAT',
> -  'data': ['qcow']}
> +  'data': ['qcow', 'luks']}
>  
>  ##
>  # QCryptoBlockOptionsBase:
> @@ -136,6 +137,40 @@
>    'data': { '*key-secret': 'str' }}
>  
>  ##
> +# QCryptoBlockOptionsLUKS:
> +#
> +# The options that apply to LUKS encryption format
> +#
> +# @key-secret: #optional the ID of a QCryptoSecret object providing the
> +#              decryption key

Maybe add "Mandatory except when probing the image for metadata only"

> +# Since: 2.6
> +##
> +{ 'struct': 'QCryptoBlockOptionsLUKS',
> +  'data': { '*key-secret': 'str' }}
> +
> +
> +##
> +# QCryptoBlockCreateOptionsLUKS:
> +#
> +# The options that apply to LUKS encryption format initialization
> +#
> +# @cipher-alg: #optional the cipher algorithm for data encryption
> +# @cipher-mode: #optional the cipher mode for data encryption
> +# @ivgen-alg: #optional the initialization vector generator
> +# @ivgen-hash-alg: #optional the initialization vector generator hash
> +# @hash-alg: #optional the master key hash algorithm

Would be nice to mention the defaults, particularly since we may later
change the default from cbc to xts.

> +# Since: 2.6
> +##
> +{ 'struct': 'QCryptoBlockCreateOptionsLUKS',
> +  'base': 'QCryptoBlockOptionsLUKS',
> +  'data': { '*cipher-alg': 'QCryptoCipherAlgorithm',
> +            '*cipher-mode': 'QCryptoCipherMode',
> +            '*ivgen-alg': 'QCryptoIVGenAlgorithm',
> +            '*ivgen-hash-alg': 'QCryptoHashAlgorithm',
> +            '*hash-alg': 'QCryptoHashAlgorithm'}}
> +

As mentioned earlier, we might also want to make stripes be customizable.

> +++ b/tests/test-crypto-block.c
> @@ -40,6 +40,69 @@ static QCryptoBlockOpenOptions qcow_open_opts = {
>      .u.qcow = &qcow_opts,
>  };
>  

> +    {
> +        .path = "/crypto/block/luks/aes-256-cbc-eesiv",
> +        .create_opts = &luks_create_opts_aes256_cbc_essiv,

s/eesiv/essiv/

Wow. A lot to read through, but looks mostly sane. v3 will probably be
ready to go; meanwhile, I spotted enough typos in the LUKS spec that I'm
half tempted to check out their source code and submit a patch.
Daniel P. Berrangé Feb. 8, 2016, 4:03 p.m. UTC | #2
On Fri, Feb 05, 2016 at 10:38:45AM -0700, Eric Blake wrote:
> On 01/20/2016 10:38 AM, Daniel P. Berrange wrote:
> > Provide a block encryption implementation that follows the
> > LUKS/dm-crypt specification.
> > 
> > This supports all combinations of hash, cipher algorithm,
> > cipher mode and iv generator that are implemented by the
> > current crypto layer.
> > 
> > The notable missing feature is support for the 'xts'
> > cipher mode, which is commonly used for disk encryption
> > instead of 'cbc'. This is because it is not provided by
> > either nettle or libgcrypt. A suitable implementation
> > will be identified & integrated later.
> > 
> > There is support for opening existing volumes formatted
> > by dm-crypt, and for formatting new volumes. In the latter
> > case it will only use key slot 0.
> > 
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>


> > +
> > +/*
> > + * Reference for format is
> > + *
> > + * docs/on-disk-format.pdf
> > + *
> > + * In 'cryptsetup' package source code
> > + *
> 
> Worth calling out the document version number? I reviewed based on
> version 1.2.1, dated Oct 2011.

Yep


> > +};
> > +
> > +
> > +struct QCryptoBlockLUKSKeySlot {
> > +    /* state of keyslot, enabled/disable */
> > +    uint32_t active;
> 
> May be worth a comment that this struct is written to disk in
> big-endian, but used in native-endian for most operations for
> convenience; to make sure future developers remember to put byteswaps in
> the correct locations.  But it looks like you handled endianness correctly.

Yes, will note it.

> 
> > +    /* iterations for PBKDF2 */
> > +    uint32_t iterations;
> > +    /* salt for PBKDF2 */
> > +    uint8_t salt[QCRYPTO_BLOCK_LUKS_SALT_LEN];
> > +    /* start sector of key material */
> > +    uint32_t key_offset;
> > +    /* number of anti-forensic stripes */
> > +    uint32_t stripes;
> > +} QEMU_PACKED;
> 
> Packed makes sense since we are writing it straight to disk; although it
> looks like natural alignment prevails and this did not need to be
> packed.  Do we want some sort of BUG_ON() that we have the right size
> struct?

Yes, I'll add QEMU_BUILD_BUG_ON

> > +
> > +struct QCryptoBlockLUKSHeader {
> > +    /* 'L', 'U', 'K', 'S', '0xBA', '0xBE' */
> > +    char magic[QCRYPTO_BLOCK_LUKS_MAGIC_LEN];
> > +
> > +    /* LUKS version, currently 1 */
> > +    uint16_t version;
> > +
> > +    /* cipher name specification (aes, etc */
> 
> Missing )
> 
> > +    char cipher_name[QCRYPTO_BLOCK_LUKS_CIPHER_NAME_LEN];
> > +
> > +    /* cipher mode specification () */
> 
> text in the ()?
> 
> > +    char cipher_mode[QCRYPTO_BLOCK_LUKS_CIPHER_MODE_LEN];
> > +
> > +    /* hash specication () */
> 
> s/specication/specification/, text in the ()?

Will fix all these examples

> 
> > +    char hash_spec[QCRYPTO_BLOCK_LUKS_HASH_SPEC_LEN];
> > +
> > +    /* start offset of the volume data (in sectors) */
> > +    uint32_t payload_offset;
> 
> Should we say "in 512-byte sectors"?  I could not see anything in the
> LUKS specification that said whether SECTOR_SIZE can be larger than 512
> - if you have a bare metal 4k sector disk, does a payload_offset of 2
> mean 1024 or 8192 bytes into the disk?  Eww. Sounds like we should
> submit a bug against the LUKS spec to have it clarified.

cryptsetup is hardcoded to use 512 byte sectors currently. IIUC
they'll need a LUKS spec update before doing 4k sectors natively.

> 
> > +
> > +    /* Number of key bytes */
> > +    uint32_t key_bytes;
> > +
> > +    /* master key checksum after PBKDF2 */
> > +    uint8_t master_key_digest[QCRYPTO_BLOCK_LUKS_DIGEST_LEN];
> > +
> > +    /* salt for master key PBKDF2 */
> > +    uint8_t master_key_salt[QCRYPTO_BLOCK_LUKS_SALT_LEN];
> > +
> > +    /* iterations for master key PBKDF2 */
> > +    uint32_t master_key_iterations;
> > +
> > +    /* UUID of the partition */
> 
> Should we say "in standard ASCII representation"?
> 
> > +    uint8_t uuid[QCRYPTO_BLOCK_LUKS_UUID_LEN];
> > +
> > +    /* key slots */
> > +    QCryptoBlockLUKSKeySlot key_slots[QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS];
> > +} QEMU_PACKED;
> 
> Again, alignment should mean that we could get away without packing; but
> the packing doesn't hurt; and a BUG_ON may be nice to ensure size.

Yep, add QEMU_BUILD_BUG_ON

> > +/* XXX doesn't QEMU already have a string -> enum val convertor ? */
> > +static int qcrypto_block_luks_name_lookup(const char *name,
> 
> Yes, spelled qapi_enum_parse().  Works if the enum was defined in a
> .json file.
> 
> 
> > +
> > +    error_setg(errp, "%s %s not supported", type, name);
> > +    return 0;
> > +}
> > +
> > +#define qcrypto_block_luks_cipher_mode_lookup(name, errp)               \
> > +    qcrypto_block_luks_name_lookup(name,                                \
> > +                                   QCryptoCipherMode_lookup,            \
> > +                                   QCRYPTO_CIPHER_MODE__MAX,            \
> > +                                   "Cipher mode",                       \
> > +                                   errp)
> > +
> > +#define qcrypto_block_luks_hash_name_lookup(name, errp)                 \
> > +    qcrypto_block_luks_name_lookup(name,                                \
> > +                                   QCryptoHashAlgorithm_lookup,         \
> > +                                   QCRYPTO_HASH_ALG__MAX,               \
> > +                                   "Hash algorithm",                    \
> > +                                   errp)
> 
> That said, your macro wrappers tweak the error message, so that it is
> perhaps more legible than the standard failure mode of
> qapi_enum_parse().  So up to you if you want to keep the wrappers.

I'll change my XXX note to mention qapi_enum_parse() as a future
todo if we can improve its error reporting in some manner.


> > +static int
> > +qcrypto_block_luks_load_key(QCryptoBlock *block,
> > +                            QCryptoBlockLUKSKeySlot *slot,
> > +                            const char *password,
> > +                            QCryptoCipherAlgorithm cipheralg,
> > +                            QCryptoCipherMode ciphermode,
> > +                            QCryptoHashAlgorithm hash,
> > +                            QCryptoIVGenAlgorithm ivalg,
> > +                            QCryptoHashAlgorithm ivhash,
> > +                            uint8_t *masterkey,
> > +                            size_t masterkeylen,
> > +                            QCryptoBlockReadFunc readfunc,
> > +                            void *opaque,
> > +                            Error **errp)
> > +{
> > +    QCryptoBlockLUKS *luks = block->opaque;
> > +    uint8_t *splitkey;
> > +    size_t splitkeylen;
> > +    uint8_t *possiblekey;
> > +    int ret = -1;
> > +    ssize_t rv;
> > +    QCryptoCipher *cipher = NULL;
> > +    uint8_t keydigest[QCRYPTO_BLOCK_LUKS_DIGEST_LEN];
> > +    QCryptoIVGen *ivgen = NULL;
> > +    size_t niv;
> > +
> > +    if (slot->active != QCRYPTO_BLOCK_LUKS_KEY_SLOT_ENABLED) {
> > +        return 0;
> > +    }
> 
> Would be worth documenting the convention of '0' (no key found, but no
> errp set), '1' (successful key), and negative (errp set).

Yes, will document that.

> > +    splitkeylen = masterkeylen * slot->stripes;
> 
> Should we do any validation that masterkeylen was in bounds and this
> doesn't overflow?  I don't want a malicious byte stream masquerading as
> a LUKS header to cause us to misbehave.

masterkeylen comes from luks->header.key_bytes. We should put some
validation in for that earlier when we first get it off disk.

> For that matter, the spec states that luks->payload_offset is a
> write-once calculation at creation time, and that it could be recomputed
> from key layout.  Should we validate that the offset listed makes
> algorithmic sense with what we know about header and key data sizes for
> the parameters listed in the header?

While it says that the offsets could be recomputed, from a technical POV
there's no reason why all impls must use the same layout / alignment for
the key slots. So I feel it safer to just rely on the header values and
not try to enforce that they match a particular offset.

> Side note: I'm a bit surprised the upstream LUKS spec does not include
> any checksum over the header contents - it does a great job at ensuring
> the master key is only going to be found by someone knowing a correct
> password and that a single byte error in a key material section
> invalidates that entire slot's usefulness, but it is not as careful on
> what happens with certain single-byte errors in the header.
> 
> > +    splitkey = g_new0(uint8_t, splitkeylen);
> > +    possiblekey = g_new0(uint8_t, masterkeylen);
> > +
> > +    /*
> > +     * The user password is used to generate a (possible)
> > +     * decryption key. This may or may not successfully
> > +     * decrypt the master key - we just blindly assume
> > +     * the key is correct and validate the results of
> > +     * decryption later.
> > +     */
> > +    if (qcrypto_pbkdf2(hash,
> > +                       (const uint8_t *)password, strlen(password),
> > +                       slot->salt, QCRYPTO_BLOCK_LUKS_SALT_LEN,
> > +                       slot->iterations,
> > +                       possiblekey, masterkeylen,
> > +                       errp) < 0) {
> > +        goto cleanup;
> > +    }
> > +
> > +    /*
> > +     * We need to read the master key material from the
> > +     * LUKS key material header. What we're reading is
> > +     * not the raw master key, but rather the data after
> > +     * it has been passed through AFSplit and the result
> > +     * then encrypted.
> > +     */
> > +    rv = readfunc(block,
> > +                  slot->key_offset * 512ll,
> 
> I tend to write LL rather than ll, to make it obvious I didn't typo '1'.
>  Looks like we're hard-coding our sector size (not the first time); but
> maybe a constant instead of a magic number will make it easier for the
> poor guy down the road that tries to switch to 4k sectors.

I'll introduce a QCRYPTO_LUKS_SECTOR_SIZE constant throughout for
now.


> > +     * it back together to get the actual master key.
> > +     */
> > +    if (qcrypto_afsplit_decode(hash,
> 
> ...
> > +
> > +    if (memcmp(keydigest, luks->header.master_key_digest,
> > +               QCRYPTO_BLOCK_LUKS_DIGEST_LEN) == 0) {
> > +        /* Success, we got the right master key */
> > +        ret = 1;
> > +        goto cleanup;
> > +    }
> > +
> > +    /* Fail, user's password was not valid for this key slot,
> > +     * tell caller to try another slot */
> > +    ret = 0;
> > +
> 
> Matches the spec.  Yay!
> 
> Side observation: The LUKS spec doesn't say anything about parallelizing
> the lookup loop across all 8 key slots, nor any randomization on which
> slot should be attempted first.  The whole point of PBKDF2 iteration
> count is to burn CPU time so that an attacker can't brute force things
> easily.  That means it is VERY easy to tell by timing how many active
> slots were attempted before a key was found, if slots are tried serially
> and we immediately break the loop on the first successful decryption.
> Is there any information leak caused by the timing observations when
> serially searching among 8 active slots, when the master key is only
> found in slot 7 vs. slot 0?  But I don't see it as your problem to
> solve, so much as something to ask the upstream LUKS design team.

FWIW, cryptsetup will short circuit the search as soon as it finds
a valid slot, so we're matching their behaviour at least.

> > +static int
> > +qcrypto_block_luks_open(QCryptoBlock *block,
> > +                        QCryptoBlockOpenOptions *options,
> > +                        QCryptoBlockReadFunc readfunc,
> > +                        void *opaque,
> > +                        unsigned int flags,
> > +                        Error **errp)
> > +{
> 
> > +    /*
> > +     * The cipher_mode header contains a string that we have
> > +     * to further parse of the format
> 
> s/parse/parse,/
> 
> > +     *
> > +     *    <cipher-mode>-<iv-generator>[:<iv-hash>]
> > +     *
> > +     * eg  cbc-essiv:sha256, cbc-plain64
> > +     */
> > +    ivgen_name = strchr(luks->header.cipher_mode, '-');
> > +    if (!ivgen_name) {
> > +        ret = -EINVAL;
> > +        error_setg(errp, "Unexpected cipher mode string format %s",
> > +                   luks->header.cipher_mode);
> > +        goto fail;
> > +    }
> > +    *ivgen_name = '\0';
> > +    ivgen_name++;
> 
> We're modifying luks->header in place; we'd better not write it back out
> to disk in the modified form.  I guess you are okay for now - your code
> only reads existing LUKS disks, and can only create a single key in slot
> 0 on conversion (that is, I don't see code for key management of an
> existing image).  Probably things we should add later, though, at which
> point we'll need to be careful that we don't overwrite too much in the
> header.

Yeah, certainly something to be careful of later.

> > +    block->payload_offset = luks->header.payload_offset;
> 
> Earlier, I argued that block->payload_offset should be in bytes.  You
> are constrained that luks->header.payload_offset is in sectors, but we
> need not propagate that craziness any higher.

Yes, I've made that change.

> > +static int
> > +qcrypto_block_luks_create(QCryptoBlock *block,
> > +                          QCryptoBlockCreateOptions *options,
> > +                          QCryptoBlockInitFunc initfunc,
> > +                          QCryptoBlockWriteFunc writefunc,
> > +                          void *opaque,
> > +                          Error **errp)
> > +{
> 
> > +
> > +    memcpy(&luks_opts, options->u.luks, sizeof(luks_opts));
> > +    if (!luks_opts.has_cipher_alg) {
> > +        luks_opts.cipher_alg = QCRYPTO_CIPHER_ALG_AES_256;
> > +    }
> > +    if (!luks_opts.has_cipher_mode) {
> > +        /* XXX switch to XTS */
> > +        luks_opts.cipher_mode = QCRYPTO_CIPHER_MODE_CBC;
> > +    }
> 
> Yeah, my (quick) reading made it seem like XTS is a slightly more secure
> default than CBC.  But as you said, XTS implementation will come later.

Yes, I still hope to post an XTS impl before 2.6, so if this code
merges before 2.6, hopefully we'll stil have time to use XTS as
the default too, matching cryptsetup

> > +    luks = g_new0(QCryptoBlockLUKS, 1);
> > +    block->opaque = luks;
> > +
> > +    memcpy(luks->header.magic, qcrypto_block_luks_magic,
> > +           QCRYPTO_BLOCK_LUKS_MAGIC_LEN);
> > +
> > +    luks->header.version = QCRYPTO_BLOCK_LUKS_VERSION;
> 
> Maybe a comment here that endianness will be addressed later, as a lot
> of code appears between here and the actual write-to-disk.

Will do

> > +    uuid_generate(uuid);
> > +    uuid_unparse(uuid, (char *)luks->header.uuid);
> > +
> > +    switch (luks_opts.cipher_alg) {
> > +    case QCRYPTO_CIPHER_ALG_AES_128:
> > +    case QCRYPTO_CIPHER_ALG_AES_192:
> > +    case QCRYPTO_CIPHER_ALG_AES_256:
> > +        cipher_alg = "aes";
> > +        break;
> > +    default:
> > +        error_setg(errp, "Cipher algorithm is not supported");
> > +        goto error;
> 
> That is, we aren't supporting "twofish", "serpent", "cast5", or "cast6".
>  Fine for now.
> 
> > +    }
> > +
> > +    cipher_mode = QCryptoCipherMode_lookup[luks_opts.cipher_mode];
> > +    ivgen_alg = QCryptoIVGenAlgorithm_lookup[luks_opts.ivgen_alg];
> > +    if (luks_opts.has_ivgen_hash_alg) {
> > +        ivgen_hash_alg = QCryptoHashAlgorithm_lookup[luks_opts.ivgen_hash_alg];
> > +        cipher_mode_spec = g_strdup_printf("%s-%s:%s", cipher_mode, ivgen_alg,
> > +                                           ivgen_hash_alg);
> > +    } else {
> > +        cipher_mode_spec = g_strdup_printf("%s-%s", cipher_mode, ivgen_alg);
> > +    }
> 
> Should we validate that the mode spec is among the set documented by the
> LUKS spec ("ecb", "cbc-plain", "cbc-essiv:hash", "xts-plain64") (well,
> we don't support xts yet), and reject combinations that aren't supported
> ("cbc-plain64" comes to mind as something the LUKS spec doesn't allow,
> even though we have the pieces to logically make it happen)?

The spec appears to be non-exhaustive, in that cryptsetup / linux allow
for cbc-plain64 and other combinations that aren't listed. So QEMU should
follow that for interoperability.

> > +    hash_alg = QCryptoHashAlgorithm_lookup[luks_opts.hash_alg];
> 
> Likewise, should we validate that the hash is one of the specified names
> ("sha1", "sha256", "sha512", "ripemd160")?

The only one we're permitting that's not listed there is 'md5' and
that should fail anyway since it provides too small a digest size
but I see we're not validating the digest size.

> > +
> > +    memcpy(luks->header.cipher_name, cipher_alg,
> > +           strlen(cipher_alg) + 1);
> > +    memcpy(luks->header.cipher_mode, cipher_mode_spec,
> > +           strlen(cipher_mode_spec) + 1);
> > +    memcpy(luks->header.hash_spec, hash_alg,
> > +           strlen(hash_alg) + 1);
> 
> Couldn't these just be strcpy()?

Yes


> > +    /* Although LUKS has multiple key slots, we're just going
> > +     * to use the first key slot */
> > +
> > +    splitkeylen = luks->header.key_bytes * QCRYPTO_BLOCK_LUKS_STRIPES;
> 
> The LUKS spec says that it is okay to allow the user to specify stripes.
>  Should that be one of our options, with a sane default?  But it can be
> added later as a followup, this patch is already quite big and I'm fine
> with hardcoding stripes for now.

Cryptsetup doesn't allow the user to specify the stripes value, just
fixing it at 4k, so I figure we're best todo the same, otherwise we'll
create images that cant be read by linux.

> 
> > +    for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
> > +        luks->header.key_slots[i].active = i == 0 ?
> > +            QCRYPTO_BLOCK_LUKS_KEY_SLOT_ENABLED :
> > +            QCRYPTO_BLOCK_LUKS_KEY_SLOT_DISABLED;
> > +        luks->header.key_slots[i].stripes = QCRYPTO_BLOCK_LUKS_STRIPES;
> > +        luks->header.key_slots[i].key_offset =
> > +            (QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET / 512) +
> > +            (ROUND_UP(((splitkeylen + 511) / 512),
> > +                      (QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET / 512)) * i);
> 
> Eww. The spec seems buggy, saying:
> 
> > // integer divisions, result rounded down:
> > baseOffset = (size of phdr)/SECTOR SIZE + 1
> > keymaterialSectors = (stripes*masterKeyLength)/SECTOR SIZE + 1
> 
> First, look at the calculation of baseOffset.  Since size of phdr is 592
> bytes, and that is NOT a SECTOR SIZE in any disk I know of, that makes
> sense that baseOffset will be at least 2 (512-byte) sectors into the
> disk, or, if SECTOR SIZE is 4, that will result in an offset of 1
> (4096-byte) sector.  However, you've defined
> QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET as 4096, which means for our 512-byte
> sector usage, you are setting the first key material at sector 4 no
> matter what.  I guess that matches what cryptsetup does?  Maybe worth a
> comment?
> 
> Now, look at the calculation of keymaterial Sectors.  The algorithm in
> the spec mishandles the case where stripes happens to be an exact
> multiple of sector size (that is, for a keysize of 20 bytes coupled with
> 512 stripes, it would reserve 21 sectors rather than the 20 actually
> required).  I think your use of ROUND_UP() makes more sense, but it
> would be nice to file a bug against the LUKS spec to have them fix their
> math, and/or document that it is okay to use values larger than the
> absolute minimum.

Yeah, what I am doing matches the cryptsetup code. I had originally
written it to be more or less what the spec suggests, and noticed
that my images were different from those created by cryptsetup. So
I switched to use the same algorithm. I figure it is probably an
optimization for on disk alignment to stop crossing sector boundaries.


> > +     * slot headers, rounded up to the nearest sector, combined with
> > +     * the size of each master key material region, also rounded up
> > +     * to the nearest sector */
> > +    luks->header.payload_offset =
> > +        (QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET / 512) +
> > +        (ROUND_UP(((splitkeylen + 511) / 512),
> > +                  (QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET / 512)) *
> > +         QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS);
> > +
> > +    block->payload_offset = luks->header.payload_offset;
> 
> Again, I argue that block->payload_offset would be saner in bytes.

Agreed

> > +    /* Reserve header space to match payload offset */
> > +    initfunc(block, block->payload_offset * 512, &local_err, opaque);
> 
> Especially since initfunc() takes bytes.
> 


> > +
> > +static int
> > +qcrypto_block_luks_decrypt(QCryptoBlock *block,
> > +                           uint64_t startsector,
> > +                           uint8_t *buf,
> > +                           size_t len,
> > +                           Error **errp)
> > +{
> > +    return qcrypto_block_decrypt_helper(block->cipher,
> > +                                        block->niv, block->ivgen,
> > +                                        startsector, buf, len, errp);
> > +}
> 
> What happens when we try to read a LUKS device with 4k sectors?  Worth
> worrying about, maybe just as a comment that we are hard-coded to 512
> bytes at the moment?

qcrypto_block_decrypt_helper() method processes 'buf' in units of 512
so even if the underling dev is 4k, we'll also do 512 byte I/O

> >  ##
> > +# QCryptoBlockOptionsLUKS:
> > +#
> > +# The options that apply to LUKS encryption format
> > +#
> > +# @key-secret: #optional the ID of a QCryptoSecret object providing the
> > +#              decryption key
> 
> Maybe add "Mandatory except when probing the image for metadata only"

Yes


> > +# Since: 2.6
> > +##
> > +{ 'struct': 'QCryptoBlockOptionsLUKS',
> > +  'data': { '*key-secret': 'str' }}
> > +
> > +
> > +##
> > +# QCryptoBlockCreateOptionsLUKS:
> > +#
> > +# The options that apply to LUKS encryption format initialization
> > +#
> > +# @cipher-alg: #optional the cipher algorithm for data encryption
> > +# @cipher-mode: #optional the cipher mode for data encryption
> > +# @ivgen-alg: #optional the initialization vector generator
> > +# @ivgen-hash-alg: #optional the initialization vector generator hash
> > +# @hash-alg: #optional the master key hash algorithm
> 
> Would be nice to mention the defaults, particularly since we may later
> change the default from cbc to xts.

Yes, will do


Regards,
Daniel
diff mbox

Patch

diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs
index 25ca0c7..3299923 100644
--- a/crypto/Makefile.objs
+++ b/crypto/Makefile.objs
@@ -20,6 +20,7 @@  crypto-obj-y += ivgen-plain64.o
 crypto-obj-y += afsplit.o
 crypto-obj-y += block.o
 crypto-obj-y += block-qcow.o
+crypto-obj-y += block-luks.o
 
 # Let the userspace emulators avoid linking gnutls/etc
 crypto-aes-obj-y = aes.o
diff --git a/crypto/block-luks.c b/crypto/block-luks.c
new file mode 100644
index 0000000..47630ee
--- /dev/null
+++ b/crypto/block-luks.c
@@ -0,0 +1,1105 @@ 
+/*
+ * QEMU Crypto block device encryption LUKS format
+ *
+ * Copyright (c) 2015-2016 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "config-host.h"
+
+#include "crypto/block-luks.h"
+
+#include "crypto/hash.h"
+#include "crypto/afsplit.h"
+#include "crypto/pbkdf.h"
+#include "crypto/secret.h"
+#include "crypto/random.h"
+
+#ifdef CONFIG_UUID
+#include <uuid/uuid.h>
+#endif
+
+#include "qemu/coroutine.h"
+
+/*
+ * Reference for format is
+ *
+ * docs/on-disk-format.pdf
+ *
+ * In 'cryptsetup' package source code
+ *
+ */
+
+typedef struct QCryptoBlockLUKS QCryptoBlockLUKS;
+typedef struct QCryptoBlockLUKSHeader QCryptoBlockLUKSHeader;
+typedef struct QCryptoBlockLUKSKeySlot QCryptoBlockLUKSKeySlot;
+
+#define QCRYPTO_BLOCK_LUKS_VERSION 1
+
+#define QCRYPTO_BLOCK_LUKS_MAGIC_LEN 6
+#define QCRYPTO_BLOCK_LUKS_CIPHER_NAME_LEN 32
+#define QCRYPTO_BLOCK_LUKS_CIPHER_MODE_LEN 32
+#define QCRYPTO_BLOCK_LUKS_HASH_SPEC_LEN 32
+#define QCRYPTO_BLOCK_LUKS_DIGEST_LEN 20
+#define QCRYPTO_BLOCK_LUKS_SALT_LEN 32
+#define QCRYPTO_BLOCK_LUKS_UUID_LEN 40
+#define QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS 8
+#define QCRYPTO_BLOCK_LUKS_STRIPES 4000
+#define QCRYPTO_BLOCK_LUKS_MIN_SLOT_KEY_ITERS 1000
+#define QCRYPTO_BLOCK_LUKS_MIN_MASTER_KEY_ITERS 1000
+#define QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET 4096
+
+#define QCRYPTO_BLOCK_LUKS_KEY_SLOT_DISABLED 0x0000DEAD
+#define QCRYPTO_BLOCK_LUKS_KEY_SLOT_ENABLED 0x00AC71F3
+
+static const char qcrypto_block_luks_magic[QCRYPTO_BLOCK_LUKS_MAGIC_LEN] = {
+    'L', 'U', 'K', 'S', 0xBA, 0xBE
+};
+
+typedef struct QCryptoBlockLUKSNameMap QCryptoBlockLUKSNameMap;
+struct QCryptoBlockLUKSNameMap {
+    const char *name;
+    int id;
+};
+
+typedef struct QCryptoBlockLUKSCipherSizeMap QCryptoBlockLUKSCipherSizeMap;
+struct QCryptoBlockLUKSCipherSizeMap {
+    uint32_t key_bytes;
+    int id;
+};
+typedef struct QCryptoBlockLUKSCipherNameMap QCryptoBlockLUKSCipherNameMap;
+struct QCryptoBlockLUKSCipherNameMap {
+    const char *name;
+    const QCryptoBlockLUKSCipherSizeMap *sizes;
+};
+
+
+static const QCryptoBlockLUKSCipherSizeMap
+qcrypto_block_luks_cipher_size_map_aes[] = {
+    { 16, QCRYPTO_CIPHER_ALG_AES_128 },
+    { 24, QCRYPTO_CIPHER_ALG_AES_192 },
+    { 32, QCRYPTO_CIPHER_ALG_AES_256 },
+    { 0, 0 },
+};
+
+static const QCryptoBlockLUKSCipherNameMap
+qcrypto_block_luks_cipher_name_map[] = {
+    { "aes", qcrypto_block_luks_cipher_size_map_aes, },
+};
+
+
+struct QCryptoBlockLUKSKeySlot {
+    /* state of keyslot, enabled/disable */
+    uint32_t active;
+    /* iterations for PBKDF2 */
+    uint32_t iterations;
+    /* salt for PBKDF2 */
+    uint8_t salt[QCRYPTO_BLOCK_LUKS_SALT_LEN];
+    /* start sector of key material */
+    uint32_t key_offset;
+    /* number of anti-forensic stripes */
+    uint32_t stripes;
+} QEMU_PACKED;
+
+struct QCryptoBlockLUKSHeader {
+    /* 'L', 'U', 'K', 'S', '0xBA', '0xBE' */
+    char magic[QCRYPTO_BLOCK_LUKS_MAGIC_LEN];
+
+    /* LUKS version, currently 1 */
+    uint16_t version;
+
+    /* cipher name specification (aes, etc */
+    char cipher_name[QCRYPTO_BLOCK_LUKS_CIPHER_NAME_LEN];
+
+    /* cipher mode specification () */
+    char cipher_mode[QCRYPTO_BLOCK_LUKS_CIPHER_MODE_LEN];
+
+    /* hash specication () */
+    char hash_spec[QCRYPTO_BLOCK_LUKS_HASH_SPEC_LEN];
+
+    /* start offset of the volume data (in sectors) */
+    uint32_t payload_offset;
+
+    /* Number of key bytes */
+    uint32_t key_bytes;
+
+    /* master key checksum after PBKDF2 */
+    uint8_t master_key_digest[QCRYPTO_BLOCK_LUKS_DIGEST_LEN];
+
+    /* salt for master key PBKDF2 */
+    uint8_t master_key_salt[QCRYPTO_BLOCK_LUKS_SALT_LEN];
+
+    /* iterations for master key PBKDF2 */
+    uint32_t master_key_iterations;
+
+    /* UUID of the partition */
+    uint8_t uuid[QCRYPTO_BLOCK_LUKS_UUID_LEN];
+
+    /* key slots */
+    QCryptoBlockLUKSKeySlot key_slots[QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS];
+} QEMU_PACKED;
+
+struct QCryptoBlockLUKS {
+    QCryptoBlockLUKSHeader header;
+};
+
+
+static int qcrypto_block_luks_cipher_name_lookup(const char *name,
+                                                 uint32_t key_bytes,
+                                                 Error **errp)
+{
+    const QCryptoBlockLUKSCipherNameMap *map =
+        qcrypto_block_luks_cipher_name_map;
+    size_t maplen = G_N_ELEMENTS(qcrypto_block_luks_cipher_name_map);
+    size_t i, j;
+    for (i = 0; i < maplen; i++) {
+        if (!g_str_equal(map[i].name, name)) {
+            continue;
+        }
+        for (j = 0; j < map[i].sizes[j].key_bytes; j++) {
+            if (map[i].sizes[j].key_bytes == key_bytes) {
+                return map[i].sizes[j].id;
+            }
+        }
+    }
+
+    error_setg(errp, "Algorithm %s with key size %d bytes not supported",
+               name, key_bytes);
+    return 0;
+}
+
+/* XXX doesn't QEMU already have a string -> enum val convertor ? */
+static int qcrypto_block_luks_name_lookup(const char *name,
+                                          const char *const *map,
+                                          size_t maplen,
+                                          const char *type,
+                                          Error **errp)
+{
+    size_t i;
+    for (i = 0; i < maplen; i++) {
+        if (g_str_equal(map[i], name)) {
+            return i;
+        }
+    }
+
+    error_setg(errp, "%s %s not supported", type, name);
+    return 0;
+}
+
+#define qcrypto_block_luks_cipher_mode_lookup(name, errp)               \
+    qcrypto_block_luks_name_lookup(name,                                \
+                                   QCryptoCipherMode_lookup,            \
+                                   QCRYPTO_CIPHER_MODE__MAX,            \
+                                   "Cipher mode",                       \
+                                   errp)
+
+#define qcrypto_block_luks_hash_name_lookup(name, errp)                 \
+    qcrypto_block_luks_name_lookup(name,                                \
+                                   QCryptoHashAlgorithm_lookup,         \
+                                   QCRYPTO_HASH_ALG__MAX,               \
+                                   "Hash algorithm",                    \
+                                   errp)
+
+#define qcrypto_block_luks_ivgen_name_lookup(name, errp)                \
+    qcrypto_block_luks_name_lookup(name,                                \
+                                   QCryptoIVGenAlgorithm_lookup,        \
+                                   QCRYPTO_IVGEN_ALG__MAX,              \
+                                   "IV generator",                      \
+                                   errp)
+
+
+static gboolean
+qcrypto_block_luks_has_format(const uint8_t *buf,
+                              size_t buf_size)
+{
+    const QCryptoBlockLUKSHeader *luks_header = (const void *)buf;
+
+    if (buf_size >= offsetof(QCryptoBlockLUKSHeader, cipher_name) &&
+        memcmp(luks_header->magic, qcrypto_block_luks_magic,
+               QCRYPTO_BLOCK_LUKS_MAGIC_LEN) == 0 &&
+        be16_to_cpu(luks_header->version) == QCRYPTO_BLOCK_LUKS_VERSION) {
+        return true;
+    } else {
+        return false;
+    }
+}
+
+
+/*
+ * Given a key slot, and user password, this will attempt to unlock
+ * the master encryption key from the key slot
+ */
+static int
+qcrypto_block_luks_load_key(QCryptoBlock *block,
+                            QCryptoBlockLUKSKeySlot *slot,
+                            const char *password,
+                            QCryptoCipherAlgorithm cipheralg,
+                            QCryptoCipherMode ciphermode,
+                            QCryptoHashAlgorithm hash,
+                            QCryptoIVGenAlgorithm ivalg,
+                            QCryptoHashAlgorithm ivhash,
+                            uint8_t *masterkey,
+                            size_t masterkeylen,
+                            QCryptoBlockReadFunc readfunc,
+                            void *opaque,
+                            Error **errp)
+{
+    QCryptoBlockLUKS *luks = block->opaque;
+    uint8_t *splitkey;
+    size_t splitkeylen;
+    uint8_t *possiblekey;
+    int ret = -1;
+    ssize_t rv;
+    QCryptoCipher *cipher = NULL;
+    uint8_t keydigest[QCRYPTO_BLOCK_LUKS_DIGEST_LEN];
+    QCryptoIVGen *ivgen = NULL;
+    size_t niv;
+
+    if (slot->active != QCRYPTO_BLOCK_LUKS_KEY_SLOT_ENABLED) {
+        return 0;
+    }
+
+    splitkeylen = masterkeylen * slot->stripes;
+    splitkey = g_new0(uint8_t, splitkeylen);
+    possiblekey = g_new0(uint8_t, masterkeylen);
+
+    /*
+     * The user password is used to generate a (possible)
+     * decryption key. This may or may not successfully
+     * decrypt the master key - we just blindly assume
+     * the key is correct and validate the results of
+     * decryption later.
+     */
+    if (qcrypto_pbkdf2(hash,
+                       (const uint8_t *)password, strlen(password),
+                       slot->salt, QCRYPTO_BLOCK_LUKS_SALT_LEN,
+                       slot->iterations,
+                       possiblekey, masterkeylen,
+                       errp) < 0) {
+        goto cleanup;
+    }
+
+    /*
+     * We need to read the master key material from the
+     * LUKS key material header. What we're reading is
+     * not the raw master key, but rather the data after
+     * it has been passed through AFSplit and the result
+     * then encrypted.
+     */
+    rv = readfunc(block,
+                  slot->key_offset * 512ll,
+                  splitkey, splitkeylen,
+                  errp,
+                  opaque);
+    if (rv < 0) {
+        goto cleanup;
+    }
+
+
+    /* Setup the cipher/ivgen that we'll use to try to decrypt
+     * the split master key material */
+    cipher = qcrypto_cipher_new(cipheralg, ciphermode,
+                                possiblekey, masterkeylen,
+                                errp);
+    if (!cipher) {
+        goto cleanup;
+    }
+
+    niv = qcrypto_cipher_get_iv_len(cipheralg,
+                                    ciphermode);
+    ivgen = qcrypto_ivgen_new(ivalg,
+                              cipheralg,
+                              ivhash,
+                              possiblekey, masterkeylen,
+                              errp);
+    if (!ivgen) {
+        goto cleanup;
+    }
+
+
+    /*
+     * The master key needs to be decrypted in the same
+     * way that the block device payload will be decrypted
+     * later. In particular we'll be using the IV generator
+     * to reset the encryption cipher every time the master
+     * key crosses a sector boundary.
+     */
+    if (qcrypto_block_decrypt_helper(cipher,
+                                     niv,
+                                     ivgen,
+                                     0,
+                                     splitkey,
+                                     splitkeylen,
+                                     errp) < 0) {
+        goto cleanup;
+    }
+
+    /*
+     * Now we're decrypted the split master key, join
+     * it back together to get the actual master key.
+     */
+    if (qcrypto_afsplit_decode(hash,
+                               masterkeylen,
+                               slot->stripes,
+                               splitkey,
+                               masterkey,
+                               errp) < 0) {
+        goto cleanup;
+    }
+
+
+    /*
+     * We still don't know that the masterkey we got is valid,
+     * because we just blindly assumed the user's password
+     * was correct. This is where we now verify it. We are
+     * creating a hash of the master key using PBKDF and
+     * then comparing that to the hash stored in the key slot
+     * header
+     */
+    if (qcrypto_pbkdf2(hash,
+                       masterkey, masterkeylen,
+                       luks->header.master_key_salt,
+                       QCRYPTO_BLOCK_LUKS_SALT_LEN,
+                       luks->header.master_key_iterations,
+                       keydigest, G_N_ELEMENTS(keydigest),
+                       errp) < 0) {
+        goto cleanup;
+    }
+
+    if (memcmp(keydigest, luks->header.master_key_digest,
+               QCRYPTO_BLOCK_LUKS_DIGEST_LEN) == 0) {
+        /* Success, we got the right master key */
+        ret = 1;
+        goto cleanup;
+    }
+
+    /* Fail, user's password was not valid for this key slot,
+     * tell caller to try another slot */
+    ret = 0;
+
+ cleanup:
+    qcrypto_ivgen_free(ivgen);
+    qcrypto_cipher_free(cipher);
+    g_free(splitkey);
+    g_free(possiblekey);
+    return ret;
+}
+
+
+/*
+ * Given a user password, this will iterate over all key
+ * slots and try to unlock each active key slot using the
+ * password until it successfully obtains a master key.
+ */
+static int
+qcrypto_block_luks_find_key(QCryptoBlock *block,
+                            const char *password,
+                            QCryptoCipherAlgorithm cipheralg,
+                            QCryptoCipherMode ciphermode,
+                            QCryptoHashAlgorithm hash,
+                            QCryptoIVGenAlgorithm ivalg,
+                            QCryptoHashAlgorithm ivhash,
+                            uint8_t **masterkey,
+                            size_t *masterkeylen,
+                            QCryptoBlockReadFunc readfunc,
+                            void *opaque,
+                            Error **errp)
+{
+    QCryptoBlockLUKS *luks = block->opaque;
+    size_t i;
+    int rv;
+
+    *masterkey = g_new0(uint8_t, luks->header.key_bytes);
+    *masterkeylen = luks->header.key_bytes;
+
+    for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
+        rv = qcrypto_block_luks_load_key(block,
+                                         &luks->header.key_slots[i],
+                                         password,
+                                         cipheralg,
+                                         ciphermode,
+                                         hash,
+                                         ivalg,
+                                         ivhash,
+                                         *masterkey,
+                                         *masterkeylen,
+                                         readfunc,
+                                         opaque,
+                                         errp);
+        if (rv < 0) {
+            goto error;
+        }
+        if (rv == 1) {
+            return 0;
+        }
+    }
+
+    error_setg(errp, "Invalid password, cannot unlock any keyslot");
+
+ error:
+    g_free(*masterkey);
+    *masterkey = NULL;
+    *masterkeylen = 0;
+    return -1;
+}
+
+
+static int
+qcrypto_block_luks_open(QCryptoBlock *block,
+                        QCryptoBlockOpenOptions *options,
+                        QCryptoBlockReadFunc readfunc,
+                        void *opaque,
+                        unsigned int flags,
+                        Error **errp)
+{
+    QCryptoBlockLUKS *luks;
+    Error *local_err = NULL;
+    int ret = 0;
+    size_t i;
+    ssize_t rv;
+    uint8_t *masterkey = NULL;
+    size_t masterkeylen;
+    char *ivgen_name, *ivhash_name;
+    QCryptoCipherMode ciphermode;
+    QCryptoCipherAlgorithm cipheralg;
+    QCryptoIVGenAlgorithm ivalg;
+    QCryptoHashAlgorithm hash;
+    QCryptoHashAlgorithm ivhash;
+    char *password = NULL;
+
+    if (!(flags & QCRYPTO_BLOCK_OPEN_NO_IO)) {
+        if (!options->u.luks->key_secret) {
+            error_setg(errp, "Parameter 'key-secret' is required for cipher");
+            return -1;
+        }
+        password = qcrypto_secret_lookup_as_utf8(
+            options->u.luks->key_secret, errp);
+        if (!password) {
+            return -1;
+        }
+    }
+
+    luks = g_new0(QCryptoBlockLUKS, 1);
+    block->opaque = luks;
+
+    /* Read the entire LUKS header, minus the key material from
+     * the underling device */
+    rv = readfunc(block, 0,
+                  (uint8_t *)&luks->header,
+                  sizeof(luks->header),
+                  errp,
+                  opaque);
+    if (rv < 0) {
+        ret = rv;
+        goto fail;
+    }
+
+    /* The header is always stored in big-endian format, so
+     * convert everything to native */
+    be16_to_cpus(&luks->header.version);
+    be32_to_cpus(&luks->header.payload_offset);
+    be32_to_cpus(&luks->header.key_bytes);
+    be32_to_cpus(&luks->header.master_key_iterations);
+
+    for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
+        be32_to_cpus(&luks->header.key_slots[i].active);
+        be32_to_cpus(&luks->header.key_slots[i].iterations);
+        be32_to_cpus(&luks->header.key_slots[i].key_offset);
+        be32_to_cpus(&luks->header.key_slots[i].stripes);
+    }
+
+    if (memcmp(luks->header.magic, qcrypto_block_luks_magic,
+               QCRYPTO_BLOCK_LUKS_MAGIC_LEN) != 0) {
+        error_setg(errp, "Volume is not in LUKS format");
+        ret = -EINVAL;
+        goto fail;
+    }
+    if (luks->header.version != QCRYPTO_BLOCK_LUKS_VERSION) {
+        error_setg(errp, "LUKS version %" PRIu32 " is not supported",
+                   luks->header.version);
+        ret = -ENOTSUP;
+        goto fail;
+    }
+
+    /*
+     * The cipher_mode header contains a string that we have
+     * to further parse of the format
+     *
+     *    <cipher-mode>-<iv-generator>[:<iv-hash>]
+     *
+     * eg  cbc-essiv:sha256, cbc-plain64
+     */
+    ivgen_name = strchr(luks->header.cipher_mode, '-');
+    if (!ivgen_name) {
+        ret = -EINVAL;
+        error_setg(errp, "Unexpected cipher mode string format %s",
+                   luks->header.cipher_mode);
+        goto fail;
+    }
+    *ivgen_name = '\0';
+    ivgen_name++;
+
+    ivhash_name = strchr(ivgen_name, ':');
+    if (!ivhash_name) {
+        ivhash = 0;
+    } else {
+        *ivhash_name = '\0';
+        ivhash_name++;
+
+        ivhash = qcrypto_block_luks_hash_name_lookup(ivhash_name,
+                                                     &local_err);
+        if (local_err) {
+            ret = -ENOTSUP;
+            error_propagate(errp, local_err);
+            goto fail;
+        }
+    }
+
+    ciphermode = qcrypto_block_luks_cipher_mode_lookup(luks->header.cipher_mode,
+                                                       &local_err);
+    if (local_err) {
+        ret = -ENOTSUP;
+        error_propagate(errp, local_err);
+        goto fail;
+    }
+
+    cipheralg = qcrypto_block_luks_cipher_name_lookup(luks->header.cipher_name,
+                                                      luks->header.key_bytes,
+                                                      &local_err);
+    if (local_err) {
+        ret = -ENOTSUP;
+        error_propagate(errp, local_err);
+        goto fail;
+    }
+
+    hash = qcrypto_block_luks_hash_name_lookup(luks->header.hash_spec,
+                                               &local_err);
+    if (local_err) {
+        ret = -ENOTSUP;
+        error_propagate(errp, local_err);
+        goto fail;
+    }
+
+    ivalg = qcrypto_block_luks_ivgen_name_lookup(ivgen_name,
+                                                 &local_err);
+    if (local_err) {
+        ret = -ENOTSUP;
+        error_propagate(errp, local_err);
+        goto fail;
+    }
+
+
+    if (!(flags & QCRYPTO_BLOCK_OPEN_NO_IO)) {
+        /* Try to find which key slot our password is valid for
+         * and unlock the master key from that slot.
+         */
+        if (qcrypto_block_luks_find_key(block,
+                                        password,
+                                        cipheralg, ciphermode,
+                                        hash,
+                                        ivalg,
+                                        ivhash,
+                                        &masterkey, &masterkeylen,
+                                        readfunc, opaque,
+                                        errp) < 0) {
+            ret = -EACCES;
+            goto fail;
+        }
+
+        /* We have a valid master key now, so can setup the
+         * block device payload decryption objects
+         */
+        block->kdfhash = hash;
+        block->niv = qcrypto_cipher_get_iv_len(cipheralg,
+                                               ciphermode);
+        block->ivgen = qcrypto_ivgen_new(ivalg,
+                                         cipheralg,
+                                         ivhash,
+                                         masterkey, masterkeylen,
+                                         errp);
+        if (!block->ivgen) {
+            ret = -ENOTSUP;
+            goto fail;
+        }
+
+        block->cipher = qcrypto_cipher_new(cipheralg,
+                                           ciphermode,
+                                           masterkey, masterkeylen,
+                                           errp);
+        if (!block->cipher) {
+            ret = -ENOTSUP;
+            goto fail;
+        }
+    }
+
+    block->payload_offset = luks->header.payload_offset;
+
+    g_free(masterkey);
+    g_free(password);
+
+    return 0;
+
+ fail:
+    g_free(masterkey);
+    qcrypto_cipher_free(block->cipher);
+    qcrypto_ivgen_free(block->ivgen);
+    g_free(luks);
+    g_free(password);
+    return ret;
+}
+
+
+static int
+qcrypto_block_luks_create(QCryptoBlock *block,
+                          QCryptoBlockCreateOptions *options,
+                          QCryptoBlockInitFunc initfunc,
+                          QCryptoBlockWriteFunc writefunc,
+                          void *opaque,
+                          Error **errp)
+{
+    QCryptoBlockLUKS *luks;
+    QCryptoBlockCreateOptionsLUKS luks_opts;
+    Error *local_err = NULL;
+    uint8_t *masterkey = NULL;
+    uint8_t *slotkey = NULL;
+    uint8_t *splitkey = NULL;
+    size_t splitkeylen = NULL;
+    size_t i;
+    QCryptoCipher *cipher = NULL;
+    QCryptoIVGen *ivgen = NULL;
+    char *password;
+    const char *cipher_alg;
+    const char *cipher_mode;
+    const char *ivgen_alg;
+    const char *ivgen_hash_alg = NULL;
+    const char *hash_alg;
+    char *cipher_mode_spec = NULL;
+    uuid_t uuid;
+
+    memcpy(&luks_opts, options->u.luks, sizeof(luks_opts));
+    if (!luks_opts.has_cipher_alg) {
+        luks_opts.cipher_alg = QCRYPTO_CIPHER_ALG_AES_256;
+    }
+    if (!luks_opts.has_cipher_mode) {
+        /* XXX switch to XTS */
+        luks_opts.cipher_mode = QCRYPTO_CIPHER_MODE_CBC;
+    }
+    if (!luks_opts.has_ivgen_alg) {
+        luks_opts.ivgen_alg = QCRYPTO_IVGEN_ALG_ESSIV;
+        if (!luks_opts.has_ivgen_hash_alg) {
+            luks_opts.ivgen_hash_alg = QCRYPTO_HASH_ALG_SHA256;
+            luks_opts.has_ivgen_hash_alg = true;
+        }
+    }
+    if (!luks_opts.has_hash_alg) {
+        luks_opts.hash_alg = QCRYPTO_HASH_ALG_SHA256;
+    }
+
+    if (!options->u.luks->key_secret) {
+        error_setg(errp, "Parameter 'key-secret' is required for cipher");
+        return -1;
+    }
+    password = qcrypto_secret_lookup_as_utf8(luks_opts.key_secret, errp);
+    if (!password) {
+        return -1;
+    }
+
+    luks = g_new0(QCryptoBlockLUKS, 1);
+    block->opaque = luks;
+
+    memcpy(luks->header.magic, qcrypto_block_luks_magic,
+           QCRYPTO_BLOCK_LUKS_MAGIC_LEN);
+
+    luks->header.version = QCRYPTO_BLOCK_LUKS_VERSION;
+    uuid_generate(uuid);
+    uuid_unparse(uuid, (char *)luks->header.uuid);
+
+    switch (luks_opts.cipher_alg) {
+    case QCRYPTO_CIPHER_ALG_AES_128:
+    case QCRYPTO_CIPHER_ALG_AES_192:
+    case QCRYPTO_CIPHER_ALG_AES_256:
+        cipher_alg = "aes";
+        break;
+    default:
+        error_setg(errp, "Cipher algorithm is not supported");
+        goto error;
+    }
+
+    cipher_mode = QCryptoCipherMode_lookup[luks_opts.cipher_mode];
+    ivgen_alg = QCryptoIVGenAlgorithm_lookup[luks_opts.ivgen_alg];
+    if (luks_opts.has_ivgen_hash_alg) {
+        ivgen_hash_alg = QCryptoHashAlgorithm_lookup[luks_opts.ivgen_hash_alg];
+        cipher_mode_spec = g_strdup_printf("%s-%s:%s", cipher_mode, ivgen_alg,
+                                           ivgen_hash_alg);
+    } else {
+        cipher_mode_spec = g_strdup_printf("%s-%s", cipher_mode, ivgen_alg);
+    }
+    hash_alg = QCryptoHashAlgorithm_lookup[luks_opts.hash_alg];
+
+
+    if (strlen(cipher_alg) >= QCRYPTO_BLOCK_LUKS_CIPHER_NAME_LEN) {
+        error_setg(errp, "Cipher name '%s' is too long for LUKS header",
+                   cipher_alg);
+        goto error;
+    }
+    if (strlen(cipher_mode_spec) >= QCRYPTO_BLOCK_LUKS_CIPHER_MODE_LEN) {
+        error_setg(errp, "Cipher mode '%s' is too long for LUKS header",
+                   cipher_mode_spec);
+        goto error;
+    }
+    if (strlen(hash_alg) >= QCRYPTO_BLOCK_LUKS_HASH_SPEC_LEN) {
+        error_setg(errp, "Hash name '%s' is too long for LUKS header",
+                   hash_alg);
+        goto error;
+    }
+
+    memcpy(luks->header.cipher_name, cipher_alg,
+           strlen(cipher_alg) + 1);
+    memcpy(luks->header.cipher_mode, cipher_mode_spec,
+           strlen(cipher_mode_spec) + 1);
+    memcpy(luks->header.hash_spec, hash_alg,
+           strlen(hash_alg) + 1);
+
+    luks->header.key_bytes = qcrypto_cipher_get_key_len(luks_opts.cipher_alg);
+
+    /* Generate the salt used for hashing the master key
+     * with PBKDF later
+     */
+    if (qcrypto_random_bytes(luks->header.master_key_salt,
+                             QCRYPTO_BLOCK_LUKS_SALT_LEN,
+                             errp) < 0) {
+        goto error;
+    }
+
+    /* Generate random master key */
+    masterkey = g_new0(uint8_t, luks->header.key_bytes);
+    if (qcrypto_random_bytes(masterkey,
+                             luks->header.key_bytes, errp) < 0) {
+        goto error;
+    }
+
+
+    /* Setup the block device payload encryption objects */
+    block->cipher = qcrypto_cipher_new(luks_opts.cipher_alg,
+                                       luks_opts.cipher_mode,
+                                       masterkey, luks->header.key_bytes,
+                                       errp);
+    if (!block->cipher) {
+        goto error;
+    }
+
+    block->kdfhash = luks_opts.hash_alg;
+    block->niv = qcrypto_cipher_get_iv_len(luks_opts.cipher_alg,
+                                           luks_opts.cipher_mode);
+    block->ivgen = qcrypto_ivgen_new(luks_opts.ivgen_alg,
+                                     luks_opts.cipher_alg,
+                                     luks_opts.ivgen_hash_alg,
+                                     masterkey, luks->header.key_bytes,
+                                     errp);
+
+    if (!block->ivgen) {
+        goto error;
+    }
+
+
+    /* Determine how many iterations we need to hash the master
+     * key with in order to have 1 second of compute time used
+     */
+    luks->header.master_key_iterations =
+        qcrypto_pbkdf2_count_iters(luks_opts.hash_alg,
+                                   masterkey, luks->header.key_bytes,
+                                   luks->header.master_key_salt,
+                                   QCRYPTO_BLOCK_LUKS_SALT_LEN,
+                                   &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        goto error;
+    }
+
+    /* Why /= 8 ?  That matches cryptsetup, but there's no
+     * explanation why they chose /= 8... */
+    luks->header.master_key_iterations /= 8;
+    luks->header.master_key_iterations = MAX(
+        luks->header.master_key_iterations,
+        QCRYPTO_BLOCK_LUKS_MIN_MASTER_KEY_ITERS);
+
+
+    /* Hash the master key, saving the result in the LUKS
+     * header. This hash is used when opening the encrypted
+     * device to verify that the user password unlocked a
+     * valid master key
+     */
+    if (qcrypto_pbkdf2(luks_opts.hash_alg,
+                       masterkey, luks->header.key_bytes,
+                       luks->header.master_key_salt,
+                       QCRYPTO_BLOCK_LUKS_SALT_LEN,
+                       luks->header.master_key_iterations,
+                       luks->header.master_key_digest,
+                       QCRYPTO_BLOCK_LUKS_DIGEST_LEN,
+                       errp) < 0) {
+        goto error;
+    }
+
+
+    /* Although LUKS has multiple key slots, we're just going
+     * to use the first key slot */
+
+    splitkeylen = luks->header.key_bytes * QCRYPTO_BLOCK_LUKS_STRIPES;
+    for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
+        luks->header.key_slots[i].active = i == 0 ?
+            QCRYPTO_BLOCK_LUKS_KEY_SLOT_ENABLED :
+            QCRYPTO_BLOCK_LUKS_KEY_SLOT_DISABLED;
+        luks->header.key_slots[i].stripes = QCRYPTO_BLOCK_LUKS_STRIPES;
+        luks->header.key_slots[i].key_offset =
+            (QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET / 512) +
+            (ROUND_UP(((splitkeylen + 511) / 512),
+                      (QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET / 512)) * i);
+    }
+
+    if (qcrypto_random_bytes(luks->header.key_slots[0].salt,
+                             QCRYPTO_BLOCK_LUKS_SALT_LEN,
+                             errp) < 0) {
+        goto error;
+    }
+
+    /* Again we determine how many iterations are required to
+     * hash the user password while consuming 1 second of compute
+     * time */
+    luks->header.key_slots[0].iterations =
+        qcrypto_pbkdf2_count_iters(luks_opts.hash_alg,
+                                   (uint8_t *)password, strlen(password),
+                                   luks->header.key_slots[0].salt,
+                                   QCRYPTO_BLOCK_LUKS_SALT_LEN,
+                                   &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        goto error;
+    }
+    /* Why /= 2 ?  That matches cryptsetup, but there's no
+     * explanation why they chose /= 2... */
+    luks->header.key_slots[0].iterations /= 2;
+    luks->header.key_slots[0].iterations = MAX(
+        luks->header.key_slots[0].iterations,
+        QCRYPTO_BLOCK_LUKS_MIN_SLOT_KEY_ITERS);
+
+
+    /* Generate a key that we'll use to encrypt the master
+     * key, from the user's password
+     */
+    slotkey = g_new0(uint8_t, luks->header.key_bytes);
+    if (qcrypto_pbkdf2(luks_opts.hash_alg,
+                       (uint8_t *)password, strlen(password),
+                       luks->header.key_slots[0].salt,
+                       QCRYPTO_BLOCK_LUKS_SALT_LEN,
+                       luks->header.key_slots[0].iterations,
+                       slotkey, luks->header.key_bytes,
+                       errp) < 0) {
+        goto error;
+    }
+
+
+    /* Setup the encryption objects needed to encrypt the
+     * master key material
+     */
+    cipher = qcrypto_cipher_new(luks_opts.cipher_alg,
+                                luks_opts.cipher_mode,
+                                slotkey, luks->header.key_bytes,
+                                errp);
+    if (!cipher) {
+        goto error;
+    }
+
+    ivgen = qcrypto_ivgen_new(luks_opts.ivgen_alg,
+                              luks_opts.cipher_alg,
+                              luks_opts.ivgen_hash_alg,
+                              slotkey, luks->header.key_bytes,
+                              errp);
+    if (!ivgen) {
+        goto error;
+    }
+
+    /* Before storing the master key, we need to vastly
+     * increase its size, as protection against forensic
+     * disk data recovery */
+    splitkey = g_new0(uint8_t, splitkeylen);
+
+    if (qcrypto_afsplit_encode(luks_opts.hash_alg,
+                               luks->header.key_bytes,
+                               luks->header.key_slots[0].stripes,
+                               masterkey,
+                               splitkey,
+                               errp) < 0) {
+        goto error;
+    }
+
+    /* Now we encrypt the split master key with the key generated
+     * from the user's password, before storing it */
+    if (qcrypto_block_encrypt_helper(cipher, block->niv, ivgen,
+                                     0,
+                                     splitkey,
+                                     splitkeylen,
+                                     errp) < 0) {
+        goto error;
+    }
+
+
+
+    /* The total size of the LUKS headers is the partition header + key
+     * slot headers, rounded up to the nearest sector, combined with
+     * the size of each master key material region, also rounded up
+     * to the nearest sector */
+    luks->header.payload_offset =
+        (QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET / 512) +
+        (ROUND_UP(((splitkeylen + 511) / 512),
+                  (QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET / 512)) *
+         QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS);
+
+    block->payload_offset = luks->header.payload_offset;
+
+    /* Reserve header space to match payload offset */
+    initfunc(block, block->payload_offset * 512, &local_err, opaque);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        goto error;
+    }
+
+    /* Everything on disk uses Big Endian, so flip header fields
+     * before writing them */
+    cpu_to_be16s(&luks->header.version);
+    cpu_to_be32s(&luks->header.payload_offset);
+    cpu_to_be32s(&luks->header.key_bytes);
+    cpu_to_be32s(&luks->header.master_key_iterations);
+
+    for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
+        cpu_to_be32s(&luks->header.key_slots[i].active);
+        cpu_to_be32s(&luks->header.key_slots[i].iterations);
+        cpu_to_be32s(&luks->header.key_slots[i].key_offset);
+        cpu_to_be32s(&luks->header.key_slots[i].stripes);
+    }
+
+
+    /* Write out the partition header and key slot headers */
+    writefunc(block, 0,
+              (const uint8_t *)&luks->header,
+              sizeof(luks->header),
+              &local_err,
+              opaque);
+
+    /* Delay checking local_err until we've byte-swapped */
+
+    /* Byte swap the header back to native, in case we need
+     * to read it again later */
+    be16_to_cpus(&luks->header.version);
+    be32_to_cpus(&luks->header.payload_offset);
+    be32_to_cpus(&luks->header.key_bytes);
+    be32_to_cpus(&luks->header.master_key_iterations);
+
+    for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
+        be32_to_cpus(&luks->header.key_slots[i].active);
+        be32_to_cpus(&luks->header.key_slots[i].iterations);
+        be32_to_cpus(&luks->header.key_slots[i].key_offset);
+        be32_to_cpus(&luks->header.key_slots[i].stripes);
+    }
+
+    if (local_err) {
+        error_propagate(errp, local_err);
+        goto error;
+    }
+
+    /* Write out the master key material, starting at the
+     * sector immediately following the partition header. */
+    if (writefunc(block,
+                  luks->header.key_slots[0].key_offset * 512,
+                  splitkey, splitkeylen,
+                  errp,
+                  opaque) != splitkeylen) {
+        goto error;
+    }
+
+    memset(masterkey, 0, luks->header.key_bytes);
+    g_free(masterkey);
+    memset(slotkey, 0, luks->header.key_bytes);
+    g_free(slotkey);
+    g_free(splitkey);
+    g_free(password);
+    g_free(cipher_mode_spec);
+
+    qcrypto_ivgen_free(ivgen);
+    qcrypto_cipher_free(cipher);
+
+    return 0;
+
+ error:
+    if (masterkey) {
+        memset(masterkey, 0, luks->header.key_bytes);
+    }
+    g_free(masterkey);
+    if (slotkey) {
+        memset(slotkey, 0, luks->header.key_bytes);
+    }
+    g_free(slotkey);
+    g_free(splitkey);
+    g_free(password);
+    g_free(cipher_mode_spec);
+
+    qcrypto_ivgen_free(ivgen);
+    qcrypto_cipher_free(cipher);
+
+    g_free(luks);
+    return -1;
+}
+
+
+static void qcrypto_block_luks_cleanup(QCryptoBlock *block)
+{
+    g_free(block->opaque);
+}
+
+
+static int
+qcrypto_block_luks_decrypt(QCryptoBlock *block,
+                           uint64_t startsector,
+                           uint8_t *buf,
+                           size_t len,
+                           Error **errp)
+{
+    return qcrypto_block_decrypt_helper(block->cipher,
+                                        block->niv, block->ivgen,
+                                        startsector, buf, len, errp);
+}
+
+
+static int
+qcrypto_block_luks_encrypt(QCryptoBlock *block,
+                           uint64_t startsector,
+                           uint8_t *buf,
+                           size_t len,
+                           Error **errp)
+{
+    return qcrypto_block_encrypt_helper(block->cipher,
+                                        block->niv, block->ivgen,
+                                        startsector, buf, len, errp);
+}
+
+
+const QCryptoBlockDriver qcrypto_block_driver_luks = {
+    .open = qcrypto_block_luks_open,
+    .create = qcrypto_block_luks_create,
+    .cleanup = qcrypto_block_luks_cleanup,
+    .decrypt = qcrypto_block_luks_decrypt,
+    .encrypt = qcrypto_block_luks_encrypt,
+    .has_format = qcrypto_block_luks_has_format,
+};
diff --git a/crypto/block-luks.h b/crypto/block-luks.h
new file mode 100644
index 0000000..0934138
--- /dev/null
+++ b/crypto/block-luks.h
@@ -0,0 +1,28 @@ 
+/*
+ * QEMU Crypto block device encryption LUKS format
+ *
+ * Copyright (c) 2015-2016 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#ifndef QCRYPTO_BLOCK_LUKS_H__
+#define QCRYPTO_BLOCK_LUKS_H__
+
+#include "crypto/blockpriv.h"
+
+extern const QCryptoBlockDriver qcrypto_block_driver_luks;
+
+#endif /* QCRYPTO_BLOCK_LUKS_H__ */
diff --git a/crypto/block.c b/crypto/block.c
index 757e28a..860ecce 100644
--- a/crypto/block.c
+++ b/crypto/block.c
@@ -20,9 +20,11 @@ 
 
 #include "crypto/blockpriv.h"
 #include "crypto/block-qcow.h"
+#include "crypto/block-luks.h"
 
 static const QCryptoBlockDriver *qcrypto_block_drivers[] = {
     [Q_CRYPTO_BLOCK_FORMAT_QCOW] = &qcrypto_block_driver_qcow,
+    [Q_CRYPTO_BLOCK_FORMAT_LUKS] = &qcrypto_block_driver_luks,
 };
 
 
diff --git a/qapi/crypto.json b/qapi/crypto.json
index b4b0a95..5e9014d 100644
--- a/qapi/crypto.json
+++ b/qapi/crypto.json
@@ -102,12 +102,13 @@ 
 #
 # @qcow: QCow/QCow2 built-in AES-CBC encryption. Use only
 #        for liberating data from old images.
+# @luks: LUKS encryption format. Recommended for new images
 #
 # Since: 2.6
 ##
 { 'enum': 'QCryptoBlockFormat',
 #  'prefix': 'QCRYPTO_BLOCK_FORMAT',
-  'data': ['qcow']}
+  'data': ['qcow', 'luks']}
 
 ##
 # QCryptoBlockOptionsBase:
@@ -136,6 +137,40 @@ 
   'data': { '*key-secret': 'str' }}
 
 ##
+# QCryptoBlockOptionsLUKS:
+#
+# The options that apply to LUKS encryption format
+#
+# @key-secret: #optional the ID of a QCryptoSecret object providing the
+#              decryption key
+# Since: 2.6
+##
+{ 'struct': 'QCryptoBlockOptionsLUKS',
+  'data': { '*key-secret': 'str' }}
+
+
+##
+# QCryptoBlockCreateOptionsLUKS:
+#
+# The options that apply to LUKS encryption format initialization
+#
+# @cipher-alg: #optional the cipher algorithm for data encryption
+# @cipher-mode: #optional the cipher mode for data encryption
+# @ivgen-alg: #optional the initialization vector generator
+# @ivgen-hash-alg: #optional the initialization vector generator hash
+# @hash-alg: #optional the master key hash algorithm
+# Since: 2.6
+##
+{ 'struct': 'QCryptoBlockCreateOptionsLUKS',
+  'base': 'QCryptoBlockOptionsLUKS',
+  'data': { '*cipher-alg': 'QCryptoCipherAlgorithm',
+            '*cipher-mode': 'QCryptoCipherMode',
+            '*ivgen-alg': 'QCryptoIVGenAlgorithm',
+            '*ivgen-hash-alg': 'QCryptoHashAlgorithm',
+            '*hash-alg': 'QCryptoHashAlgorithm'}}
+
+
+##
 # QCryptoBlockOpenOptions:
 #
 # The options that are available for all encryption formats
@@ -146,7 +181,8 @@ 
 { 'union': 'QCryptoBlockOpenOptions',
   'base': 'QCryptoBlockOptionsBase',
   'discriminator': 'format',
-  'data': { 'qcow': 'QCryptoBlockOptionsQCow' } }
+  'data': { 'qcow': 'QCryptoBlockOptionsQCow',
+            'luks': 'QCryptoBlockOptionsLUKS' } }
 
 
 ##
@@ -160,4 +196,5 @@ 
 { 'union': 'QCryptoBlockCreateOptions',
   'base': 'QCryptoBlockOptionsBase',
   'discriminator': 'format',
-  'data': { 'qcow': 'QCryptoBlockOptionsQCow' } }
+  'data': { 'qcow': 'QCryptoBlockOptionsQCow',
+            'luks': 'QCryptoBlockCreateOptionsLUKS' } }
diff --git a/tests/test-crypto-block.c b/tests/test-crypto-block.c
index 54216e9..5fe5d05 100644
--- a/tests/test-crypto-block.c
+++ b/tests/test-crypto-block.c
@@ -40,6 +40,69 @@  static QCryptoBlockOpenOptions qcow_open_opts = {
     .u.qcow = &qcow_opts,
 };
 
+
+static QCryptoBlockOptionsLUKS luks_open_opts_int = {
+    .has_key_secret = true,
+    .key_secret = (char *)"sec0",
+};
+
+static QCryptoBlockOpenOptions luks_open_opts = {
+    .format = Q_CRYPTO_BLOCK_FORMAT_LUKS,
+    .u.luks = &luks_open_opts_int,
+};
+
+
+/* Creation with all default values */
+static QCryptoBlockCreateOptionsLUKS luks_create_opts_default_int = {
+    .has_key_secret = true,
+    .key_secret = (char *)"sec0",
+};
+
+static QCryptoBlockCreateOptions luks_create_opts_default = {
+    .format = Q_CRYPTO_BLOCK_FORMAT_LUKS,
+    .u.luks = &luks_create_opts_default_int,
+};
+
+
+static QCryptoBlockCreateOptionsLUKS luks_create_opts_aes256_cbc_plain64_int = {
+    .has_key_secret = true,
+    .key_secret = (char *)"sec0",
+    .has_cipher_alg = true,
+    .cipher_alg = QCRYPTO_CIPHER_ALG_AES_256,
+    .has_cipher_mode = true,
+    .cipher_mode = QCRYPTO_CIPHER_MODE_CBC,
+    .has_ivgen_alg = true,
+    .ivgen_alg = QCRYPTO_IVGEN_ALG_PLAIN64,
+};
+
+static QCryptoBlockCreateOptions luks_create_opts_aes256_cbc_plain64 = {
+    .format = Q_CRYPTO_BLOCK_FORMAT_LUKS,
+    .u.luks = &luks_create_opts_aes256_cbc_plain64_int,
+};
+
+
+
+static QCryptoBlockCreateOptionsLUKS luks_create_opts_aes256_cbc_essiv_int = {
+    .has_key_secret = true,
+    .key_secret = (char *)"sec0",
+    .has_cipher_alg = true,
+    .cipher_alg = QCRYPTO_CIPHER_ALG_AES_256,
+    .has_cipher_mode = true,
+    .cipher_mode = QCRYPTO_CIPHER_MODE_CBC,
+    .has_ivgen_alg = true,
+    .ivgen_alg = QCRYPTO_IVGEN_ALG_ESSIV,
+    .has_ivgen_hash_alg = true,
+    .ivgen_hash_alg = QCRYPTO_HASH_ALG_SHA256,
+    .has_hash_alg = true,
+    .hash_alg = QCRYPTO_HASH_ALG_SHA1,
+};
+
+static QCryptoBlockCreateOptions luks_create_opts_aes256_cbc_essiv = {
+    .format = Q_CRYPTO_BLOCK_FORMAT_LUKS,
+    .u.luks = &luks_create_opts_aes256_cbc_essiv_int,
+};
+
+
 static struct QCryptoBlockTestData {
     const char *path;
     QCryptoBlockCreateOptions *create_opts;
@@ -66,6 +129,47 @@  static struct QCryptoBlockTestData {
 
         .ivgen_alg = QCRYPTO_IVGEN_ALG_PLAIN64,
     },
+    {
+        .path = "/crypto/block/luks/default",
+        .create_opts = &luks_create_opts_default,
+        .open_opts = &luks_open_opts,
+
+        .expect_header = true,
+
+        .cipher_alg = QCRYPTO_CIPHER_ALG_AES_256,
+        .cipher_mode = QCRYPTO_CIPHER_MODE_CBC,
+        .hash_alg = QCRYPTO_HASH_ALG_SHA256,
+
+        .ivgen_alg = QCRYPTO_IVGEN_ALG_ESSIV,
+        .ivgen_hash = QCRYPTO_HASH_ALG_SHA256,
+    },
+    {
+        .path = "/crypto/block/luks/aes-256-cbc-plain64",
+        .create_opts = &luks_create_opts_aes256_cbc_plain64,
+        .open_opts = &luks_open_opts,
+
+        .expect_header = true,
+
+        .cipher_alg = QCRYPTO_CIPHER_ALG_AES_256,
+        .cipher_mode = QCRYPTO_CIPHER_MODE_CBC,
+        .hash_alg = QCRYPTO_HASH_ALG_SHA256,
+
+        .ivgen_alg = QCRYPTO_IVGEN_ALG_PLAIN64,
+    },
+    {
+        .path = "/crypto/block/luks/aes-256-cbc-eesiv",
+        .create_opts = &luks_create_opts_aes256_cbc_essiv,
+        .open_opts = &luks_open_opts,
+
+        .expect_header = true,
+
+        .cipher_alg = QCRYPTO_CIPHER_ALG_AES_256,
+        .cipher_mode = QCRYPTO_CIPHER_MODE_CBC,
+        .hash_alg = QCRYPTO_HASH_ALG_SHA1,
+
+        .ivgen_alg = QCRYPTO_IVGEN_ALG_ESSIV,
+        .ivgen_hash = QCRYPTO_HASH_ALG_SHA256,
+    },
 };