linux-kernel - Re: Conflict with Mickaël Salaün's blacklist patches [was [PATCH v5 0/4] Add EFI_CERT_X509

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 9 Feb 2021 22:53:34 +0100
From:   Mickaël Salaün <mic@...ikod.net>
To:     Eric Snowberg <eric.snowberg@...cle.com>,
        David Howells <dhowells@...hat.com>
Cc:     dwmw2@...radead.org, Jarkko Sakkinen <jarkko@...nel.org>,
        James.Bottomley@...senPartnership.com, masahiroy@...nel.org,
        michal.lkml@...kovi.net, jmorris@...ei.org, serge@...lyn.com,
        ardb@...nel.org, Mimi Zohar <zohar@...ux.ibm.com>,
        lszubowi@...hat.com, javierm@...hat.com, keyrings@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-kbuild@...r.kernel.org,
        linux-security-module@...r.kernel.org,
        Tyler Hicks <tyhicks@...ux.microsoft.com>
Subject: Re: Conflict with Mickaël Salaün's blacklist patches [was [PATCH v5 0/4] Add EFI_CERT_X509_GUID support for dbx/mokx entries]


On 09/02/2021 00:05, Eric Snowberg wrote:
> 
>> On Feb 6, 2021, at 11:30 AM, Mickaël Salaün <mic@...ikod.net> wrote:
>>
>> On 06/02/2021 02:14, Eric Snowberg wrote:
>>
>>> I have done some additional testing, I am seeing a regression. The blacklist 
>>> keyring is no longer picking up any of the hashes from the dbx during boot. 
>>> I backed out the merge with my changes  (fdbbe7ceeb95090d09c33ce0497e0394c82aa33d) 
>>> and still see the regression.  I then backed out Mickaël merge
>>> (5bf1adccf5c41dbdd51d1f4de220d335d9548598) and it fixes the regression.
>>>
>>> On a x86 with the updated dbx from uefi.org, I’d expect to see 234 bin hash entries
>>> in the blacklist keyring.  With the current merged code, there is none.
>>
>> Hum, I missed a part in refactoring (commit
>> f78e50c8f750c0ac6767ac1ed006360cf77c56c4). :/
>> Could you please test the following patch?
>>
>> diff --git a/certs/blacklist.c b/certs/blacklist.c
>> index 07c592ae5307..f998a2e85ddc 100644
>> --- a/certs/blacklist.c
>> +++ b/certs/blacklist.c
>> @@ -197,13 +197,16 @@ int mark_hash_blacklisted(const u8 *hash, size_t
>> hash_len,
>>                enum blacklist_hash_type hash_type)
>> {
>>        const char *buffer;
>> +       int err;
>>
>>        buffer = get_raw_hash(hash, hash_len, hash_type);
>>        if (IS_ERR(buffer))
>>                return PTR_ERR(buffer);
>> +       err = mark_raw_hash_blacklisted(buffer);
>>        kfree(buffer);
>> -       return 0;
>> +       return err;
>> }
> 
> I applied this patch, it works better, but there is still a regression. 
> Most of the hashes show up in the blacklist keyring now.  However some 
> do not, here is what I see in the log during boot:
> 
> [    2.321876] blacklist: Problem blacklisting hash (-13)
> [    2.322729] blacklist: Problem blacklisting hash (-13)
> [    2.323549] blacklist: Problem blacklisting hash (-13)
> [    2.324369] blacklist: Problem blacklisting hash (-13)
> 
>> Is it possible to test these kind of dbx blacklist with Qemu?
> 
> Yes, just use OVMF. 
> 

My changes (with the fix) don't change the previous semantic. I just
tested without my changes and with my changes (and the fix), and I get
the same result: 184 bin hashes with
https://uefi.org/sites/default/files/resources/dbxupdate_x64.bin

Could you please re-test and if there is still an issue bisect and share
the certificates causing this issue?

David, do you want me to send the two new patches or an updated full
patch series?