Bug 252629 - Need infrastructure to set ELF feature note flags via elfctl
Summary: Need infrastructure to set ELF feature note flags via elfctl
Status: New
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Ports Framework (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Port Management Team
URL:
Keywords:
Depends on: 239873 252689
Blocks:
  Show dependency treegraph
 
Reported: 2021-01-13 03:15 UTC by Ed Maste
Modified: 2021-01-22 19:41 UTC (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ed Maste freebsd_committer 2021-01-13 03:15:08 UTC
[PR for issue tracking; I would like to discuss the desired approach before starting implementation]

We have elfctl(1) which can set ELF feature flags, to indicate that a binary is incompatible with certain vulnerability mitigations.

The current list of flags is:
% elfctl -l
Known features are:
aslr            Disable ASLR
protmax         Disable implicit PROT_MAX
stackgap        Disable stack gap
wxneeded        Requires W+X mappings

I would like to have some canonical way to set these flags in ports, but I am not sure what the right interface is.  It could be done in an ad-hoc manner in individual post-install targets, but I believe we'll want common infrastructure as different sets of flags will be available on different releases. Within an individual port we may have sets of files with different flags to be set.
Comment 1 Ed Maste freebsd_committer 2021-01-13 03:33:15 UTC
Although maybe we can allow elfctl to silently ignore unknown flags and use the same invocation across all releases - see https://reviews.freebsd.org/D28130.
Comment 2 Ed Maste freebsd_committer 2021-01-13 03:34:08 UTC
My world was out of date, currently known flags are:

Known features are:
aslr            Disable ASLR
protmax         Disable implicit PROT_MAX
stackgap        Disable stack gap
wxneeded        Requires W+X mappings
la48            amd64: Limit user VA to 48bit
aslrstkgap      Disable ASLR stack gap
Comment 3 commit-hook freebsd_committer 2021-01-13 05:12:37 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=f6d95a01103a49a94c876d5a51bb4be25c06d964

commit f6d95a01103a49a94c876d5a51bb4be25c06d964
Author:     Ed Maste <emaste@FreeBSD.org>
AuthorDate: 2021-01-13 03:24:52 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2021-01-13 05:10:13 +0000

    elftcl: add -i flag to ignore unknown flags

    This may allow an identical elfctl invocation to be used on multiple
    FreeBSD versions, with features not implemented on older releases being
    silently ignored.

    PR:             252629 (related)
    Reviewed by:    kib
    MFC after:      1 week
    Sponsored by:   The FreeBSD Foundation
    Differential Revision: https://reviews.freebsd.org/D28130

 usr.bin/elfctl/elfctl.1 |  6 +++++-
 usr.bin/elfctl/elfctl.c | 13 ++++++++++---
 2 files changed, 15 insertions(+), 4 deletions(-)
Comment 4 Emmanuel Vadot freebsd_committer 2021-01-13 08:54:36 UTC
Adding support in pkg(8) might be the way.
Then adding some keyworks support in the plist like 
@elfctl aslr /path/to/bin
@elfctl wxneeded /path/to/other/bin
Comment 5 Emmanuel Vadot freebsd_committer 2021-01-13 08:59:53 UTC
While here, I think that it's a bit strange to do
elfctl +aslr /path/to/bin to disable it
Comment 6 Emmanuel Vadot freebsd_committer 2021-01-13 09:05:10 UTC
After looking at elfctl it seems that if we can set multiple features at a time we cannot do something like
elfctl -e +aslr,-wxneeded /path/to/bin
I think that would be good to have.
For the keywords something like :
@elfctl set <featurelist> /path/to/bin
@elfctl clear <featurelist> /path/to/bin

Would be nicer to what I've said earlier.
Comment 7 Emmanuel Vadot freebsd_committer 2021-01-13 09:14:13 UTC
So what's needed is :

 - in pkg(8) add a lua callable function pkg.elfctl that takes a list of features and a path
 - In the portstree add a Keywords/elfctl.ucl that calls this lua function
 - Then in the plist of the ports just @elfctl <featurelist> /path/to/bin

I would prefer if the featurelist is more intuitive that what's in elfctl
"aslr" would enable it while "noaslr" would disable it etc ...
Comment 8 Baptiste Daroussin freebsd_committer 2021-01-13 09:17:35 UTC
I agree with manu this is the simplest way and the cleanest way
Comment 9 Ed Maste freebsd_committer 2021-01-13 15:25:19 UTC
(In reply to Emmanuel Vadot from comment #6)
> elfctl -e +aslr,-wxneeded /path/to/bin

For tagging at build time I expect we'd use = and just set the bits we want. In general I think there would not be a need to turn off bits in ports builds.
Comment 10 dewayne 2021-01-13 21:52:24 UTC
(In reply to Emmanuel Vadot from comment #7)
Please bear in mind that lua may not be functional in some environments.  In src.conf, I have
WITHOUT_LOADER_LUA=YES
which I suspect is the cause of pkg being unable to process lua scripts (on FreeBSD12.2 r368820M).

Is lua a prerequisite for pkg use on FreeBSD?
Comment 11 Kyle Evans freebsd_committer 2021-01-13 21:53:56 UTC
(In reply to dewayne from comment #10)

pkg has its own lua, this knob is irrelevant.
Comment 12 commit-hook freebsd_committer 2021-01-19 02:50:30 UTC
A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=369a4023e671c35390574d42b1b409b55946faf8

commit 369a4023e671c35390574d42b1b409b55946faf8
Author:     Ed Maste <emaste@FreeBSD.org>
AuthorDate: 2021-01-13 03:24:52 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2021-01-19 02:49:55 +0000

    elftcl: add -i flag to ignore unknown flags

    This may allow an identical elfctl invocation to be used on multiple
    FreeBSD versions, with features not implemented on older releases being
    silently ignored.

    PR:             252629 (related)
    Reviewed by:    kib
    Sponsored by:   The FreeBSD Foundation
    Differential Revision: https://reviews.freebsd.org/D28130

    (cherry picked from commit f6d95a01103a49a94c876d5a51bb4be25c06d964)

 usr.bin/elfctl/elfctl.1 |  6 +++++-
 usr.bin/elfctl/elfctl.c | 13 ++++++++++---
 2 files changed, 15 insertions(+), 4 deletions(-)
Comment 13 Ed Maste freebsd_committer 2021-01-22 17:31:19 UTC
Another proposed change to allow flags to be set by value: https://reviews.freebsd.org/D28284

then we could do e.g.

NT_FREEBSD_FCTL_ASLR_DISABLE=0x1
NT_FREEBSD_FCTL_PROTMAX_DISABLE=0x2
NT_FREEBSD_FCTL_STKGAP_DISABLE=0x4

elfctl -e =${NT_FREEBSD_FCTL_ASLR_DISABLE},${NT_FREEBSD_FCTL_STKGAP_DISABLE} /path/to/file
Comment 14 commit-hook freebsd_committer 2021-01-22 19:41:32 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=86f33b5fcf6087bf4439881011b920ff99e6e300

commit 86f33b5fcf6087bf4439881011b920ff99e6e300
Author:     Ed Maste <emaste@FreeBSD.org>
AuthorDate: 2021-01-22 17:22:35 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2021-01-22 19:38:52 +0000

    elfctl: allow features to be specified by value

    This will allow elfctl on older releases to set bits that are not yet
    known there, so that the binary will have the correct settings applied
    if run on a later FreeBSD version.

    PR:             252629 (related)
    Suggested by:   kib
    Reviewed by:    gbe (manpage, earlier), kib
    MFC after:      3 days
    Sponsored by:   The FreeBSD Foundation
    Differential Revision: https://reviews.freebsd.org/D28284

 usr.bin/elfctl/elfctl.1 | 15 +++++++++++++--
 usr.bin/elfctl/elfctl.c | 25 ++++++++++++++++++++++---
 2 files changed, 35 insertions(+), 5 deletions(-)