umask 0077 breaks build-using-dockerfile with USER statement

[praiskup]

BUG REPORT INFORMATION

$ cat Dockerfile
FROM registry.fedoraproject.org/fedora:29
RUN useradd jdoe
USER jdoe
RUN id
$ buildah bud .
STEP 1: FROM registry.fedoraproject.org/fedora:29
STEP 2: RUN useradd jdoe
STEP 3: USER jdoe
STEP 4: RUN id
container_linux.go:344: starting container process caused "chdir to cwd (\"/\") set in config.json failed: permission denied"
error running container: error creating container for [/bin/sh -c id]: : exit status 1
error building at step {Env:[DISTTAG=f29container FGC=f29  PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin] Command:run Args:[id] Flags:[] Attrs:map[] Message:RUN id Original:RUN id}: error while running runtime: exit status 1
ERRO[0001] exit status 1
$ rpm -q buildah
buildah-1.6-2.git5f95bd9.fc29.x86_64

Description

Ideally, buildah should make sure that the created / directory is created a standard way (say chmod 666 per filesystem.rpm content), or if it at least warned properly.

Describe the results you received:

buildah bud ends with strange permission error with umask 0077 after USER command/statement.

Describe the results you expected:

buildah bud either works under those contitions.

Output of rpm -q buildah or apt list buildah:

buildah-1.6-2.git5f95bd9.fc29.x86_64

Output of buildah version:

Version:         1.6
Go Version:      go1.11.4
Image Spec:      1.0.0
Runtime Spec:    1.0.0
CNI Spec:        0.4.0
libcni Version:  
Git Commit:      
Built:           Thu Jan  1 01:00:00 1970
OS/Arch:         linux/amd64

Output of cat /etc/*release:

NAME=Fedora
VERSION="29 (Twenty Nine)"
ID=fedora
VERSION_ID=29
VERSION_CODENAME=""
PLATFORM_ID="platform:f29"
PRETTY_NAME="Fedora 29 (Twenty Nine)"
ANSI_COLOR="0;34"
LOGO=fedora-logo-icon
CPE_NAME="cpe:/o:fedoraproject:fedora:29"
HOME_URL="https://fedoraproject.org/"
DOCUMENTATION_URL="https://docs.fedoraproject.org/en-US/fedora/f29/system-administrators-guide/"
SUPPORT_URL="https://fedoraproject.org/wiki/Communicating_and_getting_help"
BUG_REPORT_URL="https://bugzilla.redhat.com/"
REDHAT_BUGZILLA_PRODUCT="Fedora"
REDHAT_BUGZILLA_PRODUCT_VERSION=29
REDHAT_SUPPORT_PRODUCT="Fedora"
REDHAT_SUPPORT_PRODUCT_VERSION=29
PRIVACY_POLICY_URL="https://fedoraproject.org/wiki/Legal:PrivacyPolicy"
Fedora release 29 (Twenty Nine)
Fedora release 29 (Twenty Nine)

Output of cat /etc/containers/storage.conf:

[storage]
driver = "overlay"
runroot = "/var/run/containers/storage"
graphroot = "/var/lib/containers/storage"
[storage.options]
additionalimagestores = [
]
size = ""
override_kernel_check = "true"
mountopt = "nodev"
[storage.options.thinpool]
ostree_repo = ""
skip_mount_home = "false"

[TomSweeneyRedHat]

@giuseppe any thoughts?

[giuseppe]

I recently fixed an issue in fuse-overlayfs not honoring umask. It is released as part of fuse-overlayfs 0.3:
containers/fuse-overlayfs#40

What version of fuse-overlayfs are you using?

[giuseppe]

I see the problem now.

The same thing happens when running as root though. Should buildah stop honoring umask and override it? I think that if umask is set it should be honored, perhaps we can add a warning.

@rhatdan @nalind what do you think?

[praiskup]

@giuseppe wrote:

I think that if umask is set it should be honored, perhaps we can add a warning.

I'd agree if that was manually created directory, but in this case -> I just inherited from
fedora:29 image, I did RUN useradd foo -> USER foo -> and then any RUN
command issued later (as foo user) fails because foo is unable to read / directory.
It's unexpected, warning in this case doesn't seem to be convenient enough.

[rhatdan]

I think we should make sure that the / of the image is searchable. 0755. If that is what the issue is?

[giuseppe]

I am still not sure we should override the umask if it is set. If that is what the user expects, we are going to silently override it.

I think we should just add a warning when umask is set

[rhatdan]

Is this just leaking through to containers/storage? IE when it writes the image / ends up with two tight of permissions. I think we could eaisly override the permissions on /.

[giuseppe]

we can simply solve it as:

$ git diff
diff --git a/cmd/buildah/main.go b/cmd/buildah/main.go
index 1927eace..cdee1a33 100644
--- a/cmd/buildah/main.go
+++ b/cmd/buildah/main.go
@@ -4,6 +4,7 @@ import (
        "fmt"
        "github.com/containers/buildah"
        "os"
+       "syscall"
 
        "github.com/containers/libpod/pkg/util"
        "github.com/containers/storage"
@@ -63,6 +64,8 @@ func init() {
                defaultStoreDriverOptions = optionSlice
        }
 
+       syscall.Umask(0)
+
        cobra.OnInitialize(initConfig)
        //rootCmd.TraverseChildren = true
        rootCmd.Version = fmt.Sprintf("%s (image-spec %s, runtime-spec %s)", buildah.Version, ispecs.Version, rspecs.Version)

but I am still not convinced we should silently do it.

Perhaps we should just fail the build as having a umask in place breaks the reproducibility of builds as different umask generates different images

[praiskup]

Perhaps we should just fail the build as having a umask in place breaks the reproducibility of builds as different umask generates different images

This would require a definition of "expected umask value". I have (intentionally) 0077 for years on my box, but users can have any value there (default on Fedora is 0022 afaik).

[giuseppe]

I think the only "expected umask value" should be 0. Anything else modifies how the image looks like, so we need to either error out, or force it to 0 and throw a warning

[praiskup]

OK, I agree disagree that it should be zero It should be probably 0002, but bothering errors everywhere (because the distributions defaults) don't make sense IMO. Enforcing umask 0 some default umask makes more sense at least to me.

[giuseppe]

Proposed fix: #1325

I'll probably need to do the same thing for Podman