[PATCH v3 1/2] Revert "iputils: Fix build determinism"

[PATCH v3 1/2] Revert "iputils: Fix build determinism"

[Jate Sujjavanich]

This reverts commit d10da5f6e6d6d3600645dbe43ed412ff23b55095.
---
 meta/recipes-extended/iputils/iputils_s20200821.bb | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/meta/recipes-extended/iputils/iputils_s20200821.bb b/meta/recipes-extended/iputils/iputils_s20200821.bb
index 28dd194a12..8b63a23c61 100644
--- a/meta/recipes-extended/iputils/iputils_s20200821.bb
+++ b/meta/recipes-extended/iputils/iputils_s20200821.bb
@@ -38,8 +38,7 @@ PACKAGECONFIG[docs] = "-DBUILD_HTML_MANS=true -DBUILD_MANS=true,-DBUILD_HTML_MAN
 
 inherit meson systemd update-alternatives
 
-# Have to disable setcap/suid as its not deterministic
-EXTRA_OEMESON += "--prefix=${root_prefix}/ -DNO_SETCAP_OR_SUID=true"
+EXTRA_OEMESON += "--prefix=${root_prefix}/"
 
 ALTERNATIVE_PRIORITY = "100"
 
-- 
2.25.1

[PATCH v3 2/2] iputils: Fix setcap/setuid of executables

[Jate Sujjavanich]

Limit setcap search to PATH for cross builds preventing host
contamination. Add DEPENDS for libcap-native to supply this for
PACKAGECONFIG libcap.

The previous setting of NO_SETCAP_OR_SUID broke setuid or setcap of
/bin/ping and other executables.

Signed-off-by: Jate Sujjavanich <jatedev@gmail.com>
---
 ...tcap-search-to-PATH-for-cross-builds.patch | 33 +++++++++++++++++++
 .../iputils/iputils_s20200821.bb              |  3 +-
 2 files changed, 35 insertions(+), 1 deletion(-)
 create mode 100644 meta/recipes-extended/iputils/iputils/0001-Limit-setcap-search-to-PATH-for-cross-builds.patch

diff --git a/meta/recipes-extended/iputils/iputils/0001-Limit-setcap-search-to-PATH-for-cross-builds.patch b/meta/recipes-extended/iputils/iputils/0001-Limit-setcap-search-to-PATH-for-cross-builds.patch
new file mode 100644
index 0000000000..b09cdb5f13
--- /dev/null
+++ b/meta/recipes-extended/iputils/iputils/0001-Limit-setcap-search-to-PATH-for-cross-builds.patch
@@ -0,0 +1,33 @@
+From aedb7811d1e9b0eae5d33b5bed39f6219a06a8dc Mon Sep 17 00:00:00 2001
+From: Jate Sujjavanich <jatedev@gmail.com>
+Date: Thu, 18 Feb 2021 21:35:03 -0500
+Subject: [PATCH] Limit setcap search to PATH for cross builds
+
+Upstream-Status: Pending
+---
+ meson.build | 9 ++++++++-
+ 1 file changed, 8 insertions(+), 1 deletion(-)
+
+diff --git a/meson.build b/meson.build
+index aff75a2..16bbfb6 100644
+--- a/meson.build
++++ b/meson.build
+@@ -215,7 +215,14 @@ config_h = configure_file(
+ 	output : 'config.h',
+ 	configuration : conf)
+ 
+-setcap = find_program('setcap', '/usr/sbin/setcap', '/sbin/setcap', required : false)
++if meson.is_cross_build()
++	message('limit setcap search to PATH')
++	path_setcap = ['setcap']
++else
++	message('search for setcap in PATH and admin dirs')
++	path_setcap = ['setcap', '/usr/sbin/setcap', '/sbin/setcap']
++endif
++setcap = find_program(path_setcap, required : false)
+ if cap_dep.found() and setcap.found()
+ 	perm_type = 'caps'
+ 	setcap_path = setcap.path()
+-- 
+2.30.0
+
diff --git a/meta/recipes-extended/iputils/iputils_s20200821.bb b/meta/recipes-extended/iputils/iputils_s20200821.bb
index 8b63a23c61..31fe1acdaf 100644
--- a/meta/recipes-extended/iputils/iputils_s20200821.bb
+++ b/meta/recipes-extended/iputils/iputils_s20200821.bb
@@ -12,6 +12,7 @@ DEPENDS = "gnutls"
 
 SRC_URI = "git://github.com/iputils/iputils \
            file://0001-rarpd-rdisc-Drop-PrivateUsers.patch \
+           file://0001-Limit-setcap-search-to-PATH-for-cross-builds.patch \
            "
 SRCREV = "23c3782ae0c7f9c6ae59dbed8ad9204f8758542b"
 
@@ -26,7 +27,7 @@ CVE_CHECK_WHITELIST += "CVE-2000-1213 CVE-2000-1214"
 PACKAGECONFIG ??= "libcap rarpd \
                    ${@bb.utils.contains('DISTRO_FEATURES', 'ipv6', 'ninfod traceroute6', '', d)} \
                    ${@bb.utils.filter('DISTRO_FEATURES', 'systemd', d)}"
-PACKAGECONFIG[libcap] = "-DUSE_CAP=true, -DUSE_CAP=false, libcap"
+PACKAGECONFIG[libcap] = "-DUSE_CAP=true, -DUSE_CAP=false, libcap libcap-native"
 PACKAGECONFIG[libidn] = "-DUSE_IDN=true, -DUSE_IDN=false, libidn2"
 PACKAGECONFIG[gettext] = "-DUSE_GETTEXT=true, -DUSE_GETTEXT=false, gettext"
 PACKAGECONFIG[ninfod] = "-DBUILD_NINFOD=true,-DBUILD_NINFOD=false,"
-- 
2.25.1

Re: [PATCH v3 2/2] iputils: Fix setcap/setuid of executables

[Richard Purdie]

