api.go doesn't respect nsswitch.conf

[eliasp]

Due to the implementation of api.go which reads only /etc/group, it is impossible to rely on other auth mechanisms (e.g. LDAP, NIS).

groups, err := ioutil.ReadFile("/etc/group")
if err != nil {
        return err
}
re := regexp.MustCompile("(^|\n)docker:.*?:([0-9]+)")
if gidMatch := re.FindStringSubmatch(string(groups)); gidMatch != nil {
        gid, err := strconv.Atoi(gidMatch[2])
        if err != nil {
                return err
        }
        utils.Debugf("docker group found. gid: %d", gid)
        if err := os.Chown(addr, 0, gid); err != nil {
                return err
        }
}

Instead, the getgrnam syscall should be used to determine, whether a docker group exists or not.

The implemenation of lookup_unix.go in the user Go package might serve as inspiration on how to do this.

[crosbymichael]

@eliasp It sounds like you know exactly what to do, a pull request would be great. ;)

[eliasp]

@crosbymichael Well, I know (more or less) exactly what to do, I just completely lack any Go skills.
I might try to come up with something though. :-)

[crosbymichael]

@eliasp Try it out and if you need any help just let me know.

[eliasp]

Currently, the tests for api.go fail:

# _/tmp/docker
./api.go:93: cannot use auth.AuthConfig literal (type *auth.AuthConfig) as type *auth.ConfigFile in assignment
./api.go:95: authConfig.Username undefined (type *auth.ConfigFile has no field or method Username)
./api.go:95: authConfig.Email undefined (type *auth.ConfigFile has no field or method Email)
./api.go:111: cannot use false (type bool) as type *utils.HTTPRequestFactory in function argument
./api.go:122: localAuthConfig.Username undefined (type *auth.ConfigFile has no field or method Username)
./api.go:123: localAuthConfig.Password undefined (type *auth.ConfigFile has no field or method Password)
./api.go:126: undefined: auth.NewAuthConfig
./api.go:127: cannot use true (type bool) as type *utils.HTTPRequestFactory in function argument
./api.go:424: cannot use localAuthConfig (type *auth.ConfigFile) as type *auth.AuthConfig in assignment
./api.go:457: not enough arguments in call to utils.CheckLocalDns
./api.go:457: too many errors
FAIL        _/tmp/docker [build failed]

I feel uncomfortable to start doing changes here and fixing these tests is a little too much for me as a start in Docker/Go… maybe someone could have a look at these failing tests first.

[jpetazzo]

I'm not sure, but — since we're using static linking (and therefore, not
using CGO) we are probably using "built-in" versions of the resolving
functions, and I don't know if they are fancy enough to do that kind of
resolving. I'm sorry if what I said doesn't make sense; ask me to elaborate
and I'll try to explain better!

On Wed, Aug 28, 2013 at 5:10 PM, Elias Probst notifications@github.comwrote:

Currently, the tests for api.go fail:

_/tmp/docker

./api.go:93: cannot use auth.AuthConfig literal (type *auth.AuthConfig) as type *auth.ConfigFile in assignment
./api.go:95: authConfig.Username undefined (type *auth.ConfigFile has no field or method Username)
./api.go:95: authConfig.Email undefined (type *auth.ConfigFile has no field or method Email)
./api.go:111: cannot use false (type bool) as type *utils.HTTPRequestFactory in function argument
./api.go:122: localAuthConfig.Username undefined (type *auth.ConfigFile has no field or method Username)
./api.go:123: localAuthConfig.Password undefined (type *auth.ConfigFile has no field or method Password)
./api.go:126: undefined: auth.NewAuthConfig
./api.go:127: cannot use true (type bool) as type *utils.HTTPRequestFactory in function argument
./api.go:424: cannot use localAuthConfig (type *auth.ConfigFile) as type *auth.AuthConfig in assignment
./api.go:457: not enough arguments in call to utils.CheckLocalDns
./api.go:457: too many errors
FAIL _/tmp/docker [build failed]

I feel uncomfortable to start doing changes here and fixing these tests is
a little too much for me as a start in Docker/Go… maybe someone could have
a look at these failing tests first.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/1715#issuecomment-23458539
.

[creack]

@eliasp The tests does not fail on master. What branch are you using?

@jpetazzo is right, we can't rely on C functions. In order to support more than /etc/group, we'd need to implement it.

[eliasp]

@creack Sorry, turned out it was my fault… I fetched upstream into my clone, but didn't merge it…

@jpetazzo It seems to be possible. But I'm wondering why docker is built statically linked at all? Are there any (possibly Go specific) reasons for that?

[jpetazzo]

The stackoverflow question is a very useful pointer, thanks!

Regarding "why do we build docker statically?", that's because docker is
bind-mounted inside every container, and used as a trampoline to start the
target process in the container; if docker is linked dynamically, it means
that we also have to bring in the libraries it depends on, and those could
clash with existing libraries in the container.

