* [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 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.