* [Buildroot] [PATCH] package/sudo: new config to add sudo group and rule @ 2019-11-03 21:41 Stephan Henningsen 2019-11-05 18:51 ` Yann E. MORIN 0 siblings, 1 reply; 3+ messages in thread From: Stephan Henningsen @ 2019-11-03 21:41 UTC (permalink / raw) To: buildroot From: Stephan Henningsen <stephan+buildroot@asklandd.dk> Signed-off-by: Stephan Henningsen <stephan+buildroot@asklandd.dk> --- package/sudo/Config.in | 15 +++++++++++++++ package/sudo/sudo.mk | 13 +++++++++++++ 2 files changed, 28 insertions(+) diff --git a/package/sudo/Config.in b/package/sudo/Config.in index cbef15d67b..403d634ceb 100644 --- a/package/sudo/Config.in +++ b/package/sudo/Config.in @@ -9,3 +9,18 @@ config BR2_PACKAGE_SUDO but still allow people to get their work done. http://www.sudo.ws/sudo/ + + +if BR2_PACKAGE_SUDO + +config BR2_PACKAGE_SUDO_GROUP_AND_RULE + bool "add group 'sudo' and enable associated sudo rule" + select BR2_PACKAGE_SUDO_GROUP + help + Creates a group named 'sudo', and enables the following rule + in the /etc/sudoers configuration file that allows members of + group 'sudo' to execute any command as root: + + %sudo ALL=(ALL) ALL + +endif diff --git a/package/sudo/sudo.mk b/package/sudo/sudo.mk index cf8b63b1db..5df39b193e 100644 --- a/package/sudo/sudo.mk +++ b/package/sudo/sudo.mk @@ -64,4 +64,17 @@ define SUDO_PERMISSIONS /usr/bin/sudo f 4755 0 0 - - - - - endef +ifeq ($(BR2_PACKAGE_SUDO_GROUP_AND_RULE),y) +define SUDO_USERS + - -1 sudo -1 - - - - +endef + +define SUDO_ENABLE_SUDO_GROUP_RULE + $(SED) '/^# \%sudo\tALL=(ALL) ALL/s/^# //' $(TARGET_DIR)/etc/sudoers +endef + +SUDO_POST_INSTALL_TARGET_HOOKS += SUDO_ENABLE_SUDO_GROUP_RULE + +endif + $(eval $(autotools-package)) -- 2.17.1 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* [Buildroot] [PATCH] package/sudo: new config to add sudo group and rule 2019-11-03 21:41 [Buildroot] [PATCH] package/sudo: new config to add sudo group and rule Stephan Henningsen @ 2019-11-05 18:51 ` Yann E. MORIN 2019-11-05 20:42 ` Stephan Henningsen 0 siblings, 1 reply; 3+ messages in thread From: Yann E. MORIN @ 2019-11-05 18:51 UTC (permalink / raw) To: buildroot Stephan, All, On 2019-11-03 22:41 +0100, Stephan Henningsen spake thusly: > From: Stephan Henningsen <stephan+buildroot@asklandd.dk> > > Signed-off-by: Stephan Henningsen <stephan+buildroot@asklandd.dk> > --- > package/sudo/Config.in | 15 +++++++++++++++ > package/sudo/sudo.mk | 13 +++++++++++++ > 2 files changed, 28 insertions(+) > > diff --git a/package/sudo/Config.in b/package/sudo/Config.in > index cbef15d67b..403d634ceb 100644 > --- a/package/sudo/Config.in > +++ b/package/sudo/Config.in > @@ -9,3 +9,18 @@ config BR2_PACKAGE_SUDO > but still allow people to get their work done. > > http://www.sudo.ws/sudo/ > + > + > +if BR2_PACKAGE_SUDO > + > +config BR2_PACKAGE_SUDO_GROUP_AND_RULE > + bool "add group 'sudo' and enable associated sudo rule" > + select BR2_PACKAGE_SUDO_GROUP > + help > + Creates a group named 'sudo', and enables the following rule > + in the /etc/sudoers configuration file that allows members of > + group 'sudo' to execute any command as root: > + > + %sudo ALL=(ALL) ALL I thought the conclusion from the previous iteration was that the addition of the group and sudo rules were to be non-optional. > +endif > diff --git a/package/sudo/sudo.mk b/package/sudo/sudo.mk > index cf8b63b1db..5df39b193e 100644 > --- a/package/sudo/sudo.mk > +++ b/package/sudo/sudo.mk > @@ -64,4 +64,17 @@ define SUDO_PERMISSIONS > /usr/bin/sudo f 4755 0 0 - - - - - > endef > > +ifeq ($(BR2_PACKAGE_SUDO_GROUP_AND_RULE),y) > +define SUDO_USERS > + - -1 sudo -1 - - - - When the username is '-', even the uid is ignored, we usually make it '-' too. Also, there are too many spaces (see PERMISSIONS above). So, I removed the condition (and thus the Config.in option), tweaked the USERS variable, and pushed to master now. Thanks! Regards, Yann E. MORIN. > +endef > + > +define SUDO_ENABLE_SUDO_GROUP_RULE > + $(SED) '/^# \%sudo\tALL=(ALL) ALL/s/^# //' $(TARGET_DIR)/etc/sudoers > +endef > + > +SUDO_POST_INSTALL_TARGET_HOOKS += SUDO_ENABLE_SUDO_GROUP_RULE > + > +endif > + > $(eval $(autotools-package)) > -- > 2.17.1 > > _______________________________________________ > buildroot mailing list > buildroot at busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot -- .-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 561 099 427 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------' ^ permalink raw reply [flat|nested] 3+ messages in thread
* [Buildroot] [PATCH] package/sudo: new config to add sudo group and rule 2019-11-05 18:51 ` Yann E. MORIN @ 2019-11-05 20:42 ` Stephan Henningsen 0 siblings, 0 replies; 3+ messages in thread From: Stephan Henningsen @ 2019-11-05 20:42 UTC (permalink / raw) To: buildroot Hey, On Tue, Nov 5, 2019 at 7:51 PM Yann E. MORIN <yann.morin.1998@free.fr> wrote: > On 2019-11-03 22:41 +0100, Stephan Henningsen spake thusly: > > +if BR2_PACKAGE_SUDO > > + > > +config BR2_PACKAGE_SUDO_GROUP_AND_RULE > > + bool "add group 'sudo' and enable associated sudo rule" > > + select BR2_PACKAGE_SUDO_GROUP > > + help > > + Creates a group named 'sudo', and enables the following rule > > + in the /etc/sudoers configuration file that allows members of > > + group 'sudo' to execute any command as root: > > + > > + %sudo ALL=(ALL) ALL > > I thought the conclusion from the previous iteration was that the > addition of the group and sudo rules were to be non-optional. Sorry, I thought the idea was to combine the rule and group together into one option, since they make perfect sense together, and aren't very useful separately. I don't agree that they should be non-optional (or default). The sudo group+rule is just one of many use-cases for the sudo program. Others include granting only access to a specific user (root, joe, etc.) or existing system groups (wheel, daemon, etc.). People who rely on these features and are concerned about security, would need to clean up the mess that the non-optional BR2_PACKAGE_SUDO_GROUP_AND_RULE brought in, and revert the size of the attack surface. In fact, since this is now the default, most users will most likely not notice this has been added. If some already added the 'sudo' system group and their own (different) rules, their system might break or even open for permission elevation silently. I don't see how anything good can come out of this. (Except for my particular case maybe, because I'm the one who had the need and made the patch with the option ;). I really think it should be an option, just like the patch provided. > > +endif > > diff --git a/package/sudo/sudo.mk b/package/sudo/sudo.mk > > index cf8b63b1db..5df39b193e 100644 > > --- a/package/sudo/sudo.mk > > +++ b/package/sudo/sudo.mk > > @@ -64,4 +64,17 @@ define SUDO_PERMISSIONS > > /usr/bin/sudo f 4755 0 0 - - - - - > > endef > > > > +ifeq ($(BR2_PACKAGE_SUDO_GROUP_AND_RULE),y) > > +define SUDO_USERS > > + - -1 sudo -1 - - - - > > When the username is '-', even the uid is ignored, we usually make it > '-' too. Makes sense. > Also, there are too many spaces (see PERMISSIONS above). Even though I usually like spaces, I agree here =). I don't know how they got there. Perhaps I had a header there for personal documentation. > So, I removed the condition (and thus the Config.in option), tweaked the > USERS variable, and pushed to master now. Please reconsider only cleaning up the USERS variable. I think adding potentially unused non-standard system groups, sudo rules, and silenting, inadvertently allowing running commands as root is a bad idea. -- Regards, Stephan ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-11-05 20:42 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-11-03 21:41 [Buildroot] [PATCH] package/sudo: new config to add sudo group and rule Stephan Henningsen 2019-11-05 18:51 ` Yann E. MORIN 2019-11-05 20:42 ` Stephan Henningsen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox