[Buildroot] [PATCH 1/1] iputils: set the permissions with IPUTILS_PERMISSIONS

[Buildroot] [PATCH 1/1] iputils: set the permissions with IPUTILS_PERMISSIONS

[Petr Vorel]

for arping, clockdiff and ping. These require root permission for
socket(). Upstream solution (script setcap-setuid.sh) requires
setcap binary on the host and cap_net_raw+ep + libcap or setting
setuid root executable which is dangerous).

This restores behavior used for version s20180629 (which was used
for ping and traceroute6, but we don't build traceroute6 since s20190515).

Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
---
Hi,

it'd be great also create symlink ping -> ping6, but don't know how.

Kind regards,
Petr
---
 package/iputils/iputils.mk | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/package/iputils/iputils.mk b/package/iputils/iputils.mk
index d9a51ece2a..8be54b4788 100644
--- a/package/iputils/iputils.mk
+++ b/package/iputils/iputils.mk
@@ -52,4 +52,12 @@ endif
 # XSL Stylesheets for DocBook 5 not packaged for buildroot
 IPUTILS_CONF_OPTS += -DBUILD_MANS=false -DBUILD_HTML_MANS=false
 
+# handle permissions ourselves
+IPUTILS_CONF_OPTS += -DNO_SETCAP_OR_SUID=true
+define IPUTILS_PERMISSIONS
+	/bin/arping      f 4755 0 0 - - - - -
+	/bin/clockdiff   f 4755 0 0 - - - - -
+	/bin/ping        f 4755 0 0 - - - - -
+endef
+
 $(eval $(meson-package))
-- 
2.20.1

[Buildroot] [PATCH 1/1] iputils: set the permissions with IPUTILS_PERMISSIONS

[Arnout Vandecappelle]

On 10/06/2019 01:07, Petr Vorel wrote:
> for arping, clockdiff and ping. These require root permission for
> socket(). Upstream solution (script setcap-setuid.sh) requires
> setcap binary on the host and cap_net_raw+ep + libcap or setting
> setuid root executable which is dangerous).
> 
> This restores behavior used for version s20180629 (which was used
> for ping and traceroute6, but we don't build traceroute6 since s20190515).
> 
> Signed-off-by: Petr Vorel <petr.vorel@gmail.com>

 Applied to master, thanks. I updated the commit message a little.



> ---
> Hi,
> 
> it'd be great also create symlink ping -> ping6, but don't know how.

 This can be done with a post-install hook.

 Also, the bump to s20190515 should not have removed the traceroute6 program.
Could you make a second patch that adds it back?

 Regards,
 Arnout

> 
> Kind regards,
> Petr
> ---
>  package/iputils/iputils.mk | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/package/iputils/iputils.mk b/package/iputils/iputils.mk
> index d9a51ece2a..8be54b4788 100644
> --- a/package/iputils/iputils.mk
> +++ b/package/iputils/iputils.mk
> @@ -52,4 +52,12 @@ endif
>  # XSL Stylesheets for DocBook 5 not packaged for buildroot
>  IPUTILS_CONF_OPTS += -DBUILD_MANS=false -DBUILD_HTML_MANS=false
>  
> +# handle permissions ourselves
> +IPUTILS_CONF_OPTS += -DNO_SETCAP_OR_SUID=true
> +define IPUTILS_PERMISSIONS
> +	/bin/arping      f 4755 0 0 - - - - -
> +	/bin/clockdiff   f 4755 0 0 - - - - -
> +	/bin/ping        f 4755 0 0 - - - - -
> +endef
> +
>  $(eval $(meson-package))
> 

[Buildroot] [PATCH 1/1] iputils: set the permissions with IPUTILS_PERMISSIONS

[Petr Vorel]

Hi Arnout,

>  Applied to master, thanks. I updated the commit message a little.
Thanks! You've been faster, I might try to think about setcap (xattr) use if
USE_CAP=true, and setuid if USE_CAP=false. Is there an easy way to detect
iso9660 usage? (+ setcap being available on host).
I'm not aware any other xattr limitations thatn iso9660.

> > it'd be great also create symlink ping -> ping6, but don't know how.

>  This can be done with a post-install hook.
Thanks, I'll do it in separate patch sometimes this week.

>  Also, the bump to s20190515 should not have removed the traceroute6 program.
> Could you make a second patch that adds it back?
Sure, I'll try to do it this week.

>  Regards,
>  Arnout

Kind regards,
Petr

[Buildroot] [PATCH 1/1] iputils: set the permissions with IPUTILS_PERMISSIONS

[Arnout Vandecappelle]

On 10/06/2019 22:06, Petr Vorel wrote:
> Hi Arnout,
> 
>>  Applied to master, thanks. I updated the commit message a little.
> Thanks! You've been faster, I might try to think about setcap (xattr) use if
> USE_CAP=true, and setuid if USE_CAP=false.

 Great!

> Is there an easy way to detect
> iso9660 usage?

 BR2_TARGET_ROOTFS_ISO9660 of course. Well, that one is special actually because
in case of BR2_TARGET_ROOTFS_INITRAMFS or BR2_TARGET_ROOTFS_ISO9660_INITRD, the
rootfs is not actually stored in the iso9660.

 But I don't know which other filesystems lack support for xattrs.

 So maybe just assume the filesystem supports xattrs, we can fix things later if
needed.

> (+ setcap being available on host).

 setcap can't be used, since setting xattrs can only be done by root and we're
not root while building. It could be used when running in a fakeroot
environment, but I believe fakeroot is a bit flakey when it comes to
capabilities. So the appropriate xattrs should instead be set with
IPUTILS_PERMISSIONS.

 Regards,
 Arnout


> I'm not aware any other xattr limitations thatn iso9660.
> 
>>> it'd be great also create symlink ping -> ping6, but don't know how.
> 
>>  This can be done with a post-install hook.
> Thanks, I'll do it in separate patch sometimes this week.
> 
>>  Also, the bump to s20190515 should not have removed the traceroute6 program.
>> Could you make a second patch that adds it back?
> Sure, I'll try to do it this week.
> 
>>  Regards,
>>  Arnout
> 
> Kind regards,
> Petr
> 

[Buildroot] [PATCH 1/1] iputils: set the permissions with IPUTILS_PERMISSIONS

[Peter Korsgaard]

>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:

 > On 10/06/2019 22:06, Petr Vorel wrote:
 >> Hi Arnout,
 >> 
 >>> Applied to master, thanks. I updated the commit message a little.
 >> Thanks! You've been faster, I might try to think about setcap (xattr) use if
 >> USE_CAP=true, and setuid if USE_CAP=false.

 >  Great!

 >> Is there an easy way to detect
 >> iso9660 usage?

 >  BR2_TARGET_ROOTFS_ISO9660 of course. Well, that one is special actually because
 > in case of BR2_TARGET_ROOTFS_INITRAMFS or BR2_TARGET_ROOTFS_ISO9660_INITRD, the
 > rootfs is not actually stored in the iso9660.

 >  But I don't know which other filesystems lack support for xattrs.

axfs, cramfs, romfs and yaffs2 as well afaik.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH 1/1] iputils: set the permissions with IPUTILS_PERMISSIONS

[Petr Vorel]

Hi Peter,

>  >> Is there an easy way to detect
>  >> iso9660 usage?

>  >  BR2_TARGET_ROOTFS_ISO9660 of course. Well, that one is special actually because
>  > in case of BR2_TARGET_ROOTFS_INITRAMFS or BR2_TARGET_ROOTFS_ISO9660_INITRD, the
>  > rootfs is not actually stored in the iso9660.

>  >  But I don't know which other filesystems lack support for xattrs.

> axfs, cramfs, romfs and yaffs2 as well afaik.
Thanks a lot.

I guess BR2_TARGET_ROOTFS_TAR (and other tarballs supported)
supports xattrs (--xattrs-include='*' in fs/tar/tar.mk).

It'd be nice if buildroot has BR2_TARGET_ROOTFS_HAS_XATTRS.

Kind regards,
Petr

[Buildroot] [PATCH 1/1] iputils: set the permissions with IPUTILS_PERMISSIONS

[Peter Korsgaard]

>>>>> "Petr" == Petr Vorel <petr.vorel@gmail.com> writes:

 > Hi Peter,
 >> >> Is there an easy way to detect
 >> >> iso9660 usage?

 >> >  BR2_TARGET_ROOTFS_ISO9660 of course. Well, that one is special actually because
 >> > in case of BR2_TARGET_ROOTFS_INITRAMFS or BR2_TARGET_ROOTFS_ISO9660_INITRD, the
 >> > rootfs is not actually stored in the iso9660.

 >> >  But I don't know which other filesystems lack support for xattrs.

 >> axfs, cramfs, romfs and yaffs2 as well afaik.
 > Thanks a lot.

 > I guess BR2_TARGET_ROOTFS_TAR (and other tarballs supported)
 > supports xattrs (--xattrs-include='*' in fs/tar/tar.mk).

 > It'd be nice if buildroot has BR2_TARGET_ROOTFS_HAS_XATTRS.

