add back check to enforce root/sudo startup.

[martincox]

I couldn't work out what the first check was being done for. Where we were checking for non-root startup and not su'ing to riak.

I don't think that has any impact? I've run both ways (service & sudo) startup, and there's no difference. Unless there's something obvious I'm missing?

[martincox]

I think we might have an issue with the service script - I don't think it actually runs the process as riak.

If I add in the --chuid flag to start-stop-daemon in the do_start function, with this change, the service won't start - because at the point where the /usr/sbin/riak script is run, it's now being run as riak and fails the root UID check.

[martincox]

I don't know if I'm misinterpreting the switches for start-stop-daemon, but to me it reads like --user just performs a check, as opposed to starting the process as a designated user. I'm looking at this man page - http://manpages.ubuntu.com/manpages/trusty/man8/start-stop-daemon.8.html

But then looking at the previous build, with node_package, it's exactly the same - https://github.com/basho/node_package/blob/a541c79bb65fb4ac6f60197c333f841065d0aff6/priv/templates/deb/init.script#L48

I'm a bit confused.

[martincox]

node_package has a check_user function (https://github.com/basho/node_package/blob/a541c79bb65fb4ac6f60197c333f841065d0aff6/priv/base/env.sh#L223) in env.sh which will su to the {{runner_user}}.

So the service script has never run as the riak user, and the su is handled in the wrapper script - /usr/sbin/riak in our case.

In which case, the setup now, with this change, mimics what previous builds did (I think).

[martinsumner]

From a debian perspective we wanted the service to run the script as riak, as if we ran the script as root and then su'd to riak as we ran riak - the service would only give root access to the PIDfile (as it had started the script as root), and so riak would not be able to write the PID in the pre-start hook as required by the service.

So the script is meant to be run either as riak, or as root. If it is run as riak (i.e. if called directly by someone running as riak, or run by the service) it shouldn't switch user, and if it has been started as a service systemd will now give write access for the pid file to the riak user. If it is run as root, it should switch user, but assume it is not systemd starting - and so create the PID folder and make sure the riak user has write access.

So the check I was expecting to be added was not to confirm the script was being run as root, but to confirm that it is either being run as riak or root - and not some other user.

[martincox]

Yeah, so the first check in the script is to handle when it is being invoked by the riak user - but under the current configuration, that never gets called, so we are always su'ing.

In order for that to become effective, we have to modify the service script to run as the riak user, which as far as I'm aware, is done with the --chuid switch.

And then, we can modify the user check in the wrapper script to validate both root and riak users.

Does that sound correct to you?

[martinsumner]

Sorry, this looks like it has been sitting with me. Did we talk about this offline?

I don't have an intuitive understanding if this is going to break how thinks work stopping starting with systemd using the riak user. I can try and experiment when I'm cutting 3.0.2.

[Bob-The-Marauder]

Previously, there was a security issue where the Riak user could edit Riak files and potentially inject malicious code that would be executed when they started Riak. There was later a fix included to restrict access to root only for all Riak files that could be abused like this. I am wondering whether this check is related to that in any way.

[martinsumner]

@martincox, just experimented with the package and this will break riak with systemd on debian/ubuntu.

This will start riak as the riak user (i.e. run the riak script as the riak user), so even though it is systemd starting the service at the point of running usr/sbin/riak it is the riak user. I think getting systemd to start as root is a bad alternative from a security perspective.

Can we leave this out of 3.0.2?

[martincox]

Yes, leave it out until I can find time to test / change it to work properly 👍