Bug 2044850

Spec file defined CFLAGS and CXXFLAGS variables prior to calling %make_build, this might be the culprit. While being at it:

* Replace the 'sed' call to fix for failing RPC header search with upstream patch (and follow-up)
* Call autoreconf since above patches change configure.ac
* Drop old unused patch files from dist-git

I don't get it:

Trying to verify my changes, I did a local build as well as a scratch build and annocheck complains for both. Since the package's build system is autotools-based, things should just work. To verify I'm really loosing compiler flags along the way, I did this:

| --- a/conntrack-tools.spec
| +++ b/conntrack-tools.spec
| @@ -52,6 +52,8 @@ show an event message (one line) per newly established connection.
|  %autosetup -p1
|  
|  %build
| +echo "CFLAGS: %{build_cflags}"
| +echo "LDFLAGS: %{build_ldflags}"
|  autoreconf -fi
|  rm -Rf autom4te*.cache config.h.in~
|  %configure --disable-static --enable-systemd

Here's the relevant excerpt from 'centpkg local':

| Executing(%build): /bin/sh -e /var/tmp/rpm-tmp.8HHPpf
| + umask 022
| + cd /home/psutter/centpkg/conntrack-tools
| + cd conntrack-tools-1.4.5
| + echo 'CFLAGS: -O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS  -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -m64  -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection'
| CFLAGS: -O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS  -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -m64  -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection
| + echo 'LDFLAGS: -Wl,-z,relro -Wl,--as-needed   -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -Wl,--build-id=sha1'
| LDFLAGS: -Wl,-z,relro -Wl,--as-needed   -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -Wl,--build-id=sha1
| + autoreconf -fi

There is no -Wl'-z'now among linker flags. So if this isn't a general problem with RPM build system (which is pretty unlikely), I'm hunting heisenbugs here and local as well as scratch builds just don't match candidate builds (which is a pity).

Jiji, please consider this ticket for qa_ack+ and select an ITM as you see fit. I already created a merge request, let's just hope my changes are effective and the candidate build will be fine.

@fweimer , would you like to review Phil's results above?

https://fedora.softwarefactory-project.io/rpminspect-report/?url=https://centos.softwarefactory-project.io/logs//2/2/759e58a7df7b7602ba50500918bebf49361fd469/check/rpm-rpminspect/1b59048/result.json still shows failures.

The build log shows that -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld is missing from the linker invocation:

2022-01-25 14:27:54.242985 | cloud-host | DEBUG: libtool: link: gcc -shared  -fPIC -DPIC  .libs/ct_helper_amanda_la-amanda.o   -lnetfilter_conntrack -lnfnetlink  -O2 -flto=auto -g -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -march=x86-64-v2 -mtune=generic -Wl,-z -Wl,lazy -Wl,-z -Wl,relro -Wl,--as-needed -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1   -Wl,-soname -Wl,ct_helper_amanda.so -o .libs/ct_helper_amanda.so

