iputils: fix ping and traceroute6 executable permissions

PATCH New tolvupostur at gmail.com 6 participants 9 messages Jan 13, 2018 – Jan 31, 2018

lore

Share & tools

lore

tolvupostur at gmail.com

author

2018-01-13 20:19 UTC #0 lore raw

PATCH 1/1 iputils: fix ping and traceroute6 executable permissions

From: Einar Jon Gunnarsson <[email protected]>

The iputils executables are installed without the setuid bit set, which prevents some programs from working.

This patch adds a post-install hook to fix the permissions of the ping and traceroute6 executables.

Signed-off-by Einar Jon Gunnarsson <[email protected]>

---
 package/iputils/iputils.mk | 5 +++++
 1 file changed, 5 insertions(+)

package/iputils/iputils.mk +5 -0

index b20cd12..13e3389 100644
@@ -69,4 +69,9 @@ define IPUTILS_INSTALL_TARGET_CMDS
 	$(INSTALL) -D -m 755 $(@D)/traceroute6 $(TARGET_DIR)/bin/traceroute6
 endef
 
+define IPUTILS_PERMISSIONS
+	/bin/ping        f 4755 0 0 - - - - -
+	/bin/traceroute6 f 4755 0 0 - - - - -
+endef
+
 $(eval $(generic-package))

Signature · 1 line

1.9.1

Matthew Weber

2018-01-13 21:54 UTC #1 lore raw

PATCH 1/1 iputils: fix ping and traceroute6 executable permissions

Einar,

Quoted 5 lines · On Sat, Jan 13, 2018 at 2:19 PM, <[email protected]> wrote:

From: Einar Jon Gunnarsson <[email protected]>
The iputils executables are installed without the setuid bit set, which prevents some programs from working.

Does your use case involve a system with non-root users?

Could you describe what you mean by "some programs"?

The landscape of how ping gets elevated privileges for raw socket access has a number of options (setuid / cap_net_raw capability / new
socket type). The backwards compatible fix would be to use setuid
but from a security hardening aspect, I wish we could set capabilities for this instead. The issue I see is the filesystem type dependency
so we can pre-set the capabilities in xattribs. I'll have to ask
around if setuid vs capabilities has come up before but as most buildroot systems run as root, I'm guessing it hasn't been a hot topic.

Some backstory on Ubuntu's situation, I believe as of 16.04 they still did setuid but have selectively transitioned to not.
https://bugs.launchpad.net/ubuntu/+source/iputils/+bug/534341

package/iputils/iputils.mk define IPUTILS_INSTALL_TARGET_CMDS · line 75

Quoted 4 lines

package/iputils/iputils.mk +4 -0

+define IPUTILS_PERMISSIONS
+       /bin/ping        f 4755 0 0 - - - - -
+       /bin/traceroute6 f 4755 0 0 - - - - -
+endef

The package installs other binaries when IPUTILS_INSTALL_TARGET_CMDS executes, did you confirm that none of the others also require it?

Matt

Yann E. MORIN

2018-01-13 22:37 UTC #2 lore raw

PATCH 1/1 iputils: fix ping and traceroute6 executable permissions

Matt, Einar, All,

On 2018-01-13 15:54 -0600, Matthew Weber spake thusly:

Quoted 20 lines · On Sat, Jan 13, 2018 at 2:19 PM, <[email protected]> wrote:

Quoted 4 lines
From: Einar Jon Gunnarsson <[email protected]>
The iputils executables are installed without the setuid bit set, which prevents some programs from working.
Does your use case involve a system with non-root users?
Could you describe what you mean by "some programs"?
The landscape of how ping gets elevated privileges for raw socket access has a number of options (setuid / cap_net_raw capability / new
socket type). The backwards compatible fix would be to use setuid
but from a security hardening aspect, I wish we could set capabilities for this instead. The issue I see is the filesystem type dependency
so we can pre-set the capabilities in xattribs. I'll have to ask
around if setuid vs capabilities has come up before but as most buildroot systems run as root, I'm guessing it hasn't been a hot topic.

Quoted 3 lines

Some backstory on Ubuntu's situation, I believe as of 16.04 they still did setuid but have selectively transitioned to not.
https://bugs.launchpad.net/ubuntu/+source/iputils/+bug/534341

Still the case in 17.10, and if I remove the setuid bit, it fails:

    $ ping some-host
    ping: socket: Operation not permitted

Regards,
Yann E. MORIN.

Quoted 13 lines

