2720. permissions function incorrectly specified for symlinks

Section: 31.12.13.27 [fs.op.permissions] Status: C++17 Submitter: Eric Fiselier Opened: 2016-05-28 Last modified: 2017-07-30

Priority: 2

View all other issues in [fs.op.permissions].

View all issues with C++17 status.

Discussion:

Currently when adding or removing permissions the permissions(p, prms, [...]) function always determines the current permissions for a file p using status(p).permissions(). This means that it resolves symlinks even when perms::resolve_symlinks was not specified.

I believe this is the incorrect behavior. Instead symlink_status(p).permissions() should be used unless perms::resolve_symlinks is specified.

Previous resolution [SUPERSEDED]:

This wording is relative to N4582.

  1. In 31.12.13.27 [fs.op.permissions] change Table 150 — "Effects of permission bits" as indicated:

    Table 150 — Effects of permission bits
    Bits present in prms Effective bits applied
    Neither add_perms nor remove_perms prms & perms::mask
    add_perms and resolve_symlinks status(p).permissions() | (prms & perms::mask)
    remove_perms and resolve_symlinks status(p).permissions() & (prms & perms::mask)
    add_perms and not resolve_symlinks symlink_status(p).permissions() | (prms & perms::mask)
    remove_perms and not resolve_symlinks symlink_status(p).permissions() & ~(prms & perms::mask)

[2016-06, Oulu — Jonathan comments and provides alternative wording]

We agree there is an issue here, but I don't like the proposed resolution. If Eric's P/R is accepted then it changes the default behaviour (when users do not set the perms::resolve_symlinks bit) to modify the permissions of the symlink itself.

I claim that modifying the permissions of a symlink (rather than what it points to) is not a sensible default. It is not supported by the POSIX chmod system call. To change permissions of a symlink with POSIX you must use the newer fchmodat function and the AT_SYMLINK_NOFOLLOW flag, see here.

Changing permissions of a symlink is not possible using the GNU chmod util, see here:

"chmod never changes the permissions of symbolic links, since the chmod system call cannot change their permissions. This is not a problem since the permissions of symbolic links are never used."

BSD chmod does provide a switch to change a symlink's permissions, but it's not the default.

I suggest that we should replace the filesystem::perms::resolve_symlinks enumerator with filesystem::perms::symlink_nofollow (paint the bikeshed!), so that the default is sensible, and the uncommon, useless alternative of changing the symlink itself requires setting a bit in the flags explicitly.

resolve_symlinks is unused in the spec today, the only mention is its definition in Table 147.

[2016-06, Oulu]

There exists a slightly related issue, 2728.

[2016-06 Oulu]

Tuesday: Move to Ready. JW and Eric to implement and report back if problems found.

Friday: status to Immediate

Proposed resolution:

This wording is relative to N4594.

  1. Change Table 147 — "Enum class perms" as indicated:

    Table 147 — Enum class perms
    Name Value
    (octal)    		 POSIX
    macro    		 Definition or notes
    symlink_nofollowresolve_symlinks 0x40000 permissions() shall change the permissions of symbolic linksresolve symlinks

  2. Edit 31.12.13.27 [fs.op.permissions]:

    void permissions(const path& p, perms prms);
void permissions(const path& p, perms prms, error_code& ec) noexcept;

    -1- Requires: !((prms & perms::add_perms) != perms::none && (prms & perms::remove_perms) != perms::none).

    -2- Effects: Applies the effective permissions bits from prms to the file p resolves to, or if that file is a symbolic link and symlink_nofollow is not set in prms, the file that it points to, as if by POSIX fchmodat(). The effective permission bits are determined as specified in Table 150, where s is the result of (prms & perms::symlink_nofollow) != perms::none ? symlink_status(p) : status(p).

  3. Change Table 150 — "Effects of permission bits" as indicated:

    [Drafting note: Very recently the project editor had already fixed a typo in Table 150 editorially, the applied change effectively was: 

    status(p).permissions() & ~(prms & perms::mask)

    ]

    Table 150 — Effects of permission bits
    Bits present in prms Effective bits applied
    Neither add_perms nor remove_perms prms & perms::mask
    add_perms status(p).permissions() | (prms & perms::mask)
    remove_perms status(p).permissions() & (prms & perms::mask)