linux-kernel - Re: Problem with new X.509 is_hash

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

Message-ID: <CAKv+Gu9KuogQkfgqKYJt40oW2tvuuVi+w29tp_2KLUqcWvM83A@mail.gmail.com>
Date:   Wed, 21 Jun 2017 15:07:31 +0200
From:   Ard Biesheuvel <ard.biesheuvel@...aro.org>
To:     David Howells <dhowells@...hat.com>
Cc:     James Bottomley <James.Bottomley@...senpartnership.com>,
        keyrings@...r.kernel.org,
        linux-security-module <linux-security-module@...r.kernel.org>,
        "linux-efi@...r.kernel.org" <linux-efi@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: Problem with new X.509 is_hash_blacklisted() interface

On 21 June 2017 at 14:49, David Howells <dhowells@...hat.com> wrote:
> Ard Biesheuvel <ard.biesheuvel@...aro.org> wrote:
>
>> >      This can be told to skip a particular algorithm for when the caller
>> >      has one precalculated.  The precalculated hash can be passed to
>> >      is_hash_blacklisted().  This would typically be the case for a signed
>> >      X.509 message.
>>
>> This last part seems a premature optimization to me. Is there a
>> performance concern preventing us from using (4) only?
>
> Crypto stuff is relatively slow - and in the case of X.509 and PKCS#7 the
> caller will already have calculated a hash.  The most likely situation
> currently, I think, is that we will only have sha256 hashes in the blacklist,
> and whatever we're checking will have a sha256 hash also.
>
> Possibly, I could just pass the precalculated hash into is_data_blacklisted()
> and so avoid having to call is_hash_blacklisted() from outside.
>

That would be cleaner, yes. As I said, it looks correct to me, but I
would simply prefer to keep the code as simple as possible. If there
is a good reason for the additional fast path, then I have no
objections at all.

>> In any case, the approach and the code look sound to me, although I
>> think adding a hash of a type that we don't know how to calculate
>> deserves a warning at least.
>
> There are two issues with that:
>
>  (1) We don't know what hashes are available without checking to see what
>      modules are available.  However, to do this would involve loading the
>      hash algorithm module - but we might not be in a position to do this yet
>      (the blacklist is loaded before we start userspace).
>

Ah ok. That does complicate matters, indeed.

>  (2) A module implementing a hash algorithm might be blacklisted by the hash
>      that we've been given to add to the blacklist.  I think this is a more
>      general problem - and might require us to restrict blacklisting to hash
>      algorithms that are built in.
>

That makes sense, but deserves another big fat warning when it turns
out that the blacklist contains an entry we cannot verify against.