package/iputils/iputils.mk define IPUTILS_INSTALL_TARGET_CMDS · line 75
Quoted 4 lines
package/iputils/iputils.mk +4 -0
+define IPUTILS_PERMISSIONS
+       /bin/ping        f 4755 0 0 - - - - -
+       /bin/traceroute6 f 4755 0 0 - - - - -
+endef
The package installs other binaries when IPUTILS_INSTALL_TARGET_CMDS executes, did you confirm that none of the others also require it?
Matt
_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Signature · 6 lines

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

Baruch Siach

2018-01-14 05:50 UTC #3 lore raw

PATCH 1/1 iputils: fix ping and traceroute6 executable permissions

Hi Yann,

Quoted 23 lines · Yann E. MORIN

Matt, Einar, All,
On 2018-01-13 15:54 -0600, Matthew Weber spake thusly:
Quoted 20 lines · On Sat, Jan 13, 2018 at 2:19 PM, <[email protected]> wrote:
Quoted 4 lines
From: Einar Jon Gunnarsson <[email protected]>
The iputils executables are installed without the setuid bit set, which prevents some programs from working.
Does your use case involve a system with non-root users?
Could you describe what you mean by "some programs"?
The landscape of how ping gets elevated privileges for raw socket access has a number of options (setuid / cap_net_raw capability / new
socket type). The backwards compatible fix would be to use setuid
but from a security hardening aspect, I wish we could set capabilities for this instead. The issue I see is the filesystem type dependency
so we can pre-set the capabilities in xattribs. I'll have to ask
around if setuid vs capabilities has come up before but as most buildroot systems run as root, I'm guessing it hasn't been a hot topic.

Quoted 8 lines

Quoted 3 lines
Some backstory on Ubuntu's situation, I believe as of 16.04 they still did setuid but have selectively transitioned to not.
https://bugs.launchpad.net/ubuntu/+source/iputils/+bug/534341
Still the case in 17.10, and if I remove the setuid bit, it fails:
    $ ping some-host
    ping: socket: Operation not permitted

The Debian iputils changelog[1] reads:

iputils (3:20121221-6) UNRELEASED; urgency=medium

* Remove setuid bits on binaries if setcap succeeds during

    reconfigure. (Closes: 778500)

setuid seems not to be set on my Debian testing machine:

$ ls -l /bin/ping
-rwxr-xr-x 1 root root 61240 Nov 10  2016 /bin/ping

The Ubuntu changelog[2] says this:

iputils (3:20150815-2ubuntu1) yakkety; urgency=low

