linux-kernel - Re:[RFC 1/1] Add dm verity root hash pkcs7 sig validation.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Date:   Mon, 3 Jun 2019 13:08:45 -0700 (PDT)
From:   jaskarankhurana@...ux.microsoft.com
To:     linux-kernel@...r.kernel.org,
        linux-security-module@...r.kernel.org, gmazyland@...il.com
cc:     agk@...hat.com, snitzer@...hat.com, dm-deval@...hat.com,
        jmorris@...ei.org, scottsh@...rosoft.com
Subject: Re:[RFC 1/1] Add dm verity root hash pkcs7 sig validation.



On Tue, 21 May 2019, Milan Broz wrote:
> On 20/05/2019 23:54, Jaskaran Khurana wrote:
>> Adds in-kernel pkcs7 signature checking for the roothash of
>> the dm-verity hash tree.
>>
>>
>> Adds DM_VERITY_VERIFY_ROOTHASH_SIG_FORCE: roothash signature *must* be
>> specified for all dm verity volumes and verification must succeed prior
>> to creation of device mapper block device.
>
> I am not sure this is a good idea. If I understand it correctly, this will
> block creating another dm-verity mappings without PKCS7 signature, and these
> are used in many other environments and applications that could possibly
> run on that system later.
>
> (But I have no idea how to solve it better though :-)
>
> ...
>
>> +	/* Root hash signature is  a optional parameter*/
>> +	r = verity_verify_root_hash(root_hash_digest_to_validate,
>> +				    strlen(root_hash_digest_to_validate),
>> +				    verify_args.sig,
>> +				    verify_args.sig_size);
>> +	if (r < 0) {
>> +		ti->error = "Root hash verification failed";
>> +		goto bad;
>> +	}
>
> You are sending the PKCS7 signature as a (quite large) binary blob inside the mapping table.
>
> I am not sure if it is possible here (I guess so), but why not put this it kernel keyring
> and then just reference it from mapping table?
> (We use kernel keyring in libcryptsetup already for dm-crypt.)
>
> It will also solve an issue in userspace patch, when you are reading the signature
> file too late (devices can be suspended in that moment, so I would prefer to download
> sig file to keyring in advance, and then just reference it in mapping table).
>
> (I guess you will send merge request for veritysetup userspace part later.)

I have made the changes for passing the signature bytes using the keyring 
and I had sent an updated patch for the same last week. I have given a 
link to the veritysetup changes which I used to test this in the patch and I will 
cleanup and send those for review next.

Please take a look and provide code review feedback for the kernel 
changes.

>
> Milan
>
Regards,
Jaskaran