[PATCH 1/2] MODSIGN: check hash of kernel module in blacklist

[Josh Boyer]

On Fri, Dec 13, 2013 at 12:34:11PM +0800, joeyli wrote:
> 於 四，2013-12-12 於 08:33 -0500，Josh Boyer 提到：
> > On Thu, Dec 12, 2013 at 11:46:22AM +0800, joeyli wrote:
> > > Hi Josh, 
> > > 
> > > Thanks for your review and suggestions!
> > > 
> > > 於 三，2013-12-11 於 11:07 -0500，Josh Boyer 提到：
> > > > On Wed, Dec 11, 2013 at 03:26:16PM +0800, Lee, Chun-Yi wrote:
> > > > > This patch introduces a blacklist list of kernel module's hash. It check
> > > > > the blacklist before checking kernel module signature.
> > > > > It didn't limit what hash algorithm used but the module of hash algorithm
> > > > > need build-in or put in initrd for verify kernel module in initrd.
> > > > > 
> > > ...
> > > > > +	const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
> > > > > +
> > > > > +	/* truncate the module to discard the signature when it signed */
> > > > > +	if (modlen > markerlen &&
> > > > > +	    memcmp(mod + modlen - markerlen, MODULE_SIG_STRING, markerlen) == 0) {
> > > > > +		modlen -= markerlen;
> > > > > +		if (modlen <= sizeof(ms))
> > > > > +			return -EBADMSG;
> > > > > +		memcpy(&ms, mod + (modlen - sizeof(ms)), sizeof(ms));
> > > > > +		modlen -= sizeof(ms);
> > > > > +		sig_len = be32_to_cpu(ms.sig_len);
> > > > > +		if (sig_len >= modlen)
> > > > > +			return -EBADMSG;
> > > > > +		modlen -= sig_len;
> > > > > +		if ((size_t)ms.signer_len + ms.key_id_len >= modlen)
> > > > > +			return -EBADMSG;
> > > > > +		modlen -= (size_t)ms.signer_len + ms.key_id_len;
> > > > > +	}
> > > > 
> > > > Hm.  Why do we discard the signature before we calculate the hash?  It
> > > > seems we might need to check for a hash of both the signed and unsigned
> > > > module, correct?
> > > > 
> > > 
> > > Yes, the reason of blacklisted a kernel module is there have security
> > > weakness in the code of module. So, no matter who signed the kernel
> > > module or even the module didn't sign, we don't want it loaded by
> > > kernel.
> > > 
> > > For another situation, if we want revoke a KEY, then just direct import
> > > the public key to MOKx but not add hash of signed kernel module one by
> > > one.
> > 
> > That is all true, but we don't necessarily control what hash is actually
> > stored in dbx/MokXRT.  If a user (or in the case of dbx, the CA)
> > happened to hash the module with the signature attached and enrolled
> > that hash into UEFI/Mok, then doing a comparison with the signature
> > stripped against that will fail, won't it?  That is why I was suggesting
> > we needed to compare against both.
> > 
> > I agree that the ideal situation would be for the enrolled hash to be
> > free of signatures, but there's nothing that guarantees that will be the
> > case.
> 
> OK, I will also computing the hash with signature and compare.

OK.  I realize it's unfortunate to have to do that, but I don't see
another way at the moment.

> > (I also think the vast majority of blacklisting will be with certs, not
> > with individual modules so this is somewhat minor.  I think that even
> > small build-time variances will make module blacklisting difficult to
> > actually make viable.)
> > 
> > josh
> > 
> 
> For the situation we don't want revoke key of certificate, it's the way
> we need carry out.

Yes, agreed.  I suspect it will be most useful for 3rd party drivers
rather than individual modules a distribution ships.

Thanks again!

josh