* Merge from Debian unstable (LP: #1592425). Remaining changes:
- Support cross-building
- Mark ping and ping6 setuid again as there's currently no good ways to have capabilities be kept in all our images.

[1] https://tracker.debian.org/media/packages/i/iputils/changelog-3%3A20161105-1

[2] https://launchpad.net/ubuntu/+source/iputils/+changelog

Quoted 7 lines

Quoted 7 lines
package/iputils/iputils.mk define IPUTILS_INSTALL_TARGET_CMDS · line 75
Quoted 4 lines
package/iputils/iputils.mk +4 -0
+define IPUTILS_PERMISSIONS
+       /bin/ping        f 4755 0 0 - - - - -
+       /bin/traceroute6 f 4755 0 0 - - - - -
+endef
The package installs other binaries when IPUTILS_INSTALL_TARGET_CMDS executes, did you confirm that none of the others also require it?

baruch

Signature · 3 lines

     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

Einar Jón

author

2018-01-14 10:28 UTC #4 lore raw

PATCH 1/1 iputils: fix ping and traceroute6 executable permissions

Hi guys,

Quoted 14 lines · Baruch Siach

Hi Yann,
Quoted 11 lines · Yann E. MORIN
Matt, Einar, All,
On 2018-01-13 15:54 -0600, Matthew Weber spake thusly:
Quoted 8 lines · On Sat, Jan 13, 2018 at 2:19 PM, <[email protected]> wrote:
Quoted 4 lines
From: Einar Jon Gunnarsson <[email protected]>
The iputils executables are installed without the setuid bit set, which prevents some programs from working.
Does your use case involve a system with non-root users?

Yes, we have no root login, and most access is done with LDAP users.

Quoted 1 line

Quoted 1 line
Quoted 1 line
Could you describe what you mean by "some programs"?

I mean some of the iputils programs, i.e. the ones I'm fixing. That's just plagiarism from the sudo commit fixing a similar issue 0e8dafc - sudo: fix main executable permissions (5 years ago)<Simon Dawson>|

Quoted 20 lines

Quoted 11 lines
Quoted 11 lines
The landscape of how ping gets elevated privileges for raw socket access has a number of options (setuid / cap_net_raw capability / new
socket type). The backwards compatible fix would be to use setuid
but from a security hardening aspect, I wish we could set capabilities for this instead. The issue I see is the filesystem type dependency
so we can pre-set the capabilities in xattribs. I'll have to ask
around if setuid vs capabilities has come up before but as most buildroot systems run as root, I'm guessing it hasn't been a hot topic.
Quoted 8 lines
Quoted 3 lines
Some backstory on Ubuntu's situation, I believe as of 16.04 they still did setuid but have selectively transitioned to not.
https://bugs.launchpad.net/ubuntu/+source/iputils/+bug/534341
Still the case in 17.10, and if I remove the setuid bit, it fails:
    $ ping some-host
    ping: socket: Operation not permitted

Quoted 7 lines

The Debian iputils changelog[1] reads:
iputils (3:20121221-6) UNRELEASED; urgency=medium
* Remove setuid bits on binaries if setcap succeeds during
    reconfigure. (Closes: 778500)

I'm running iputils-s20160308 (current master), and I can confirm that ping does need
sudo (or similar superuser privileges).

Quoted 9 lines

setuid seems not to be set on my Debian testing machine:
$ ls -l /bin/ping
-rwxr-xr-x 1 root root 61240 Nov 10  2016 /bin/ping
The Ubuntu changelog[2] says this:
iputils (3:20150815-2ubuntu1) yakkety; urgency=low

Quoted 16 lines

* Merge from Debian unstable (LP: #1592425). Remaining changes:
- Support cross-building
- Mark ping and ping6 setuid again as there's currently no good ways to have capabilities be kept in all our images.
[1] https://tracker.debian.org/media/packages/i/iputils/changelog-3%3A20161105-1
[2] https://launchpad.net/ubuntu/+source/iputils/+changelog
Quoted 7 lines
Quoted 7 lines
package/iputils/iputils.mk define IPUTILS_INSTALL_TARGET_CMDS · line 75
Quoted 4 lines
package/iputils/iputils.mk +4 -0
+define IPUTILS_PERMISSIONS
+       /bin/ping        f 4755 0 0 - - - - -
+       /bin/traceroute6 f 4755 0 0 - - - - -
+endef
The package installs other binaries when IPUTILS_INSTALL_TARGET_CMDS executes, did you confirm that none of the others also require it?

I actually just checked the suid bits set in Ubuntu and used those. There it is only traceroute6 and ping.

The full list is
/sbin/arping - needs root on my machine /sbin/rarpd - daemon
/bin/clockdiff - needs root on my machine /bin/ping - needs root on my machine /sbin/rdisc - daemon
/usr/sbin/in.tftpd - daemon
/bin/tracepath - works without root /bin/traceroute6 - Dunno. Needs IPv6 on my machine.

The daemons do require superuser privileges, of course, so I don't mess with those.
arping is in sbin, so I left that alone.
I forgot about clockdiff, to be honest.
I don't have IPv6 support, so traceroute6 gives me an error, but the man page of tracepath says this:
- It is *similar to traceroute, only does not require superuser
privileges* and has no fancy options.

From the man pages:
- ping requires CAP_NET_RAW capability to be executed 1) if the
program is used for non-echo queries (See -N option), or 2) if kernel does not
support non-raw ICMP sockets, or 3) if the user is not allowed
to create an ICMP echo socket. The program may be used as set-uid root.
- tracepath6 requires CAP_NET_RAW capability to be executed. It is
safe to be used as set-uid root.
- clockdiff requires CAP_NET_RAW capability to be executed. It is
safe to be used as set-uid root.
- arping requires CAP_NET_RAW capability to be executed. It is not
recommended to be used as set-uid root, because it allows user to modify ARP
caches of neighbour hosts.

traceroute6 also has suid bit set in ubuntu, and claims to need it in the man pages, so I added it in.
I should test if it is really needed.

So the question is if clockdiff should also get suid bits, or if we solve this a different way.

Quoted 7 lines

baruch
--
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

Signature · 2 lines

Regards
Einar J?n

Einar Jón

author

2018-01-15 09:50 UTC #5 lore raw

PATCH 1/1 iputils: fix ping and traceroute6 executable permissions

Hi again

