fix secret mounts for env vars when using chroot isolation

[jonahbull]

What type of PR is this?

/kind bug

What this PR does / why we need it:

As discussed in #5185, attempting to build an image that uses environmental variables as build secrets fails with the message:

error running subprocess: remounting "/var/tmp/buildah3153589124/mnt/rootfs/run/secrets/MYSECRET" in mount namespace with flags 0x1 instead of 0x0: operation not permitted

In my particular use case, this is using the VFS storage driver and chroot isolation. I haven't been able to reproduce using other isolation types, not sure if other folks have.

As far as I can tell, the cause is that before #5083, when running with chroot isolation ro mounts like secrets from env vars would explicitly have the unix.MS_NOEXEC, unix.MS_NOSUID, unix.MS_NODEV flags set when they were remounted: https://github.com/containers/buildah/pull/5083/files#diff-c287fcc22d32f0775a98f65ec2db880f887c6c392f8e5ed6d7320a24560c1949L468.

Now when running with chroot isolation ro mounts like secrets from env vars are not getting those same flags set and so the remount operation fails. Specifically it looks like in this case we are missing the unix.MS_NOSUID and unix.MS_NODEV flags.

I think the flags aren't being set because when we calculate effectiveUnimportantFlags here, we end up with 0b100000. Forgive the possibly overly-verbose notes here, but I'm not well-versed in these kinds of bitwise operations so it was helpful for me to add as much detail as possible when debugging.

possibleImportantFlags := uintptr(unix.ST_NODEV | unix.ST_NOEXEC | unix.ST_NOSUID | unix.ST_RDONLY)
// fs.Flags: 0b100110 possibleImportantFlags: 0b1111 ^possibleImportantFlags: 0b10000
effectiveUnimportantFlags := uintptr(fs.Flags) & ^possibleImportantFlags
// effectiveUnimportantFlags: 0b100000

When we pass effectiveUnimportantFlags to mountFlagsForFSFlags, none of the possible fsFlags match 0b100000, so it returns 0b0.

This change adds special handling for read-only mounts when we need to do a remount to try to get the desired flags to stick. If we've requested a read-only mount (unix.ST_RDONLY is set in requestFlags), then we add any possibleImportantFlags that are set in fs.Flags to remountFlags so the remount operation doesn't fail because they are missing

How to verify it

I've added a test to bud.bats that reproduces the issue when run locally with a version of buildah that doesn't have the fix applied.

I could definitely have missed something, but this change appears to fix the issue in #5185 without breaking other use cases, at least from my limited local testing (running the bud.bats and chroot.bats suites).

Which issue(s) this PR fixes:

Fixes #5185

Special notes for your reviewer:

Does this PR introduce a user-facing change?

None

[jonahbull]

Looks like my initial naive attempt doesn't quite work. I'll poke at getting the unit tests passing (they are failing on TestMounts/dev,exec,suid,rw) but if anyone who happens by has a different solution in mind I'm all ears.

[rhatdan]

Thanks @jonahbull
LGTM
/approve
@flouthoc @nalind @giuseppe @mheon PTAL

[giuseppe]

/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: giuseppe, jonahbull, rhatdan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [giuseppe,rhatdan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment