qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v9 0/4] delete created files when block_crypto_co_create_opts


From: Daniel Henrique Barboza
Subject: Re: [PATCH v9 0/4] delete created files when block_crypto_co_create_opts_luks fails
Date: Wed, 4 Mar 2020 11:36:04 -0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0

Ping

On 1/30/20 6:39 PM, Daniel Henrique Barboza wrote:
The version 8 of this patch series got buried and it's now
conflicting with master. Rebase and re-sending it.

Also, I contemplated the idea of moving/copying the password
verification in qcrypto_block_luks_create() all the way back to the
start of block_crypto_co_create_opts_luks(), failing early before the
bdrv_create_file(), avoiding the problem altogether without the
need of a delete_file API I'm trying to push here (see patch 03
commit message for detailed info about the bug).

This idea was dropped after I saw that:

- We would need to store the resulting password, now being retrieved
early in block_crypto_co_create_opts_luks(), in a new
QCryptoBlockCreateOptions string to be used inside
qcrypto_block_luks_create() as intended. An alternative would be to
call qcrypto_secret_lookup_as_utf8() twice, discarding the first
string;

- There are a lot of ways to fail in qcrypto_block_luks_create()
other than a non-UTF8 password that would trigger the same problem.
A more appropiate way of doing what I intended, instead of
copying/hacking code around to fail before bdrv_create(), is some sort
of bdrv_validate() API that would encapsulate everything that is
related to user input validation for the security drivers. This
API could then be called before the file creation (maybe inside
bdrv_create itself) and fail early if needed. This is too overkill
for what I'm trying to fix here, and I'm not sure if it would be
a net gain compared to the delete_file API.


All that said, I believe that this patch series presents a sane
solution with the code we have ATM.


changes in this version:
- rebase with current master at 204aa60b37
- previous version:
https://lists.gnu.org/archive/html/qemu-devel/2019-11/msg01551.html


Daniel Henrique Barboza (4):
   block: introducing 'bdrv_co_delete_file' interface
   block.c: adding bdrv_co_delete_file
   crypto.c: cleanup created file when block_crypto_co_create_opts_luks
     fails
   qemu-iotests: adding LUKS cleanup for non-UTF8 secret error

  block.c                    | 26 +++++++++++++++
  block/crypto.c             | 18 ++++++++++
  block/file-posix.c         | 23 +++++++++++++
  include/block/block.h      |  1 +
  include/block/block_int.h  |  4 +++
  tests/qemu-iotests/282     | 67 ++++++++++++++++++++++++++++++++++++++
  tests/qemu-iotests/282.out | 11 +++++++
  tests/qemu-iotests/group   |  1 +
  8 files changed, 151 insertions(+)
  create mode 100755 tests/qemu-iotests/282
  create mode 100644 tests/qemu-iotests/282.out




reply via email to

[Prev in Thread] Current Thread [Next in Thread]