2019-07-01 18:24:51

by Jaskaran Singh Khurana

[permalink] [raw]
Subject: [RFC PATCH v6 0/1] Add dm verity root hash pkcs7 sig validation.

Changes in v6:

Address comments from Milan Broz and Eric Biggers on v5.

-Keep the verification code under config DM_VERITY_VERIFY_ROOTHASH_SIG.

-Change the command line parameter to requires_signatures(bool) which will
force root hash to be signed and trusted if specified.

-Fix the signature not being present in verity_status. Merged the
https://git.kernel.org/pub/scm/linux/kernel/git/mbroz/linux.git/commit/?h=dm-cryptsetup&id=a26c10806f5257e255b6a436713127e762935ad3
made by Milan Broz and tested it.


Jaskaran Khurana (1):
Add dm verity root hash pkcs7 sig validation.

Documentation/device-mapper/verity.txt | 7 ++
drivers/md/Kconfig | 12 +++
drivers/md/Makefile | 5 +
drivers/md/dm-verity-target.c | 43 +++++++-
drivers/md/dm-verity-verify-sig.c | 133 +++++++++++++++++++++++++
drivers/md/dm-verity-verify-sig.h | 60 +++++++++++
drivers/md/dm-verity.h | 2 +
7 files changed, 257 insertions(+), 5 deletions(-)
create mode 100644 drivers/md/dm-verity-verify-sig.c
create mode 100644 drivers/md/dm-verity-verify-sig.h

--
2.17.1


2019-07-12 17:34:59

by Jaskaran Singh Khurana

[permalink] [raw]
Subject: Re: [RFC PATCH v6 0/1] Add dm verity root hash pkcs7 sig validation.


Hello Milan,

> Changes in v6:
>
> Address comments from Milan Broz and Eric Biggers on v5.
>
> -Keep the verification code under config DM_VERITY_VERIFY_ROOTHASH_SIG.
>
> -Change the command line parameter to requires_signatures(bool) which will
> force root hash to be signed and trusted if specified.
>
> -Fix the signature not being present in verity_status. Merged the
> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fmbroz%2Flinux.git%2Fcommit%2F%3Fh%3Ddm-cryptsetup%26id%3Da26c10806f5257e255b6a436713127e762935ad3&data=02%7C01%7CJaskaran.Khurana%40microsoft.com%7C18f92445e46940aeebb008d6fe50c610%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636976020210890638&sdata=aY0V9%2FBz2RHryIvoftGKUGnyPp9Fsc1JY4FZbHfW4hg%3D&reserved=0
> made by Milan Broz and tested it.
>
>

Could you please provide feedback on this v6 version.

Regards,
Jaskaran

2019-07-16 13:01:15

by Milan Broz

[permalink] [raw]
Subject: Re: [RFC PATCH v6 0/1] Add dm verity root hash pkcs7 sig validation.

On 12/07/2019 19:33, Jaskaran Singh Khurana wrote:
>
> Hello Milan,
>
>> Changes in v6:
>>
>> Address comments from Milan Broz and Eric Biggers on v5.
>>
>> -Keep the verification code under config DM_VERITY_VERIFY_ROOTHASH_SIG.
>>
>> -Change the command line parameter to requires_signatures(bool) which will
>> force root hash to be signed and trusted if specified.
>>
>> -Fix the signature not being present in verity_status. Merged the
>> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fmbroz%2Flinux.git%2Fcommit%2F%3Fh%3Ddm-cryptsetup%26id%3Da26c10806f5257e255b6a436713127e762935ad3&data=02%7C01%7CJaskaran.Khurana%40microsoft.com%7C18f92445e46940aeebb008d6fe50c610%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636976020210890638&sdata=aY0V9%2FBz2RHryIvoftGKUGnyPp9Fsc1JY4FZbHfW4hg%3D&reserved=0
>> made by Milan Broz and tested it.
>>
>>
>
> Could you please provide feedback on this v6 version.

Hi,

I am ok with the v6 patch; I think Mike will return to it in 5.4 reviews.

But the documentation is very brief. I spent quite a long time to configure the system properly.
I think you should add more description (at least to patch header) how to use this feature in combination with system keyring.

Do I understand correctly that these steps need to be done?

- user configures a certificate and adds it in kernel builtin keyring (I used CONFIG_SYSTEM_TRUSTED_KEYS option).
- the dm-verity device root hash is signed directly by a key of this cert
- the signature is uploaded to the user keyring
- reference to signature in keyring is added as an optional dm-verity table parameter root_hash_sig_key_desc
- optionally, require_signatures dm-verity module is set to enforce signatures.

For reference, below is the bash script I used (with unpatched veritysetup to generate working DM table), is the expected workflow here?

#!/bin/bash

NAME=test
DEV=/dev/sdc
DEV_HASH=/dev/sdd
ROOT_HASH=778fccab393842688c9af89cfd0c5cde69377cbe21ed439109ec856f2aa8a423
SIGN=sign.txt
SIGN_NAME=verity:$NAME

# get unsigned device-mapper table using unpatched veritysetup
veritysetup open $DEV $NAME $DEV_HASH $ROOT_HASH
TABLE=$(dmsetup table $NAME)
veritysetup close $NAME

# Generate self-signed CA key, must be in .config as CONFIG_SYSTEM_TRUSTED_KEYS="path/ca.pem"
#openssl req -x509 -newkey rsa:1024 -keyout ca_key.pem -out ca.pem -nodes -days 365 -set_serial 01 -subj /CN=example.com

# sign root hash directly by CA cert
echo -n $ROOT_HASH | openssl smime -sign -nocerts -noattr -binary -inkey ca_key.pem -signer ca.pem -outform der -out $SIGN

