Re: [Qemu-devel] [PATCH v2 05/17] crypto: add support for anti-forensic split algorithm

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 7037 bytes --]

On 01/20/2016 10:38 AM, Daniel P. Berrange wrote:
> The LUKS format specifies an anti-forensic split algorithm which
> is used to artificially expand the size of the key material on
> disk. This is an implementation of that algorithm.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  crypto/Makefile.objs        |   1 +
>  crypto/afsplit.c            | 162 ++++++++++++++++++++++++++++++++++++++++
>  include/crypto/afsplit.h    | 135 +++++++++++++++++++++++++++++++++
>  tests/.gitignore            |   1 +
>  tests/Makefile              |   2 +
>  tests/test-crypto-afsplit.c | 176 ++++++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 477 insertions(+)
>  create mode 100644 crypto/afsplit.c
>  create mode 100644 include/crypto/afsplit.h
>  create mode 100644 tests/test-crypto-afsplit.c
> 

In addition to Fam's findings,

> +++ b/crypto/afsplit.c
> @@ -0,0 +1,162 @@
> +/*
> + * QEMU Crypto anti forensic information splitter
> + *
> + * Copyright (c) 2015-2016 Red Hat, Inc.
> + *
> + * Derived from cryptsetup package lib/lusk1/af.c
> + *
> + * Copyright (C) 2004, Clemens Fruhwirth <clemens@endorphin.org>
> + * Copyright (C) 2009-2012, Red Hat, Inc. All rights reserved.
> + *

> +
> +static void qcrypto_afsplit_xor(size_t blocklen,
> +                                const uint8_t *in1,
> +                                const uint8_t *in2,
> +                                uint8_t *out)
> +{
> +    size_t i;
> +    for (i = 0; i < blocklen; i++) {
> +        out[i] = in1[i] ^ in2[i];
> +    }

Could be made faster using larger data types, if we know that things are
properly aligned.  I'm not sure if the compiler can automatically
optimize this.  I'm also not sure if this is ever on a hot path where it
matters.

> +}
> +
> +
> +static int qcrypto_afsplit_hash(QCryptoHashAlgorithm hash,
> +                                size_t blocklen,
> +                                uint8_t *block,
> +                                Error **errp)
> +{
> +    size_t digestlen = qcrypto_hash_digest_len(hash);
> +
> +    size_t hashcount = blocklen / digestlen;
> +    size_t finallen = blocklen % digestlen;
> +    uint32_t i;
> +
> +    if (finallen) {
> +        hashcount++;
> +    } else {
> +        finallen = blocklen;
> +    }
> +
> +    for (i = 0; i < hashcount; i++) {
> +        uint8_t *out = NULL;
> +        size_t outlen = 0;
> +        uint32_t iv = cpu_to_be32(i);
> +        struct iovec in[] = {
> +            { .iov_base = &iv,
> +              .iov_len = sizeof(iv) },
> +            { .iov_base = block + (i * digestlen),
> +              .iov_len = (i == (hashcount - 1)) ? finallen : digestlen },
> +        };
> +
> +        if (qcrypto_hash_bytesv(hash,
> +                                in,
> +                                G_N_ELEMENTS(in),
> +                                &out, &outlen,
> +                                errp) < 0) {
> +            return -1;
> +        }
> +
> +        if (outlen != digestlen) {
> +            error_setg(errp, "Hash output %zu not %zu",
> +                       outlen, digestlen);
> +            g_free(out);

Is this error even possible, or can it be an assert?

> +            return -1;
> +        }
> +        memcpy(block + (i * digestlen), out,
> +               (i == (hashcount - 1)) ? finallen : digestlen);
> +        g_free(out);
> +    }
> +
> +    return 0;
> +}
> +
> +


> 
> +int qcrypto_afsplit_encode(QCryptoHashAlgorithm hash,
> +                           size_t blocklen,
> +                           uint32_t stripes,
> +                           const uint8_t *in,
> +                           uint8_t *out,
> +                           Error **errp)
> +{
> +    uint8_t *block = g_new0(uint8_t, blocklen);
> +    size_t i;

> 
> +
> +        qcrypto_afsplit_xor(blocklen,
> +                            out + (i * blocklen),
> +                            block,
> +                            block);

Should we ensure no overflow in the multiplication here (or rather, that
the input blocklen * stripes is reasonable on input)?


> +++ b/include/crypto/afsplit.h

> +
> +/**
> + * This module implements the anti-forensic splitter that is specified
> + * as part of the LUKS format:
> + *
> + *   http://clemens.endorphin.org/cryptography
> + *   http://clemens.endorphin.org/TKS1-draft.pdf
> + *
> + * The core idea is to take a short piece of data (key material)
> + * and process it to expand it to a much larger piece of data.
> + * The expansion process is reversable, to obtain the original

s/reversable/reversible/

> + * short data. The key property of the expansion is that if any
> + * byte in the larger data set is changed / missing, it should be
> + * impossible to recreate the original short data.
> + *

> +
> +/**
> + * qcrypto_afsplit_encode:
> + * @hash: the hash algorithm to use for data expansion
> + * @blocklen: the size of @in in bytes
> + * @stripes: the number of times to expand @in in size
> + * @in: the master key to be expanded in size
> + * @out: preallocted buffer to hold the split key

s/preallocted/preallocated/

> + * @errp: pointer to a NULL-initialized error object
> + *
> + * Split the data in @in, which is @blocklen bytes in
> + * size, to form a larger piece of data @out, which is
> + * @blocklen * @stripes bytes in size.
> + *
> + * Returns: 0 on success, -1 on error;
> + */
> +int qcrypto_afsplit_encode(QCryptoHashAlgorithm hash,
> +                           size_t blocklen,
> +                           uint32_t stripes,
> +                           const uint8_t *in,
> +                           uint8_t *out,
> +                           Error **errp);
> +
> +/**
> + * qcrypto_afsplit_decode:
> + * @hash: the hash algorithm to use for data compression
> + * @blocklen: the size of @out in bytes
> + * @stripes: the number of times to decrease @in in size
> + * @in: the master key to be expanded in size
> + * @out: preallocted buffer to hold the split key

and again

> + * @errp: pointer to a NULL-initialized error object
> + *
> + * Join the data in @in, which is @blocklen * @stripes
> + * bytes in size, to form the original small piece o
> + * data @out, which is @blocklen bytes in size.
> + *
> + * Returns: 0 on success, -1 on error;
> + */
> +int qcrypto_afsplit_decode(QCryptoHashAlgorithm hash,
> +                           size_t blocklen,
> +                           uint32_t stripes,
> +                           const uint8_t *in,
> +                           uint8_t *out,
> +                           Error **errp);

Markus may have an opinion on whether these functions could return void
and just use the errp pointer to report errors; but I'm fine with how
you've done it.


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]