[Rpm-maint] [PATCH v2 1/4] ima-plugin: Have executable configuration files signed

[Stefan Berger]

Panu Matilainen <pmatilai at laiskiainen.org> wrote on 09/23/2016 02:44:48 
AM:

> From: Panu Matilainen <pmatilai at laiskiainen.org>
> To: Stefan Berger <stefanb at linux.vnet.ibm.com>, rpm-maint at lists.rpm.org
> Cc: Stefan Berger/Watson/IBM at IBMUS, fionnuala.gunter at gmail.com
> Date: 09/23/2016 02:45 AM
> Subject: Re: [Rpm-maint] [PATCH v2 1/4] ima-plugin: Have executable 
> configuration files signed
> 
> On 09/22/2016 08:30 PM, Stefan Berger wrote:
> > Some configuration files are executables and so they require the
> > signature in the extended attribute. If they are not executable,
> > they can be skipped.
> >
> > Examples for configuration files that are also executables are
> > the grub files in /etc/grub.d.
> >
> > Signed-off-by: Stefan Berger <stefanb at linux.vnet.ibm.com>
> > ---
> >  plugins/ima.c | 25 +++++++++++++++++--------
> >  1 file changed, 17 insertions(+), 8 deletions(-)
> >
> > diff --git a/plugins/ima.c b/plugins/ima.c
> > index be15ecf..81ed194 100644
> > --- a/plugins/ima.c
> > +++ b/plugins/ima.c
> > @@ -41,7 +41,8 @@ static rpmRC ima_psm_post(rpmPlugin plugin, 
> rpmte te, int res)
> >     const char *fpath;
> >     const unsigned char * fsig = NULL;
> >     size_t len;
> > -   int rc = 0;
> > +   int rc = 0, n;
> > +   struct stat statbuf;
> >
> >     if (fi == NULL) {
> >         rc = RPMERR_BAD_MAGIC;
> > @@ -49,13 +50,21 @@ static rpmRC ima_psm_post(rpmPlugin plugin, 
> rpmte te, int res)
> >     }
> >
> >     while (rpmfiNext(fi) >= 0) {
> > -       /* Don't install signatures for (mutable) config files */
> > -       if (!(rpmfiFFlags(fi) & RPMFILE_CONFIG)) {
> > -      fpath = rpmfiFN(fi);
> > -      fsig = rpmfiFSignature(fi, &len);
> > -      if (fsig && (check_zero_hdr(fsig, len) == 0)) {
> > -          lsetxattr(fpath, XATTR_NAME_IMA, fsig, len, 0);
> > -      }
> > +       /* Don't install signatures for (mutable) files marked
> > +        * as config files unless they are also executable.
> > +        */
> > +       if (rpmfiFFlags(fi) & RPMFILE_CONFIG) {
> > +           n = rpmfiStat(fi, 0, &statbuf);
> > +           if (n != 0)
> > +               continue;
> > +           if ((statbuf.st_mode & 0111) == 0)
> > +               continue;
> 
> rpmfiStat() is not wrong, but it does far more than you need here and 
> complicates things a little. You could just use rpmfiFMode() for the 
> mode bits.
> 
> Also generally it's preferred to avoid magic numbers when it can be 
> easily expressed with defined names, (S_IXUSR|S_IXGRP|S_IXOTH) is easier 

> for the reader than 0111.

Ok, I'll fix that.

> 
> As for the change itself, I think it also points to
> a) packaging bug in grub2-tools - except for those *_custom files these 
> are *not* configuration files that you're supposed to touch
> b) overall insanity of grub2 - inspect any of the files in /etc/grub.d, 
> trying to keep in mind these are bootloader "configuration"...
> 
> All that aside, I wonder about this change: what will happen now if the 
> user legitimately edits one of those executable %config files? The 
> signature no longer match, does that mean it'll fail to execute?

The change is necessary. There are two ways to run these scripts:

bash 00_header  or  ./00_header

The latter needs the signature, while the former does (currently) not need 
it. Following tests with grub the scripts seem to be executed using the 
latter method and fail if they don't have a signature.

Once IMA Appraisal is active, you would have to regard the system as being 
in lock-down mode. If you make changes to these files, you'd have to sign 
them, otherwise they will fail executing.

   Stefan

> 
>    - Panu -
> 


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rpm.org/pipermail/rpm-maint/attachments/20160923/50a746e4/attachment-0001.html>