[PATCH v2 0/5] Allow xuser to shutdown

[PATCH v2 0/5] Allow xuser to shutdown

[Laurentiu Palcu]

Changes in v2:
 * created a separate recipe for xuser creation, located in
   meta/recipes-support/user-creation/. If there are other users we might need
   to add in the future, we can put the recipes here;
 * removed xuser creation from connman and xserver-noadm-init and added the
   new 'xuser-account' recipe in RDEPENDS;

Laurentiu

CC: sgw@linux.intel.com
CC: martin.jansa@gmail.com

The following changes since commit 8a186a6b3853fc1a7dcf342d421c8926c38949c9:

  bitbake: hob: save button from settings called a nonexisting method (2013-07-03 08:13:35 +0100)

are available in the git repository at:

  git://git.yoctoproject.org/poky-contrib lpalcu/b4345_shutdown_icon_rootless_x_v2
  http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=lpalcu/b4345_shutdown_icon_rootless_x_v2

Laurentiu Palcu (5):
  sysvinit: allow users in shutdown group to perform halt/reboot
  xuser-account: create separate recipe for xuser creation
  connman: remove xuser creation, RDEPEND on xuser-account
  xserver-nodm-init: remove xuser creation, RDEPEND on xuser-account
  shutdown-desktop: give entire path in Exec field

 meta/recipes-connectivity/connman/connman.inc      |   10 ++------
 meta/recipes-core/sysvinit/sysvinit_2.88dsf.bb     |   12 ++++++---
 .../x11-common/xserver-nodm-init.bb                |   13 ++--------
 .../shutdown-desktop/shutdown-desktop.bb           |    2 ++
 .../user-creation/xuser-account_0.1.bb             |   27 ++++++++++++++++++++
 5 files changed, 42 insertions(+), 22 deletions(-)
 create mode 100644 meta/recipes-support/user-creation/xuser-account_0.1.bb

-- 
1.7.9.5

[PATCH v2 1/5] sysvinit: allow users in shutdown group to perform halt/reboot

[Laurentiu Palcu]

For this to happen:
 * 'shutdown' group has been created;
 * changed ownership group for /sbin/halt and /sbin/shutdown to 'shutdown';
 * deny execution rights to other users except 'root' and those belonging
   to 'shutdown' group;
 * set setuid bit to both apps;

So, basically, in order for a normal user to be able to shutdown/reboot
the machine, it must be a member of 'shutdown' group.

Other changes:
 * fixed identetion for 2 lines that used spaces instead of tabs;

