All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH v13 4/8] util-linux: selinux, audit, and pam support
Date: Tue, 6 Dec 2016 21:25:08 +0100	[thread overview]
Message-ID: <20161206212508.44fc1232@free-electrons.com> (raw)
In-Reply-To: <1477423570-15694-4-git-send-email-bryce.ferguson@rockwellcollins.com>

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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

  parent reply	other threads:[~2016-12-06 20:25 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-25 19:26 [Buildroot] [PATCH v13 1/8] policycoreutils: new package Bryce Ferguson
2016-10-25 19:26 ` [Buildroot] [PATCH v13 2/8] refpolicy: " Bryce Ferguson
2016-12-12 22:21   ` Thomas Petazzoni
2016-12-13 22:08     ` Thomas Petazzoni
2016-12-21 14:45       ` Bryce Ferguson
2017-01-05 18:11     ` Bryce Ferguson
2016-10-25 19:26 ` [Buildroot] [PATCH v13 3/8] linux-pam: add system auth file and host variant Bryce Ferguson
2016-12-06 20:27   ` [Buildroot] [PATCH 1/2] linux-pam: adjust login pam file for SELinux Thomas Petazzoni
2016-12-06 20:27     ` [Buildroot] [PATCH 2/2] util-linux: add selinux support Thomas Petazzoni
2016-12-22 16:32       ` Bryce Ferguson
2016-12-22 16:25     ` [Buildroot] [PATCH 1/2] linux-pam: adjust login pam file for SELinux Bryce Ferguson
2017-01-25 10:06     ` Thomas Petazzoni
2016-10-25 19:26 ` [Buildroot] [PATCH v13 4/8] util-linux: selinux, audit, and pam support Bryce Ferguson
2016-10-26  6:02   ` Rahul Bedarkar
2016-12-06 20:25   ` Thomas Petazzoni [this message]
2017-03-10 21:59   ` Thomas Petazzoni
2016-10-25 19:26 ` [Buildroot] [PATCH v13 5/8] busybox: applets as individual binaries Bryce Ferguson
2016-10-25 19:26 ` [Buildroot] [PATCH v13 6/8] qemu x86 selinux: base br defconfig Bryce Ferguson
2016-10-25 19:26 ` [Buildroot] [PATCH v13 7/8] qemu x86 selinux: added common selinux support files Bryce Ferguson
2016-10-25 19:26 ` [Buildroot] [PATCH v13 8/8] skeleton: busybox individual apps no symlink Bryce Ferguson
2016-12-10 14:59 ` [Buildroot] [PATCH v13 1/8] policycoreutils: new package Thomas Petazzoni
2016-12-12 19:11   ` Matthew Weber
2016-12-12 19:28   ` Bryce Ferguson
2016-12-12 20:14     ` Thomas Petazzoni

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161206212508.44fc1232@free-electrons.com \
    --to=thomas.petazzoni@free-electrons.com \
    --cc=buildroot@busybox.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.