diff mbox series

dm-crypt: Allow to specify the integrity key size as option

Message ID 20240816112133.2100537-1-ifranzki@linux.ibm.com (mailing list archive)
State Accepted, archived
Delegated to: Mikulas Patocka
Headers show
Series dm-crypt: Allow to specify the integrity key size as option | expand

Commit Message

Ingo Franzki Aug. 16, 2024, 11:21 a.m. UTC
For the MAC based integrity operation, the integrity key size (i.e.
key_mac_size) is currently set to the digest size of the used digest.

For wrapped key HMAC algorithms, the key size is independent of the
cryptographic key size. So there is no known size of the mac key in
such cases. The desired key size can optionally be specified as argument
when the dm-crypt device is configured via 'integrity_key_size:%u'.
If no integrity_key_size argument is specified, the mac key size
is still set to the digest size, as before.

Increase version number to 1.28.0 so that support for the new
argument can be detected by user space (i.e. cryptsetup).

Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
---
 Documentation/admin-guide/device-mapper/dm-crypt.rst |  4 ++++
 drivers/md/dm-crypt.c                                | 11 +++++++++--
 2 files changed, 13 insertions(+), 2 deletions(-)

Comments

Mikulas Patocka Aug. 20, 2024, 3:56 p.m. UTC | #1
On Fri, 16 Aug 2024, Ingo Franzki wrote:

> For the MAC based integrity operation, the integrity key size (i.e.
> key_mac_size) is currently set to the digest size of the used digest.
> 
> For wrapped key HMAC algorithms, the key size is independent of the
> cryptographic key size. So there is no known size of the mac key in
> such cases. The desired key size can optionally be specified as argument
> when the dm-crypt device is configured via 'integrity_key_size:%u'.
> If no integrity_key_size argument is specified, the mac key size
> is still set to the digest size, as before.
> 
> Increase version number to 1.28.0 so that support for the new
> argument can be detected by user space (i.e. cryptsetup).

Hi

I know you already discussed it with Milan. I'd like to ask, what's the 
reason for this patch? Milan said that you need it for mainframes - 
please, describe the specific configuration when this patch is needed.

Mikulas


> Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
> ---
>  Documentation/admin-guide/device-mapper/dm-crypt.rst |  4 ++++
>  drivers/md/dm-crypt.c                                | 11 +++++++++--
>  2 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/admin-guide/device-mapper/dm-crypt.rst b/Documentation/admin-guide/device-mapper/dm-crypt.rst
> index e625830d335e..636b47c582f0 100644
> --- a/Documentation/admin-guide/device-mapper/dm-crypt.rst
> +++ b/Documentation/admin-guide/device-mapper/dm-crypt.rst
> @@ -160,6 +160,10 @@ iv_large_sectors
>     The <iv_offset> must be multiple of <sector_size> (in 512 bytes units)
>     if this flag is specified.
>  
> +integrity_key_size:<bytes>
> +   Use an integrity key of <bytes> size instead of using an integrity key size
> +   of the digest size of the used HMAC algorithm.
> +
>  
>  Module parameters::
>  max_read_size
> diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
> index 348b4b26c272..c4c706115870 100644
> --- a/drivers/md/dm-crypt.c
> +++ b/drivers/md/dm-crypt.c
> @@ -2937,7 +2937,8 @@ static int crypt_ctr_auth_cipher(struct crypt_config *cc, char *cipher_api)
>  	if (IS_ERR(mac))
>  		return PTR_ERR(mac);
>  
> -	cc->key_mac_size = crypto_ahash_digestsize(mac);
> +	if (!cc->key_mac_size)
> +		cc->key_mac_size = crypto_ahash_digestsize(mac);
>  	crypto_free_ahash(mac);
>  
>  	cc->authenc_key = kmalloc(crypt_authenckey_size(cc), GFP_KERNEL);
> @@ -3219,6 +3220,12 @@ static int crypt_ctr_optional(struct dm_target *ti, unsigned int argc, char **ar
>  			cc->cipher_auth = kstrdup(sval, GFP_KERNEL);
>  			if (!cc->cipher_auth)
>  				return -ENOMEM;
> +		} else if (sscanf(opt_string, "integrity_key_size:%u", &val) == 1) {
> +			if (val == 0) {
> +				ti->error = "Invalid integrity_key_size argument";
> +				return -EINVAL;
> +			}
> +			cc->key_mac_size = val;
>  		} else if (sscanf(opt_string, "sector_size:%hu%c", &cc->sector_size, &dummy) == 1) {
>  			if (cc->sector_size < (1 << SECTOR_SHIFT) ||
>  			    cc->sector_size > 4096 ||
> @@ -3758,7 +3765,7 @@ static void crypt_io_hints(struct dm_target *ti, struct queue_limits *limits)
>  
>  static struct target_type crypt_target = {
>  	.name   = "crypt",
> -	.version = {1, 27, 0},
> +	.version = {1, 28, 0},
>  	.module = THIS_MODULE,
>  	.ctr    = crypt_ctr,
>  	.dtr    = crypt_dtr,
> -- 
> 2.43.0
>
Ingo Franzki Aug. 21, 2024, 7:48 a.m. UTC | #2
On 20.08.2024 17:56, Mikulas Patocka wrote:
> 
> 
> On Fri, 16 Aug 2024, Ingo Franzki wrote:
> 
>> For the MAC based integrity operation, the integrity key size (i.e.
>> key_mac_size) is currently set to the digest size of the used digest.
>>
>> For wrapped key HMAC algorithms, the key size is independent of the
>> cryptographic key size. So there is no known size of the mac key in
>> such cases. The desired key size can optionally be specified as argument
>> when the dm-crypt device is configured via 'integrity_key_size:%u'.
>> If no integrity_key_size argument is specified, the mac key size
>> is still set to the digest size, as before.
>>
>> Increase version number to 1.28.0 so that support for the new
>> argument can be detected by user space (i.e. cryptsetup).
> 
> Hi
> 
> I know you already discussed it with Milan. I'd like to ask, what's the 
> reason for this patch? Milan said that you need it for mainframes - 
> please, describe the specific configuration when this patch is needed.
> 
> Mikulas

Hi Mikulas,

thanks for looking into this.

In short: Yes, we need it for a new function on Linux on IBM Z platform (aka s390x), but the general concept of using wrapped keys is not limited to that platform but can be used by other platforms as well. Furthermore, the proposed change can also be beneficial for clear key HMAC integrity protection, to allow choosing the size of the integrity key.

As a little background:

Quite some time ago I worked with Milan to add support for using HSM (Hardware Security Module) protected keys (or in general wrapped keys) with dm-crypt and cryptsetup.

HSM protected keys are keys that are wrapped (i.e. encrypted) by an HSM master key. This increases security because such keys can only be used with the proper HSM being available, and the clear key values of such keys are never revealed in clear outside of the HSM. LUKS does a very good job in protecting the volume keys, but once the volume has been opened the key appears in clear in kernel memory. When using HSM protected keys, only the wrapped key appears in memory, but never the clear key value. 

Such HSM protected keys usually have a physical size that is much larger than a clear key. We usually speak about a key blob here, because only the HSM really understands how to use that key blob, for anything outside of the HSM it is just an opaque key blob. The point is, that the physical size of such HSM protected keys are not directly related to the cryptographic size of the key they contain.

For the Linux on IBM Z platform (aka s390x) we provide a special kernel cipher named 'PAES' that implements the AES algorithm but takes such HSM protected key blobs as argument. While this works transparently with dm-crypt, it needed some changes in the cprytpsetup tooling, because cryptsetup assumed that the physical size of a key relates directly to its cryptographic size, which is not true for such HSM protected keys. Milan was so kind to make the required changes back then in cryptsetup to make this work. 

So, with this, one can now setup an encrypted disk using plain or LUKS2 by specifying a cipher like 'paes-xts-plain64' to benefit from the added security of HSM protected keys. If you like, you can read more about this here: https://public.dhe.ibm.com/software/dw/linux390/docu/l5n3dc04.pdf 


Reasons for the proposed change in dm-crypt: 

We now want to provide a similar functionality for integrity protected volumes. Similar to PAES, we provide a kernel HMAC cipher named 'PHMAC' that implements the HMAC algorithm but takes HSM protected key blobs as argument. So, one would use a cipher like 'phmac-sha256' to setup an integrity protected disk to benefit from the added security of HSM protected keys.

The use of such HSM protected HMAC keys work transparently with the dm-integrity target (only needs some changes in the user space tooling). However, there is also the combined encrypt and integrity protection capability of dm-crypt, either using HMAC or AEAD. We focus on HMAC based integrity protection here. And for that, we do need the proposed change in dm-crypt.

In case of combined encryption and HMAC based integrity, dm-crypt gets a single key blob and needs to split it into two parts. The first part is the encryption key, the second part is the integrity key. Currently, dm-crypt assumes that the size of the integrity key part is the digest-size of the HMAC algorithm. With this, the integrity key size is known, and the 2nd part of the key blob can be split off and used as integrity key. The remaining first part is used as encryption key. 

While with this logic, the encryption key can already be an opaque key blob (i.e. a HSM protected key blob) of any length, the integrity key part can not, because its size is assumed to be directly related to the HAMC digest used. 

To support opaque integrity key blobs as well, the size of the integrity key must be passed to dm-crypt from user space. This needs changes in the cryptsetup tool as well, I am working with Milan on this. If an integrity key size is specified in the dm-crypt parameters, it uses that size to split the key into the 2 parts, otherwise it uses the digest size as before.

With this, both, the encryption key as well as the integrity key can then be opaque HSM protected key blobs of arbitrary sizes. 
So one can use a command like the following to format a encrypted and integrity protected volume using HSM protected keys:

# cryptsetup luksFormat --type luks2 --master-key-file <volume-key> --key-size <encrypt-key-size-in-bits> --cipher paes-xts-plain64 --integrity phmac-sha256 <device>


As mentioned above, the proposed change is not directly related to a specific platform, although Linux on IBM Z (s390x) is the first to exploit this. It could be utilized by any platform for wrapped key ciphers. It can also be used for clear key HMAC integrity when one wants to use an HMAC key size that is different from the digest size. 


I hope this answers your questions. 

Kind regards, Ingo


[snip]
Mikulas Patocka Aug. 22, 2024, 4:21 p.m. UTC | #3
On Wed, 21 Aug 2024, Ingo Franzki wrote:

> On 20.08.2024 17:56, Mikulas Patocka wrote:
> > 
> > 
> > On Fri, 16 Aug 2024, Ingo Franzki wrote:
> > 
> >> For the MAC based integrity operation, the integrity key size (i.e.
> >> key_mac_size) is currently set to the digest size of the used digest.
> >>
> >> For wrapped key HMAC algorithms, the key size is independent of the
> >> cryptographic key size. So there is no known size of the mac key in
> >> such cases. The desired key size can optionally be specified as argument
> >> when the dm-crypt device is configured via 'integrity_key_size:%u'.
> >> If no integrity_key_size argument is specified, the mac key size
> >> is still set to the digest size, as before.
> >>
> >> Increase version number to 1.28.0 so that support for the new
> >> argument can be detected by user space (i.e. cryptsetup).
> > 
> > Hi
> > 
> > I know you already discussed it with Milan. I'd like to ask, what's the 
> > reason for this patch? Milan said that you need it for mainframes - 
> > please, describe the specific configuration when this patch is needed.
> > 
> > Mikulas
> 
> Hi Mikulas,
> 
> thanks for looking into this.
> 
> In short: Yes, we need it for a new function on Linux on IBM Z platform 
> (aka s390x), but the general concept of using wrapped keys is not 
> limited to that platform but can be used by other platforms as well. 
> Furthermore, the proposed change can also be beneficial for clear key 
> HMAC integrity protection, to allow choosing the size of the integrity 
> key.

Hi

Thanks for the explanation. I discussed it with Milan and we concluded 
that the patch is OK and that we can stage it for the kernel 6.12.

I added the patch to the device mapper repository. You can get it from 
"git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git", 
checkout branch "remotes/origin/dm-6.12".

I fixed two bugs in the patch:
1. crypt_status must report the new argument in its table line
2. sscanf(opt_string, "integrity_key_size:%u"...) should really be 
   sscanf(opt_string, "integrity_key_size:%u%c"...), so that we report 
   syntax error if there are trailing characters after the number.

Please, download the updated patch from the "linux-dm.git" repository and 
test it.

Mikulas
Ingo Franzki Aug. 23, 2024, 7:26 a.m. UTC | #4
On 22.08.2024 18:21, Mikulas Patocka wrote:
> 
> 
> On Wed, 21 Aug 2024, Ingo Franzki wrote:
> 
>> On 20.08.2024 17:56, Mikulas Patocka wrote:
>>>
>>>
>>> On Fri, 16 Aug 2024, Ingo Franzki wrote:
>>>
>>>> For the MAC based integrity operation, the integrity key size (i.e.
>>>> key_mac_size) is currently set to the digest size of the used digest.
>>>>
>>>> For wrapped key HMAC algorithms, the key size is independent of the
>>>> cryptographic key size. So there is no known size of the mac key in
>>>> such cases. The desired key size can optionally be specified as argument
>>>> when the dm-crypt device is configured via 'integrity_key_size:%u'.
>>>> If no integrity_key_size argument is specified, the mac key size
>>>> is still set to the digest size, as before.
>>>>
>>>> Increase version number to 1.28.0 so that support for the new
>>>> argument can be detected by user space (i.e. cryptsetup).
>>>
>>> Hi
>>>
>>> I know you already discussed it with Milan. I'd like to ask, what's the 
>>> reason for this patch? Milan said that you need it for mainframes - 
>>> please, describe the specific configuration when this patch is needed.
>>>
>>> Mikulas
>>
>> Hi Mikulas,
>>
>> thanks for looking into this.
>>
>> In short: Yes, we need it for a new function on Linux on IBM Z platform 
>> (aka s390x), but the general concept of using wrapped keys is not 
>> limited to that platform but can be used by other platforms as well. 
>> Furthermore, the proposed change can also be beneficial for clear key 
>> HMAC integrity protection, to allow choosing the size of the integrity 
>> key.
> 
> Hi
> 
> Thanks for the explanation. I discussed it with Milan and we concluded 
> that the patch is OK and that we can stage it for the kernel 6.12.
> 
> I added the patch to the device mapper repository. You can get it from 
> "git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git", 
> checkout branch "remotes/origin/dm-6.12".
> 
> I fixed two bugs in the patch:
> 1. crypt_status must report the new argument in its table line
> 2. sscanf(opt_string, "integrity_key_size:%u"...) should really be 
>    sscanf(opt_string, "integrity_key_size:%u%c"...), so that we report 
>    syntax error if there are trailing characters after the number.
> 
> Please, download the updated patch from the "linux-dm.git" repository and 
> test it.

Thanks for fixing these bugs. They all make very much sense. 

I have downloaded the patch from https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-6.12&id=4441686b24a1d7acf9834ca95864d67e3f97666a and tested it successfully on my system. Works great. 'dmsetup table' now also shows the 'integrity_key_size' option. 

@Milan: I noticed that now that dm-crypt also reports the integrity_key_size param in the status query, I need to also support that in my cryptsetup changes (i.e. in function _dm_target_query_crypt() in lib/libdevmapper.c). With this additional change, luksFormat and friends work fine with the updated dm-crypt module.

> 
> Mikulas
>
diff mbox series

Patch

diff --git a/Documentation/admin-guide/device-mapper/dm-crypt.rst b/Documentation/admin-guide/device-mapper/dm-crypt.rst
index e625830d335e..636b47c582f0 100644
--- a/Documentation/admin-guide/device-mapper/dm-crypt.rst
+++ b/Documentation/admin-guide/device-mapper/dm-crypt.rst
@@ -160,6 +160,10 @@  iv_large_sectors
    The <iv_offset> must be multiple of <sector_size> (in 512 bytes units)
    if this flag is specified.
 
+integrity_key_size:<bytes>
+   Use an integrity key of <bytes> size instead of using an integrity key size
+   of the digest size of the used HMAC algorithm.
+
 
 Module parameters::
 max_read_size
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 348b4b26c272..c4c706115870 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -2937,7 +2937,8 @@  static int crypt_ctr_auth_cipher(struct crypt_config *cc, char *cipher_api)
 	if (IS_ERR(mac))
 		return PTR_ERR(mac);
 
-	cc->key_mac_size = crypto_ahash_digestsize(mac);
+	if (!cc->key_mac_size)
+		cc->key_mac_size = crypto_ahash_digestsize(mac);
 	crypto_free_ahash(mac);
 
 	cc->authenc_key = kmalloc(crypt_authenckey_size(cc), GFP_KERNEL);
@@ -3219,6 +3220,12 @@  static int crypt_ctr_optional(struct dm_target *ti, unsigned int argc, char **ar
 			cc->cipher_auth = kstrdup(sval, GFP_KERNEL);
 			if (!cc->cipher_auth)
 				return -ENOMEM;
+		} else if (sscanf(opt_string, "integrity_key_size:%u", &val) == 1) {
+			if (val == 0) {
+				ti->error = "Invalid integrity_key_size argument";
+				return -EINVAL;
+			}
+			cc->key_mac_size = val;
 		} else if (sscanf(opt_string, "sector_size:%hu%c", &cc->sector_size, &dummy) == 1) {
 			if (cc->sector_size < (1 << SECTOR_SHIFT) ||
 			    cc->sector_size > 4096 ||
@@ -3758,7 +3765,7 @@  static void crypt_io_hints(struct dm_target *ti, struct queue_limits *limits)
 
 static struct target_type crypt_target = {
 	.name   = "crypt",
-	.version = {1, 27, 0},
+	.version = {1, 28, 0},
 	.module = THIS_MODULE,
 	.ctr    = crypt_ctr,
 	.dtr    = crypt_dtr,