# load signature to keyring
keyctl padd user $SIGN_NAME @u <$SIGN

# add device-mapper table, now with sighed root hash optional argument
dmsetup create -r $NAME --table "$TABLE 2 root_hash_sig_key_desc $SIGN_NAME"
dmsetup table $NAME

# cleanup
dmsetup remove $NAME
keyctl clear @u


Milan

2019-07-16 18:09:23

by Jaskaran Singh Khurana

[permalink] [raw]
Subject: Re: [RFC PATCH v6 0/1] Add dm verity root hash pkcs7 sig validation.


Hello Milan,
On Tue, 16 Jul 2019, Milan Broz wrote:

> On 12/07/2019 19:33, Jaskaran Singh Khurana wrote:
>>
>> Hello Milan,
>>
>>> Changes in v6:
>>>
>>> Address comments from Milan Broz and Eric Biggers on v5.
>>>
>>> -Keep the verification code under config DM_VERITY_VERIFY_ROOTHASH_SIG.
>>>
>>> -Change the command line parameter to requires_signatures(bool) which will
>>> force root hash to be signed and trusted if specified.
>>>
>>> -Fix the signature not being present in verity_status. Merged the
>>> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fmbroz%2Flinux.git%2Fcommit%2F%3Fh%3Ddm-cryptsetup%26id%3Da26c10806f5257e255b6a436713127e762935ad3&amp;data=02%7C01%7CJaskaran.Khurana%40microsoft.com%7C18f92445e46940aeebb008d6fe50c610%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636976020210890638&amp;sdata=aY0V9%2FBz2RHryIvoftGKUGnyPp9Fsc1JY4FZbHfW4hg%3D&amp;reserved=0
>>> made by Milan Broz and tested it.
>>>
>>>
>>
>> Could you please provide feedback on this v6 version.
>
> Hi,
>
> I am ok with the v6 patch; I think Mike will return to it in 5.4 reviews.
>

Thanks for the help and also for reviewing this patch. Could you please
add Reviewed-by/Tested-by tag to the patch.

> But the documentation is very brief. I spent quite a long time to configure the system properly.
> I think you should add more description (at least to patch header) how to use this feature in combination with system keyring.
>

I will add more documentation to the patch header describing the steps
required for setup.

> Do I understand correctly that these steps need to be done?
>
> - user configures a certificate and adds it in kernel builtin keyring (I used CONFIG_SYSTEM_TRUSTED_KEYS option).
> - the dm-verity device root hash is signed directly by a key of this cert
> - the signature is uploaded to the user keyring
> - reference to signature in keyring is added as an optional dm-verity table parameter root_hash_sig_key_desc
> - optionally, require_signatures dm-verity module is set to enforce signatures.
>
> For reference, below is the bash script I used (with unpatched veritysetup to generate working DM table), is the expected workflow here?

The steps and workflow is correct. I will send the cryptsetup changes for
review.

>
> #!/bin/bash
>
> NAME=test
> DEV=/dev/sdc
> DEV_HASH=/dev/sdd
> ROOT_HASH=778fccab393842688c9af89cfd0c5cde69377cbe21ed439109ec856f2aa8a423
> SIGN=sign.txt
> SIGN_NAME=verity:$NAME
>
> # get unsigned device-mapper table using unpatched veritysetup
> veritysetup open $DEV $NAME $DEV_HASH $ROOT_HASH
> TABLE=$(dmsetup table $NAME)
> veritysetup close $NAME
>
> # Generate self-signed CA key, must be in .config as CONFIG_SYSTEM_TRUSTED_KEYS="path/ca.pem"
> #openssl req -x509 -newkey rsa:1024 -keyout ca_key.pem -out ca.pem -nodes -days 365 -set_serial 01 -subj /CN=example.com
>
> # sign root hash directly by CA cert
> echo -n $ROOT_HASH | openssl smime -sign -nocerts -noattr -binary -inkey ca_key.pem -signer ca.pem -outform der -out $SIGN
>
> # load signature to keyring
> keyctl padd user $SIGN_NAME @u <$SIGN
>
> # add device-mapper table, now with sighed root hash optional argument
> dmsetup create -r $NAME --table "$TABLE 2 root_hash_sig_key_desc $SIGN_NAME"
> dmsetup table $NAME
>
> # cleanup
> dmsetup remove $NAME
> keyctl clear @u
>
>

Thanks for testing the changes and all the guidance here.

> Milan
>
Regards,
Jaskaran.

2019-07-17 13:08:59

by Milan Broz

[permalink] [raw]
Subject: Re: [RFC PATCH v6 0/1] Add dm verity root hash pkcs7 sig validation.

Hi,

On 16/07/2019 20:08, Jaskaran Singh Khurana wrote:
>>> Could you please provide feedback on this v6 version.
>>
>> Hi,
>>
>> I am ok with the v6 patch; I think Mike will return to it in 5.4 reviews.
>>
>
> Thanks for the help and also for reviewing this patch. Could you please
> add Reviewed-by/Tested-by tag to the patch.

ok, you can add
Tested-and-Reviewed-by: Milan Broz <[email protected]>

or just use the version on my git, I already updated few lines because
of recent kernel changes, mainly the revert of keyring changes, tested patch is here

https://git.kernel.org/pub/scm/linux/kernel/git/mbroz/linux.git/commit/?h=dm-cryptsetup&id=266f7c9c74b23e4cb2e67ceb813dd707061c1641
...

> The steps and workflow is correct. I will send the cryptsetup changes for
> review.

ok, I'll probably try to use our existing userspace libcryptsetup API to avoid introducing new calls,
but that is not important for now - the kernel bits must be in the mainline kernel first.

Thanks,
Milan