On Fri, 2021-02-19 at 17:53 +0000, Jate Sujjavanich wrote:
> Limit setcap search to PATH for cross builds preventing host
> contamination. Add DEPENDS for libcap-native to supply this for
> PACKAGECONFIG libcap.
> 
> The previous setting of NO_SETCAP_OR_SUID broke setuid or setcap of
> /bin/ping and other executables.
> 
> Signed-off-by: Jate Sujjavanich <jatedev@gmail.com>
> ---
>  ...tcap-search-to-PATH-for-cross-builds.patch | 33 +++++++++++++++++++
>  .../iputils/iputils_s20200821.bb              |  3 +-
>  2 files changed, 35 insertions(+), 1 deletion(-)
>  create mode 100644 meta/recipes-extended/iputils/iputils/0001-Limit-setcap-search-to-PATH-for-cross-builds.patch
> 
> diff --git a/meta/recipes-extended/iputils/iputils/0001-Limit-setcap-search-to-PATH-for-cross-builds.patch b/meta/recipes-extended/iputils/iputils/0001-Limit-setcap-search-to-PATH-for-cross-builds.patch
> new file mode 100644
> index 0000000000..b09cdb5f13
> --- /dev/null
> +++ b/meta/recipes-extended/iputils/iputils/0001-Limit-setcap-search-to-PATH-for-cross-builds.patch
> @@ -0,0 +1,33 @@
> +From aedb7811d1e9b0eae5d33b5bed39f6219a06a8dc Mon Sep 17 00:00:00 2001
> +From: Jate Sujjavanich <jatedev@gmail.com>
> +Date: Thu, 18 Feb 2021 21:35:03 -0500
> +Subject: [PATCH] Limit setcap search to PATH for cross builds
> +
> +Upstream-Status: Pending
> +---
> + meson.build | 9 ++++++++-
> + 1 file changed, 8 insertions(+), 1 deletion(-)
> +
> +diff --git a/meson.build b/meson.build
> +index aff75a2..16bbfb6 100644
> +--- a/meson.build
> ++++ b/meson.build
> +@@ -215,7 +215,14 @@ config_h = configure_file(
> + 	output : 'config.h',
> + 	configuration : conf)
> + 
> +-setcap = find_program('setcap', '/usr/sbin/setcap', '/sbin/setcap', required : false)
> ++if meson.is_cross_build()
> ++	message('limit setcap search to PATH')
> ++	path_setcap = ['setcap']
> ++else
> ++	message('search for setcap in PATH and admin dirs')
> ++	path_setcap = ['setcap', '/usr/sbin/setcap', '/sbin/setcap']
> ++endif
> ++setcap = find_program(path_setcap, required : false)
> + if cap_dep.found() and setcap.found()
> + 	perm_type = 'caps'
> + 	setcap_path = setcap.path()
> +-- 
> 


This does look like ultimately the better solution, could you send it to 
iputils and see what they make of it? I'm keen to try and reduce the number
of patches we carry and this one looks like a good candiate for them covering
the key edge cases as far as I can tell!

Cheers,

Richard

Re: [PATCH v3 2/2] iputils: Fix setcap/setuid of executables

[Jose Quaresma]