Some inconsistencies in my last email. Fixed below.
My wife was not happy that I was spending my Sunday morning on this, so I sent it too soon...

Recap:
The man pages say that these programs require CAP_NET_RAW capability. Confirmed with ping and clockdiff - they fail without root privileges. I can't test traceroute6 on my machines since they are IPv4 only. That needs to be addressed one way or the other.
Other tools, like daemons and arping, need root privileges but should not get a suid pid.
Man pages say that setting SUID bits safe for these 3:
ping, traceroute6 and clockdiff.

Questions:
Shall I make a new patch with suid bit also set for clockdiff? Can someone verify traceroute6?

------------
[user at arm ~]$ping -c1 localhost
ping: socket: Operation not permitted
ping: socket: Address family not supported by protocol
[user at arm ~]$sudo ping -c1 localhost
PING localhost (127.0.0.1) 56(84) bytes of data.
64 bytes from localhost (127.0.0.1): icmp_seq=1 ttl=64 time=0.207 ms [user at arm ~]$clockdiff localhost
clockdiff: socket: Operation not permitted
[user at arm ~]$sudo clockdiff localhost
...
host=localhost rtt=421(315)ms/0ms delta=0ms/0ms Mon Jan 15 10:14:38 2018

Quoted 50 lines · Einar J?n

Hi guys,
Quoted 14 lines · Baruch Siach
Hi Yann,
Quoted 11 lines · Yann E. MORIN
Matt, Einar, All,
On 2018-01-13 15:54 -0600, Matthew Weber spake thusly:
Quoted 8 lines · On Sat, Jan 13, 2018 at 2:19 PM, <[email protected]> wrote:
Quoted 4 lines
From: Einar Jon Gunnarsson <[email protected]>
The iputils executables are installed without the setuid bit set, which prevents some programs from working.
Does your use case involve a system with non-root users?
Yes, we have no root login, and most access is done with LDAP users.
Quoted 1 line
Quoted 1 line
Quoted 1 line
Could you describe what you mean by "some programs"?
I mean some of the iputils programs, i.e. the ones I'm fixing. That's just plagiarism from the sudo commit fixing a similar issue 0e8dafc - sudo: fix main executable permissions (5 years ago)<Simon Dawson>|
Quoted 24 lines
Quoted 11 lines
Quoted 11 lines
The landscape of how ping gets elevated privileges for raw socket access has a number of options (setuid / cap_net_raw capability / new
socket type). The backwards compatible fix would be to use setuid
but from a security hardening aspect, I wish we could set capabilities for this instead. The issue I see is the filesystem type dependency
so we can pre-set the capabilities in xattribs. I'll have to ask
around if setuid vs capabilities has come up before but as most buildroot systems run as root, I'm guessing it hasn't been a hot topic.
Quoted 8 lines
Quoted 3 lines
Some backstory on Ubuntu's situation, I believe as of 16.04 they still did setuid but have selectively transitioned to not.
https://bugs.launchpad.net/ubuntu/+source/iputils/+bug/534341
Still the case in 17.10, and if I remove the setuid bit, it fails:
    $ ping some-host
    ping: socket: Operation not permitted
The Debian iputils changelog[1] reads:
iputils (3:20121221-6) UNRELEASED; urgency=medium

Quoted 7 lines

Quoted 3 lines
* Remove setuid bits on binaries if setcap succeeds during
    reconfigure. (Closes: 778500)
I'm running iputils-s20160308 (current master), and I can confirm that ping does need
sudo (or similar superuser privileges).

I'm running iputils-s20160308 (2017.02), and I can confirm that ping does need sudo (or similar superuser privileges).
I also built current master, iputils-s20161105, and it behaves the same

Quoted 49 lines

