[Buildroot] [PATCH 0/2 v7] package/iputils: use capabilties (branch yem/iputils-caps)

[Buildroot] [PATCH 0/2 v7] package/iputils: use capabilties (branch yem/iputils-caps)

[Yann E. MORIN]

Hello All!

This small series changes iputils to use capabilities if support for
extended attributes is enabled.

Additionally, arping can be used for arp poisoning, so it should not be
usable by a non-root user. As such, we remove the setuid bit, and do not
assign it a capability either.


Regards,
Yann E. MORIN.


The following changes since commit 152e160bf6fcbb8f73c60944107e3a82f91edf50

  package/netsurf: add optional dependency on webp (2019-08-01 16:54:04 +0200)


are available in the git repository at:

  git://git.buildroot.org/~ymorin/git/buildroot.git

for you to fetch changes up to 7357ef816dae35990f461175f35135fb4996eb54

  package/iputils: use capabilities if possible (2019-08-01 18:18:07 +0200)


----------------------------------------------------------------
Petr Vorel (1):
      package/iputils: use capabilities if possible

Yann E. MORIN (1):
      package/iputils: drop setuid on arping

 package/iputils/iputils.mk | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 1/2 v7] package/iputils: drop setuid on arping

[Yann E. MORIN]

arping can be used for arp poisoning, so it should really not be setuid.

Reported-by: Petr Vorel <petr.vorel@gmail.com>
Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
---
 package/iputils/iputils.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/iputils/iputils.mk b/package/iputils/iputils.mk
index b28222138f..7482bbdca1 100644
--- a/package/iputils/iputils.mk
+++ b/package/iputils/iputils.mk
@@ -80,7 +80,7 @@ IPUTILS_POST_INSTALL_TARGET_HOOKS += IPUTILS_CREATE_PING6_SYMLINK
 # handle permissions ourselves
 IPUTILS_CONF_OPTS += -DNO_SETCAP_OR_SUID=true
 define IPUTILS_PERMISSIONS
-	/usr/sbin/arping      f 4755 0 0 - - - - -
+	/usr/sbin/arping      f  755 0 0 - - - - -
 	/usr/bin/clockdiff    f 4755 0 0 - - - - -
 	/bin/ping             f 4755 0 0 - - - - -
 	/usr/bin/traceroute6  f 4755 0 0 - - - - -
-- 
2.20.1

[Buildroot] [PATCH 2/2 v7] package/iputils: use capabilities if possible

[Yann E. MORIN]

From: Petr Vorel <petr.vorel@gmail.com>

If support for extended attributes is enabled, then we can use them to
store capabilities. If not, we keep using the setuid bit.

arping does not get a capability, as it can be used for arp poisoning.

Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
[yann.morin.1998 at free.fr:
  - resort to using q full-fledged conditional block
]
Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>

---
Changes v6 -> v7:
  - use a full if-block
---
 package/iputils/iputils.mk | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/package/iputils/iputils.mk b/package/iputils/iputils.mk
index 7482bbdca1..cbcce89684 100644
--- a/package/iputils/iputils.mk
+++ b/package/iputils/iputils.mk
@@ -79,11 +79,23 @@ IPUTILS_POST_INSTALL_TARGET_HOOKS += IPUTILS_CREATE_PING6_SYMLINK
 
 # handle permissions ourselves
 IPUTILS_CONF_OPTS += -DNO_SETCAP_OR_SUID=true
+ifeq ($(BR2_ROOTFS_DEVICE_TABLE_SUPPORTS_EXTENDED_ATTRIBUTES),y)
+define IPUTILS_PERMISSIONS
+	/usr/sbin/arping      f 755 0 0 - - - - -
+	/usr/bin/clockdiff    f 755 0 0 - - - - -
+	|xattr cap_net_raw+p
+	/bin/ping             f 755 0 0 - - - - -
+	|xattr cap_net_raw+p
+	/usr/bin/traceroute6  f 755 0 0 - - - - -
+	|xattr cap_net_raw+p
+endef
+else
 define IPUTILS_PERMISSIONS
 	/usr/sbin/arping      f  755 0 0 - - - - -
 	/usr/bin/clockdiff    f 4755 0 0 - - - - -
 	/bin/ping             f 4755 0 0 - - - - -
 	/usr/bin/traceroute6  f 4755 0 0 - - - - -
 endef
+endif
 
 $(eval $(meson-package))
-- 
2.20.1

[Buildroot] [PATCH 1/2 v7] package/iputils: drop setuid on arping

[Petr Vorel]