The problem is that you can enable several rootfs formats at the same
time (E.G. tar and cramfs), so we would need to only use xattrs if no
file system without xattrs support is enabled.

And things would break if you do a build with E.G. only tar rootfs
support and then afterwards enable cramfs without doing a clean
rebuild - Yes, I know you are not supposed to do that, but it does
happen.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH 1/1] iputils: set the permissions with IPUTILS_PERMISSIONS

[Petr Vorel]

Hi Peter,

>  >> >> Is there an easy way to detect
>  >> >> iso9660 usage?

>  >> >  BR2_TARGET_ROOTFS_ISO9660 of course. Well, that one is special actually because
>  >> > in case of BR2_TARGET_ROOTFS_INITRAMFS or BR2_TARGET_ROOTFS_ISO9660_INITRD, the
>  >> > rootfs is not actually stored in the iso9660.

>  >> >  But I don't know which other filesystems lack support for xattrs.

>  >> axfs, cramfs, romfs and yaffs2 as well afaik.
>  > Thanks a lot.

>  > I guess BR2_TARGET_ROOTFS_TAR (and other tarballs supported)
>  > supports xattrs (--xattrs-include='*' in fs/tar/tar.mk).

>  > It'd be nice if buildroot has BR2_TARGET_ROOTFS_HAS_XATTRS.

> The problem is that you can enable several rootfs formats at the same
> time (E.G. tar and cramfs), so we would need to only use xattrs if no
> file system without xattrs support is enabled.

> And things would break if you do a build with E.G. only tar rootfs
> support and then afterwards enable cramfs without doing a clean
> rebuild - Yes, I know you are not supposed to do that, but it does
> happen.

Thanks for detailed info. I guess in that case is setuid really the only option.

Kind regards,
Petr

[Buildroot] [PATCH 1/1] iputils: set the permissions with IPUTILS_PERMISSIONS

[Arnout Vandecappelle]

On 18/06/2019 22:52, Petr Vorel wrote:
> Hi Peter,
> 
>>  >> >> Is there an easy way to detect
>>  >> >> iso9660 usage?
> 
>>  >> >  BR2_TARGET_ROOTFS_ISO9660 of course. Well, that one is special actually because
>>  >> > in case of BR2_TARGET_ROOTFS_INITRAMFS or BR2_TARGET_ROOTFS_ISO9660_INITRD, the
>>  >> > rootfs is not actually stored in the iso9660.
> 
>>  >> >  But I don't know which other filesystems lack support for xattrs.
> 
>>  >> axfs, cramfs, romfs and yaffs2 as well afaik.
>>  > Thanks a lot.
> 
>>  > I guess BR2_TARGET_ROOTFS_TAR (and other tarballs supported)
>>  > supports xattrs (--xattrs-include='*' in fs/tar/tar.mk).
> 
>>  > It'd be nice if buildroot has BR2_TARGET_ROOTFS_HAS_XATTRS.
> 
>> The problem is that you can enable several rootfs formats at the same
>> time (E.G. tar and cramfs), so we would need to only use xattrs if no
>> file system without xattrs support is enabled.

 So maybe we could add a system option BR2_SYSTEM_XATTR that enables the use of
xattr.

 Currently we have nothing using xattr, but there are quite a few packages that
could benefit from it, e.g. libpcap, and SELinux stuff.

 We could use that option to enable xattr instead of setuid where relevant, and
to disable filesystems that don't support xattr.


>> And things would break if you do a build with E.G. only tar rootfs
>> support and then afterwards enable cramfs without doing a clean
>> rebuild - Yes, I know you are not supposed to do that, but it does
>> happen.

 I don't think we need to worry about that. But actually, with the
BR2_SYSTEM_XATTR option, it would even work since it's only taken into account
during finalize.


> Thanks for detailed info. I guess in that case is setuid really the only option.

 It isn't, but the alternatives are a lot of work :-)

 Regards,
 Arnout

[Buildroot] [PATCH 1/1] iputils: set the permissions with IPUTILS_PERMISSIONS

