[Buildroot] [PATCH] iputils: fix ping and traceroute6 executable permissions

[Buildroot] [PATCH] iputils: fix ping and traceroute6 executable permissions

[tolvupostur at gmail.com]

From: Einar Jon Gunnarsson <tolvupostur@gmail.com>

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 <tolvupostur@gmail.com>
---
 package/iputils/iputils.mk | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/package/iputils/iputils.mk b/package/iputils/iputils.mk
index b20cd12..13e3389 100644
--- a/package/iputils/iputils.mk
+++ b/package/iputils/iputils.mk
@@ -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))
-- 
1.9.1

[Buildroot] [PATCH] iputils: fix ping and traceroute6 executable permissions

[Matthew Weber]

Einar,

On Sat, Jan 13, 2018 at 2:19 PM,  <tolvupostur@gmail.com> wrote:
> From: Einar Jon Gunnarsson <tolvupostur@gmail.com>
>
> 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

> +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] [PATCH] iputils: fix ping and traceroute6 executable permissions

[Yann E. MORIN]

Matt, Einar, All,

On 2018-01-13 15:54 -0600, Matthew Weber spake thusly:
> On Sat, Jan 13, 2018 at 2:19 PM,  <tolvupostur@gmail.com> wrote:
> > From: Einar Jon Gunnarsson <tolvupostur@gmail.com>
> > 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

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.

> > +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

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

[Buildroot] [PATCH] iputils: fix ping and traceroute6 executable permissions

[Baruch Siach]

Hi Yann,

On Sat, Jan 13, 2018 at 11:37:09PM +0100, Yann E. MORIN wrote:
> Matt, Einar, All,
> 
> On 2018-01-13 15:54 -0600, Matthew Weber spake thusly:
> > On Sat, Jan 13, 2018 at 2:19 PM,  <tolvupostur@gmail.com> wrote:
> > > From: Einar Jon Gunnarsson <tolvupostur@gmail.com>
> > > 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
> 
> 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

> > > +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

-- 
     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 -

[Buildroot] [PATCH] iputils: fix ping and traceroute6 executable permissions

[Einar Jón]

Hi guys,

On 14 January 2018 at 06:50, Baruch Siach <baruch@tkos.co.il> wrote:
> Hi Yann,
>
> On Sat, Jan 13, 2018 at 11:37:09PM +0100, Yann E. MORIN wrote:
>> Matt, Einar, All,
>>
>> On 2018-01-13 15:54 -0600, Matthew Weber spake thusly:
>> > On Sat, Jan 13, 2018 at 2:19 PM,  <tolvupostur@gmail.com> wrote:
>> > > From: Einar Jon Gunnarsson <tolvupostur@gmail.com>
>> > > 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.

>> > 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>|

>> >
>> > 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
>>
>> 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)
>

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

> 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
>
>> > > +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.

>
> 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

[Buildroot] [PATCH] iputils: fix ping and traceroute6 executable permissions

[Einar Jón]

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

On 14 January 2018 at 11:28, Einar J?n <tolvupostur@gmail.com> wrote:
> Hi guys,
>
> On 14 January 2018 at 06:50, Baruch Siach <baruch@tkos.co.il> wrote:
>> Hi Yann,
>>
>> On Sat, Jan 13, 2018 at 11:37:09PM +0100, Yann E. MORIN wrote:
>>> Matt, Einar, All,
>>>
>>> On 2018-01-13 15:54 -0600, Matthew Weber spake thusly:
>>> > On Sat, Jan 13, 2018 at 2:19 PM,  <tolvupostur@gmail.com> wrote:
>>> > > From: Einar Jon Gunnarsson <tolvupostur@gmail.com>
>>> > > 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.
>
>>> > 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>|
>
>>> >
>>> > 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
>>>
>>> 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)
>>
>
> 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

>
>> 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
>>
>>> > > +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.

 - traceROUTE6 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.
>
>>
>> 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



-- 
Regards
Einar J?n
+31 610 957234

[Buildroot] [PATCH] iputils: fix ping and traceroute6 executable permissions

[Thomas Petazzoni]

Hello,

On Sat, 13 Jan 2018 21:19:42 +0100, tolvupostur at gmail.com wrote:
> From: Einar Jon Gunnarsson <tolvupostur@gmail.com>
> 
> 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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH] iputils: fix ping and traceroute6 executable permissions

[Peter Korsgaard]

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:

 > Hello,
 > On Sat, 13 Jan 2018 21:19:42 +0100, tolvupostur at gmail.com wrote:
 >> From: Einar Jon Gunnarsson <tolvupostur@gmail.com>
 >> 
 >> 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!

Committed to 2017.11.x, thanks.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH] iputils: fix ping and traceroute6 executable permissions

[Peter Korsgaard]

>>>>> "tolvupostur" == tolvupostur  <tolvupostur@gmail.com> writes:

 > From: Einar Jon Gunnarsson <tolvupostur@gmail.com>
 > 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 <tolvupostur@gmail.com>

Committed to 2017.02.x, thanks.

-- 
Bye, Peter Korsgaard