Some files missing when image is built on singularity hub

[bbarker]

Link to Container Collection Log, Build, or Collection (in that order)

https://www.singularity-hub.org/containers/3586/log

Behavior when Building Locally

I do not see the error:

/bin/cp: cannot stat '../../Utils/persist-env.sh': No such file or directory

Also, the container runs as expected.

Error on Singularity Hub

I see the above error, and the container does not run as expected due to missing files.

What do you think is going on?

I'm not sure, though I imagine it may have something to do with which files are copied from a collection repo: perhaps files that are not in the same folder as one of the singularity recipes? At least, that is the case, here.

[bbarker]

Note that docker assumes all files must be under the CWD of the build root, but I assume singularity hub expects the CWD = the same directory where the recipe is. I had already adjusted my build script for my recipe to make this assumption, but singularity hub seems to be doing something different.

[vsoch]

Can you please show me the recipe that is building the container? The built itself is happening in a contained environment, so you wouldn't have access to a file (that looks like it's on a mac, given the Utils folder) from there.

[bbarker]

@vsoch sure, it is here: https://github.com/federatedcloud/NixTemplates/blob/master/Base/OpenMPI/Singularity.nix_alpine_openmpi_3cdef444dbb9648d67074bbaab8216851d0dd11d

That particular recipe is generated and built locally by doing ./build-singularity.sh from the same directory, though I don't think that is of interest to this issue.

Not sure what you mean about Utils, as I've not much experience with Mac - I just made a directory called Utils. Will that class with some OS-Xism?

[bbarker]

@vsoch if the contained environement is an issue, is there a way for singularity hub to check and allow any directories that remain under the repository root?

Otherwise, what is the correct pattern for where you have a file X shared by two different singularity recipes in two different directories? I haven't tried symbolic links, not sure if that would work (or work easily)

[vsoch]

Ah yes, this makes sense now. If it's an issue that the file isn't found, the path does look correct:

https://github.com/federatedcloud/NixTemplates/blob/master/Base/OpenMPI/Singularity.nix_alpine_openmpi_3cdef444dbb9648d67074bbaab8216851d0dd11d#L17

In which case Singularity isn't letting you extend beyond the CWD. If the file is found and you believe this to be an issue, then the correct place to report this is with the Singularity software. The builder just implements that. Let me know if you have any questions.