[Petr Vorel]

Hi Arnout,

> >>  > It'd be nice if buildroot has BR2_TARGET_ROOTFS_HAS_XATTRS.

> >> The problem is that you can enable several rootfs formats at the same
> >> time (E.G. tar and cramfs), so we would need to only use xattrs if no
> >> file system without xattrs support is enabled.

>  So maybe we could add a system option BR2_SYSTEM_XATTR that enables the use of
> xattr.
+1

>  Currently we have nothing using xattr, but there are quite a few packages that
> could benefit from it, e.g. libpcap, and SELinux stuff.
And IMA+EVM kernel features.

>  We could use that option to enable xattr instead of setuid where relevant, and
> to disable filesystems that don't support xattr.


> >> And things would break if you do a build with E.G. only tar rootfs
> >> support and then afterwards enable cramfs without doing a clean
> >> rebuild - Yes, I know you are not supposed to do that, but it does
> >> happen.

>  I don't think we need to worry about that. But actually, with the
> BR2_SYSTEM_XATTR option, it would even work since it's only taken into account
> during finalize.


> > Thanks for detailed info. I guess in that case is setuid really the only option.

>  It isn't, but the alternatives are a lot of work :-)
:-). Do you plan to work on it? If not, I might do in next few weeks (I'm quite
busy during summer).

>  Regards,
>  Arnout

Kind regards,
Petr

[Buildroot] [PATCH 1/1] iputils: set the permissions with IPUTILS_PERMISSIONS

[Arnout Vandecappelle]

On 19/06/2019 07:01, Petr Vorel wrote:
> Hi Arnout,
> 
>>>>  > It'd be nice if buildroot has BR2_TARGET_ROOTFS_HAS_XATTRS.
> 
>>>> The problem is that you can enable several rootfs formats at the same
>>>> time (E.G. tar and cramfs), so we would need to only use xattrs if no
>>>> file system without xattrs support is enabled.
> 
>>  So maybe we could add a system option BR2_SYSTEM_XATTR that enables the use of
>> xattr.
> +1
> 
>>  Currently we have nothing using xattr, but there are quite a few packages that
>> could benefit from it, e.g. libpcap, and SELinux stuff.
> And IMA+EVM kernel features.
> 
>>  We could use that option to enable xattr instead of setuid where relevant, and
>> to disable filesystems that don't support xattr.
> 
> 
>>>> And things would break if you do a build with E.G. only tar rootfs
>>>> support and then afterwards enable cramfs without doing a clean
>>>> rebuild - Yes, I know you are not supposed to do that, but it does
>>>> happen.
> 
>>  I don't think we need to worry about that. But actually, with the
>> BR2_SYSTEM_XATTR option, it would even work since it's only taken into account
>> during finalize.
> 
> 
>>> Thanks for detailed info. I guess in that case is setuid really the only option.
> 
>>  It isn't, but the alternatives are a lot of work :-)
> :-). Do you plan to work on it? If not, I might do in next few weeks (I'm quite
> busy during summer).

 I don't do any work myself, I just occasionally apply patches :-) And I think
for Peter it's pretty much the same thing.

 Regards,
 Arnout

[Buildroot] [PATCH 1/1] iputils: set the permissions with IPUTILS_PERMISSIONS

[Petr Vorel]

Hi Arnout,

...
> >>> Thanks for detailed info. I guess in that case is setuid really the only option.

> >>  It isn't, but the alternatives are a lot of work :-)
> > :-). Do you plan to work on it? If not, I might do in next few weeks (I'm quite
> > busy during summer).

>  I don't do any work myself, I just occasionally apply patches :-) And I think
> for Peter it's pretty much the same thing.
Thanks for info. Adding into my todo list :)

>  Regards,
>  Arnout


Kind regards,
Petr

[Buildroot] [PATCH 1/1] iputils: set the permissions with IPUTILS_PERMISSIONS

[Peter Korsgaard]

>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:

Hi,

 >>> It isn't, but the alternatives are a lot of work :-)
 >> :-). Do you plan to work on it? If not, I might do in next few weeks (I'm quite
 >> busy during summer).

 >  I don't do any work myself, I just occasionally apply patches :-) And I think
 > for Peter it's pretty much the same thing.

I do actual development once in a while ;) - But I also don't have any
immediate plans for working on improving our xattrs support.

-- 
Bye, Peter Korsgaard