Re: [Qemu-block] [PATCH v2 09/13] qcrypto-block: extract check and parse header
On Mon, Aug 26, 2019 at 04:50:59PM +0300, Maxim Levitsky wrote: > This is just to make qcrypto_block_luks_open more > reasonable in size. > > Signed-off-by: Maxim Levitsky > --- > crypto/block-luks.c | 254 +--- > 1 file changed, 146 insertions(+), 108 deletions(-) > > diff --git a/crypto/block-luks.c b/crypto/block-luks.c > index b4dc6fc899..cc9a52c9af 100644 > --- a/crypto/block-luks.c > +++ b/crypto/block-luks.c > @@ -508,6 +508,148 @@ fail: > return ret; > } > > +/* > + * Does basic sanity checks on the LUKS header > + */ > +static int > +qcrypto_block_luks_check_header(const QCryptoBlockLUKS *luks, Error **errp) > +{ > +int ret; > + > +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; > +} Just 'return -1' here immediately - don't return an errno, as we're using Error objects for reporting. > + > +if (luks->header.version != QCRYPTO_BLOCK_LUKS_VERSION) { > +error_setg(errp, "LUKS version %" PRIu32 " is not supported", > + luks->header.version); > +ret = -ENOTSUP; > +goto fail; > +} > + > +return 0; > +fail: > +return ret; > +} > + > +/* > + * Parses the crypto parameters that are stored in the LUKS header > + */ > + > +static int > +qcrypto_block_luks_parse_header(QCryptoBlockLUKS *luks, Error **errp) > +{ > +g_autofree char *cipher_mode = g_strdup(luks->header.cipher_mode); > +char *ivgen_name, *ivhash_name; > +int ret = -1; > +Error *local_err = NULL; > + > +/* > + * The cipher_mode header contains a string that we have > + * to further parse, of the format > + * > + *-[:] > + * > + * eg cbc-essiv:sha256, cbc-plain64 > + */ > +ivgen_name = strchr(cipher_mode, '-'); > +if (!ivgen_name) { > +ret = -EINVAL; Again, don't use errnos - just return -1 in this method. > +error_setg(errp, "Unexpected cipher mode string format %s", > + luks->header.cipher_mode); > +goto out; > +} > +*ivgen_name = '\0'; > +ivgen_name++; > + > +ivhash_name = strchr(ivgen_name, ':'); > +if (!ivhash_name) { > +luks->ivgen_hash_alg = 0; > +} else { > +*ivhash_name = '\0'; > +ivhash_name++; > + > +luks->ivgen_hash_alg = > qcrypto_block_luks_hash_name_lookup(ivhash_name, > + > &local_err); > +if (local_err) { > +ret = -ENOTSUP; > +error_propagate(errp, local_err); > +goto out; > +} > +} > + > +luks->cipher_mode = qcrypto_block_luks_cipher_mode_lookup(cipher_mode, > + &local_err); > +if (local_err) { > +ret = -ENOTSUP; > +error_propagate(errp, local_err); > +goto out; > +} > + > +luks->cipher_alg = > +qcrypto_block_luks_cipher_name_lookup(luks->header.cipher_name, > + luks->cipher_mode, > + > luks->header.master_key_len, > + &local_err); > +if (local_err) { > +ret = -ENOTSUP; > +error_propagate(errp, local_err); > +goto out; > +} > + > +luks->hash_alg = > +qcrypto_block_luks_hash_name_lookup(luks->header.hash_spec, > + &local_err); > +if (local_err) { > +ret = -ENOTSUP; > +error_propagate(errp, local_err); > +goto out; > +} > + > +luks->ivgen_alg = qcrypto_block_luks_ivgen_name_lookup(ivgen_name, > + &local_err); > +if (local_err) { > +ret = -ENOTSUP; > +error_propagate(errp, local_err); > +goto out; > +} > + > +if (luks->ivgen_alg == QCRYPTO_IVGEN_ALG_ESSIV) { > +if (!ivhash_name) { > +ret = -EINVAL; > +error_setg(errp, "Missing IV generator hash specification"); > +goto out; > +} > +luks->ivgen_cipher_alg = > +qcrypto_block_luks_essiv_cipher(luks->cipher_alg, > +luks->ivgen_hash_alg, > +&local_err); > +if (local_err) { > +ret = -ENOTSUP; > +error_propagate(errp, local_err); > +goto out; > +} > +} else { > + > +/* > + * Note we parsed the ivhash_name earlier in the cipher_mode > + * spec string even with plain/plain64 ivgens, but we > + * will ignore it, since it is irrelevant for these ivgens. > + * This is for compat with
[Qemu-block] [PATCH v2 09/13] qcrypto-block: extract check and parse header
This is just to make qcrypto_block_luks_open more reasonable in size. Signed-off-by: Maxim Levitsky --- crypto/block-luks.c | 254 +--- 1 file changed, 146 insertions(+), 108 deletions(-) diff --git a/crypto/block-luks.c b/crypto/block-luks.c index b4dc6fc899..cc9a52c9af 100644 --- a/crypto/block-luks.c +++ b/crypto/block-luks.c @@ -508,6 +508,148 @@ fail: return ret; } +/* + * Does basic sanity checks on the LUKS header + */ +static int +qcrypto_block_luks_check_header(const QCryptoBlockLUKS *luks, Error **errp) +{ +int ret; + +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; +} + +return 0; +fail: +return ret; +} + +/* + * Parses the crypto parameters that are stored in the LUKS header + */ + +static int +qcrypto_block_luks_parse_header(QCryptoBlockLUKS *luks, Error **errp) +{ +g_autofree char *cipher_mode = g_strdup(luks->header.cipher_mode); +char *ivgen_name, *ivhash_name; +int ret = -1; +Error *local_err = NULL; + +/* + * The cipher_mode header contains a string that we have + * to further parse, of the format + * + *-[:] + * + * eg cbc-essiv:sha256, cbc-plain64 + */ +ivgen_name = strchr(cipher_mode, '-'); +if (!ivgen_name) { +ret = -EINVAL; +error_setg(errp, "Unexpected cipher mode string format %s", + luks->header.cipher_mode); +goto out; +} +*ivgen_name = '\0'; +ivgen_name++; + +ivhash_name = strchr(ivgen_name, ':'); +if (!ivhash_name) { +luks->ivgen_hash_alg = 0; +} else { +*ivhash_name = '\0'; +ivhash_name++; + +luks->ivgen_hash_alg = qcrypto_block_luks_hash_name_lookup(ivhash_name, + &local_err); +if (local_err) { +ret = -ENOTSUP; +error_propagate(errp, local_err); +goto out; +} +} + +luks->cipher_mode = qcrypto_block_luks_cipher_mode_lookup(cipher_mode, + &local_err); +if (local_err) { +ret = -ENOTSUP; +error_propagate(errp, local_err); +goto out; +} + +luks->cipher_alg = +qcrypto_block_luks_cipher_name_lookup(luks->header.cipher_name, + luks->cipher_mode, + luks->header.master_key_len, + &local_err); +if (local_err) { +ret = -ENOTSUP; +error_propagate(errp, local_err); +goto out; +} + +luks->hash_alg = +qcrypto_block_luks_hash_name_lookup(luks->header.hash_spec, + &local_err); +if (local_err) { +ret = -ENOTSUP; +error_propagate(errp, local_err); +goto out; +} + +luks->ivgen_alg = qcrypto_block_luks_ivgen_name_lookup(ivgen_name, + &local_err); +if (local_err) { +ret = -ENOTSUP; +error_propagate(errp, local_err); +goto out; +} + +if (luks->ivgen_alg == QCRYPTO_IVGEN_ALG_ESSIV) { +if (!ivhash_name) { +ret = -EINVAL; +error_setg(errp, "Missing IV generator hash specification"); +goto out; +} +luks->ivgen_cipher_alg = +qcrypto_block_luks_essiv_cipher(luks->cipher_alg, +luks->ivgen_hash_alg, +&local_err); +if (local_err) { +ret = -ENOTSUP; +error_propagate(errp, local_err); +goto out; +} +} else { + +/* + * Note we parsed the ivhash_name earlier in the cipher_mode + * spec string even with plain/plain64 ivgens, but we + * will ignore it, since it is irrelevant for these ivgens. + * This is for compat with dm-crypt which will silently + * ignore hash names with these ivgens rather than report + * an error about the invalid usage + */ +luks->ivgen_cipher_alg = luks->cipher_alg; +} +ret = 0; +out: +return ret; + +} + /* * Given a key slot, and user password, this will attempt to unlock * the master encryption key from the key slot. @@ -720,12 +862,9 @@ qcrypto_block_luks_open(QCryptoBlock *block, Error **errp) { QCryptoBlockLUKS *luks = NULL; -Err