RE: Weak hash algorithms allowed with DIGEST_NG

[THOBY Simon <Simon.THOBY@viveris.fr>]

Hello again,

From : Mimi Zohar <zohar@linux.ibm.com>
> Before allowing the EVM HMAC to be updated, EVM verifies the existing
> HMAC to protect against an offline attack.  It doesn't prevent online
> changes.  Additional support to prevent crypto downgrade would need to
> be added.
> 

Yes, I wasn't really worrying about EVM, but only about IMA itself. Because if a
critical file (let's say bash) was hashed by the rightful user, but with a weak
algorithm, one can imagine an attacker finding a collision (another file,
carefully crafted by the attacker to have the same hash), and replacing offline
the legitimate file buy the "malicious" one. As far as I understand, the new
file would share the same hash, so the security.ima attribute wouldn't change,
and the security.evm wouldn't either because no xattr or inode number changed.
Of course this issue isn't critical because even if people hash their files
by calling evmctl, the default is SHA1 and it should be fairly hard to find
collisions (comparatively to e.g. MD4 or MD5), so nobody in pratice
should use a very weak algorithm where collisions would be "easy".

A careful owner/device producer should have no issue, but this highlights
the value of sane defaults, and I think evmctl certainly could benefit from
defaulting to sha256 in 2021 (but there may be compatibility issues I'm
not aware of that prevent such change).

> <snip>
> 
> > Is there any way to enforce the use of the hash specified in the
> > 'ima_hash' cmdline parameter ?
> 
> The cmdline parameter overrides the compile time default hash algorithm
> used for (re-)calculating the file hash.
> 

Yes, but that only applies to the hashes performed automatically by the kernel,
not to a user relabelling his whole / with
find / \( -fstype rootfs -o -fstype ext4 \) -type f -uid 0 -exec evmctl ima_hash '{}' 2> /dev/null \;
and forgetting to specify a stronger algorithm (that's how I learned of this pitfall myself).

> > I couldn't find any glancing at the code, but I didn't read all of it
> > and I understood even less, so I secretly hope to have missed a small
> > yet critical check/option.
> > And if there is no such way, would you be opposed to a patch adding
> > an option (something like 'ima_enforce_hash_alg') that only allows
> > digest hashed with the values supplied in the 'ima_hash' parameter ?
> 
> Please keep in mind that:
> - depending on which file is not properly signed with the required
> hash, the system might not boot.

Yes, yet in a sense that is also true when deploying IMA on a system, so it shouldn't
change much is such a scenario. This could definitely break a working system however,
so I have no doubt that if such option were to exit, it should be opt-in and not opt-out
(and to think I was talking of "sane defaults" a few lines above :)).

> - limiting the hash algorithm to a single algorithm would prevent
> migrating to a stronger algorithm.
> 

Indeed, I fear migrating the system online with such an option would be quite complex.

> For embedded/IoT, these concerns might not be a problem.
> 
> thanks,
> 
> Mimi
> 

Simon