Quoted 10 lines
setuid seems not to be set on my Debian testing machine:
$ ls -l /bin/ping
-rwxr-xr-x 1 root root 61240 Nov 10  2016 /bin/ping
The Ubuntu changelog[2] says this:
iputils (3:20150815-2ubuntu1) yakkety; urgency=low
Quoted 16 lines
* Merge from Debian unstable (LP: #1592425). Remaining changes:
- Support cross-building
- Mark ping and ping6 setuid again as there's currently no good ways to have capabilities be kept in all our images.
[1] https://tracker.debian.org/media/packages/i/iputils/changelog-3%3A20161105-1
[2] https://launchpad.net/ubuntu/+source/iputils/+changelog
Quoted 7 lines
Quoted 7 lines
package/iputils/iputils.mk define IPUTILS_INSTALL_TARGET_CMDS · line 75
Quoted 4 lines
package/iputils/iputils.mk +4 -0
+define IPUTILS_PERMISSIONS
+       /bin/ping        f 4755 0 0 - - - - -
+       /bin/traceroute6 f 4755 0 0 - - - - -
+endef
The package installs other binaries when IPUTILS_INSTALL_TARGET_CMDS executes, did you confirm that none of the others also require it?
I actually just checked the suid bits set in Ubuntu and used those. There it is only traceroute6 and ping.
The full list is
/sbin/arping - needs root on my machine /sbin/rarpd - daemon
/bin/clockdiff - needs root on my machine /bin/ping - needs root on my machine /sbin/rdisc - daemon
/usr/sbin/in.tftpd - daemon
/bin/tracepath - works without root /bin/traceroute6 - Dunno. Needs IPv6 on my machine.
The daemons do require superuser privileges, of course, so I don't mess with those.
arping is in sbin, so I left that alone.
I forgot about clockdiff, to be honest.
I don't have IPv6 support, so traceroute6 gives me an error, but the man page of tracepath says this:
- It is *similar to traceroute, only does not require superuser
privileges* and has no fancy options.

Quoted 9 lines

From the man pages:
- ping requires CAP_NET_RAW capability to be executed 1) if the
program is used for non-echo queries (See -N option), or 2) if kernel does not
support non-raw ICMP sockets, or 3) if the user is not allowed
to create an ICMP echo socket. The program may be used as set-uid root.
- tracepath6 requires CAP_NET_RAW capability to be executed. It is
safe to be used as set-uid root.

- traceROUTE6 requires CAP_NET_RAW capability to be executed. It is
safe to be used as set-uid root.

Quoted 28 lines

- clockdiff requires CAP_NET_RAW capability to be executed. It is
safe to be used as set-uid root.
- arping requires CAP_NET_RAW capability to be executed. It is not
recommended to be used as set-uid root, because it allows user to modify ARP
caches of neighbour hosts.
traceroute6 also has suid bit set in ubuntu, and claims to need it in the man pages, so I added it in.
I should test if it is really needed.
So the question is if clockdiff should also get suid bits, or if we solve this a different way.
Quoted 7 lines
baruch
--
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
--
Regards
Einar J?n

Signature · 3 lines

Regards
Einar J?n
+31 610 957234

Thomas Petazzoni

Applied

2018-01-17 22:24 UTC #6 lore raw

PATCH 1/1 iputils: fix ping and traceroute6 executable permissions

In reply to tolvupostur at gmail.com

Hello,

Quoted 7 lines · tolvupostur at gmail.com

From: Einar Jon Gunnarsson <[email protected]>
The iputils executables are installed without the setuid bit set, which prevents some programs from working.
This patch adds a post-install hook to fix the permissions of the ping and traceroute6 executables.

You didn't add a post-install hook, but a permission table.

I've applied after adjusting the commit log. Thanks!

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Peter Korsgaard

2018-01-21 20:27 UTC #7 lore raw

PATCH 1/1 iputils: fix ping and traceroute6 executable permissions

Quoted 9 lines

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Thomas" == Thomas Petazzoni <[email protected]> writes:
Hello,
Quoted 5 lines · tolvupostur at gmail.com
From: Einar Jon Gunnarsson <[email protected]>
The iputils executables are installed without the setuid bit set, which prevents some programs from working.

Quoted 6 lines

Quoted 2 lines
This patch adds a post-install hook to fix the permissions of the ping and traceroute6 executables.
You didn't add a post-install hook, but a permission table.
I've applied after adjusting the commit log. Thanks!

Committed to 2017.11.x, thanks.

Signature · 1 line

Bye, Peter Korsgaard

Peter Korsgaard

2018-01-31 07:51 UTC #8 lore raw

PATCH 1/1 iputils: fix ping and traceroute6 executable permissions

In reply to tolvupostur at gmail.com

Quoted 10 lines

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"tolvupostur" == tolvupostur <[email protected]> writes:
From: Einar Jon Gunnarsson <[email protected]>
The iputils executables are installed without the setuid bit set, which prevents some programs from working.
This patch adds a post-install hook to fix the permissions of the ping and traceroute6 executables.
Signed-off-by Einar Jon Gunnarsson <[email protected]>

Committed to 2017.02.x, thanks.

Signature · 1 line

Bye, Peter Korsgaard