Singularity: Restrictive system umask prevents file binds to non-existing mountpoints

[zao]

Version 2.4.6 from own deb built from source tree.

The creation of mountpoints in userbind.c has diverging behaviour between file mountpoints and directory mountpoints.
Directories explicitly suppress the ambient umask while creating the directories, while file creation uses a naked fopen which honors the ambient umask.

With a restrictive umask like 077, the file mountpoint has so few rights that the newly rewritten singularity_mount fails early.
It first tries opening the file as a limited user, and only if it succeeds with that, it goes on to escalate and actually mount.

[dtrudg]

Closing as a umask fix was included in 2.5.0. Please feel to reopen if you experience further issues.

[zao]

This seems to propagate the forced umask into the contained process.
I've got to investigate this at work late next week to figure out if this would break our job scripts as singularity is invoked as part of a larger system.

[zao]

I do not have rights to reopen this issue, but I feel that forcing an umask onto the contained process is surprising and may have effects on end users.
This particularly is surprising if you exec to dip into a container to run individual commands.

[GodloveD]

Hi @zao. As I'm sure you know, there are many programs that do not respect a user-defined umasks. Can you elaborate on what you find to be surprising?

[DrDaveD]

What if somebody had a more restrictive umask, for example 077 to cut off all access to user & group? Singularity now opens it up more to 022. I too do not like this behavior. I think the most it should do is OR with 002.

[DrDaveD]

Sorry, now I have read the history and followed links and see that it was the intention to open up permissions on restrictive umasks. If it is needed for certain operations I can see setting it then, but I agree with @zao that it shouldn't carry through to user code running inside the container. I think the original umask should be restored at that point.

[GodloveD]

OK. I think I see what you mean @zao and @DrDaveD. This is basically the issue you are concerned about right?

$ umask 0077

$ singularity exec docker://busybox touch foo
Docker image path: index.docker.io/library/busybox:latest
Cache folder set to /home/ubuntu/.singularity/docker
Creating container runtime...
Exploding layer: sha256:f70adabe43c0cccffbae8785406d490e26855b8748fc982d14bc2b20c778b929.tar.gz
Exploding layer: sha256:29ade4f96a9909940ce0251b8182993c53f22a7e006a8c7fe868f1e0032d52b5.tar.gz

$ ll foo
-rw-r--r-- 1 ubuntu 0 May  2 17:11 foo

Yes I agree that this is less than ideal. It's probably possible to be a bit more nuanced with the umask here. I'll have a look. In the meantime, for a workaround you can set your umask in the %environemnt section of the deffile or pass it as one of the commands to exec I guess.

[zao]

Aye, that's the issue.
In particular, this is a silent change vs. the previous behaviour of keeping the umask intact, so previously deployed workflows may suddenly behave differently, both for tighter and looser masks.

Thanks for considering it.

[gnguy]

+1 in favor of considering an update to the umask handling process. Our preference would be to carry-over the user umask settings into the contained process.

We've attempted to maintain that behavior, per your suggestion, by chaining together a umask setting and then executing commands via singularity exec, but the only way we could get it to work was to construct a string with the multiple commands and then eval the string using singularity exec. Unfortunately, this errors out for a subset of use-cases (see below for an example), so our current solution can't be deployed internally. Do you have any ideas on how to better-structure our calls to singularity exec to allow this to work?

This shell script generally works, but fails if there is a parenthesis in one of the run_args:

#$ -S /bin/sh
export SINGULARITYENV_ORIG_UMASK=$(umask)
run_file="$1"; shift
run_args=$(printf %q "$*" | sed "s/[\]//g")

# /bin/sh -c allows Singularity to run multiple strung-together commands arbitrarily, as && unquoted will run the first command in Singularity and the second command outside of it
# The single quotes are because we want $ORIG_UMASK to be evaluated within the Singularity session
# The double quotes are because we want $run_file and $* to be evaluated from the current context and then passed into the singularity exec command
# The $* is used rather than $@ because for some mysterious reason, $@ only passes on the first element from an argument list when used this way
singularity exec rstudio_3501.img /bin/sh -c 'umask $ORIG_UMASK && /usr/local/bin/R <'"$run_file  --no-save --args $run_args"

sh singularity_3501.sh my_script.R arg1 arg2 'c(1990)' # Note: fails with a syntax error near unexpected token `(' if the single quotes are removed
/bin/sh: 1: Syntax error: "(" unexpected

[GodloveD]

Try this in the %environment section of the def file.

%environment
    if [ -n "${ORIG_UMASK:-}" ]; then
        umask $ORIG_UMASK
    fi

Then continue to export SINGULARITYENV_ORIG_UMASK=$(umask) before container execution.

Does that work?

[gnguy]

Thanks @GodloveD, I can give that a try tomorrow -- do you know of any way to do it without the recipe/def file? At our org, we've been building Docker images and then directly calling singularity build without using a recipe at all, so this would be a significant process change for us to now include recipes for each image conversion.

[GodloveD]

It's not recommended and not reproducible, but you could build your containers as sandboxes, cat the changes to /.singularity.d/env/90-environment.sh in the container (using the --writable flag), and then if you want (optionally) build a squashfs image from the sandbox.

[gnguy]

Thanks for the suggestions. I was able to test the %environment setting via the def file, and have confirmed that that does do the job to set umask. Unfortunately, neither suggestion works in our build pipeline, which runs through Linux machines on which we don't have root. I can explore what it would take for us refactor our build pipeline in some way to enable usage of the def file, hopefully will be able to figure out something relatively seamless.

[thiell]

Any news on this one? We have applications like Stata that relies on umask and the workaround works but it's not convenient nor very maintainable. That sounds like a basic request but this ticket hasn't evolved for a year. Thanks much.

[thiell]

Just to clarify, we need to carry-over the user umask settings into the contained process, as mentioned in this comment https://github.com/sylabs/singularity/issues/1465#issuecomment-406403997
We’re using the following workaround which requires a rebuild of containers: https://github.com/sylabs/singularity/issues/1465#issuecomment-406437118

[dtrudg]

Okay - so this is a little hard to track on this issue, since the original post on this issue was something that actually required the opposite behavior - that we ignore the umask for certain things (dirs/files that will be mount points in the container). The discussion then pivoted to a different direction after that was implemented.

I'm not sure how easy it is in the 3.x flow right now to save the external umask, force to 0022 for any build activity (which is likely required to ensure that containers built by someone are usable to others), and ensure the original umask on entry into the container - but that's what I believe we need to satisfy the request.

Could you open a new issue, detailing which version of Singularity you are using, and give an example of exactly what you want to see? It will certainly get more attention with a new specific example reported on a current Singularity version, rather than mixed up in the 2.x issue here.

Many thank!s!

[dtrudg]

Progress on the umask propagation issue has been made in a PR and discussion is now encouraged on #5214. Thanks!