[YOCTO #4345]

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@intel.com>
---
 meta/recipes-core/sysvinit/sysvinit_2.88dsf.bb |   12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/meta/recipes-core/sysvinit/sysvinit_2.88dsf.bb b/meta/recipes-core/sysvinit/sysvinit_2.88dsf.bb
index e64b67a..784d538 100644
--- a/meta/recipes-core/sysvinit/sysvinit_2.88dsf.bb
+++ b/meta/recipes-core/sysvinit/sysvinit_2.88dsf.bb
@@ -25,7 +25,7 @@ SRC_URI[sha256sum] = "60bbc8c1e1792056e23761d22960b30bb13eccc2cabff8c7310a01f4d5
 S = "${WORKDIR}/sysvinit-${PV}"
 B = "${S}/src"
 
-inherit update-alternatives
+inherit update-alternatives useradd
 DEPENDS_append = " update-rc.d-native"
 
 ALTERNATIVE_${PN} = "init mountpoint halt reboot runlevel shutdown poweroff last mesg utmpdump wall"
@@ -54,6 +54,9 @@ ALTERNATIVE_LINK_NAME[sulogin.8] = "${mandir}/man8/sulogin.8"
 ALTERNATIVE_LINK_NAME[utmpdump.1] = "${mandir}/man1/utmpdump.1"
 ALTERNATIVE_LINK_NAME[wall.1] = "${mandir}/man1/wall.1"
 
+USERADD_PACKAGES = "${PN}"
+GROUPADD_PARAM_${PN} = "--system shutdown"
+
 PACKAGES =+ "sysvinit-pidof sysvinit-sulogin"
 FILES_${PN} += "${base_sbindir}/* ${base_bindir}/*"
 FILES_sysvinit-pidof = "${base_bindir}/pidof.sysvinit ${base_sbindir}/killall5"
@@ -87,6 +90,9 @@ do_install () {
 	install -m 0755    ${WORKDIR}/bootlogd.init     ${D}${sysconfdir}/init.d/bootlogd
 	ln -sf bootlogd ${D}${sysconfdir}/init.d/stop-bootlogd
 
-        update-rc.d -r ${D} bootlogd start 07 S .
-        update-rc.d -r ${D} stop-bootlogd start 99 2 3 4 5 .
+	update-rc.d -r ${D} bootlogd start 07 S .
+	update-rc.d -r ${D} stop-bootlogd start 99 2 3 4 5 .
+
+	chown root.shutdown ${D}${base_sbindir}/halt ${D}${base_sbindir}/shutdown
+	chmod o-x,u+s ${D}${base_sbindir}/halt ${D}${base_sbindir}/shutdown
 }
-- 
1.7.9.5

[PATCH v2 2/5] xuser-account: create separate recipe for xuser creation

[Laurentiu Palcu]

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@intel.com>
---
 .../user-creation/xuser-account_0.1.bb             |   27 ++++++++++++++++++++
 1 file changed, 27 insertions(+)
 create mode 100644 meta/recipes-support/user-creation/xuser-account_0.1.bb

diff --git a/meta/recipes-support/user-creation/xuser-account_0.1.bb b/meta/recipes-support/user-creation/xuser-account_0.1.bb
new file mode 100644
index 0000000..7430d2a
--- /dev/null
+++ b/meta/recipes-support/user-creation/xuser-account_0.1.bb
@@ -0,0 +1,27 @@
+DESCRIPTION = "Creates an 'xuser' account"
+LICENSE = "MIT"
+LIC_FILES_CHKSUM = "file://${COREBASE}/meta/COPYING.MIT;md5=3da9cfbcb788c80a0384361b4de20420"
+
+SRC_URI = ""
+
+inherit useradd
+
+do_configure() {
+    :
+}
+
+do_compile() {
+    :
+}
+
+do_install() {
+    :
+}
+
+USERADD_PACKAGES = "${PN}"
+GROUPADD_PARAM_${PN} = "--system shutdown"
+USERADD_PARAM_${PN} = "--create-home \
+                       --groups video,tty,audio,input,shutdown \
+                       --user-group xuser"
+
+ALLOW_EMPTY_${PN} = "1"
-- 
1.7.9.5

Re: [PATCH v2 2/5] xuser-account: create separate recipe for xuser creation

[Burton, Ross]

On 4 July 2013 11:58, Laurentiu Palcu <laurentiu.palcu@intel.com> wrote:
> +USERADD_PARAM_${PN} = "--create-home \
> +                       --groups video,tty,audio,input,shutdown \
> +                       --user-group xuser"

I'm not liking the name "xuser" but unless someone has a much better
idea right now we can change that later.

Thinking about places where the xuser is used in other recipes (i.e.
connman for ACLs), can anyone see a problem with adding xuser to the
"users" group so those ACLS can use group membership instead of user
name?

Ross

Re: [PATCH v2 2/5] xuser-account: create separate recipe for xuser creation

[Tomas Frydrych]

On 04/07/13 12:29, Burton, Ross wrote:
> On 4 July 2013 11:58, Laurentiu Palcu <laurentiu.palcu@intel.com> wrote:
>> +USERADD_PARAM_${PN} = "--create-home \
>> +                       --groups video,tty,audio,input,shutdown \
>> +                       --user-group xuser"
> 
> I'm not liking the name "xuser" but unless someone has a much better
> idea right now we can change that later.

oeuser?

> Thinking about places where the xuser is used in other recipes (i.e.
> connman for ACLs), can anyone see a problem with adding xuser to the
> "users" group so those ACLS can use group membership instead of user
> name?

Yes, I think group would be preferred option.

Tomas

-- 
http://sleepfive.com

Re: [PATCH v2 2/5] xuser-account: create separate recipe for xuser creation

[Martin Jansa]

[-- Attachment #1: Type: text/plain, Size: 1546 bytes --]

On Thu, Jul 04, 2013 at 01:58:01PM +0300, Laurentiu Palcu wrote:
> Signed-off-by: Laurentiu Palcu <laurentiu.palcu@intel.com>
> ---
>  .../user-creation/xuser-account_0.1.bb             |   27 ++++++++++++++++++++
>  1 file changed, 27 insertions(+)
>  create mode 100644 meta/recipes-support/user-creation/xuser-account_0.1.bb
> 
> diff --git a/meta/recipes-support/user-creation/xuser-account_0.1.bb b/meta/recipes-support/user-creation/xuser-account_0.1.bb
> new file mode 100644
> index 0000000..7430d2a
> --- /dev/null
> +++ b/meta/recipes-support/user-creation/xuser-account_0.1.bb
> @@ -0,0 +1,27 @@
> +DESCRIPTION = "Creates an 'xuser' account"
> +LICENSE = "MIT"
> +LIC_FILES_CHKSUM = "file://${COREBASE}/meta/COPYING.MIT;md5=3da9cfbcb788c80a0384361b4de20420"
> +

can we inherit allarch in it?

> +SRC_URI = ""
> +
> +inherit useradd
> +
> +do_configure() {
> +    :
> +}
> +
> +do_compile() {
> +    :
> +}
> +
> +do_install() {
> +    :
> +}
> +
> +USERADD_PACKAGES = "${PN}"
> +GROUPADD_PARAM_${PN} = "--system shutdown"
> +USERADD_PARAM_${PN} = "--create-home \
> +                       --groups video,tty,audio,input,shutdown \
> +                       --user-group xuser"
> +
> +ALLOW_EMPTY_${PN} = "1"
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core

-- 
Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 205 bytes --]

[PATCH v2 3/5] connman: remove xuser creation, RDEPEND on xuser-account

[Laurentiu Palcu]

[YOCTO #4345]

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@intel.com>
---
 meta/recipes-connectivity/connman/connman.inc |   10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/meta/recipes-connectivity/connman/connman.inc b/meta/recipes-connectivity/connman/connman.inc
index e6584ff..6b5ea0b 100644
--- a/meta/recipes-connectivity/connman/connman.inc
+++ b/meta/recipes-connectivity/connman/connman.inc
@@ -60,14 +60,7 @@ SYSTEMD_SERVICE_${PN} = "connman.service"
 SYSTEMD_SERVICE_${PN}-vpn = "connman-vpn.service"
 SYSTEMD_WIRED_SETUP = "ExecStartPre=-${libdir}/connman/wired-setup"
 
-# IMPORTANT: because xuser is shared with rootless X, please make sure the
-# USERADD_PARAM is in sync with the one in xserver-nodm-init.bb
-USERADD_PACKAGES = "${PN}"
-USERADD_PARAM_${PN} = "--create-home \
-                       --groups video,tty,audio \
-                       --user-group xuser"
-
-inherit autotools gtk-doc pkgconfig systemd update-rc.d useradd
+inherit autotools gtk-doc pkgconfig systemd update-rc.d
 
 do_configure_append () {
 	sed -i "s#ExecStart=#${SYSTEMD_WIRED_SETUP}\nExecStart=#" ${S}/src/connman.service
@@ -118,6 +111,7 @@ RDEPENDS_${PN} = "\
 	${@base_contains('PACKAGECONFIG', 'bluetooth', 'bluez4', '', d)} \
 	${@base_contains('PACKAGECONFIG', 'wifi','wpa-supplicant', '', d)} \
 	${@base_contains('PACKAGECONFIG', '3g','ofono', '', d)} \
+	xuser-account \
 	"
 
 PACKAGES_DYNAMIC += "^${PN}-plugin-.*"
-- 
1.7.9.5

[PATCH v2 4/5] xserver-nodm-init: remove xuser creation, RDEPEND on xuser-account

[Laurentiu Palcu]

[YOCTO #4345]

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@intel.com>
---
 .../x11-common/xserver-nodm-init.bb                |   13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/meta/recipes-graphics/x11-common/xserver-nodm-init.bb b/meta/recipes-graphics/x11-common/xserver-nodm-init.bb
index d2797a9..8490c81 100644
--- a/meta/recipes-graphics/x11-common/xserver-nodm-init.bb
+++ b/meta/recipes-graphics/x11-common/xserver-nodm-init.bb
@@ -3,7 +3,7 @@ LICENSE = "GPLv2"
 LIC_FILES_CHKSUM = "file://COPYING;md5=751419260aa954499f7abaabaa882bbe"
 SECTION = "x11"
 PR = "r31"
-RDEPENDS_${PN} = "sudo"
+RDEPENDS_${PN} = "sudo xuser-account"
 
 SRC_URI = "file://xserver-nodm \
            file://Xusername \
@@ -23,17 +23,8 @@ do_install() {
     fi
 }
 
-inherit update-rc.d useradd
+inherit update-rc.d
 
 INITSCRIPT_NAME = "xserver-nodm"
 INITSCRIPT_PARAMS = "start 9 5 2 . stop 20 0 1 6 ."
 
-# Use fixed Xusername of xuser for now, this will need to be
-# fixed if the Xusername changes from xuser
-# IMPORTANT: because xuser is shared with connman, please make sure the
-# USERADD_PARAM is in sync with the one in connman.inc
-USERADD_PACKAGES = "${PN}"
-USERADD_PARAM_${PN} = "--create-home \
-                       --groups video,tty,audio,input \
-                       --user-group xuser"
-
-- 
1.7.9.5

[PATCH v2 5/5] shutdown-desktop: give entire path in Exec field

[Laurentiu Palcu]

A normal user does not have /sbin in its PATH, by default, so having the
entire path here allows the correct execution when run as regular user.

[YOCTO #4345]

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@intel.com>
---
 .../shutdown-desktop/shutdown-desktop.bb           |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/meta/recipes-sato/shutdown-desktop/shutdown-desktop.bb b/meta/recipes-sato/shutdown-desktop/shutdown-desktop.bb
index c5096c1..9e283e4 100644
--- a/meta/recipes-sato/shutdown-desktop/shutdown-desktop.bb
+++ b/meta/recipes-sato/shutdown-desktop/shutdown-desktop.bb
@@ -18,6 +18,8 @@ pkg_postinst_${PN} () {
     grep -q qemuarm $D${sysconfdir}/hostname && \
         sed -i $D${datadir}/applications/shutdown.desktop -e 's/^Exec=halt/Exec=reboot/' \
         || true
+
+    sed -i $D${datadir}/applications/shutdown.desktop -e 's#^Exec=\(.*\)#Exec=${base_sbindir}/\1#'
 }
 
 inherit allarch
-- 
1.7.9.5

Re: [PATCH v2 5/5] shutdown-desktop: give entire path in Exec field

[Burton, Ross]

On 4 July 2013 11:58, Laurentiu Palcu <laurentiu.palcu@intel.com> wrote:
> +    sed -i $D${datadir}/applications/shutdown.desktop -e 's#^Exec=\(.*\)#Exec=${base_sbindir}/\1#'

Doing this in postinst is pretty nasty, instead change the desktop
file to contain something like @SBIN@ and run it through sed in
do_install.

Ross

Re: [PATCH v2 5/5] shutdown-desktop: give entire path in Exec field

[Laurentiu Palcu]

On 07/04/2013 05:58 PM, Burton, Ross wrote:
> On 4 July 2013 11:58, Laurentiu Palcu <laurentiu.palcu@intel.com> wrote:
>> +    sed -i $D${datadir}/applications/shutdown.desktop -e 's#^Exec=\(.*\)#Exec=${base_sbindir}/\1#'
> 
> Doing this in postinst is pretty nasty, 
Nasty, in what way? How is this line different from the previous line in
the same postinstall? That one is also using sed to change 'halt' to
'reboot' for qemuarm. Is it the sed you're worried about? This
postinstall is executed on host anyway, at do_rootfs().

> instead change the desktop
> file to contain something like @SBIN@ and run it through sed in
> do_install.
Even though the change you're proposing is OK, involves changing the
desktop file to add the @SBIN@ pattern and move the sed line to
do_install(). Compared to having one single line added in the
postinstall... I would choose the latter, unless you elaborate on what
do you actually mean by "nasty".

Laurentiu
> 
> Ross
> 

Re: [PATCH v2 5/5] shutdown-desktop: give entire path in Exec field

[Burton, Ross]

On 4 July 2013 16:39, Laurentiu Palcu <laurentiu.palcu@intel.com> wrote:
>> Doing this in postinst is pretty nasty,
> Nasty, in what way? How is this line different from the previous line in
> the same postinstall? That one is also using sed to change 'halt' to
> 'reboot' for qemuarm. Is it the sed you're worried about? This
> postinstall is executed on host anyway, at do_rootfs().

The existing sed to manipulate the exec line is arguably a better
solution to making this package machine-specific and special-casing
just qemuarm.  (I'm not sure why it special-cases qemuarm, and
751212d5effdceab91d95705e647cf07e6820940 where it was introduced
doesn't clarify matters, but we'll ignore that for now).  Arguably,
but I'm not sure I agree.  Anyway, you've a path that is known at
build time so we can fix it at build time.  Instead of using install,
you can use sed.  Just because we *can* run postinst scripts when
generating the rootfs doesn't mean we *should*, do_install is for
installing the files we're going to package, and postinst is for
runtime changes that can't happen at any other time.

>> instead change the desktop
>> file to contain something like @SBIN@ and run it through sed in
>> do_install.
> Even though the change you're proposing is OK, involves changing the
> desktop file to add the @SBIN@ pattern and move the sed line to
> do_install(). Compared to having one single line added in the
> postinstall... I would choose the latter, unless you elaborate on what
> do you actually mean by "nasty".

We're not being invoiced based on the number of patch hunks, and
changing /sbin to @SBIN@ and install to sed isn't exactly a massive
time sink.

Ross

Re: [PATCH v2 5/5] shutdown-desktop: give entire path in Exec field

[Tomas Frydrych]

On 04/07/13 16:39, Laurentiu Palcu wrote:
> On 07/04/2013 05:58 PM, Burton, Ross wrote:
>> On 4 July 2013 11:58, Laurentiu Palcu <laurentiu.palcu@intel.com> wrote:
> Even though the change you're proposing is OK, involves changing the
> desktop file to add the @SBIN@ pattern and move the sed line to
> do_install(). Compared to having one single line added in the
> postinstall... I would choose the latter, unless you elaborate on what
> do you actually mean by "nasty".

Post install scripts should be avoided whenever possible and used only
for things that cannot be done at build or install time. This clearly
can be done at install time, so it should.

Tomas

-- 
http://sleepfive.com

Re: [PATCH v2 5/5] shutdown-desktop: give entire path in Exec field

[Laurentiu Palcu]

On Thu, Jul 04, 2013 at 04:56:45PM +0100, Tomas Frydrych wrote:
> On 04/07/13 16:39, Laurentiu Palcu wrote:
> > On 07/04/2013 05:58 PM, Burton, Ross wrote:
> >> On 4 July 2013 11:58, Laurentiu Palcu <laurentiu.palcu@intel.com> wrote:
> > Even though the change you're proposing is OK, involves changing the
> > desktop file to add the @SBIN@ pattern and move the sed line to
> > do_install(). Compared to having one single line added in the
> > postinstall... I would choose the latter, unless you elaborate on what
> > do you actually mean by "nasty".
> 
> Post install scripts should be avoided whenever possible and used only
> for things that cannot be done at build or install time. This clearly
> can be done at install time, so it should.
I agree. However, in this particular case I'm just being consistent with
what was already there. I would have personally moved the entire
postinstall to do_install, but the previous sed cannot be moved. So,
since we're stuck with that sed there, this one is just a harmless
addition. This is what I mean!

Laurentiu

> 
> Tomas
> 
> -- 
> http://sleepfive.com
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core

[PATCH v3 0/5] Allow xuser to shutdown (cover letter only)

[Laurentiu Palcu]

Changes in v3:
 * made xuser-account inherit allarch;
 * moved the sed in do_install();

Changes in v2:
 * created a separate recipe for xuser creation, located in
   meta/recipes-support/user-creation/. If there are other users we might need
   to add in the future, we can put the recipes here;
 * removed xuser creation from connman and xserver-noadm-init and added the
   new 'xuser-account' recipe in RDEPENDS;

Laurentiu

The following changes since commit 8a186a6b3853fc1a7dcf342d421c8926c38949c9:

  bitbake: hob: save button from settings called a nonexisting method (2013-07-03 08:13:35 +0100)

are available in the git repository at:

  git://git.yoctoproject.org/poky-contrib lpalcu/b4345_shutdown_icon_rootless_x_v3
  http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=lpalcu/b4345_shutdown_icon_rootless_x_v3

Laurentiu Palcu (5):
  sysvinit: allow users in shutdown group to perform halt/reboot
  xuser-account: create separate recipe for xuser creation
  connman: remove xuser creation, RDEPEND on xuser-account
  xserver-nodm-init: remove xuser creation, RDEPEND on xuser-account
  shutdown-desktop: give entire path in Exec field

 meta/recipes-connectivity/connman/connman.inc      |   10 ++------
 meta/recipes-core/sysvinit/sysvinit_2.88dsf.bb     |   12 ++++++---
 .../x11-common/xserver-nodm-init.bb                |   13 ++--------
 .../shutdown-desktop/shutdown-desktop.bb           |    4 ++-
 .../user-creation/xuser-account_0.1.bb             |   27 ++++++++++++++++++++
 5 files changed, 43 insertions(+), 23 deletions(-)
 create mode 100644 meta/recipes-support/user-creation/xuser-account_0.1.bb

-- 
1.7.9.5