Fix for chroot not getting supplemental groups

[coolacid]

linux chroot doesn't get the supplemental groups before dropping privileges. As such, we need to pull it from /etc/group and tweak it so that we can send them to chroot.

Original report came from @spuder on irc -- here is his GIST report:

https://gist.github.com/spuder/a1c3c7d10ce129507858

Spuder tested this and reported it working. Should work on other systems that use sysv init.

[coolacid]

I could "fix" logstash-web, but I see no reason why it should require the extra groups to run.

[spuder]

Before the patch logstash could not read files owned by root:adm with permissions of 640, after the patch, logstach can read those files.

Tested on ubuntu 12.04

[electrical]

Cool solution, im only afraid of the situation when there are no additional groups would the command fail then because its missing group names?

Perhaps an idea to do something like:
if CGROUPS is not empty then set EXTRA_GROUPS to '--groups $CGROUPS' and use the EXTRA_GROUPS in the command?

[jordansissel]

getent initgroups $LS_USER feels like the more correct approach here, do you agree?
Parsing /etc/group isn't best because it doesn't respect nsswitch.conf.

Thoughts?

[coolacid]

New commit tests to see if there was anything returned.

tested with my username - I get EXTRA_GROUPS, with another account it's blank as expected.

[coolacid]

I was not aware of the command - and you're right, pam could be a problem too.

The command as provided gives group numbers, which would work - let me play some more.

[coolacid]

I don't remove the already set group anymore, but I can't see that hurting anything.

[jordansissel]

+1 using id instead of grep /etc/group seems like a good approach.

[electrical]

👍 from me :-)

[spuder]

The updated pull request works great for me.
74dfd2b

chmod 640 /tmp/foo
/etc/init.d/logstash-coolacid3 start
logstash started.
echo "logstash-coolacid3 640" >> /tmp/foo
tail -n1 /tmp/bar
{"message":"logstash-coolacid3 640","@version":"1","@timestamp":"2014-05-22T20:48:15.742Z","host":"foosball.ac","path":"/tmp/foo"}

If the init script was generated by 'pleaserun', I wonder if there needs to be a bug-hancement filed on that project as well?

[electrical]

@spuder all issues that we see here with with the init script we silently try to port over to pleaserun :-)
Feel free to open the issue there so we can solve it permanently.

[spuder]

Is there an expected release for this fix? I'd really like to see it make it into 1.4.2 since logstash is broken on all Ubuntu nodes at the moment.

[victorgp]

How is possible that logstash is broken in, at least, all ubuntu nodes and this fix is not already merged?

[elasticsearch-release]

Can one of the admins verify this patch?

[jordansissel]

@victorgp I am sorry that you are negatively impacted by this. From your comment, I can tell you are excited to see this patch merged!

While I believe this patch to work (testing by hand on one OS distro version), it would be greatly useful to us to know if it works for you. Further, this script is also used on CentOS/RHEL/SLES systems, can you help us test on those? I don't have access yet to all of those systems so it is difficult to do such testing in isolation. We are always working to improve our testing infrastructure as well to help cover these situations.

Any testing you can do on this would be lovely. As a bonus, by testing this, you would apply a workaround for your own infrastructure until we provide a release with this patch (or something like it that is known to work on all supported platforms).

I would hate to merge this patch only to have you or someone else find it in the next release having caused something else to break. This is why testing is so important to us.

This is an open source project and community, and we are stronger if we work together! Any energy you can provide is most appreciated, though obviously not required :)

[victorgp]

@jordansissel sorry for the negativity, i didn't want to seem so negative. I will try to help you testing the patch in Debian and Ubuntu, i don't have access to other distros either. Let's see if i can find some free time one of these days to test it.

[victorgp]

The tests i've done tell me that this fix is not fully complete. It does work for old scripts located in /etc/init.d/ but it is still broken for upstart scripts in /etc/init/

I suppose the file logstash.upstart.ubuntu must be fixed, but it is no using chroot to run the program, why? i guess both should run in the same way...

BTW anyone knows what is the version that introduced this bug?

[electrical]

Hi,

The upstart script didn't get the love yet it deserves but we will soon do that.

[spuder]

I find it very interesting that other people are still able to use logstash despite this bug. It is an absolute blocker for me. I'm trying to setup the simplest possible logstash cluster as shown in the examples in the logstash book, but can't work around this without significant chmod hackery.

Surely every one who tries to use logstash is running into this bug, no?
Maybe every other sysadmin is just doing a chmod 777 on all their logs.
If anyone has thoughts on why more people aren't blocked by this, I'd love to hear it.

[coolacid]

As far as I can see upstart doesn't use chown -- and IIRC when I did these patches, I looked though all the startup scripts to see if they would have the same problem.

[heytchap]

Hey guys. Not sure if it's just me but the @coolacid modifications posted above still prevent me from reading anything root:adm with 640 permissions. Have modified both sysv and upstart but issue remains. Am I the only one?

[coolacid]

@heytchap Did you add logstash to the adm group?

[heytchap]

@coolacid I did. I also added the LS user to root and syslog.
to confirm, the following command gives me this output:

id logstash
uid=999(logstash) gid=999(logstash) groups=999(logstash),0(root),4(adm),104(syslog)

so Im not sure what the hangup is here.

[jordansissel]

@lasse-aagren Indeed! My hope, for now, is that setfacl is simply a workaround until we can fix it. This assumes all process-launcher systems are capable of setting supplementary groups- Supervisord, for example, appears only to support setting user, not even group, nor supplementary groups.

[jacek-berlin]

Hi,
I've just stumbled upon this bug (Logstash 1.4.2, Debian Wheezy) .

Quick and dirty fix (if you trust your system) would be also to comment out the chroot line.

#### Works fine

# Run the program!
#  nice -n ${LS_NICE} chroot --userspec $LS_USER:$LS_GROUP / 

    sh -c "
    cd $LS_HOME
    ulimit -n ${LS_OPEN_FILES}
    exec \"$program\" $args
  " > "${LS_LOG_DIR}/$name.stdout" 2> "${LS_LOG_DIR}/$name.err" &

[sdklein]

+1 for the solution. I tested this pull request on CentOS 7 and it worked as expected.

[franklinwise]

When is this getting merged?

[jhoff909]

Curious when this is going to get merged? Like others, I can't imagine that most folks using logstash to read files aren't running into this and spending a lot of time either figuring it out or reading posts like the above...

[vasti]

Variable HOME is exported without setting value to it. There's HOME=${LS_HOME} on master.
I get error No SINCEDB_DIR or HOME environment variable set, I don't know where to keep track of the files I'm watching. Either set HOME or SINCEDB_DIR in your environment, or set sincedb_path in in your Logstash config for the file input with path because of that.

[jordansissel]

I think we've had enough folks report that this patch works for them that I am ok to merge it.

[elasticsearch-bot]

Merged sucessfully into master 1.5!