Hi Yann,

> arping can be used for arp poisoning, so it should really not be setuid.

> Reported-by: Petr Vorel <petr.vorel@gmail.com>
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
Acked-by: Petr Vorel <petr.vorel@gmail.com>
+1
> ---
>  package/iputils/iputils.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

> diff --git a/package/iputils/iputils.mk b/package/iputils/iputils.mk
> index b28222138f..7482bbdca1 100644
> --- a/package/iputils/iputils.mk
> +++ b/package/iputils/iputils.mk
> @@ -80,7 +80,7 @@ IPUTILS_POST_INSTALL_TARGET_HOOKS += IPUTILS_CREATE_PING6_SYMLINK
>  # handle permissions ourselves
>  IPUTILS_CONF_OPTS += -DNO_SETCAP_OR_SUID=true
>  define IPUTILS_PERMISSIONS
> -	/usr/sbin/arping      f 4755 0 0 - - - - -
> +	/usr/sbin/arping      f  755 0 0 - - - - -
>  	/usr/bin/clockdiff    f 4755 0 0 - - - - -
>  	/bin/ping             f 4755 0 0 - - - - -
>  	/usr/bin/traceroute6  f 4755 0 0 - - - - -


Kind regards,
Petr

[Buildroot] [PATCH 2/2 v7] package/iputils: use capabilities if possible

[Petr Vorel]

Hi Yann,

> From: Petr Vorel <petr.vorel@gmail.com>

> If support for extended attributes is enabled, then we can use them to
> store capabilities. If not, we keep using the setuid bit.

> arping does not get a capability, as it can be used for arp poisoning.

> Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
> [yann.morin.1998 at free.fr:
>   - resort to using q full-fledged conditional block
> ]
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>

> ---
> Changes v6 -> v7:
>   - use a full if-block
> ---
>  package/iputils/iputils.mk | 12 ++++++++++++
>  1 file changed, 12 insertions(+)

> diff --git a/package/iputils/iputils.mk b/package/iputils/iputils.mk
> index 7482bbdca1..cbcce89684 100644
> --- a/package/iputils/iputils.mk
> +++ b/package/iputils/iputils.mk
> @@ -79,11 +79,23 @@ IPUTILS_POST_INSTALL_TARGET_HOOKS += IPUTILS_CREATE_PING6_SYMLINK

>  # handle permissions ourselves
>  IPUTILS_CONF_OPTS += -DNO_SETCAP_OR_SUID=true
> +ifeq ($(BR2_ROOTFS_DEVICE_TABLE_SUPPORTS_EXTENDED_ATTRIBUTES),y)
> +define IPUTILS_PERMISSIONS
> +	/usr/sbin/arping      f 755 0 0 - - - - -
> +	/usr/bin/clockdiff    f 755 0 0 - - - - -
> +	|xattr cap_net_raw+p
> +	/bin/ping             f 755 0 0 - - - - -
> +	|xattr cap_net_raw+p
> +	/usr/bin/traceroute6  f 755 0 0 - - - - -
> +	|xattr cap_net_raw+p
> +endef
> +else
>  define IPUTILS_PERMISSIONS
>  	/usr/sbin/arping      f  755 0 0 - - - - -
>  	/usr/bin/clockdiff    f 4755 0 0 - - - - -
>  	/bin/ping             f 4755 0 0 - - - - -
>  	/usr/bin/traceroute6  f 4755 0 0 - - - - -
>  endef
> +endif
I wanted to avoid duplicity, but this is really clearer to read!
Thanks for fixing arping setuid.

Kind regards,
Petr

>  $(eval $(meson-package))

[Buildroot] [PATCH 1/2 v7] package/iputils: drop setuid on arping

[Peter Korsgaard]

>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:

 > arping can be used for arp poisoning, so it should really not be setuid.
 > Reported-by: Petr Vorel <petr.vorel@gmail.com>
 > Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>

Committed, thanks.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH 2/2 v7] package/iputils: use capabilities if possible

[Peter Korsgaard]

>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:

 > From: Petr Vorel <petr.vorel@gmail.com>
 > If support for extended attributes is enabled, then we can use them to
 > store capabilities. If not, we keep using the setuid bit.

 > arping does not get a capability, as it can be used for arp poisoning.

 > Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
 > [yann.morin.1998 at free.fr:
 >   - resort to using q full-fledged conditional block
 > ]
 > Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>

 > ---
 > Changes v6 -> v7:
 >   - use a full if-block

Committed, thanks.

-- 
Bye, Peter Korsgaard