Re: [CRYPTO-LUKS v1 03/19] cryptodisk: Incorrect calculation of start sector for grub_disk_read in grub_cryptodisk_read.

On Sun, Aug 23, 2020 at 11:31:46PM -0500, Glenn Washburn wrote: > On Sun, 23 Aug 2020 12:39:03 +0200 > Patrick Steinhardt wrote: > > > On Fri, Jul 31, 2020 at 07:01:44AM -0500, Glenn Washburn wrote: > > > Here dev is a grub_cryptodisk_t and dev->offset is offset in > > > sectors of size native

Re: [CRYPTO-LUKS v1 03/19] cryptodisk: Incorrect calculation of start sector for grub_disk_read in grub_cryptodisk_read.

On Sun, 23 Aug 2020 12:39:03 +0200 Patrick Steinhardt wrote: > On Fri, Jul 31, 2020 at 07:01:44AM -0500, Glenn Washburn wrote: > > Here dev is a grub_cryptodisk_t and dev->offset is offset in > > sectors of size native to the cryptodisk device. The sector is > > correctly transformed into native

Re: [PATCH] v6 for detached headers and key files

On Sat, 22 Aug 2020 00:25:18 +0200 Denis 'GNUtoo' Carikli wrote: > On Wed, 19 Aug 2020 13:59:57 -0500 > Glenn Washburn wrote: > > > I'm curious, are you using a virtual machine to test grub? If I > > understand correctly the above, you're using physical machines to > > test. I'm using qemu and

Re: [PATCH 2/9] luks: Fix out-of-bounds copy of UUID

On Sun, 23 Aug 2020 12:59:57 +0200 Patrick Steinhardt wrote: > When configuring a LUKS disk, we copy over the UUID from the LUKS > header into the new `grub_cryptodisk_t` structure via `grub_memcpy > ()`. As size we mistakenly use the size of the `grub_cryptodisk_t` > UUID field, which is

[PATCH 8/9] cryptodisk: Fix cipher IV mode 'plain64' always being set as 'plain'

From: Glenn Washburn Signed-off-by: Glenn Washburn Reviewed-by: Patrick Steinhardt --- grub-core/disk/cryptodisk.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c index 76ec2d4b9..74ab54eaa 100644 ---

[PATCH 9/9] cryptodisk: Properly handle non-512 byte sized sectors

From: Glenn Washburn By default, dm-crypt internally uses an IV that corresponds to 512-byte sectors, even when a larger sector size is specified. What this means is that when using a larger sector size, the IV is incremented every sector. However, the amount the IV is incremented is the number

[PATCH 6/9] cryptodisk: Unregister cryptomount command when removing module

From: Glenn Washburn Signed-off-by: Glenn Washburn Reviewed-by: Patrick Steinhardt --- grub-core/disk/cryptodisk.c | 1 + 1 file changed, 1 insertion(+) diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c index 1897acc4b..b2c6e9a7d 100644 --- a/grub-core/disk/cryptodisk.c

[PATCH 7/9] cryptodisk: Incorrect calculation of start sector for grub_disk_read in grub_cryptodisk_read

From: Glenn Washburn Here dev is a grub_cryptodisk_t and dev->offset is offset in sectors of size native to the cryptodisk device. The sector is correctly transformed into native grub sector size, but then added to dev->offset which is not transformed. It would be nice if the type system would

[PATCH 4/9] luks2: grub_cryptodisk_t->total_length is the max number of device native sectors

From: Glenn Washburn The total_length field is named confusingly because length usually refers to bytes, whereas in this case its really the total number of sectors on the device. Also counter-intuitively, grub_disk_get_size returns the total number of device native sectors sectors. We need to

[PATCH 5/9] luks2: Improve error reporting when decrypting/verifying key

While we already set up error messages in both `luks2_verify_key()` and `luks2_decrypt_key()`, we do not ever print them. This makes it really hard to discover why a given key actually failed to decrypt a disk. Improve this by including the error message in the user-visible output.

[PATCH 3/9] luks2: Fix use of incorrect index and some error messages

From: Glenn Washburn Signed-off-by: Glenn Washburn Reviewed-by: Patrick Steinhardt --- grub-core/disk/luks2.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c index e3ff7c83d..200f81d3a 100644 ---

[PATCH 1/9] json: Remove invalid typedef redefinition

The C standard does not allow for typedef redefinitions, even if they map to the same underlying type. In order to avoid including the "jsmn.h" in "json.h" and thus exposing jsmn's internals, we have exactly such a forward-declaring typedef in "json.h". If enforcing the GNU99 C standard, clang may

[PATCH 0/9] Cryptodisk fixes for v2.06

Hi, I've sifted through the mailing list contents of the last few months to cherry-pick cryptodisk bugfixes which I think should be included in the v2.06 release. I've found the following 9 patches from Glenn and me which should probably be included, separated them out from their respective patch

[PATCH 2/9] luks: Fix out-of-bounds copy of UUID

When configuring a LUKS disk, we copy over the UUID from the LUKS header into the new `grub_cryptodisk_t` structure via `grub_memcpy ()`. As size we mistakenly use the size of the `grub_cryptodisk_t` UUID field, which is guaranteed to be strictly bigger than the LUKS UUID field we're copying. As a

Re: [CRYPTO-LUKS v1 03/19] cryptodisk: Incorrect calculation of start sector for grub_disk_read in grub_cryptodisk_read.

On Fri, Jul 31, 2020 at 07:01:44AM -0500, Glenn Washburn wrote: > Here dev is a grub_cryptodisk_t and dev->offset is offset in sectors of size > native to the cryptodisk device. The sector is correctly transformed into > native grub sector size, but then added to dev->offset which is not >

Re: [CRYPTO-LUKS v1 07/19] luks2: grub_cryptodisk_t->total_length is the max number of device native sectors.

On Fri, Jul 31, 2020 at 07:01:48AM -0500, Glenn Washburn wrote: > The total_length field is named confusingly because length usually > refers to bytes, whereas in this case its really the total number of > sectors on the device. Also counter-intuitively, grub_disk_get_size > returns the total