• Re: [gentoo-dev] [PATCH 2/2] fcaps.eclass: make binaries readable by de

    From Eli Schwartz@21:1/5 to Mike Gilbert on Sun Nov 10 23:40:01 2024
    This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --------------Rl0dljW0XIBlcdY8beCiJraA
    Content-Type: text/plain; charset=UTF-8
    Content-Transfer-Encoding: quoted-printable

    On 11/10/24 4:54 PM, Mike Gilbert wrote:
    Removing the read bit from suid binaries has questionable security
    benefit, and may cause problems for some software.

    Users may override FCAPS_CAPS_MODE and FCAPS_NOCAPS_MODE should they
    desire the old behavior.

    Bug: https://bugs.gentoo.org/938164
    Signed-off-by: Mike Gilbert <floppym@gentoo.org>
    ---
    eclass/fcaps.eclass | 4 ++--
    1 file changed, 2 insertions(+), 2 deletions(-)

    diff --git a/eclass/fcaps.eclass b/eclass/fcaps.eclass
    index bf05776ba760..da4a52099396 100644
    --- a/eclass/fcaps.eclass
    +++ b/eclass/fcaps.eclass
    @@ -70,13 +70,13 @@ esac
    # @USER_VARIABLE
    # @DESCRIPTION:
    # Mode to use when capabilities are supported.
    -: ${FCAPS_CAPS_MODE:=0711}
    +: ${FCAPS_CAPS_MODE:=0755}


    Considering the context of the linked bug, and the change offered here,
    I don't really understand the proposed solution.

    This is a very flexible variable. Way too flexible. There is no valid
    use case for setting it to anything other than removing read
    permissions, or preserving read permissions -- so why is it acceptable
    to offer users the opportunity to define

    FCAPS_CAPS_MODE="4123"
    FCAPS_NOCAPS_MODE="0644"

    Which is an error condition?

    If we want a user variable at all here, let it be

    : ${FCAPS_DENY_WORLD_READ:=no}

    But I'm not convinced any optionality is necessary at all here. If the
    expected behavior is to have read, and users are free to toggle sfperms
    at the portage level, why is it necessary to make this additionally configurable as an eclass variable?

    Either way, there is also a stale comment in the function body:

    # If everything goes well, we don't want the file to be readable
    # by people.



    # @ECLASS_VARIABLE: FCAPS_NOCAPS_MODE
    # @USER_VARIABLE
    # @DESCRIPTION:
    # Mode to use when capabilities are not supported.
    -: ${FCAPS_NOCAPS_MODE:=4711}
    +: ${FCAPS_NOCAPS_MODE:=4755}

    # @FUNCTION: fcaps
    # @USAGE: [-o <owner>] [-g <group>] [-m <mode>] [-M <caps mode>] <capabilities> <file[s]>


    --
    Eli Schwartz

    --------------Rl0dljW0XIBlcdY8beCiJraA--

    -----BEGIN PGP SIGNATURE-----

    wnsEABYIACMWIQTnFNnmK0TPZHnXm3qEp9ErcA0vVwUCZzE13gUDAAAAAAAKCRCEp9ErcA0vVwHI AQDXe0zaZKBX+PYbTUh/Fqhzh1E/L0BnZHbU7C+bn9GT3AEAqI4Ht/I9iV58e4cvFBrXjVEn70mn vOaUo8tG290gWAU=
    =6ulV
    -----END PGP SIGNATURE-----

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Mike Gilbert@21:1/5 to eschwartz@gentoo.org on Mon Nov 11 01:20:01 2024
    On Sun, Nov 10, 2024 at 5:38 PM Eli Schwartz <eschwartz@gentoo.org> wrote:

    On 11/10/24 4:54 PM, Mike Gilbert wrote:
    Removing the read bit from suid binaries has questionable security
    benefit, and may cause problems for some software.

    Users may override FCAPS_CAPS_MODE and FCAPS_NOCAPS_MODE should they
    desire the old behavior.

    Bug: https://bugs.gentoo.org/938164
    Signed-off-by: Mike Gilbert <floppym@gentoo.org>
    ---
    eclass/fcaps.eclass | 4 ++--
    1 file changed, 2 insertions(+), 2 deletions(-)

    diff --git a/eclass/fcaps.eclass b/eclass/fcaps.eclass
    index bf05776ba760..da4a52099396 100644
    --- a/eclass/fcaps.eclass
    +++ b/eclass/fcaps.eclass
    @@ -70,13 +70,13 @@ esac
    # @USER_VARIABLE
    # @DESCRIPTION:
    # Mode to use when capabilities are supported.
    -: ${FCAPS_CAPS_MODE:=0711}
    +: ${FCAPS_CAPS_MODE:=0755}


    Considering the context of the linked bug, and the change offered here,
    I don't really understand the proposed solution.

    This is a very flexible variable. Way too flexible. There is no valid
    use case for setting it to anything other than removing read
    permissions, or preserving read permissions -- so why is it acceptable
    to offer users the opportunity to define

    FCAPS_CAPS_MODE="4123"
    FCAPS_NOCAPS_MODE="0644"

    Which is an error condition?

    If we want a user variable at all here, let it be

    : ${FCAPS_DENY_WORLD_READ:=no}

    Good point, I can make this adjustment.

    But I'm not convinced any optionality is necessary at all here. If the expected behavior is to have read, and users are free to toggle sfperms
    at the portage level, why is it necessary to make this additionally configurable as an eclass variable?

    FEATURES="sfperms" modifies the files during the merge phase.

    fcaps.eclass modifies the files in pkg_postinst, after the merge phase
    is complete.
    It does this because there is no guarantee that ${D} will reside on a filesystem that supports file-based capabilities.

    I don't think we want to have fcaps.eclass start looking at FEATURES
    since this is undefined in PMS.

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)