(In reply to Florian Weimer from comment #5)
> https://fedora.softwarefactory-project.io/rpminspect-report/?url=https://
> centos.softwarefactory-project.io/logs//2/2/
> 759e58a7df7b7602ba50500918bebf49361fd469/check/rpm-rpminspect/1b59048/result.
> json still shows failures.
> 
> The build log shows that -Wl,-z,now
> -specs=/usr/lib/rpm/redhat/redhat-hardened-ld is missing from the linker
> invocation:
> 
> 2022-01-25 14:27:54.242985 | cloud-host | DEBUG: libtool: link: gcc -shared 
> -fPIC -DPIC  .libs/ct_helper_amanda_la-amanda.o   -lnetfilter_conntrack
> -lnfnetlink  -O2 -flto=auto -g -grecord-gcc-switches
> -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -march=x86-64-v2
> -mtune=generic -Wl,-z -Wl,lazy -Wl,-z -Wl,relro -Wl,--as-needed
> -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1   -Wl,-soname
> -Wl,ct_helper_amanda.so -o .libs/ct_helper_amanda.so

That's the symptom, I'm asking for the cause. Another try at identifying input
to configure call:

|  %build
| +env
| +echo "CFLAGS: %{build_cflags}"
| +echo "LDFLAGS: %{build_ldflags}"
|  autoreconf -fi

Here's build.log for x86_64 build:
https://kojihub.stream.rdu2.redhat.com/kojifiles/work/tasks/5001/905001/build.log

In summary:

- env contains:

RPM_LD_FLAGS=-Wl,-z,relro -Wl,--as-needed   -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 
RPM_OPT_FLAGS=-O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS  -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -m64 -march=x86-64-v2 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-

- build_cflags/build_ldflags are:

CFLAGS: -O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS  -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -m64 -march=x86-64-v2 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection
LDFLAGS: -Wl,-z,relro -Wl,--as-needed   -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1

So if build system is not broken, how does it pass the required linker flags to
the package build? I can work around it by setting the missing flags manually,
but would like to have an official statement before implementing such hacks.

(In reply to Phil Sutter from comment #6)
> So if build system is not broken, how does it pass the required linker flags
> to
> the package build? I can work around it by setting the missing flags
> manually,
> but would like to have an official statement before implementing such hacks.

I'm not sure what you are looking here, sorry. There is no official makefile-based build system, each package brings its own version. As a result, build flags handling varies from package to package.

I *suspect* it could be related to -specs= support, which was added upstream, but is missing from Fedora/downstream:

https://src.fedoraproject.org/rpms/libtool/pull-request/7

Frédéric, do you think we should backport this upstream enhancement?

Hmm. Maybe this is a misunderstanding, our replies seem to drift. Let me reiterate from my perspective, maybe you find the spot where I took a wrong turn:

- Annocheck fails for missing linker flag '-Wl,-z,now'

- If this is a problem with conntrack-tools' build system, the relevant flag
  must get lost inside of configure or during 'make' call, but I don't see
  where

- I assume the missing flag is passed by rpmbuild, so looking at 'env' or any
  "spec file variables" (%{...}) should show it, but it's not there

- If my package should be built with a specific linker flag, it needs to know.
  How is the information submitted?

I looked at this properly, and the spec file disables hardening explicitly:

%undefine _hardened_build
[…]
CFLAGS="${CFLAGS} -Wl,-z,lazy"
CXXFLAGS="${CXXFLAGS} -Wl,-z,lazy"

This is the result of an improper fix for bug 1413408. The root cause for this bug seems to be that the plugin API is only defined by /usr/sbin/conntrackd, which means that /usr/sbin/nfct cannot load those plugins. There are several ways to solve this: merge the command line tools into one binary, factor out these interfaces into a new DSO, or (but this is more of a quick hack) use weak symbol references.

(In reply to Florian Weimer from comment #9)
> I looked at this properly, and the spec file disables hardening explicitly:
> 
> %undefine _hardened_build

Argh, I missed this statement. Clearly explains why things didn't make sense
from my perspective. Thanks for pointing it out!

> […]
> CFLAGS="${CFLAGS} -Wl,-z,lazy"
> CXXFLAGS="${CXXFLAGS} -Wl,-z,lazy"
> 
> This is the result of an improper fix for bug 1413408. The root cause for
> this bug seems to be that the plugin API is only defined by
> /usr/sbin/conntrackd, which means that /usr/sbin/nfct cannot load those
> plugins. There are several ways to solve this: merge the command line tools
> into one binary, factor out these interfaces into a new DSO, or (but this is
> more of a quick hack) use weak symbol references.

Thanks, I'll evaluate on the possible options.

Patch to merge nfct into conntrackd sent upstream:
https://lore.kernel.org/netfilter-devel/20220126224930.31730-1-phil@nwl.cc/

Given the potentially unwanted side-effects of a much bigger nfct binary, I
submitted a new approach which adds the missing symbols to nfct if requested by
a configure option:

https://lore.kernel.org/netfilter-devel/20220208160100.27527-1-phil@nwl.cc/

Upstream promised to either come up with an own solution or accept mine within
this week.

Upstream commit to backport:

commit dc454a657f57a5cf143fddc5c1dd87a510c1790a (HEAD -> master, origin/master, origin/HEAD)
Author: Pablo Neira Ayuso <pablo>
Date:   Tue Mar 8 23:05:39 2022 +0100

    nfct: remove lazy binding
    
    Since cd5135377ac4 ("conntrackd: cthelper: Set up userspace helpers when
    daemon starts"), userspace conntrack helpers do not depend on a previous
    invocation of nfct to set up the userspace helpers.
    
    Move helper definitions to nfct-extensions/helper.c since existing
    deployments might still invoke nfct, even if not required anymore.
    
    This patch was motivated by the removal of the lazy binding.
    
    Phil Sutter says:
    
    "For security purposes, distributions might want to pass -Wl,-z,now
    linker flags to all builds, thereby disabling lazy binding globally.
    
    In the past, nfct relied upon lazy binding: It uses the helper objects'
    parsing functions without but doesn't provide all symbols the objects
    use."
    
    Acked-by: Phil Sutter <phil>
    Signed-off-by: Pablo Neira Ayuso <pablo>

Jiji, Please set ITM as you see fit.

Jiji,

Pagure is not functional at the moment[1], so let us please use a "manual"
process. Here[2] is a scratch build of the new release, could you please give
it a try and set Verified:Tested if it resolves this BZ?

Thanks!

[1] https://issues.redhat.com/browse/OSCI-3013
[2] https://brewweb.engineering.redhat.com/brew/taskinfo?taskID=43755313

Oh, indeed. Apparently I pushed releases which still contain the '%undefine
_hardened_build' directive. Sorry for the mess, I'll have to push a new release
for C9S and update the release for RHEL9.0. At least Pagure dist-git sync is
back, so we can use a PR-based workflow again.

Here it is:

https://src.osci.redhat.com/rpms/conntrack-tools/pull-request/7

It also fixes the build instructions in tests.yml - they needed adjustment due to the backported patches.

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (new packages: conntrack-tools), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2022:2777