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.
(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