[-- Attachment #1: Type: text/plain, Size: 5410 bytes --]

Hi Jate,

IMHO as I mention on the last version of this patch
the file 0001-Limit-setcap-search-to-PATH-for-cross-builds.patch is not
needed
to make the iputils deterministic, the PACKAGECONFIG[libcap] changes on
this [Patch 2/2] will do it.

On [Patch 1/2] the option -DNO_SETCAP_OR_SUID=true has been removed with
the reverted change.
This will make the meson to use the setuid when setcap and libcap not found
and -DUSE_CAP=false.

Only with the addition of the libcap-native on the DEPENDS the meson will
use the native setcap
without host contamination.

When the libcap package config is enabled (default option).
The libcap-native will provide the setcap binary in the sysoot
native STAGING_DIR_NATIVE so the meson
will find the setcaps binary on the STAGING_DIR_NATIVE.

The find_program('setcaps', ....) will return the full path of the native
setcap and not the host setcap.
Without the libcap-native the meson find_program('setcaps', ....) will fail
to find the first argument
because it not found on the PATH and will fallback to the next argument.
The next argument have a full PATH so the find_program will find on the
host in this case
and this will cause the contamination.

from the meson help:

    program find_program(program_name1, program_name2, ...)

program_name1 here is a string that can be an executable or script to be
searched for in PATH or other places inside the project.
(since 0.37.0) program_name2 and later positional arguments are used as
fallback strings to search for.
This is meant to be used for cases where the program may have many
alternative names, such as foo and foo.py.
The function will check for the arguments one by one and the first one that
is found is returned.

Jose

Jate Sujjavanich <jatedev@gmail.com> escreveu no dia sexta, 19/02/2021 à(s)
17:53:

> Limit setcap search to PATH for cross builds preventing host
> contamination. Add DEPENDS for libcap-native to supply this for
> PACKAGECONFIG libcap.
>
> The previous setting of NO_SETCAP_OR_SUID broke setuid or setcap of
> /bin/ping and other executables.
>
> Signed-off-by: Jate Sujjavanich <jatedev@gmail.com>
> ---
>  ...tcap-search-to-PATH-for-cross-builds.patch | 33 +++++++++++++++++++
>  .../iputils/iputils_s20200821.bb              |  3 +-
>  2 files changed, 35 insertions(+), 1 deletion(-)
>  create mode 100644
> meta/recipes-extended/iputils/iputils/0001-Limit-setcap-search-to-PATH-for-cross-builds.patch
>
> diff --git
> a/meta/recipes-extended/iputils/iputils/0001-Limit-setcap-search-to-PATH-for-cross-builds.patch
> b/meta/recipes-extended/iputils/iputils/0001-Limit-setcap-search-to-PATH-for-cross-builds.patch
> new file mode 100644
> index 0000000000..b09cdb5f13
> --- /dev/null
> +++
> b/meta/recipes-extended/iputils/iputils/0001-Limit-setcap-search-to-PATH-for-cross-builds.patch
> @@ -0,0 +1,33 @@
> +From aedb7811d1e9b0eae5d33b5bed39f6219a06a8dc Mon Sep 17 00:00:00 2001
> +From: Jate Sujjavanich <jatedev@gmail.com>
> +Date: Thu, 18 Feb 2021 21:35:03 -0500
> +Subject: [PATCH] Limit setcap search to PATH for cross builds
> +
> +Upstream-Status: Pending
> +---
> + meson.build | 9 ++++++++-
> + 1 file changed, 8 insertions(+), 1 deletion(-)
> +
> +diff --git a/meson.build b/meson.build
> +index aff75a2..16bbfb6 100644
> +--- a/meson.build
> ++++ b/meson.build
> +@@ -215,7 +215,14 @@ config_h = configure_file(
> +       output : 'config.h',
> +       configuration : conf)
> +
> +-setcap = find_program('setcap', '/usr/sbin/setcap', '/sbin/setcap',
> required : false)
> ++if meson.is_cross_build()
> ++      message('limit setcap search to PATH')
> ++      path_setcap = ['setcap']
> ++else
> ++      message('search for setcap in PATH and admin dirs')
> ++      path_setcap = ['setcap', '/usr/sbin/setcap', '/sbin/setcap']
> ++endif
> ++setcap = find_program(path_setcap, required : false)
> + if cap_dep.found() and setcap.found()
> +       perm_type = 'caps'
> +       setcap_path = setcap.path()
> +--
> +2.30.0
> +
> diff --git a/meta/recipes-extended/iputils/iputils_s20200821.bb
> b/meta/recipes-extended/iputils/iputils_s20200821.bb
> index 8b63a23c61..31fe1acdaf 100644
> --- a/meta/recipes-extended/iputils/iputils_s20200821.bb
> +++ b/meta/recipes-extended/iputils/iputils_s20200821.bb
> @@ -12,6 +12,7 @@ DEPENDS = "gnutls"
>
>  SRC_URI = "git://github.com/iputils/iputils \
>             file://0001-rarpd-rdisc-Drop-PrivateUsers.patch \
> +           file://0001-Limit-setcap-search-to-PATH-for-cross-builds.patch
> \
>             "
>  SRCREV = "23c3782ae0c7f9c6ae59dbed8ad9204f8758542b"
>
> @@ -26,7 +27,7 @@ CVE_CHECK_WHITELIST += "CVE-2000-1213 CVE-2000-1214"
>  PACKAGECONFIG ??= "libcap rarpd \
>                     ${@bb.utils.contains('DISTRO_FEATURES', 'ipv6',
> 'ninfod traceroute6', '', d)} \
>                     ${@bb.utils.filter('DISTRO_FEATURES', 'systemd', d)}"
> -PACKAGECONFIG[libcap] = "-DUSE_CAP=true, -DUSE_CAP=false, libcap"
> +PACKAGECONFIG[libcap] = "-DUSE_CAP=true, -DUSE_CAP=false, libcap
> libcap-native"
>  PACKAGECONFIG[libidn] = "-DUSE_IDN=true, -DUSE_IDN=false, libidn2"
>  PACKAGECONFIG[gettext] = "-DUSE_GETTEXT=true, -DUSE_GETTEXT=false,
> gettext"
>  PACKAGECONFIG[ninfod] = "-DBUILD_NINFOD=true,-DBUILD_NINFOD=false,"
> --
> 2.25.1
>
>

-- 
Best regards,

José Quaresma

