linux-kernel - RE: [PATCH 3/5] ima: Fix ima digest hash table key calculation

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]

Message-ID: <11984a05a5624f64aed1ec6b0d0b75ff@huawei.com>
Date:   Thu, 23 Apr 2020 10:21:03 +0000
From:   Roberto Sassu <roberto.sassu@...wei.com>
To:     Mimi Zohar <zohar@...ux.ibm.com>
CC:     "linux-integrity@...r.kernel.org" <linux-integrity@...r.kernel.org>,
        "linux-security-module@...r.kernel.org" 
        <linux-security-module@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Krzysztof Struczynski <krzysztof.struczynski@...wei.com>,
        Silviu Vlasceanu <Silviu.Vlasceanu@...wei.com>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: RE: [PATCH 3/5] ima: Fix ima digest hash table key calculation



HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Li Jian, Shi Yanli


> -----Original Message-----
> From: Mimi Zohar [mailto:zohar@...ux.ibm.com]
> Sent: Wednesday, April 22, 2020 10:56 PM
> To: Roberto Sassu <roberto.sassu@...wei.com>
> Cc: linux-integrity@...r.kernel.org; linux-security-module@...r.kernel.org;
> linux-kernel@...r.kernel.org; Krzysztof Struczynski
> <krzysztof.struczynski@...wei.com>; Silviu Vlasceanu
> <Silviu.Vlasceanu@...wei.com>; stable@...r.kernel.org
> Subject: Re: [PATCH 3/5] ima: Fix ima digest hash table key calculation
> 
> Hi Roberto, Krsysztof,
> 
> On Wed, 2020-03-25 at 17:11 +0100, Roberto Sassu wrote:
> > From: Krzysztof Struczynski <krzysztof.struczynski@...wei.com>
> >
> > Function hash_long() accepts unsigned long, while currently only one byte
> > is passed from ima_hash_key(), which calculates a key for ima_htable.
> Use
> > more bytes to avoid frequent collisions.
> >
> > Length of the buffer is not explicitly passed as a function parameter,
> > because this function expects a digest whose length is greater than the
> > size of unsigned long.
> 
> Somehow I missed the original report of this problem https://lore.kern
> el.org/patchwork/patch/674684/.  This patch is definitely better, but
> how many unique keys are actually being used?  Is it anywhere near
> IMA_MEASURE_HTABLE_SIZE(512)?

I did a small test (with 1043 measurements):

slots: 250, max depth: 9 (without the patch)
slots: 448, max depth: 7 (with the patch)

Then, I increased the number of bits to 10:

slots: 251, max depth: 9 (without the patch)
slots: 660, max depth: 4 (with the patch)

> Do we need a new securityfs entry to display the number used?

Probably it is useful only if the administrator can decide the number of slots.

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Li Jian, Shi Yanli


> > Cc: stable@...r.kernel.org
> > Fixes: 3323eec921ef ("integrity: IMA as an integrity service provider")
> > Signed-off-by: Krzysztof Struczynski <krzysztof.struczynski@...wei.com>
> > ---
> >  security/integrity/ima/ima.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> > index 64317d95363e..cf0022c2bc14 100644
> > --- a/security/integrity/ima/ima.h
> > +++ b/security/integrity/ima/ima.h
> > @@ -177,7 +177,7 @@ extern struct ima_h_table ima_htable;
> >
> >  static inline unsigned long ima_hash_key(u8 *digest)
> >  {
> > -	return hash_long(*digest, IMA_HASH_BITS);
> > +	return hash_long(*((unsigned long *)digest), IMA_HASH_BITS);
> >  }
> >
> >  #define __ima_hooks(hook)		\