F35 Change: Broken RPATH will fail rpmbuild (System-Wide Change proposal)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://fedoraproject.org/wiki/Changes/Broken_RPATH_will_fail_rpmbuild

== Summary ==
Enable broken RPATH detection
[https://docs.fedoraproject.org/en-US/packaging-guidelines/#_brp_buildroot_policy_scripts
buildroot policy] script by default. This will make the RPM build fail
once a broken RPATH was detected within a binary or a shared library
file. An opt-out mechanism will be provided as well.

== Owner ==
* Name: [[User:cstratak| Charalampos Stratakis]]
* Email: cstratak AT redhat.com


== Detailed Description ==
The dynamic linker and loader (ld.so) is responsible for resolving
runtime dependencies of executables and shared library files through a
search hierarchy. However some packages (usually through their
upstream buildsystems) contain a hard-coded path within their binaries
or .so files, by using the -R or -rpath flag during compilation, which
is called an RPATH. By utilizing RPATH, ELF files can point to
directories to be included in the search path, on runtime, to resolve
their dependencies.

While RPATH can be used for non-standard directories, such as a place
containing private libraries of the project, when it points to a value
already provided by the search path of ld.so, it changes the hierarchy
of the search by placing the system defaults first.

(a) DT_RPATH -> (b) LD_LIBRARY_PATH -> (c) DT_RUNPATH -> (d) cache
(/etc/ld.so.cache) -> (e) system defaults

This could present a variety of issues, such as LD_LIBRARY_PATH
overrides not working, incomplete dependency resolution, loading of
wrong libraries etc. In general, changing the default search hierarchy
could lead to unforeseen bugs and issues - in a similar manner as
adding /usr/lib64 to LD_LIBRARY_PATH.

Another problem of a hardcoded RPATH is security. When an ELF object
contains an RPATH pointed to a directory not managed by the system,
where some malicious actor has write permissions to, it's relatively
easy to execute arbitrary code.

Performance can be also affected, since probing explicitly e.g.
/usr/lib64 through RPATH adds extra open/openat system calls to the
process startup.

In Fedora the use of such RPATH is
[https://docs.fedoraproject.org/en-US/packaging-guidelines/#_beware_of_rpath
forbidden], but it was never enforced. This change intends to ratify
that by executing `/usr/lib/rpm/check-rpaths` during rpmbuild, after
%install, and fail the build if an RPATH entry was detected. This
change will not affect RPATH's pointing to private libraries.

=== Definition of a broken RPATH ===
This change will use the
[https://github.com/rpm-software-management/rpm/blob/master/scripts/check-rpaths-worker
rpm script] for checking the broken RPATH's.

The categories are:

* standard RPATHs (e.g. `/usr/lib` or `/usr/lib64`); such RPATHs are a
minor issue but are introducing redundant searchpaths without
providing a benefit. They can also cause errors in multilib
environments.
*  invalid RPATHs; these are RPATHs which are neither absolute nor
relative filenames and can therefore be a SECURITY risk
*  insecure RPATHs; these are relative RPATHs which are a SECURITY risk
*  the special `$ORIGIN` RPATHs are appearing after other RPATHs; this
is just a minor issue but usually unwanted
*  the RPATH is empty; there is no reason for such RPATHs and they
cause unneeded work while loading libraries
*  an RPATH references `..` of an absolute path; this will break the
functionality when the path before `..` is a symlink

=== Opting out ===
A standard opt-out mechanism is provided, same as the other
[https://docs.fedoraproject.org/en-US/packaging-guidelines/#_brp_buildroot_policy_scripts
buildroot policy scripts], if the script provides incorrect results
for your package. Simply add `%define __brp_check_rpaths %{nil}` on
top of your SPEC. According to the guidelines, any package that
disables a BRP script this way, MUST also note the reason in an
accompanying comment.

== Feedback ==
The change [https://pagure.io/packaging-committee/issue/886 has been
proposed] a long time ago through FPC and the general consensus is
that it needs to be done along with an overhaul of the Fedora
documentation in regards to RPATH.

An [https://lists.fedoraproject.org/archives/list/devel@xxxxxxxxxxxxxxxxxxxxxxx/thread/7ZKGVM4XJ7QFRFZXTSGUT4K2MPDVV2XY/#W7LXPX4SIB57DDXXI4PQNKCFSOQMOL4S
email thread] was also started on Fedora devel regarding this change.

There have been multiple requests in the past to enable that check, as
well as various attempts to remove RPATH's from packages in the
distro. [https://lists.fedoraproject.org/archives/list/devel@xxxxxxxxxxxxxxxxxxxxxxx/thread/WD6JWMDIORBYNL4C5UHOJQGDR3N7HZY3/#LB63Q2HSLPWRMR7UQVQOYVVTG346TDRZ
0] [https://lists.fedoraproject.org/archives/list/devel@xxxxxxxxxxxxxxxxxxxxxxx/thread/A5X7ENAITWTVZASJBLCXS5MXQ7BE2RS6/#A5X7ENAITWTVZASJBLCXS5MXQ7BE2RS6
1][https://lists.fedoraproject.org/archives/list/devel@xxxxxxxxxxxxxxxxxxxxxxx/message/YJUWD2K32CZAGCDYOAJH2ISA2WF5AMGW/
2][https://lists.fedoraproject.org/archives/list/devel@xxxxxxxxxxxxxxxxxxxxxxx/message/2GITTEQ7SC5T656AXQ3OHKDG4SLINXB6/
3]