Haha, I think Windows or Mac (I'm not entirely sure) has a directory structure with user homes under /Users/$USER, that was the rationale for my comment!

[bbarker]

@vsoch hmm but as I said, it seemed to work for me locally with singularity, though locally I was using 2.5.1, it looks like the builder is using 2.5.0.

[vsoch]

@bbarker I would stick to "best practice" which I think means generally having files that are (below in directory) to the build recipes. You can imagine this poses security issues if it allows otherwise.

I can't comment on it working locally, that sounds like a bad thing to me!

[bbarker]

@vsoch I understand the possible security implications, but this may need to be balanced somewhat with development configurations - imagine having slightly different versions or related containers that share some files in common. The only solution I can think of that doesn't involve duplicating files or using symbolic links (which may not work) is we stick the recipes all in the same directory. But this doesn't seem very modular.

[bbarker]

@vsoch Another solution that works with docker (though still isn't great for usability, but better than nothing) - allow the build root to be the root of the repository.

[vsoch]

If you have issue with the Singularity software, you should please bring it up with the developers on that repository. Singularity Hub is not going to modify Singularity for this purpose.

[dtrudg]

Hi @vsoch @bbarker - looking at this, I believe it is due to things in the secure build custom branch of Singularity that Singularity Hub is using. It's not an issue with standard Singularity.

In the secbuild.sh that implements the build approach Singularity Hub uses, the whole repo is not bound in.... it is binding in only from the directory containing the definition file...

bind path = $BUILDDEF_DIR:$REPO_DIR

I'm afraid this is outside of the scope of development on Singularity, and would need to be modified for SingularityHub specifically if behavior needs to change.

[vsoch]

The build is maintained by @cclerget, let me know if there is a new secure build to update the builders with. Thanks @dctrud !

[cclerget]

Hi @vsoch , I updated the branch https://github.com/cclerget/singularity/tree/feature-squashbuild-secbuild-2.5.0 to introduce a new option for isolated build : --isolated-root, that should resolve this issue. With --isolated-root you just have to put the root path to the cloned repository, so isolated build can access to all files in the git repositories.

[vsoch]

Thanks @cclerget! Is there any reason not to update to the latest version of Singularity?

[cclerget]

I just backported a fix from 2.6 RC for arch bootstrap code, I updated my branch with changes from 2.5.2. As long as there is no changes in build code, you don't need to update the builder.

[vsoch]

okay, then don't update the builder? I'll wait for the next version of Singularity (3.0?) Will you be updating the secure build then?

[bbarker]

If I understand correctly, doesn't the builder have to be updated to have the --isolated-root option passed in?

[vsoch]

@cclerget I don't understand how you have changed the builder, but I don't need to update it? I think the ideal (default) behavior should not be to allow moving around from root with the --isolated flag (or other). This is how it works with Dockerfile - it's a security issue if the user can move around directories, so I was surprised that Singularity allows it (in some version it seems!)

[bbarker]

+1, --isolated-root set to the repo root seems like a good option, and would also allow basically similar functionality to Docker.

[vsoch]

Did you post a message and delete it? I can't find it!

For Docker, I believe that a recipe (Dockerfile) is allowed outside of the build context, but you can't ../../ to go up directories, see https://stackoverflow.com/questions/27068596/how-to-include-files-outside-of-dockers-build-context. This is logical to me - a builder should have confidence they won't have a builder poking around the rest of their system.

[bbarker]

@vsoch Sorry, I initially misunderstood your comment (I think) which was why I deleted my response. That is interesting though, I didn't realize you could specify the Dockerfile outside of the build context, and I can't think of any harm in it.

[vsoch]

Yes! And actually this is what the builder is doing - copying your recipe to the root directory so you have access to your full repo (bit not outside of it)

[dtrudg]

Just a note here that Singularity builds being able to access files outside of the location of the def file isn't a security issue per-se, and security is not the reason you can't do this with Docker.

Docker has to package up the build context and send it to the daemon which is doing the building (you might be building using a remote docker daemon)... so you can't access things outside of the context, as they wouldn't have been sent to the remote daemon. When you singularity build it's a process you are running locally. You are not asking a daemon to do a build on your behalf.

In the Singularity case, you have to sudo to root to build from a definition file. If you can do that there's no real additional security issue from Singularity being able to go outside of where the def file is during a build.

[cclerget]

@vsoch Sorry ! I will clarify a bit, my words weren't very clear and I didn't mention how to use --isolated-root. You can update now from branch https://github.com/cclerget/singularity/tree/feature-squashbuild-secbuild-2.5.0 to address the issue, this branch integrate last changes from 2.5.2. What I was saying before is you don't need to update the builder each time there is a release out except if there is blocking issues affecting build process (like this one).

So to use --isolated-root, it's simple, you will need to run build like that :

git clone https://github.com/user/repo.git /tmp/temp-XXX
singularity build --isolated --isolated-root /tmp/temp-XXX /tmp/image.simg /tmp/temp-XXX/path/to/file.def

Hope that helps.

[vsoch]

Thanks @cclerget ! I understand, thank you for the clarification. The branch isn't re-installed each time (the builder is a pre-build, done and done image) so if you make changes to the branch, I would need to reinstall the software. Does that make sense?

[vsoch]

And one more question for you - is there any condition for which it would hurt for someone using --isolated to also have --isolated-root ? If it isn't a security issue as @dctrud mentions, and it gives the user more flexibility for the build, I don't see why I wouldn't want to make this happen for all builds that are isolated by default (and instead have the spython client provide the user an argument to not use it).

[cclerget]

Yes it make sense, that's why I mentioned I may not update my branch for each release if that's not required. Exactly, that's the purpose of --isolated-root, so you can set it for all builds, I don't think that make sense to have an option to not use it, there is no real value in term of isolation. When specified isolated-root take the whole git repository rather than taking only the folder containing Singularity definition file, it's more flexible without impacting isolation.

[vsoch]

So why have two options? Why can't we just do isolated without root (and it allows for parent folder context)

[bbarker]

what if the nesting is higher than depth 1 (from root of repo to the singularity file)?

On Thu, Jul 19, 2018, 5:05 PM Vanessa Sochat notifications@github.com wrote:

So why have two options? Why can't we just do isolated without root (and it allows for parent folder context)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/singularityhub/singularityhub.github.io/issues/139#issuecomment-406414112, or mute the thread https://github.com/notifications/unsubscribe-auth/AA37jnn22tgqJT2RTjxQy-uzIEVqa-_yks5uIPSRgaJpZM4VVCZ- .

[vsoch]

per my understanding the dep shouldn't matter, it can be anywhere on the system. My question is whether it's necessary to have two flags given that the second flag with root wouldn't harm in most situations. from a usability standpoint having the one isolated flag that also allows for building outside of the root directory would make most sense to me! And possibly then an additional flag to disable that if a user really wants to (the less likely of the two cases)

[cclerget]

@vsoch It's necessary, the main purpose of --isolated is to ensure you can't access to host filesystem during build to not harm any host filesystem files since all commands are run as root. Otherwise without --isolated-root how do you tell to Singularity where isolation begin ? you could perfectly set --isolated-root / and make --isolated flag completely useless in term of host filesystem isolation. You need to put the path via --isolated-root to explicitly tell to Singularity, I want to bind this path in the isolated build container and access to all files within the provided path by --isolated-root.

[vsoch]

Right, so I'm wondering under what circustances we wouldn't wanted to have --isolated-root. I'm asking because for spython I would just make the isolated=True flag turn on BOTH isolated and isolated-root flags, because I don't see why the user would want just --isolated if --isolated-root gives more flexibility and poses no security issue.

[vsoch]

Oh I see, there is the root following as an argument, I don't know how I missed that! I'll see what I can do.

[vsoch]

So let's talk about this logic - does it make sense?

    # The user wants to run an isolated build
    if isolated is True:
        cmd.append('--isolated')

        # And change the default root folder for isolation
        if isolated_root is not None:
            cmd = cmd + ['--isolated-root', isolated_root ]

With this functionality setting --isolated-root seems to be essential for it to work, period, otherwise you are saying that isolated is useless? So then what is the behavior before you added --isolated-root, period? Are you saying the --isolated flag has not served a purpose?

[vsoch]

Before --isolated-root is it the case that singularity uses the $PWD of the build as the isolated root? (This would be my guess)

[cclerget]

So you mean --isolated-root /some/path (implying --isolated) to be equivalent to --isolated-root /some/path --isolated, right ?

[dtrudg]

@vsoch - is your spython meant to target standard singularity, not the secbuild branch which is SingularityHub specific? If so I don't think you want to be implementing the above at all in spython.

When I say accessing things up a level or more isn't an extra security risk it's because I'm talking about standard use of Singularity, where a user is running sudo singularity build on a machine they control. The assumption is that this is done on a machine where they happily have sudo rights. With the isolated build stuff and the branch we are in a non-standard area - due to how your SingularityHub builders are working - where the person who through a GitHib commit is triggering the build should certainly not have effectively sudo rights over your builder box. It's not a standard flow in Singularity proper.

[vsoch]

spython is a wrapper for Singularity, or Singularity secure build, and it's the client that is used by the builder (with the --isolated flag for security). As of now, the only difference is the --isolated flag, which is agnostic to what the user has on the system. If the user has secbuild then --isolated would work, otherwise it would spit out the error message (from Singularity).

@dctrud yes - this is what I was saying before :) It is not secure to give the user access to build outside of the context directory.

[vsoch]

@bbarker I can update the builder for you, but it does not seem secure for me to use --isolated-root set to anything other than the build context. I think for Singularity Hub you can assume that we take the most conservative approach, which means not navigating outside of the build directory.

[bbarker]

@vsoch If I understand correctly, how is setting --isolated-root=GIT_REPO_ROOT not secure?

[vsoch]

It would be secure, but it wouldn't fix your issue. The build is already happening from the root of the repo (as would be expected) and the issue is that your files aren't outside of that. This is why I suggest to take an approach that doesn't have files with relatives paths back up to parent folders.

This doesn't work

/bin/cp: cannot stat '../../Utils/persist-env.sh': No such file or directory

but you could easily reference the actual path in the repo:

Utils/persist-env.sh

If I change it to make the $PWD to be where the recipe is, I would have an equivalent outcry that the other default behavior (building from the root) should be the case. You are the first to have issue with this approach, and I am not convinced that the change is so dire, as you are able to edit the recipe file, and doing so would have paths that are more directly understood.

[bbarker]

originally, I was using paths like that, because locally I was running "singularity build" from two levels up, but I suppose there is no convenient way to tell singularity hub about this?

If not, I'll plan to reorganize my repos. Doc changes may be warranted in singularity docs; even though singularity hub is separate, it is so closely associated to be worth mentioning this, I think.

On Fri, Jul 20, 2018, 10:01 AM Vanessa Sochat notifications@github.com wrote:

It would be secure, but it wouldn't fix your issue. The build is already happening from the root of the repo (as would be expected) and the issue is that your files aren't outside of that. This is why I suggest to take an approach that doesn't have files with relatives paths back up to parent folders.

This doesn't work

/bin/cp: cannot stat '../../Utils/persist-env.sh': No such file or directory

but you could easily reference the actual path in the repo:

Utils/persist-env.sh

If I change it to make the $PWD to be where the recipe is, I would have an equivalent outcry that the other default behavior (building from the root) should be the case. You are the first to have issue with this approach, and I am not convinced that the change is so dire, as you are able to edit the recipe file, and doing so would have paths that are more directly understood.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/singularityhub/singularityhub.github.io/issues/139#issuecomment-406609462, or mute the thread https://github.com/notifications/unsubscribe-auth/AA37jmZFhDq74tn8mVslK2pT8k62u_tiks5uIeKegaJpZM4VVCZ- .

[vsoch]

okay, thanks @bbarker , this is a plan that the robots like too :)

Generally, since funding is needed for every single container and version of a builder that I provide, I am opting to generate new builders for the major (including security) Singularity releases (but not quasi or development releases as I was doing a while back). They are infrequent enough to coincide with a good schedule for server updates too, so when the time comes around, please ping me and let's discuss changes that you think would be useful for yourself and other users for Singularity Hub. And docs are easy peezy! We can do those any time if there is something wrong.

[bbarker]

Thanks!

Actually, I just realized that what you are suggesting is that my original method should work (since you said the build is already happening from the root of the repo), so I'm going to try that again, just for the sake of understanding, and we can see if it works.

[vsoch]

Good idea! Surprisingly, it's not super common to interact with files (outside of the direct context) so it's not super surprising to see these issues come up. It will be very useful to debug and get feedback for the next update of the builders, so many thank yous in advance!

[vsoch]

And for new issues, please open each one separately so we can properly organize them (labels and colors... very important! :rainbow: )