* [dm-devel] [PATCH] dm-crypt: make printing of the key constant-time
@ 2022-04-24 20:55 Mikulas Patocka
2022-04-25 12:53 ` [dm-devel] [PATCH v2] " Mikulas Patocka
0 siblings, 1 reply; 5+ messages in thread
From: Mikulas Patocka @ 2022-04-24 20:55 UTC (permalink / raw
To: Mike Snitzer; +Cc: dm-devel, Milan Broz
The device mapper dm-crypt target is using scnprintf("%02x", cc->key[i]) to
report the current key to userspace. However, this is not constant-time
operation and it may leak information about the key via timing, via cache
access patterns or via the branch predictor.
This patch changes it to use "%c" instead. We introduce a function
hex2asc. hex2asc converts a number in the range 0 ... 15 to an ascii
character and it is coded in such a way that it contains no branches and
no memory accesses.
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Cc; stable@vger.kernel.org
---
drivers/md/dm-crypt.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
Index: linux-2.6/drivers/md/dm-crypt.c
===================================================================
--- linux-2.6.orig/drivers/md/dm-crypt.c 2022-04-24 19:44:14.000000000 +0200
+++ linux-2.6/drivers/md/dm-crypt.c 2022-04-24 19:54:13.000000000 +0200
@@ -3439,6 +3439,11 @@ static int crypt_map(struct dm_target *t
return DM_MAPIO_SUBMITTED;
}
+static char hex2asc(unsigned char c)
+{
+ return c + '0' + ((9 - c) >> 4 & 0x27);
+}
+
static void crypt_status(struct dm_target *ti, status_type_t type,
unsigned status_flags, char *result, unsigned maxlen)
{
@@ -3459,7 +3464,7 @@ static void crypt_status(struct dm_targe
DMEMIT(":%u:%s", cc->key_size, cc->key_string);
else
for (i = 0; i < cc->key_size; i++)
- DMEMIT("%02x", cc->key[i]);
+ DMEMIT("%c%c", hex2asc(cc->key[i] >> 4), hex2asc(cc->key[i] & 0xf));
} else
DMEMIT("-");
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* [dm-devel] [PATCH v2] dm-crypt: make printing of the key constant-time
2022-04-24 20:55 [dm-devel] [PATCH] dm-crypt: make printing of the key constant-time Mikulas Patocka
@ 2022-04-25 12:53 ` Mikulas Patocka
2022-05-04 14:41 ` Milan Broz
2022-05-08 4:40 ` Demi Marie Obenour
0 siblings, 2 replies; 5+ messages in thread
From: Mikulas Patocka @ 2022-04-25 12:53 UTC (permalink / raw
To: Mike Snitzer; +Cc: dm-devel, Milan Broz
This is the second version of this patch. It casts the value to unsigned
before doing the shift, because right shift of a negative number is not
defined.
Mikulas
From: Mikulas Patocka <mpatocka@redhat.com>
The device mapper dm-crypt target is using scnprintf("%02x", cc->key[i]) to
report the current key to userspace. However, this is not constant-time
operation and it may leak information about the key via timing, via cache
access patterns or via the branch predictor.
This patch changes it to use "%c" instead. We introduce a function
hex2asc. hex2asc converts a number in the range 0 ... 15 to an ascii
character and it is coded in such a way that it contains no branches and
no memory accesses.
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Cc; stable@vger.kernel.org
---
drivers/md/dm-crypt.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
Index: linux-2.6/drivers/md/dm-crypt.c
===================================================================
--- linux-2.6.orig/drivers/md/dm-crypt.c 2022-04-25 14:41:15.000000000 +0200
+++ linux-2.6/drivers/md/dm-crypt.c 2022-04-25 14:42:06.000000000 +0200
@@ -3439,6 +3439,11 @@ static int crypt_map(struct dm_target *t
return DM_MAPIO_SUBMITTED;
}
+static char hex2asc(unsigned char c)
+{
+ return c + '0' + ((unsigned)(9 - c) >> 4 & 0x27);
+}
+
static void crypt_status(struct dm_target *ti, status_type_t type,
unsigned status_flags, char *result, unsigned maxlen)
{
@@ -3459,7 +3464,7 @@ static void crypt_status(struct dm_targe
DMEMIT(":%u:%s", cc->key_size, cc->key_string);
else
for (i = 0; i < cc->key_size; i++)
- DMEMIT("%02x", cc->key[i]);
+ DMEMIT("%c%c", hex2asc(cc->key[i] >> 4), hex2asc(cc->key[i] & 0xf));
} else
DMEMIT("-");
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dm-devel] [PATCH v2] dm-crypt: make printing of the key constant-time
2022-04-25 12:53 ` [dm-devel] [PATCH v2] " Mikulas Patocka
@ 2022-05-04 14:41 ` Milan Broz
2022-05-08 4:40 ` Demi Marie Obenour
1 sibling, 0 replies; 5+ messages in thread
From: Milan Broz @ 2022-05-04 14:41 UTC (permalink / raw
To: Mikulas Patocka, Mike Snitzer; +Cc: dm-devel
On 25/04/2022 14:53, Mikulas Patocka wrote:
> This is the second version of this patch. It casts the value to unsigned
> before doing the shift, because right shift of a negative number is not
> defined.
>
> Mikulas
>
>
>
> From: Mikulas Patocka <mpatocka@redhat.com>
>
> The device mapper dm-crypt target is using scnprintf("%02x", cc->key[i]) to
> report the current key to userspace. However, this is not constant-time
> operation and it may leak information about the key via timing, via cache
> access patterns or via the branch predictor.
>
> This patch changes it to use "%c" instead. We introduce a function
> hex2asc. hex2asc converts a number in the range 0 ... 15 to an ascii
> character and it is coded in such a way that it contains no branches and
> no memory accesses.
>
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> Cc; stable@vger.kernel.org
I have run some tests with it, also cryptsetup testsuite is fine.
If it helps, you can add
Tested-by: Milan Broz <gmazyland@gmail.com>
Milan
>
> ---
> drivers/md/dm-crypt.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> Index: linux-2.6/drivers/md/dm-crypt.c
> ===================================================================
> --- linux-2.6.orig/drivers/md/dm-crypt.c 2022-04-25 14:41:15.000000000 +0200
> +++ linux-2.6/drivers/md/dm-crypt.c 2022-04-25 14:42:06.000000000 +0200
> @@ -3439,6 +3439,11 @@ static int crypt_map(struct dm_target *t
> return DM_MAPIO_SUBMITTED;
> }
>
> +static char hex2asc(unsigned char c)
> +{
> + return c + '0' + ((unsigned)(9 - c) >> 4 & 0x27);
> +}
> +
> static void crypt_status(struct dm_target *ti, status_type_t type,
> unsigned status_flags, char *result, unsigned maxlen)
> {
> @@ -3459,7 +3464,7 @@ static void crypt_status(struct dm_targe
> DMEMIT(":%u:%s", cc->key_size, cc->key_string);
> else
> for (i = 0; i < cc->key_size; i++)
> - DMEMIT("%02x", cc->key[i]);
> + DMEMIT("%c%c", hex2asc(cc->key[i] >> 4), hex2asc(cc->key[i] & 0xf));
> } else
> DMEMIT("-");
>
>
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dm-devel] [PATCH v2] dm-crypt: make printing of the key constant-time
2022-04-25 12:53 ` [dm-devel] [PATCH v2] " Mikulas Patocka
@ 2022-05-08 4:40 ` Demi Marie Obenour
2022-05-08 4:40 ` Demi Marie Obenour
1 sibling, 0 replies; 5+ messages in thread
From: Demi Marie Obenour @ 2022-05-08 4:40 UTC (permalink / raw
To: Mikulas Patocka, Mike Snitzer; +Cc: dm-devel, Milan Broz, stable
[-- Attachment #1: Type: text/plain, Size: 321 bytes --]
Nit: did you mean CC: stable@vger.kernel.org (colon instead of
semicolon)? Also, would it be better to avoid using the formatting
built-in to DMEMIT()? Finally, are there any architectures for which
gcc or clang make this non-constant-time?
--
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dm-devel] [PATCH v2] dm-crypt: make printing of the key constant-time
@ 2022-05-08 4:40 ` Demi Marie Obenour
0 siblings, 0 replies; 5+ messages in thread
From: Demi Marie Obenour @ 2022-05-08 4:40 UTC (permalink / raw
To: Mikulas Patocka, Mike Snitzer; +Cc: dm-devel, Milan Broz, stable
[-- Attachment #1.1: Type: text/plain, Size: 321 bytes --]
Nit: did you mean CC: stable@vger.kernel.org (colon instead of
semicolon)? Also, would it be better to avoid using the formatting
built-in to DMEMIT()? Finally, are there any architectures for which
gcc or clang make this non-constant-time?
--
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 98 bytes --]
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-05-09 7:53 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-04-24 20:55 [dm-devel] [PATCH] dm-crypt: make printing of the key constant-time Mikulas Patocka
2022-04-25 12:53 ` [dm-devel] [PATCH v2] " Mikulas Patocka
2022-05-04 14:41 ` Milan Broz
2022-05-08 4:40 ` Demi Marie Obenour
2022-05-08 4:40 ` Demi Marie Obenour
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.