[-- Attachment #2: Type: text/html, Size: 8229 bytes --]

Re: [PATCH v3 2/2] iputils: Fix setcap/setuid of executables

[Jate Sujjavanich]

[-- Attachment #1: Type: text/plain, Size: 3265 bytes --]

I have submitted to the upstream which I believe is the github
iputils/iputils project.

My previous thread had the wrong mailing list address, so I am replying to
this question here:
>Thinking further, how about changing the PACKAGECONFIG to:
>
>PACKAGECONFIG[libcap] = "-DUSE_CAP=true, -DUSE_CAP=false
-DNO_SETCAP_OR_SUID=true, libcap libcap-native"
>
>?
>
>Cheers,
>
>Richard

This would disable the fallback to setuid if PACKAGECONFIG libcap was
disabled.

- Jate



On Fri, Feb 19, 2021 at 12:57 PM Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:

> On Fri, 2021-02-19 at 17:53 +0000, Jate Sujjavanich wrote:
> > Limit setcap search to PATH for cross builds preventing host
> > contamination. Add DEPENDS for libcap-native to supply this for
> > PACKAGECONFIG libcap.
> >
> > The previous setting of NO_SETCAP_OR_SUID broke setuid or setcap of
> > /bin/ping and other executables.
> >
> > Signed-off-by: Jate Sujjavanich <jatedev@gmail.com>
> > ---
> >  ...tcap-search-to-PATH-for-cross-builds.patch | 33 +++++++++++++++++++
> >  .../iputils/iputils_s20200821.bb              |  3 +-
> >  2 files changed, 35 insertions(+), 1 deletion(-)
> >  create mode 100644
> meta/recipes-extended/iputils/iputils/0001-Limit-setcap-search-to-PATH-for-cross-builds.patch
> >
> > diff --git
> a/meta/recipes-extended/iputils/iputils/0001-Limit-setcap-search-to-PATH-for-cross-builds.patch
> b/meta/recipes-extended/iputils/iputils/0001-Limit-setcap-search-to-PATH-for-cross-builds.patch
> > new file mode 100644
> > index 0000000000..b09cdb5f13
> > --- /dev/null
> > +++
> b/meta/recipes-extended/iputils/iputils/0001-Limit-setcap-search-to-PATH-for-cross-builds.patch
> > @@ -0,0 +1,33 @@
> > +From aedb7811d1e9b0eae5d33b5bed39f6219a06a8dc Mon Sep 17 00:00:00 2001
> > +From: Jate Sujjavanich <jatedev@gmail.com>
> > +Date: Thu, 18 Feb 2021 21:35:03 -0500
> > +Subject: [PATCH] Limit setcap search to PATH for cross builds
> > +
> > +Upstream-Status: Pending
> > +---
> > + meson.build | 9 ++++++++-
> > + 1 file changed, 8 insertions(+), 1 deletion(-)
> > +
> > +diff --git a/meson.build b/meson.build
> > +index aff75a2..16bbfb6 100644
> > +--- a/meson.build
> > ++++ b/meson.build
> > +@@ -215,7 +215,14 @@ config_h = configure_file(
> > +     output : 'config.h',
> > +     configuration : conf)
> > +
> > +-setcap = find_program('setcap', '/usr/sbin/setcap', '/sbin/setcap',
> required : false)
> > ++if meson.is_cross_build()
> > ++    message('limit setcap search to PATH')
> > ++    path_setcap = ['setcap']
> > ++else
> > ++    message('search for setcap in PATH and admin dirs')
> > ++    path_setcap = ['setcap', '/usr/sbin/setcap', '/sbin/setcap']
> > ++endif
> > ++setcap = find_program(path_setcap, required : false)
> > + if cap_dep.found() and setcap.found()
> > +     perm_type = 'caps'
> > +     setcap_path = setcap.path()
> > +--
> >
>
>
> This does look like ultimately the better solution, could you send it to
> iputils and see what they make of it? I'm keen to try and reduce the number
> of patches we carry and this one looks like a good candiate for them
> covering
> the key edge cases as far as I can tell!
>
> Cheers,
>
> Richard
>
>

[-- Attachment #2: Type: text/html, Size: 4354 bytes --]

Re: [PATCH v3 2/2] iputils: Fix setcap/setuid of executables

[Richard Purdie]

On Mon, 2021-02-22 at 10:37 -0500, Jate Sujjavanich wrote:
> I have submitted to the upstream which I believe is the github iputils/iputils project.
> 
> My previous thread had the wrong mailing list address, so I am replying to this question here:
> > Thinking further, how about changing the PACKAGECONFIG to:
> > 
> > PACKAGECONFIG[libcap] = "-DUSE_CAP=true, -DUSE_CAP=false -DNO_SETCAP_OR_SUID=true, libcap libcap-native"
> > 
> > ?
> > 
> > Cheers,
> > 
> > Richard
> 
> This would disable the fallback to setuid if PACKAGECONFIG libcap was disabled.

I have fuzzy recollection that the setuid didn't always work on some hosts 
so had determinism issues of its own which was why it ended up disabled. It
also presented a little less of a security issue that way, although would
make certain operations harder.

Cheers,

Richard

Re: [PATCH v3 2/2] iputils: Fix setcap/setuid of executables

[Jate Sujjavanich]

[-- Attachment #1: Type: text/plain, Size: 6023 bytes --]

If libcap is disabled in PACKAGECONFIG, then arguments two and three mean
find_program find could setcap on the build machine. This would mean it is
not strictly determined by the recipe and it's determined by the
configuration of the build machine instead.

find_program('setcap', '/usr/sbin/setcap', '/sbin/setcap', required : false)

- Jate

On Sat, Feb 20, 2021 at 6:01 PM Jose Quaresma <quaresma.jose@gmail.com>
wrote:

> Hi Jate,
>
> IMHO as I mention on the last version of this patch
> the file 0001-Limit-setcap-search-to-PATH-for-cross-builds.patch is not
> needed
> to make the iputils deterministic, the PACKAGECONFIG[libcap] changes on
> this [Patch 2/2] will do it.
>
> On [Patch 1/2] the option -DNO_SETCAP_OR_SUID=true has been removed with
> the reverted change.
> This will make the meson to use the setuid when setcap and libcap not
> found and -DUSE_CAP=false.
>
> Only with the addition of the libcap-native on the DEPENDS the meson will
> use the native setcap
> without host contamination.
>
> When the libcap package config is enabled (default option).
> The libcap-native will provide the setcap binary in the sysoot
> native STAGING_DIR_NATIVE so the meson
> will find the setcaps binary on the STAGING_DIR_NATIVE.
>
> The find_program('setcaps', ....) will return the full path of the native
> setcap and not the host setcap.
> Without the libcap-native the meson find_program('setcaps', ....) will
> fail to find the first argument
> because it not found on the PATH and will fallback to the next argument.
> The next argument have a full PATH so the find_program will find on the
> host in this case
> and this will cause the contamination.
>
> from the meson help:
>
>     program find_program(program_name1, program_name2, ...)
>
> program_name1 here is a string that can be an executable or script to be
> searched for in PATH or other places inside the project.
> (since 0.37.0) program_name2 and later positional arguments are used as
> fallback strings to search for.
> This is meant to be used for cases where the program may have many
> alternative names, such as foo and foo.py.
> The function will check for the arguments one by one and the first one
> that is found is returned.
>
> Jose
>
> Jate Sujjavanich <jatedev@gmail.com> escreveu no dia sexta, 19/02/2021
> à(s) 17:53:
>
>> Limit setcap search to PATH for cross builds preventing host
>> contamination. Add DEPENDS for libcap-native to supply this for
>> PACKAGECONFIG libcap.
>>
>> The previous setting of NO_SETCAP_OR_SUID broke setuid or setcap of
>> /bin/ping and other executables.
>>
>> Signed-off-by: Jate Sujjavanich <jatedev@gmail.com>
>> ---
>>  ...tcap-search-to-PATH-for-cross-builds.patch | 33 +++++++++++++++++++
>>  .../iputils/iputils_s20200821.bb              |  3 +-
>>  2 files changed, 35 insertions(+), 1 deletion(-)
>>  create mode 100644
>> meta/recipes-extended/iputils/iputils/0001-Limit-setcap-search-to-PATH-for-cross-builds.patch
>>
>> diff --git
>> a/meta/recipes-extended/iputils/iputils/0001-Limit-setcap-search-to-PATH-for-cross-builds.patch
>> b/meta/recipes-extended/iputils/iputils/0001-Limit-setcap-search-to-PATH-for-cross-builds.patch
>> new file mode 100644
>> index 0000000000..b09cdb5f13
>> --- /dev/null
>> +++
>> b/meta/recipes-extended/iputils/iputils/0001-Limit-setcap-search-to-PATH-for-cross-builds.patch
>> @@ -0,0 +1,33 @@
>> +From aedb7811d1e9b0eae5d33b5bed39f6219a06a8dc Mon Sep 17 00:00:00 2001
>> +From: Jate Sujjavanich <jatedev@gmail.com>
>> +Date: Thu, 18 Feb 2021 21:35:03 -0500
>> +Subject: [PATCH] Limit setcap search to PATH for cross builds
>> +
>> +Upstream-Status: Pending
>> +---
>> + meson.build | 9 ++++++++-
>> + 1 file changed, 8 insertions(+), 1 deletion(-)
>> +
>> +diff --git a/meson.build b/meson.build
>> +index aff75a2..16bbfb6 100644
>> +--- a/meson.build
>> ++++ b/meson.build
>> +@@ -215,7 +215,14 @@ config_h = configure_file(
>> +       output : 'config.h',
>> +       configuration : conf)
>> +
>> +-setcap = find_program('setcap', '/usr/sbin/setcap', '/sbin/setcap',
>> required : false)
>> ++if meson.is_cross_build()
>> ++      message('limit setcap search to PATH')
>> ++      path_setcap = ['setcap']
>> ++else
>> ++      message('search for setcap in PATH and admin dirs')
>> ++      path_setcap = ['setcap', '/usr/sbin/setcap', '/sbin/setcap']
>> ++endif
>> ++setcap = find_program(path_setcap, required : false)
>> + if cap_dep.found() and setcap.found()
>> +       perm_type = 'caps'
>> +       setcap_path = setcap.path()
>> +--
>> +2.30.0
>> +
>> diff --git a/meta/recipes-extended/iputils/iputils_s20200821.bb
>> b/meta/recipes-extended/iputils/iputils_s20200821.bb
>> index 8b63a23c61..31fe1acdaf 100644
>> --- a/meta/recipes-extended/iputils/iputils_s20200821.bb
>> +++ b/meta/recipes-extended/iputils/iputils_s20200821.bb
>> @@ -12,6 +12,7 @@ DEPENDS = "gnutls"
>>
>>  SRC_URI = "git://github.com/iputils/iputils \
>>             file://0001-rarpd-rdisc-Drop-PrivateUsers.patch \
>> +
>>  file://0001-Limit-setcap-search-to-PATH-for-cross-builds.patch \
>>             "
>>  SRCREV = "23c3782ae0c7f9c6ae59dbed8ad9204f8758542b"
>>
>> @@ -26,7 +27,7 @@ CVE_CHECK_WHITELIST += "CVE-2000-1213 CVE-2000-1214"
>>  PACKAGECONFIG ??= "libcap rarpd \
>>                     ${@bb.utils.contains('DISTRO_FEATURES', 'ipv6',
>> 'ninfod traceroute6', '', d)} \
>>                     ${@bb.utils.filter('DISTRO_FEATURES', 'systemd', d)}"
>> -PACKAGECONFIG[libcap] = "-DUSE_CAP=true, -DUSE_CAP=false, libcap"
>> +PACKAGECONFIG[libcap] = "-DUSE_CAP=true, -DUSE_CAP=false, libcap
>> libcap-native"
>>  PACKAGECONFIG[libidn] = "-DUSE_IDN=true, -DUSE_IDN=false, libidn2"
>>  PACKAGECONFIG[gettext] = "-DUSE_GETTEXT=true, -DUSE_GETTEXT=false,
>> gettext"
>>  PACKAGECONFIG[ninfod] = "-DBUILD_NINFOD=true,-DBUILD_NINFOD=false,"
>> --
>> 2.25.1
>>
>>
>
> --
> Best regards,
>
> José Quaresma
>

[-- Attachment #2: Type: text/html, Size: 9024 bytes --]

Re: [PATCH v3 2/2] iputils: Fix setcap/setuid of executables

[Jose Quaresma]

[-- Attachment #1: Type: text/plain, Size: 6734 bytes --]

Hi,

Jate Sujjavanich <jatedev@gmail.com> escreveu no dia segunda, 22/02/2021
à(s) 15:58:

> If libcap is disabled in PACKAGECONFIG, then arguments two and three mean
> find_program find could setcap on the build machine. This would mean it is
> not strictly determined by the recipe and it's determined by the
> configuration of the build machine instead.
>

In this case the meson will not use the setcap from the build machine
because the libcap not found on the target sysroot.
The libcap is a dependencie when PACKAGECONFIG+="libcap" and in this case
the meson it will fallback to use setuid.
https://github.com/iputils/iputils/blob/f0c64bbb2b1563647bfeeed8d82bc4d2d2312d8a/meson.build#L221

But as Richard says the setuid can be problematic too!


> find_program('setcap', '/usr/sbin/setcap', '/sbin/setcap', required :
> false)
>
> - Jate
>
> On Sat, Feb 20, 2021 at 6:01 PM Jose Quaresma <quaresma.jose@gmail.com>
> wrote:
>
>> Hi Jate,
>>
>> IMHO as I mention on the last version of this patch
>> the file 0001-Limit-setcap-search-to-PATH-for-cross-builds.patch is not
>> needed
>> to make the iputils deterministic, the PACKAGECONFIG[libcap] changes on
>> this [Patch 2/2] will do it.
>>
>> On [Patch 1/2] the option -DNO_SETCAP_OR_SUID=true has been removed with
>> the reverted change.
>> This will make the meson to use the setuid when setcap and libcap not
>> found and -DUSE_CAP=false.
>>
>> Only with the addition of the libcap-native on the DEPENDS the meson
>> will use the native setcap
>> without host contamination.
>>
>> When the libcap package config is enabled (default option).
>> The libcap-native will provide the setcap binary in the sysoot
>> native STAGING_DIR_NATIVE so the meson
>> will find the setcaps binary on the STAGING_DIR_NATIVE.
>>
>> The find_program('setcaps', ....) will return the full path of the native
>> setcap and not the host setcap.
>> Without the libcap-native the meson find_program('setcaps', ....) will
>> fail to find the first argument
>> because it not found on the PATH and will fallback to the next argument.
>> The next argument have a full PATH so the find_program will find on the
>> host in this case
>> and this will cause the contamination.
>>
>> from the meson help:
>>
>>     program find_program(program_name1, program_name2, ...)
>>
>> program_name1 here is a string that can be an executable or script to be
>> searched for in PATH or other places inside the project.
>> (since 0.37.0) program_name2 and later positional arguments are used as
>> fallback strings to search for.
>> This is meant to be used for cases where the program may have many
>> alternative names, such as foo and foo.py.
>> The function will check for the arguments one by one and the first one
>> that is found is returned.
>>
>> Jose
>>
>> Jate Sujjavanich <jatedev@gmail.com> escreveu no dia sexta, 19/02/2021
>> à(s) 17:53:
>>
>>> Limit setcap search to PATH for cross builds preventing host
>>> contamination. Add DEPENDS for libcap-native to supply this for
>>> PACKAGECONFIG libcap.
>>>
>>> The previous setting of NO_SETCAP_OR_SUID broke setuid or setcap of
>>> /bin/ping and other executables.
>>>
>>> Signed-off-by: Jate Sujjavanich <jatedev@gmail.com>
>>> ---
>>>  ...tcap-search-to-PATH-for-cross-builds.patch | 33 +++++++++++++++++++
>>>  .../iputils/iputils_s20200821.bb              |  3 +-
>>>  2 files changed, 35 insertions(+), 1 deletion(-)
>>>  create mode 100644
>>> meta/recipes-extended/iputils/iputils/0001-Limit-setcap-search-to-PATH-for-cross-builds.patch
>>>
>>> diff --git
>>> a/meta/recipes-extended/iputils/iputils/0001-Limit-setcap-search-to-PATH-for-cross-builds.patch
>>> b/meta/recipes-extended/iputils/iputils/0001-Limit-setcap-search-to-PATH-for-cross-builds.patch
>>> new file mode 100644
>>> index 0000000000..b09cdb5f13
>>> --- /dev/null
>>> +++
>>> b/meta/recipes-extended/iputils/iputils/0001-Limit-setcap-search-to-PATH-for-cross-builds.patch
>>> @@ -0,0 +1,33 @@
>>> +From aedb7811d1e9b0eae5d33b5bed39f6219a06a8dc Mon Sep 17 00:00:00 2001
>>> +From: Jate Sujjavanich <jatedev@gmail.com>
>>> +Date: Thu, 18 Feb 2021 21:35:03 -0500
>>> +Subject: [PATCH] Limit setcap search to PATH for cross builds
>>> +
>>> +Upstream-Status: Pending
>>> +---
>>> + meson.build | 9 ++++++++-
>>> + 1 file changed, 8 insertions(+), 1 deletion(-)
>>> +
>>> +diff --git a/meson.build b/meson.build
>>> +index aff75a2..16bbfb6 100644
>>> +--- a/meson.build
>>> ++++ b/meson.build
>>> +@@ -215,7 +215,14 @@ config_h = configure_file(
>>> +       output : 'config.h',
>>> +       configuration : conf)
>>> +
>>> +-setcap = find_program('setcap', '/usr/sbin/setcap', '/sbin/setcap',
>>> required : false)
>>> ++if meson.is_cross_build()
>>> ++      message('limit setcap search to PATH')
>>> ++      path_setcap = ['setcap']
>>> ++else
>>> ++      message('search for setcap in PATH and admin dirs')
>>> ++      path_setcap = ['setcap', '/usr/sbin/setcap', '/sbin/setcap']
>>> ++endif
>>> ++setcap = find_program(path_setcap, required : false)
>>> + if cap_dep.found() and setcap.found()
>>> +       perm_type = 'caps'
>>> +       setcap_path = setcap.path()
>>> +--
>>> +2.30.0
>>> +
>>> diff --git a/meta/recipes-extended/iputils/iputils_s20200821.bb
>>> b/meta/recipes-extended/iputils/iputils_s20200821.bb
>>> index 8b63a23c61..31fe1acdaf 100644
>>> --- a/meta/recipes-extended/iputils/iputils_s20200821.bb
>>> +++ b/meta/recipes-extended/iputils/iputils_s20200821.bb
>>> @@ -12,6 +12,7 @@ DEPENDS = "gnutls"
>>>
>>>  SRC_URI = "git://github.com/iputils/iputils \
>>>             file://0001-rarpd-rdisc-Drop-PrivateUsers.patch \
>>> +
>>>  file://0001-Limit-setcap-search-to-PATH-for-cross-builds.patch \
>>>             "
>>>  SRCREV = "23c3782ae0c7f9c6ae59dbed8ad9204f8758542b"
>>>
>>> @@ -26,7 +27,7 @@ CVE_CHECK_WHITELIST += "CVE-2000-1213 CVE-2000-1214"
>>>  PACKAGECONFIG ??= "libcap rarpd \
>>>                     ${@bb.utils.contains('DISTRO_FEATURES', 'ipv6',
>>> 'ninfod traceroute6', '', d)} \
>>>                     ${@bb.utils.filter('DISTRO_FEATURES', 'systemd', d)}"
>>> -PACKAGECONFIG[libcap] = "-DUSE_CAP=true, -DUSE_CAP=false, libcap"
>>> +PACKAGECONFIG[libcap] = "-DUSE_CAP=true, -DUSE_CAP=false, libcap
>>> libcap-native"
>>>  PACKAGECONFIG[libidn] = "-DUSE_IDN=true, -DUSE_IDN=false, libidn2"
>>>  PACKAGECONFIG[gettext] = "-DUSE_GETTEXT=true, -DUSE_GETTEXT=false,
>>> gettext"
>>>  PACKAGECONFIG[ninfod] = "-DBUILD_NINFOD=true,-DBUILD_NINFOD=false,"
>>> --
>>> 2.25.1
>>>
>>>
>>
>> --
>> Best regards,
>>
>> José Quaresma
>>
>

-- 
Best regards,

José Quaresma

[-- Attachment #2: Type: text/html, Size: 10334 bytes --]

Re: [PATCH v3 2/2] iputils: Fix setcap/setuid of executables

[Richard Purdie]

On Mon, 2021-02-22 at 17:39 +0000, Jose Quaresma wrote:
> Hi,
> 
> Jate Sujjavanich <jatedev@gmail.com> escreveu no dia segunda, 22/02/2021 à(s) 15:58:
> > If libcap is disabled in PACKAGECONFIG, then arguments two and three mean 
> > find_program find could setcap on the build machine. This would mean it is
> >  not strictly determined by the recipe and it's determined by the 
> > configuration of the build machine instead.
> > 
> 
> In this case the meson will not use the setcap from the build machine because 
> the libcap not found on the target sysroot. The libcap is a dependencie when 
> PACKAGECONFIG+="libcap" and in this case the meson it will fallback to use
> setuid.
> https://github.com/iputils/iputils/blob/f0c64bbb2b1563647bfeeed8d82bc4d2d2312d8a/meson.build#L221

Whilst it may not use libcap, I seem to remember issues with the output 
still not being deterministic since there was something in the out which 
indicated the presence of setcap (or not).

Cheers,

Richard

Re: [PATCH v3 2/2] iputils: Fix setcap/setuid of executables

[Jate Sujjavanich]

[-- Attachment #1: Type: text/plain, Size: 2630 bytes --]

I tested removing the cross-compile detection patch and removing libcap
from PACKAGECONFIG. The build does find setcap on the build machine.

log.do_configure:
CONFIGURATION
Capatiblity (with libcap): false
...
Program /usr/sbin/setcap found: YES (/usr/sbin/setcap)

However, meson did pass perm_type = setuid to setcap-setuid.sh.

log.do_install
[1/2]
/workdir/poky/build/tmp/work/core2-64-poky-linux/iputils/s20200821-r0/reci
pe-sysroot-native/usr/bin/meson install --no-rebuild
setcap-setuid.sh: changing
'/workdir/poky/build/tmp/work/core2-64-poky-linux/ipu
tils/s20200821-r0/image//bin/ping' to be setuid root executable
ownership of
'/workdir/poky/build/tmp/work/core2-64-poky-linux/iputils/s20200821-r0/image//bin/ping'
retained as root

The conditional code in meson.build does fall back to setuid mode based on
the presence of libcap stored in cap_dep.found(). So I think you are right,
Jose.
if cap_dep.found() and setcap.found()
        perm_type = 'caps'
        setcap_path = setcap.path()
else
        perm_type = 'setuid'
        setcap_path = '/dev/null'
endif

And setcap-setuid.sh uses a simple chown and chmod. I *think* these are
universal in posix and that we don't have to worry about host contamination.
        setuid)
                _log "changing '$exec_path' to be setuid root executable"
                chown -v root "$exec_path"
                chmod -v u+s "$exec_path"

- Jate


On Mon, Feb 22, 2021 at 3:26 PM Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:

> On Mon, 2021-02-22 at 17:39 +0000, Jose Quaresma wrote:
> > Hi,
> >
> > Jate Sujjavanich <jatedev@gmail.com> escreveu no dia segunda,
> 22/02/2021 à(s) 15:58:
> > > If libcap is disabled in PACKAGECONFIG, then arguments two and three
> mean
> > > find_program find could setcap on the build machine. This would mean
> it is
> > >  not strictly determined by the recipe and it's determined by the
> > > configuration of the build machine instead.
> > >
> >
> > In this case the meson will not use the setcap from the build machine
> because
> > the libcap not found on the target sysroot. The libcap is a dependencie
> when
> > PACKAGECONFIG+="libcap" and in this case the meson it will fallback to
> use
> > setuid.
> >
> https://github.com/iputils/iputils/blob/f0c64bbb2b1563647bfeeed8d82bc4d2d2312d8a/meson.build#L221
>
> Whilst it may not use libcap, I seem to remember issues with the output
> still not being deterministic since there was something in the out which
> indicated the presence of setcap (or not).
>
> Cheers,
>
> Richard
>
>

[-- Attachment #2: Type: text/html, Size: 4237 bytes --]

Re: [PATCH v3 2/2] iputils: Fix setcap/setuid of executables

[Jose Quaresma]

[-- Attachment #1: Type: text/plain, Size: 2949 bytes --]

Hi Jate,

Is that behavior that you explain that I am trying to said.

Jate Sujjavanich <jatedev@gmail.com> escreveu no dia quarta, 24/02/2021
à(s) 17:19:

> I tested removing the cross-compile detection patch and removing libcap
> from PACKAGECONFIG. The build does find setcap on the build machine.
>
> log.do_configure:
> CONFIGURATION
> Capatiblity (with libcap): false
> ...
> Program /usr/sbin/setcap found: YES (/usr/sbin/setcap)
>
> However, meson did pass perm_type = setuid to setcap-setuid.sh.
>
> log.do_install
> [1/2]
> /workdir/poky/build/tmp/work/core2-64-poky-linux/iputils/s20200821-r0/reci
> pe-sysroot-native/usr/bin/meson install --no-rebuild
> setcap-setuid.sh: changing
> '/workdir/poky/build/tmp/work/core2-64-poky-linux/ipu
> tils/s20200821-r0/image//bin/ping' to be setuid root executable
> ownership of
> '/workdir/poky/build/tmp/work/core2-64-poky-linux/iputils/s20200821-r0/image//bin/ping'
> retained as root
>
> The conditional code in meson.build does fall back to setuid mode based on
> the presence of libcap stored in cap_dep.found(). So I think you are right,
> Jose.
> if cap_dep.found() and setcap.found()
>         perm_type = 'caps'
>         setcap_path = setcap.path()
> else
>         perm_type = 'setuid'
>         setcap_path = '/dev/null'
> endif
>
> And setcap-setuid.sh uses a simple chown and chmod. I *think* these are
> universal in posix and that we don't have to worry about host contamination.
>         setuid)
>                 _log "changing '$exec_path' to be setuid root executable"
>                 chown -v root "$exec_path"
>                 chmod -v u+s "$exec_path"
>
> - Jate
>
>
> On Mon, Feb 22, 2021 at 3:26 PM Richard Purdie <
> richard.purdie@linuxfoundation.org> wrote:
>
>> On Mon, 2021-02-22 at 17:39 +0000, Jose Quaresma wrote:
>> > Hi,
>> >
>> > Jate Sujjavanich <jatedev@gmail.com> escreveu no dia segunda,
>> 22/02/2021 à(s) 15:58:
>> > > If libcap is disabled in PACKAGECONFIG, then arguments two and three
>> mean
>> > > find_program find could setcap on the build machine. This would mean
>> it is
>> > >  not strictly determined by the recipe and it's determined by the
>> > > configuration of the build machine instead.
>> > >
>> >
>> > In this case the meson will not use the setcap from the build machine
>> because
>> > the libcap not found on the target sysroot. The libcap is a dependencie
>> when
>> > PACKAGECONFIG+="libcap" and in this case the meson it will fallback to
>> use
>> > setuid.
>> >
>> https://github.com/iputils/iputils/blob/f0c64bbb2b1563647bfeeed8d82bc4d2d2312d8a/meson.build#L221
>>
>> Whilst it may not use libcap, I seem to remember issues with the output
>> still not being deterministic since there was something in the out which
>> indicated the presence of setcap (or not).
>>
>> Cheers,
>>
>> Richard
>>
>>

-- 
Best regards,

José Quaresma

[-- Attachment #2: Type: text/html, Size: 4861 bytes --]

Re: [PATCH v3 2/2] iputils: Fix setcap/setuid of executables

[Jate Sujjavanich]

[-- Attachment #1: Type: text/plain, Size: 3298 bytes --]

Jose,

Yes, I forgot to add a sentence saying that my testing confirmed what you
said. Your explanation was helpful in learning about this meson build.

- Jate

On Wed, Mar 3, 2021 at 8:41 AM Jose Quaresma <quaresma.jose@gmail.com>
wrote:

> Hi Jate,
>
> Is that behavior that you explain that I am trying to said.
>
> Jate Sujjavanich <jatedev@gmail.com> escreveu no dia quarta, 24/02/2021
> à(s) 17:19:
>
>> I tested removing the cross-compile detection patch and removing libcap
>> from PACKAGECONFIG. The build does find setcap on the build machine.
>>
>> log.do_configure:
>> CONFIGURATION
>> Capatiblity (with libcap): false
>> ...
>> Program /usr/sbin/setcap found: YES (/usr/sbin/setcap)
>>
>> However, meson did pass perm_type = setuid to setcap-setuid.sh.
>>
>> log.do_install
>> [1/2]
>> /workdir/poky/build/tmp/work/core2-64-poky-linux/iputils/s20200821-r0/reci
>> pe-sysroot-native/usr/bin/meson install --no-rebuild
>> setcap-setuid.sh: changing
>> '/workdir/poky/build/tmp/work/core2-64-poky-linux/ipu
>> tils/s20200821-r0/image//bin/ping' to be setuid root executable
>> ownership of
>> '/workdir/poky/build/tmp/work/core2-64-poky-linux/iputils/s20200821-r0/image//bin/ping'
>> retained as root
>>
>> The conditional code in meson.build does fall back to setuid mode based
>> on the presence of libcap stored in cap_dep.found(). So I think you are
>> right, Jose.
>> if cap_dep.found() and setcap.found()
>>         perm_type = 'caps'
>>         setcap_path = setcap.path()
>> else
>>         perm_type = 'setuid'
>>         setcap_path = '/dev/null'
>> endif
>>
>> And setcap-setuid.sh uses a simple chown and chmod. I *think* these are
>> universal in posix and that we don't have to worry about host contamination.
>>         setuid)
>>                 _log "changing '$exec_path' to be setuid root executable"
>>                 chown -v root "$exec_path"
>>                 chmod -v u+s "$exec_path"
>>
>> - Jate
>>
>>
>> On Mon, Feb 22, 2021 at 3:26 PM Richard Purdie <
>> richard.purdie@linuxfoundation.org> wrote:
>>
>>> On Mon, 2021-02-22 at 17:39 +0000, Jose Quaresma wrote:
>>> > Hi,
>>> >
>>> > Jate Sujjavanich <jatedev@gmail.com> escreveu no dia segunda,
>>> 22/02/2021 à(s) 15:58:
>>> > > If libcap is disabled in PACKAGECONFIG, then arguments two and three
>>> mean
>>> > > find_program find could setcap on the build machine. This would mean
>>> it is
>>> > >  not strictly determined by the recipe and it's determined by the
>>> > > configuration of the build machine instead.
>>> > >
>>> >
>>> > In this case the meson will not use the setcap from the build machine
>>> because
>>> > the libcap not found on the target sysroot. The libcap is a
>>> dependencie when
>>> > PACKAGECONFIG+="libcap" and in this case the meson it will fallback to
>>> use
>>> > setuid.
>>> >
>>> https://github.com/iputils/iputils/blob/f0c64bbb2b1563647bfeeed8d82bc4d2d2312d8a/meson.build#L221
>>>
>>> Whilst it may not use libcap, I seem to remember issues with the output
>>> still not being deterministic since there was something in the out which
>>> indicated the presence of setcap (or not).
>>>
>>> Cheers,
>>>
>>> Richard
>>>
>>>
>
> --
> Best regards,
>
> José Quaresma
>

[-- Attachment #2: Type: text/html, Size: 5439 bytes --]