Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow deploying s6-overlay in images without nobody/nogroup #15

Closed
glerchundi opened this issue Mar 13, 2015 · 14 comments
Closed

allow deploying s6-overlay in images without nobody/nogroup #15

glerchundi opened this issue Mar 13, 2015 · 14 comments

Comments

@glerchundi
Copy link
Member

Images like busybox:ubuntu-14.04 don't include neither nobody in /etc/passwd nor norgroup in /etc/group and init fails to start outputting this:

s6-envuidgid: fatal: unknown user: nobody
if: fatal: child crashed with signal 13

!!!!!
 init-stage2 failed.
 Logs are in /var/log/s6-uncaught-logs
!!!!!

As our overlay can just deploy static files in the destination image the solution would be to perform these actions in runtime:

  • check if nobody/nogroup exists
  • create them if they didn't exist.

What do you think?

@skarnet
Copy link
Contributor

skarnet commented Mar 13, 2015

If we're going to create our own users, no need to care about nobody/nogroup in particular. We can create users with the name we want - and it's better, since we're sure our processes will be the only ones running as those users.

The only worry that I have is: is it worth it? I initially picked "nobody" and "daemon" because it was easy - those users are almost guaranteed to exist on every system, and it was a simple way of using non-root uids without having to work too hard on it.
If we need to do ugly stuff such as potentially modifying /etc/passwd on every boot - which means / cannot be read-only anymore - then I think it's too heavy and risky, especially since it has to be done in stage 1.