As for other distributions, Debian [https://wiki.debian.org/RpathIssue
discourages] the use of RPATH, openSUSE
[https://en.opensuse.org/openSUSE:Packaging_checks#Beware_of_Rpath
forbids it] by running the check from rpmlint after every package
build and Arch and Gentoo point out to possible insecure usage at
their respective documentation pages.

Also there has been a relevant [https://bugs.python.org/issue36659
discussion] in the upstream python community.

== Benefit to Fedora ==

The main benefit of this change is avoiding bugs that might stem from
hardcoded RPATH values which include but not limited to: loading of
wrong or malicious code, wrong dependency resolution of object files
etc. There will also be a performance increase by derived from
avoiding unnecessary  system calls required for handling RPATH's.

In addition the RPATH related packaging guidelines will be enforced by
the build system.

== Scope ==
* Proposal owners: Merge the
[https://src.fedoraproject.org/rpms/redhat-rpm-config/pull-request/132
pull request] to redhat-rpm-config to enable running the check-rpaths
script after %install.

* Other developers: After merging the changes to redhat-rpm-config the
affected package maintainers that will see their packages' builds
fail, will need to review their usage of RPATH and either remove it or
workaround the issue. The packages currently failing to build due to
RPATH issues, so far, are listed in the wiki page

* Release engineering: This change doesn't require coordination with
rel-eng, as any issues will be caught during the regular mass rebuild
of packages.

* Policies and guidelines: TODO: The guidelines will be overhauled to
take into account accepted usage or RPATH, clarification of the policy
and ways to opt-out.
FPC ticket: https://pagure.io/packaging-committee/issue/886
* Trademark approval: N/A (not needed for this Change)

== Upgrade/compatibility impact ==
There will be no visible impact to non-packagers. Packagers will need
to fix their packages if an broken RPATH entry was detected, as a
broken RPATH will make the rpmbuild fail either in koji or locally.

== How To Test ==
* Mock build testing:
Initiate a Fedora rawhide mock chroot and install the modified
redhat-rpm-config:
  mock -r fedora-rawhide-x86_64 --install
https://download.copr.fedorainfracloud.org/results/cstratak/rpath/fedora-rawhide-x86_64/02160930-redhat-rpm-config/redhat-rpm-config-190-1.fc35.noarch.rpm

Build your package with the --no-clean option:
 mock -r fedora-rawhide-x86_64 --no-clean <srpm>

* Local testing: Building rpm's locally should already reveal the
issue as the .rpmmacros file defines the RPATH check. Sample of a
vanilla .rpmmacros file:

 %_topdir %(echo $HOME)/rpmbuild

 %__arch_install_post \
    [ "%{buildarch}" = "noarch" ] || QA_CHECK_RPATHS=1 ; \
    case "${QA_CHECK_RPATHS:-}" in [1yY]*) /usr/lib/rpm/check-rpaths ;; esac \
    /usr/lib/rpm/check-buildroot

* rpmlint test: The binary-or-shlib-defines-rpath test from the
BinariesCheck module will reveal RPATH usage.

  rpmlint -c BinariesCheck <binary rpm>

  e.g.: rpmlint -c BinariesCheck -v audiofile-0.3.6-27.fc34.x86_64.rpm
        audiofile.x86_64: I: checking
        audiofile.x86_64: E: binary-or-shlib-defines-rpath
/usr/bin/sfconvert ['/usr/lib64']
        audiofile.x86_64: E: binary-or-shlib-defines-rpath
/usr/bin/sfinfo ['/usr/lib64']
        1 packages and 0 specfiles checked; 2 errors, 0 warnings.

== User Experience ==
N/A (While this is a system wide change the impact is not visible to
regular users, only to packagers as described above).

== Dependencies ==
The change depends on modifying redhat-rpm-config.

== Contingency Plan ==
* Contingency mechanism: The change to redhat-rpm-config will be reverted
* Contingency deadline: Beta freeze
* Blocks release? No

== Documentation ==
TODO:

The documentation of the packaging guidelines will be updated to
reflect the changes that this change brings. Updated guidelines will
be https://docs.fedoraproject.org/en-US/packaging-guidelines/#_beware_of_rpath


-- 
Ben Cotton
He / Him / His
Fedora Program Manager
Red Hat
TZ=America/Indiana/Indianapolis
_______________________________________________
devel mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxx
Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: https://lists.fedoraproject.org/archives/list/devel@xxxxxxxxxxxxxxxxxxxxxxx
Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Fedora Announce]     [Fedora Users]     [Fedora Kernel]     [Fedora Testing]     [Fedora Formulas]     [Fedora PHP Devel]     [Kernel Development]     [Fedora Legacy]     [Fedora Maintainers]     [Fedora Desktop]     [PAM]     [Red Hat Development]     [Gimp]     [Yosemite News]

  Powered by Linux