Ignition ignores special file mode bits (sticky, setuid, setgid)

[cheesesashimi]

Bug

Operating System Version

[core@localhost ~]$ cat /etc/os-release
NAME="Fedora Linux"
VERSION="35.20211215.3.0 (CoreOS)"
ID=fedora
VERSION_ID=35
VERSION_CODENAME=""
PLATFORM_ID="platform:f35"
PRETTY_NAME="Fedora CoreOS 35.20211215.3.0"
ANSI_COLOR="0;38;2;60;110;180"
LOGO=fedora-logo-icon
CPE_NAME="cpe:/o:fedoraproject:fedora:35"
HOME_URL="https://getfedora.org/coreos/"
DOCUMENTATION_URL="https://docs.fedoraproject.org/en-US/fedora-coreos/"
SUPPORT_URL="https://github.com/coreos/fedora-coreos-tracker/"
BUG_REPORT_URL="https://github.com/coreos/fedora-coreos-tracker/"
REDHAT_BUGZILLA_PRODUCT="Fedora"
REDHAT_BUGZILLA_PRODUCT_VERSION=35
REDHAT_SUPPORT_PRODUCT="Fedora"
REDHAT_SUPPORT_PRODUCT_VERSION=35
PRIVACY_POLICY_URL="https://fedoraproject.org/wiki/Legal:PrivacyPolicy"
VARIANT="CoreOS"
VARIANT_ID=coreos
OSTREE_VERSION='35.20211215.3.0'
DEFAULT_HOSTNAME=localhost

Ignition Version

3.3.0 - though I think this is applicable to all Ignition versions.

Environment

I'm creating a new FCOS VM using QEMU on a MacOS host, following these instructions: https://docs.fedoraproject.org/en-US/fedora-coreos/provisioning-qemu/

Expected Behavior

When a file or directory is specified in the Ignition config with any of the special file mode bits (sticky, setuid, setgid), the file or directory should be created with those bits set. Alternatively, if setting those file mode bits is unsupported, I would have expected the Ignition config to fail validation with an error message explaining that. Additionally, I would have expected the Ignition (and Butane) docs to make it clear that Ignition does not support those file modes.

Actual Behavior

If one creates a file or directory with a special file mode bit set (e.g., 01755 / decimal 1005), one would expect the created file or directory to have that bit set. However, only the file permission bits 0755 are set. The special file mode bits are ignored.

Reproduction Steps

1. Create a new Ignition config:

{
  "ignition": {
    "version": "3.3.0"
  },
  "storage": {
    "directories": [
      {
        "path": "/etc/a-new-dir",
        "mode": 1005
      }
    ],
    "files": [
      {
        "path": "/etc/a-config-file",
        "contents": {
          "source": "data:,hello%20world"
        },
        "mode": 1005
      }
    ]
  }
}

2. Create a new FCOS instance with the above config.
3. Connect to the new instance and stat the newly created file and directory:

[core@localhost ~]$ stat /etc/a-new-dir
  File: /etc/a-new-dir
  Size: 6               Blocks: 0          IO Block: 4096   directory
Device: fc04h/64516d    Inode: 18874496    Links: 2
Access: (0755/drwxr-xr-x)  Uid: (    0/    root)   Gid: (    0/    root)
Context: system_u:object_r:etc_t:s0
Access: 2022-01-11 15:04:06.481655180 +0000
Modify: 2022-01-11 15:04:03.460655180 +0000
Change: 2022-01-11 15:04:06.481655180 +0000
 Birth: 2022-01-11 15:04:03.460655180 +0000

[core@localhost ~]$ stat /etc/a-config-file
  File: /etc/a-config-file
  Size: 11              Blocks: 8          IO Block: 4096   regular file
Device: fc04h/64516d    Inode: 878894      Links: 1
Access: (0755/-rwxr-xr-x)  Uid: (    0/    root)   Gid: (    0/    root)
Context: system_u:object_r:etc_t:s0
Access: 2022-01-11 15:04:03.468655180 +0000
Modify: 2022-01-11 15:04:03.472655180 +0000
Change: 2022-01-11 15:04:06.483655180 +0000
 Birth: 2022-01-11 15:04:03.468655180 +0000

4. Compare the access mode to what was specified in the Ignition config.

Other Information

At first glance, it appears that Ignition supports file modes up to 07777, as evidenced by the file mode validator.

The root cause of this is because the file mode types in the Golang stdlib have their own internal representation. Additionally, when os.Chmod() runs, it actively discards anything past the first 9 bits (owner, group, other / read, write, execute) and uses its own internal representation to set the special file mode bits. Consequently, when one specifies mode 01755 in their Ignition config, the file is created with mode 0755 because the special file mode bits were discarded. To set 01755, one must convert that representation to Golangs internal representation (e.g., 0755 | os.ModeSticky or 04000755) before calling os.Chmod().

I'm happy to provide any additional information, if needed.

[bgilbert]

Thanks for the report and analysis! This is clearly a bug, and the technical fix should be straightforward. Two concerns though:

• I'm always a little hesitant to change behavior retroactively in existing specs, because someone might write e.g. a 3.0.0 config with setuid files and expect it to apply correctly on all Ignition versions supporting that spec. In this case, it'll fail silently.
• If there are configs that asked for setuid/setgid binaries, didn't get them, and didn't notice, suddenly allowing those binaries to escalate privilege might have security implications.

I see three options:

1. Just fix it.
2. Fix it in the experimental spec, warn and ignore in stable specs, and add it to the spec upgrade notes.
3. Fix it in the experimental spec, fail validation in stable specs, and add it to the spec upgrade notes. This is more likely to catch configs that will be surprised at getting setuid bits, but it also breaks existing configs, which we don't do.

cc @jlebon and maybe @dustymabe for thoughts.

[dustymabe]

I think I'm in the 1. Just fix it. camp. Maybe I need to understand a little bit more the implications of that option before saying for sure.

[travier]

I think this might need a wider discussion. I think we should do 2. Users that would have really needed it before would have realized that this was broken so we don't want to surprisingly add it to existing configuration versions.

[bgilbert]

@jlebon and I discussed this OOB. This doesn't seem worth breaking existing configs (case 3), which has a very high bar. On the other hand, case 1 might be surprising for multiple reasons, including that configs that work on current FCOS would silently fail to set a security-sensitive mode bit when applied to e.g. older RHCOS.

@dustymabe and I also discussed this a bit on IRC, leading to:

<dustymabe> I think I'm still in the `1.` camp 
<dustymabe> but it's a weakly held opinion

At this point I think we should proceed with option 2. We can raise this at a meeting, though, if desired.

[cgwalters]

I'm fine with 2.

I think as far as warnings go though, we actually want to be doing that in e.g. butane right? Or would that happen implicitly because butane uses ignition's Go bits for this?

[bgilbert]

Yeah, Butane runs Ignition validation after translating. Since this is a distro-independent issue (and we want ignition-validate to catch it too), Ignition validation is the right place for it.