The possibilities I really want to explore are:

  • guaranteeing certain basic uids are present on every image
  • building an /etc/passwd at build time
  • if all else fails, how practical it would be to document that s6-enabled images need a few specific uids in the user database, or maybe to reserve fixed numerical uids (which isn't that ugly if you don't look at ps too hard - the important thing is that there's no collision).

@jprjr
Copy link
Member

jprjr commented Mar 13, 2015

I would vote against trying to manipulate user databases at runtime, or even at build-time. The overlay is just a .tar.gz - if we got into having some type of "post-extract" step, I think we're getting into the realm of inventing a package management system.

If the goal is to provide a sort of "best practices"-oriented project, I think it's reasonable to document what user(s) each service requires.

Alongside that, you would have documentation on how to add a new service. This could include creating a service user, using envdirs effectively, logging, and so on.

@dreamcat4
Copy link

I agree with both points of view. None of the possible solution is optimum for the reasons you guys are saying. But most of all I think it is worth pursuing to uphold the claim that the s6-overlay can work with any image whatsoever. And including FROM scratch also.

The problems you guys are highlighting is the reason I think is 'best efforts' to fall back to a hardcoded UID number when the user does not exist. Which realistically shall only happen for exceptional cases (e.g. mis-configured images like busybox). With also a decent warning message to stderr whenever that happens. It isn't very elegant solution I admit. But it just does require us to interfere as much with the image, or suffer the other problems ^^ you are hitting here.

@dreamcat4
Copy link

Once Docker fixes ADD cmd. Such a build-time solution would eventually look like this, for Docker users

[EDIT] minor correction to docker's future ADD cmd.

FROM ubuntu-debootstrap:14.04
ADD --extract https://<download_url>/s6-overlay.tar.gz /
RUN <some s6 command to create necessary users>
ENTRYPOINT ["/init"]

vs

FROM ubuntu-debootstrap:14.04
ADD --extract https://<download_url>/s6-overlay.tar.gz /
ENTRYPOINT ["/init"]

So - what I am saying is that the cost of having more elegant user accounts comes at the price of less elegance in the Dockerfile. It is a trade. But actually we can to BOTH things. And document the option to the users.

  1. Provide the <some s6 command to create necessary users>. Being sure that the command is available on any non-existed or default minimum $PATH at docker build time.

  2. Provide a simple hard-coded uid:gid as a failover. If users did not know they needed to specify the extra RUN command in their dockerfile. Or else spit to stderr on missing user failure what they should do (what missing RUN line they need to go and type into their dockerfile).

  3. However if user is using a popular and good distribution. Then they do not need to add the optional RUN line in their Dockerfile. Because the default user accounts are already set up properly.

N.B. Another point I think we need to remember is the USER directive is also possible in Dockerfile. We must remember and should test for that also. Because in that situation, anything after the USER directive can only be run as regular user. So no chown, or spawning different processes to multiple accounts, no modifying of /etc/passwd or add new users, etc.

What do you think?

@glerchundi
Copy link
Member Author

Hi all,

Let's discard some options in order to start focusing on feasible solutions:

  • As we're providing a static file as an overlay for an existing OS, build-time user/group creation would incur in unexpected behaviours and therefore issues for us. For example:
FROM ubuntu
ADD --extract https://<url>/s6-overlay.tar.gz /
ENTRYPOINT [ "/init" ]

This would override existing /etc/{passwd,group} and probably users required by that concrete OS. @skarnet, @jprjr, @dreamcat4, please correct me If i'm missing something. Perhaps you were thinking in something that I cannot see right now.

  • This brings us to the only solution for docker: create them (or a custom ones) if needed, in runtime. I really dislike the idea to do such hack in runtime, because among other things, and as Laurent says, it's not going to be read-only capable anymore. And who knows how is going to be used this overlay, for now, our idea is to include it in docker images but maybe someone could find it interesting for another kind of solution and, of course, no need to explain why this would be a gain for us.

In conclusion, the only solutions would be:

  • Provide a solution to create them in runtime, Is it worth it @skarnet, how do you see this implementation? do you think this would mess all the init script? Maybe we tackle this problem with a command line provided argument like: /init --ensure-vital-requirements $@. Maybe this opens the door for different kind of operations in init like --output-to-stdout.
  • Do a good documentation job and ask for specific requirements in the destination image/OS/whatever.

We can do a proof-of-concept, and decide if it's really worth it.

@skarnet
Copy link
Contributor

skarnet commented Mar 13, 2015

No, performing /etc/passwd manipulation at runtime in stage 1, or option parsing in /init, is clearly not worth it: too slow, too complex, too brittle.
I much, much prefer relying on the existence of certain uids in the initial image's /etc/passwd, and failing that, simply hardcoding uids - if users can't bother to read instructions, they'll still get a secure system, but it won't be pretty.

I'll modify s6-envuidgid to be able to specify default values when the user isn't found in the uid database.

@skarnet
Copy link
Contributor

skarnet commented Mar 13, 2015

There. If you use s6-2.1.3.0, you'll be able to write "s6-envuidgid -D uid:gid blah..." to get default values.
With that, we can just:

  • document that a few s6-specific uids should exist in /etc/passwd
  • use hardcoded numeric ids if it's not the case despite the requirement.

We need at least one id for the catch-all logger (s6log ?) and one for the fd-holding daemon (fdholder ?). I suggest reserving ids like 32768 and 32769: nobody ever uses those (it's either on the low range or close to 65535, never in the middle).
What do you guys think ?

@dreamcat4
Copy link

@skarnet +1 I agree with this.

While talking on the same subject about setuidgid, we might want to check beforehand, when starting up if the process UID of PID 1 is not 0. Since in that situation we are not allowed to. That is a different issue though I just wanted to briefly mention it.

@jprjr
Copy link
Member

jprjr commented Mar 13, 2015

I'd recommend you just try changing user ID but if it fails just move on.

On March 13, 2015 5:13:59 PM CDT, Dreamcat4 notifications@github.com wrote:

@skarnet +1 I agree with this.

While talking on the same subject about setuidgid, we might want to
check beforehand, when starting up if the process UID of PID 1 is not
0. Since in that situation we are not allowed to. That is a different
issue though I just wanted to briefly mention it.


Reply to this email directly or view it on GitHub:
#15 (comment)

Sent from my Android device with K-9 Mail. Please excuse my brevity.

@skarnet
Copy link
Contributor

skarnet commented Mar 13, 2015

Yeah, this one isn't going to be pretty.

I'm not changing s6-applyuidgid to allow it to go on even if it does not succeed in dropping its privileges. You know that if there's an option to do something insecure, people will use that option in contexts where it is not safe, and will create security weaknesses.

So yes, special-casing a non-root pid 1 will be needed, because all the s6-setuidgid lines will need to be removed in that case. I think we should create a specific version of /etc/s6/service with run scripts tailored to a non-root USER.

We could even make those run scripts write condescending and patronizing messages to the logs. Maybe have a special service that spams outside of the container in the most annoying way possible. "Do you know you've been running that container in an UNSAFE AND MORONIC WAY for $seconds seconds ?"

Just kidding. But sometimes, the dark side is oh so tempting.

@dreamcat4
Copy link

@skarnet I do agree such things should not be rolled into s6-applyuidgid program. But instead handled by the init-stage* scripts. If or should we choose to support it.

More discussion can be had about it over on separate issue, raised today - issue #19

#19

@glerchundi
Copy link
Member Author

@skarnet i think you have missed something parsing in setuidgid or passing it to envuidgid...

#> s6-envuidgid nobody import -u UID s6-echo \${UID}
65534
#> s6-envuidgid -D 12345 unexisting import -u UID s6-echo \${UID}
12345
#> s6-setuidgid nobody id
uid=65534(nobody) gid=65534(nogroup) groups=65534(nogroup)
#> s6-setuidgid -D 65534 unexisting id
s6-envuidgid: usage: s6-envuidgid [ -i | -D defaultuid:defaultgid ] username prog...

UPDATE 1: but now thinking it twice (sorry because i woke up now...), is it really possible to convert in a user that doesn't exist? :
UPDATE 2: although i think this really, really sucks, I made an example just to see what happens:

#include <stdio.h>
#include <stdlib.h>

void
do_setuid (uid_t uid)
{ int status;
  status = setuid (uid) ;
  if (status < 0) {
    fprintf (stderr, "Couldn't set uid.\n") ;
    exit (status);
  }
}

int
main (int argc, char **argv)
{
  if (argc != 2) {
    exit (1) ;
  }
  unsigned int uid = (unsigned int)atoi(argv[1]) ;
  do_setuid(uid) ;

  char *exec_argv[] = { "id", NULL } ;
  execvp("id", exec_argv) ;
}

And it works:

#> ./a.out 65534
uid=65534(nobody) gid=0(root) groups=65534(nogroup)
#> ./a.out 12345
uid=12345 gid=0(root)

@jprjr
Copy link
Member

jprjr commented Mar 14, 2015

@glerchundi yeah, it's possible to change id to a non-existant user, it happens fairly often.

glerchundi added a commit to glerchundi/s6-overlay-builder that referenced this issue Mar 14, 2015
@glerchundi
Copy link
Member Author

@skarnet , I don't know if you were refering to the last comment of @dreamcat4 or just to the whole issue discussion. Anyway, i've changed the implementation which uses s6-setuidgid for s6-envuidgid + s6-applyuidgid.

So, i'm going to close this and move forward. If you want to change s6 source code and include this, I will happily change it again :-)

glerchundi added a commit that referenced this issue Mar 14, 2015
fixes #15, if vital users don't exist, defaults to custom uid/gid
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants