diff mbox series

[1/1] package/tpm2-tss: needs host-acl

Message ID 20211013065446.1857356-1-fontaine.fabrice@gmail.com
State Rejected
Headers show
Series [1/1] package/tpm2-tss: needs host-acl | expand

Commit Message

Fabrice Fontaine Oct. 13, 2021, 6:54 a.m. UTC
Add host-acl mandatory dependency for setfacl to avoid the following
build failure since bump to version 3.1.0 in commit
470e2e9bc5211184431fbc868359a9d695e624f4 and
https://github.com/tpm2-software/tpm2-tss/commit/9d42f4dbde6a94724e95d57c333cda7711dda57c:

configure: error: Missing required program 'setfacl': ensure it is installed and on PATH.

Fixes:
 - http://autobuild.buildroot.org/results/eab44622f8d8ff4fbf464b5a98856382f019c2cb

Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
---
 package/tpm2-tss/tpm2-tss.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Thomas Petazzoni Oct. 18, 2021, 8:34 p.m. UTC | #1
On Wed, 13 Oct 2021 08:54:46 +0200
Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:

> Add host-acl mandatory dependency for setfacl to avoid the following
> build failure since bump to version 3.1.0 in commit
> 470e2e9bc5211184431fbc868359a9d695e624f4 and
> https://github.com/tpm2-software/tpm2-tss/commit/9d42f4dbde6a94724e95d57c333cda7711dda57c:

So this commit adds:

# Check all tools used by make install
AS_IF([test "$HOSTOS" = "Linux"],
      [ERROR_IF_NO_PROG([groupadd])
       ERROR_IF_NO_PROG([useradd])
       ERROR_IF_NO_PROG([id])
       ERROR_IF_NO_PROG([chown])
       ERROR_IF_NO_PROG([chmod])
       ERROR_IF_NO_PROG([mkdir])
       ERROR_IF_NO_PROG([setfacl])])

But in the context of Buildroot, using groupadd, useradd, chown, chmod,
setfacl at install time cannot work, since we're not doing "make
install" as root.

So are we sure that adding host-acl as a dependency to get a host
variant of setfacl is really the good solution?

How is it handled for groupadd/useradd/chown/chmod today ?

Thomas
Fabrice Fontaine Oct. 19, 2021, 9:28 p.m. UTC | #2
Le lun. 18 oct. 2021 à 22:35, Thomas Petazzoni
<thomas.petazzoni@bootlin.com> a écrit :
>
> On Wed, 13 Oct 2021 08:54:46 +0200
> Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:
>
> > Add host-acl mandatory dependency for setfacl to avoid the following
> > build failure since bump to version 3.1.0 in commit
> > 470e2e9bc5211184431fbc868359a9d695e624f4 and
> > https://github.com/tpm2-software/tpm2-tss/commit/9d42f4dbde6a94724e95d57c333cda7711dda57c:
>
> So this commit adds:
>
> # Check all tools used by make install
> AS_IF([test "$HOSTOS" = "Linux"],
>       [ERROR_IF_NO_PROG([groupadd])
>        ERROR_IF_NO_PROG([useradd])
>        ERROR_IF_NO_PROG([id])
>        ERROR_IF_NO_PROG([chown])
>        ERROR_IF_NO_PROG([chmod])
>        ERROR_IF_NO_PROG([mkdir])
>        ERROR_IF_NO_PROG([setfacl])])
>
> But in the context of Buildroot, using groupadd, useradd, chown, chmod,
> setfacl at install time cannot work, since we're not doing "make
> install" as root.
>
> So are we sure that adding host-acl as a dependency to get a host
> variant of setfacl is really the good solution?
>
> How is it handled for groupadd/useradd/chown/chmod today ?
groupadd/useradd/chown/chmod are called since version 3.0.0 and
https://github.com/tpm2-software/tpm2-tss/commit/d01c24e6cb095e18c5e29af4a3018010b3230a6d

setfacl is called since version 3.0.0 and
https://github.com/tpm2-software/tpm2-tss/commit/811e451c94254496338d36783cda44a7729b085a

But more importantly, all those functions are not even used on a
target without systemd and a host with
systemd-{sysusers,tmpfiles} because tpm2-tss try to install files in
$(TARGET_DIR)/etc/{sysusers,tmpfiles}.d which don't even exist if
BR2_INIT_SYSTEMD is not set.
Even if BR2_INIT_SYSTEMD is set, the path is wrong as it should be
installed in $(TARGET_DIR)/usr/lib/{sysusers,tmpfiles}.d

To conclude, I would say that install of tpm2-tss is partially broken
since at least bump to version 3.0.0 in commit
89f3d90e24bb24b70fc4a37bf8cff6186672475c.
>
> Thomas
> --
> Thomas Petazzoni, co-owner and CEO, Bootlin
> Embedded Linux and Kernel engineering and training
> https://bootlin.com
Best Regards,

Fabrice
Yann E. MORIN Nov. 6, 2021, 2:56 p.m. UTC | #3
Fabrice, Thomas, Yair, All,

On 2021-10-19 23:28 +0200, Fabrice Fontaine spake thusly:
> Le lun. 18 oct. 2021 à 22:35, Thomas Petazzoni
> <thomas.petazzoni@bootlin.com> a écrit :
> > On Wed, 13 Oct 2021 08:54:46 +0200
> > Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:
> > > Add host-acl mandatory dependency for setfacl to avoid the following
> > > build failure since bump to version 3.1.0 in commit
> > > 470e2e9bc5211184431fbc868359a9d695e624f4 and
> > > https://github.com/tpm2-software/tpm2-tss/commit/9d42f4dbde6a94724e95d57c333cda7711dda57c:
> > So this commit adds:
> > # Check all tools used by make install
> > AS_IF([test "$HOSTOS" = "Linux"],
> >       [ERROR_IF_NO_PROG([groupadd])
> >        ERROR_IF_NO_PROG([useradd])
> >        ERROR_IF_NO_PROG([id])
> >        ERROR_IF_NO_PROG([chown])
> >        ERROR_IF_NO_PROG([chmod])
> >        ERROR_IF_NO_PROG([mkdir])
> >        ERROR_IF_NO_PROG([setfacl])])
> >
> > But in the context of Buildroot, using groupadd, useradd, chown, chmod,
> > setfacl at install time cannot work, since we're not doing "make
> > install" as root.
> >
> > So are we sure that adding host-acl as a dependency to get a host
> > variant of setfacl is really the good solution?
> >
> > How is it handled for groupadd/useradd/chown/chmod today ?
> groupadd/useradd/chown/chmod are called since version 3.0.0 and
> https://github.com/tpm2-software/tpm2-tss/commit/d01c24e6cb095e18c5e29af4a3018010b3230a6d
> 
> setfacl is called since version 3.0.0 and
> https://github.com/tpm2-software/tpm2-tss/commit/811e451c94254496338d36783cda44a7729b085a
> 
> But more importantly, all those functions are not even used on a
> target without systemd and a host with
> systemd-{sysusers,tmpfiles} because tpm2-tss try to install files in
> $(TARGET_DIR)/etc/{sysusers,tmpfiles}.d which don't even exist if
> BR2_INIT_SYSTEMD is not set.
> Even if BR2_INIT_SYSTEMD is set, the path is wrong as it should be
> installed in $(TARGET_DIR)/usr/lib/{sysusers,tmpfiles}.d
> 
> To conclude, I would say that install of tpm2-tss is partially broken
> since at least bump to version 3.0.0 in commit
> 89f3d90e24bb24b70fc4a37bf8cff6186672475c.

Agreed, the install is borked because it is looking for programs at
configure time, so it finds those on the host if they exist, or do not
find any at all, which can very well differ from what will be present
on the target.

But I would say that this is not totally unreasonable: there is no way,
at cross-configure time, for a package to find the tools that will be
present at runtime.

All we can do in such a case it force the path to such tools.

However, in this case, tpm2-tss only uses setfacl if systemd-tmpfiles is
not available. If the call to setfacl fails, the install does not fail
(split on two lines for readability):

    @-$(call make_fapi_dirs) && $(call set_fapi_permissions) \
    || echo "WARNING Failed to create the FAPI directories with the correct permissions"

set_fapi_permissions is a macro that eventually expands to:

    (chown -R tss:tss "$1") && \
    (chmod -R 2775 "$1") && \
    (setfacl -m default:group:tss:rwx "$1")

So the call to setfacl will not even be ever attempted, because the
chown will fail first.

We have three solutions to that:

  - either we force the use of systemd-tmpfiles when tpm2-tss is
    enabled,

  - or we disable tpm2-tss if systemd-tmpfiles is not enabled,

  - or we just configure tpm2-tss with ac_cv_prog_result_setfacl=true,
    and let the install use the fallback path and be done with it.

So, this would fix the immediate issue.

Now there would remain the issue that some features of tpm2-tss sill be
incorrectly enabled/disabled depending on whther the host has or is
missing systemd-users and/or systemd-tmpfiles:

    # Check for systemd helper tools used by make install
    AC_CHECK_PROG(systemd_sysusers, systemd-sysusers, yes)
    AM_CONDITIONAL(SYSD_SYSUSERS, test "x$systemd_sysusers" = "xyes")
    AC_CHECK_PROG(systemd_tmpfiles, systemd-tmpfiles, yes)
    AM_CONDITIONAL(SYSD_TMPFILES, test "x$systemd_tmpfiles" = "xyes")

In which case we would need to pass ac_cv_prog_systemd_sysusers=yes and
systemd_tmpfiles=yes when they are enabled in our config.

This would be for another patch, of course.

I'll see to send appropriate patches soonish.

Regards,
Yann E. MORIN.
diff mbox series

Patch

diff --git a/package/tpm2-tss/tpm2-tss.mk b/package/tpm2-tss/tpm2-tss.mk
index 8e701933c4..58f3fa38cc 100644
--- a/package/tpm2-tss/tpm2-tss.mk
+++ b/package/tpm2-tss/tpm2-tss.mk
@@ -11,7 +11,7 @@  TPM2_TSS_LICENSE_FILES = LICENSE
 TPM2_TSS_CPE_ID_VENDOR = tpm2_software_stack_project
 TPM2_TSS_CPE_ID_PRODUCT = tpm2_software_stack
 TPM2_TSS_INSTALL_STAGING = YES
-TPM2_TSS_DEPENDENCIES = liburiparser openssl host-pkgconf
+TPM2_TSS_DEPENDENCIES = liburiparser openssl host-acl host-pkgconf
 TPM2_TSS_CONF_OPTS = --with-crypto=ossl --disable-doxygen-doc --disable-defaultflags
 # 0001-configure-Only-use-CXX-when-fuzzing.patch
 TPM2_TSS_AUTORECONF = YES