| Message ID | 1477423570-15694-4-git-send-email-bryce.ferguson@rockwellcollins.com |
|---|---|
| State | Rejected |
| Headers | show |
Hello, On Wednesday 26 October 2016 12:56 AM, Bryce Ferguson wrote: > > diff --git a/package/util-linux/util-linux.mk b/package/util-linux/util-linux.mk > index bd6508c..934fe2f 100644 > --- a/package/util-linux/util-linux.mk > +++ b/package/util-linux/util-linux.mk > @@ -63,6 +63,20 @@ endif > UTIL_LINUX_CONF_ENV += LIBS="$(UTIL_LINUX_LIBS)" > UTIL_LINUX_MAKE_OPTS += LIBS="$(UTIL_LINUX_LIBS)" > > +ifeq ($(BR2_PACKAGE_LIBSELINUX),y) > +UTIL_LINUX_DEPENDENCIES += libselinux > +UTIL_LINUX_CONF_OPTS += --with-selinux > +else > +UTIL_LINUX_CONF_OPTS += --without-selinux > +endif > + > +ifeq ($(BR2_PACKAGE_AUDIT),y) > +UTIL_LINUX_DEPENDENCIES += audit > +UTIL_LINUX_CONF_OPTS += --with-audit > +else > +UTIL_LINUX_CONF_OPTS += --without-audit > +endif > + util-linux already has optional dependency on audit package. Regards, Rahul
Hello, On Tue, 25 Oct 2016 14:26:06 -0500, Bryce Ferguson wrote: > From: Matt Weber <matthew.weber@rockwellcollins.com> > > This patch adds optional libselinux ,audit and pam support to linux utilities. > > Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com> > Signed-off-by: Clayton Shotwell <clayton.shotwell@rockwellcollins.com> > Reviewed-by: Samuel Martin <s.martin49@gmail.com> > Signed-off-by: Niranjan Reddy <niranjan.reddy@rockwellcollins.com> There are a few things I don't like in this patch. I'm going to submit two alternative patches as a replacement. I'm interested in your feedback about those replacement patches (especially if they work fine for you). > +ifeq ($(BR2_PACKAGE_LIBSELINUX),y) > +UTIL_LINUX_DEPENDENCIES += libselinux > +UTIL_LINUX_CONF_OPTS += --with-selinux > +else > +UTIL_LINUX_CONF_OPTS += --without-selinux > +endif > + > +ifeq ($(BR2_PACKAGE_AUDIT),y) > +UTIL_LINUX_DEPENDENCIES += audit > +UTIL_LINUX_CONF_OPTS += --with-audit > +else > +UTIL_LINUX_CONF_OPTS += --without-audit > +endif As was already stated, optional audit support is already in util-linux.mk. > + > # Used by cramfs utils > UTIL_LINUX_DEPENDENCIES += $(if $(BR2_PACKAGE_ZLIB),zlib) > > @@ -179,9 +193,25 @@ define UTIL_LINUX_INSTALL_PAMFILES > $(INSTALL) -m 0644 package/util-linux/su.pam \ > $(TARGET_DIR)/etc/pam.d/su-l > endef > + > +# Add the required hooks to the pam files if SELinux is enabled > +# SED expression adds these lines to /etc/pam.d/login,/etc/pam.d/su-l and /etc/pam.d/su files > +# session required pam_selinux.so close > +# session required pam_selinux.so open > +ifeq ($(BR2_PACKAGE_LIBSELINUX),y) > +define UTIL_LINUX_FIXUP_PAMFILES > + for file in login su su-l ; do \ > + $(SED) '/selinux/d' $(TARGET_DIR)/etc/pam.d/$${file}; \ > + $(SED) '0,/session/s/session/session required pam_selinux.so close\nsession/' $(TARGET_DIR)/etc/pam.d/$${file}; \ > + echo "session required pam_selinux.so open" >> $(TARGET_DIR)/etc/pam.d/$${file}; \ > + done > +endef > +endif I don't like this, for two reasons: - The SED expressions are really really complicated. - You're tweaking /etc/pam.d/login, which has not been installed by this package, but by the linux-pam package. See my alternate proposal, which I'll send in a few minutes. Thanks! Thomas
Hello, On Tue, 25 Oct 2016 14:26:06 -0500, Bryce Ferguson wrote: > From: Matt Weber <matthew.weber@rockwellcollins.com> > > This patch adds optional libselinux ,audit and pam support to linux utilities. > > Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com> > Signed-off-by: Clayton Shotwell <clayton.shotwell@rockwellcollins.com> > Reviewed-by: Samuel Martin <s.martin49@gmail.com> > Signed-off-by: Niranjan Reddy <niranjan.reddy@rockwellcollins.com> Instead of this patch, we have applied commit 373b7efdb308fcb7607cbef63ee4d2644b46136f, which serves the same purpose. So I've marked this patch as Rejected in patchwork. Best regards, Thomas
diff --git a/package/util-linux/util-linux.mk b/package/util-linux/util-linux.mk index bd6508c..934fe2f 100644 --- a/package/util-linux/util-linux.mk +++ b/package/util-linux/util-linux.mk @@ -63,6 +63,20 @@ endif UTIL_LINUX_CONF_ENV += LIBS="$(UTIL_LINUX_LIBS)" UTIL_LINUX_MAKE_OPTS += LIBS="$(UTIL_LINUX_LIBS)" +ifeq ($(BR2_PACKAGE_LIBSELINUX),y) +UTIL_LINUX_DEPENDENCIES += libselinux +UTIL_LINUX_CONF_OPTS += --with-selinux +else +UTIL_LINUX_CONF_OPTS += --without-selinux +endif + +ifeq ($(BR2_PACKAGE_AUDIT),y) +UTIL_LINUX_DEPENDENCIES += audit +UTIL_LINUX_CONF_OPTS += --with-audit +else +UTIL_LINUX_CONF_OPTS += --without-audit +endif + # Used by cramfs utils UTIL_LINUX_DEPENDENCIES += $(if $(BR2_PACKAGE_ZLIB),zlib) @@ -179,9 +193,25 @@ define UTIL_LINUX_INSTALL_PAMFILES $(INSTALL) -m 0644 package/util-linux/su.pam \ $(TARGET_DIR)/etc/pam.d/su-l endef + +# Add the required hooks to the pam files if SELinux is enabled +# SED expression adds these lines to /etc/pam.d/login,/etc/pam.d/su-l and /etc/pam.d/su files +# session required pam_selinux.so close +# session required pam_selinux.so open +ifeq ($(BR2_PACKAGE_LIBSELINUX),y) +define UTIL_LINUX_FIXUP_PAMFILES + for file in login su su-l ; do \ + $(SED) '/selinux/d' $(TARGET_DIR)/etc/pam.d/$${file}; \ + $(SED) '0,/session/s/session/session required pam_selinux.so close\nsession/' $(TARGET_DIR)/etc/pam.d/$${file}; \ + echo "session required pam_selinux.so open" >> $(TARGET_DIR)/etc/pam.d/$${file}; \ + done +endef +endif + endif UTIL_LINUX_POST_INSTALL_TARGET_HOOKS += UTIL_LINUX_INSTALL_PAMFILES +UTIL_LINUX_POST_INSTALL_TARGET_HOOKS += UTIL_LINUX_FIXUP_PAMFILES # Install agetty->getty symlink to avoid breakage when there's no busybox ifeq ($(BR2_PACKAGE_UTIL_LINUX_AGETTY),y)