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 <stef...@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.

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?

        - Panu -
_______________________________________________
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Reply via email to