But if we can find a way to link with extra libs and still get a static
binary... Then we're golden :-)

On Thu, Aug 29, 2013 at 4:52 PM, Elias Probst notifications@github.comwrote:

@creack https://github.com/creack Sorry, turned out it was my fault… I
fetched upstream into my clone, but didn't merge it…

@jpetazzo https://github.com/jpetazzo It seems to be possiblehttp://stackoverflow.com/questions/16747021/how-do-you-statically-link-a-c-library-in-go-using-cgo.
But I'm wondering why docker is built statically linked at all? Are there
any (possibly Go specific) reasons for that?

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/1715#issuecomment-23532062
.

[eliasp]

@jpetazzo Thanks for the explanation, that's actually one of the few situations, where static-linking is preferred.

FYI: I won't have time until next Tuesday to work on the implementation of this issue.

[shykes]

Any takers?

I'll mark this as an easy fix for aspiring contributors.

[mrallen1]

I found this: https://github.com/proxypoke/group.go but no license sadly.

[tianon]

See:
https://groups.google.com/forum/#!topic/golang-nuts/WIYnds6TuEA
https://codereview.appspot.com/4589049
https://codereview.appspot.com/13454043

So potentially a future version of Go will have group lookup functions to go with the user lookup stubs. My guess is that they'll look to start reviewing stuff like this after they finally get the feature-frozen Go 1.2 out the door, then we'll just have to wait until n+1 gets released. Until then, we could implement the same code applied there if it's important enough.

[shykes]

It looks like nobody is actively working on this.

For the record this is open to contributors.

[crosbymichael]

ping @tianon I think your groups stuff fixes this?

[dwlocks]

I've run into this bug in .9: NIS group membership is not respected, but /etc/group membership is. So if @tianon 's groups stuff is in .9, it does not fix this issue.

[tianon]

I can confirm that my groups stuff definitely does not fix anything related to NSS, which has been an issue in the past specifically because we link statically, and linking the nss libraries statically is a problem. This might be something we could look to enable now, but I'm not positive.

[eliasp]

@tianon Is linking the NSS libraries a general problem, or specific to them? Have you looked at this for static linking of system libraries?

[tianon]

We're static linking several libraries already, but aren't the NSS libraries specific to the version of NSS that's currently on the system? I'll admit that my knowledge of NSS doesn't go much further than hacking through the configuration files to get them to do what I needed.

See also #4450 (comment) for a discussion of why we can't just use Go's own "os/user" (which is actually a case of it being used and failing because it ends up depending on libc, and thus will require the same version of libc at runtime).

[alexlarsson]

The problem is that NSS is plugin based, so it will dlopen() some module that in turn links to the dynamic glibc.
Bam, you now have two copies of glibc loaded! epic win! everyone gets free candy and crashes!

[alexlarsson]

Note, i think typically that would work, because the module only sees the dynamic glibc and the app the static one. However, if the two libcs are of a very different version, then its possible that some of the internal ABIs have changed (maybe the NSS plugin structs where modified) which can cause problems. This is why it says:

warning: Using 'getpwnam_r' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking

I.e. you need at runtime the same version of glibc as during the (static) linktime.

[vbatts]

This sounds like another use case that could leverage a separate binary, for a command like docker tool <CMD> [ARGS...], where is a binary somewhere like libexec/docker/CMD.
There could be an internal library to handle calling these tools. And since it's not calling linux/windows/osx commands, it stays platform independent. If the command returns non-zero, the server can handle the error, but not have to deal with a dlopen glibc crash in the daemon.

[aus]

This can probably be closed now that libcontainer handles group look ups.

[runcom]

seems like there's a new patch set but still in review since ~11 months https://codereview.appspot.com/101310044

and also #11377 is merged now as a workaround

[prune998]

Any news on this ?
Group lookup is still done in the staticaly defined /etc/group file without any support to NSS lookups.
Any chance to have Docker support basic Linux features ?

Thanks.

[GordonTheTurtle]

USER POLL

The best way to get notified of updates is to use the Subscribe button on this page.

Please don't use "+1" or "I have this too" comments on issues. We automatically
collect those comments to keep the thread short.

The people listed below have upvoted this issue by leaving a +1 comment:

@rmrf

[vdemeester]

Given the activity level on this issue, I'm going to close it as it's either fixed, a duplicate or not a request anymore. If you think I'm mistaken, feel free to discuss it there 😉

[nicoulaj]

Not sure why this would be closed, this bug is still there. See docker/for-linux#186 and analysis by @mjameswh.

[prune998]

Please fix before closing !

[mjameswh]

@vdemeester Could you please consider reopening this issue?

I think the PR I submitted yesterday should fix it, but I'm afraid that at present, it won't get any attention because it has been closed a long time ago...