[Nagiosplug-devel] Security discussion - don't run as root plugins

[Andreas Ericsson]

Thomas Guyot-Sionnest wrote:
> 
> | Imo, this snippet could work well:
> |
> | const char *run_as_uid;
> | if (!(setuid() | seteuid()) { /* yes, we're actually root, not just
> setsuid */
> | 	int i;
> | 	for (i = 1; i < argc; i++)
> | 		char *arg = argv[i];
> | 		if (!strncmp("--run-as=", arg, 9)) {
> | 			run_as_uid = arg + 9;
> | 			break;
> | 		}
> | 	}
> | }
> |
> | if (run_as_uid && setuid(strtol(run_as_uid)) < 0)
> | 	error("Failed to setuid(%s): %s\n", run_as_uid, strerror(errno));
> 
> Sure, but now you're going to update every plugin to no-op on that
> argument... Unless you re-write argv and update argc.
> 

Yes.

> | Relying on the environment is not very stable imo, as it's one area where
> | memory exhaustion is so easily accomplished.
> 
> We only read there... It's not as we were adding new environment values.
> 

Reading is actually the dangerous part, as "reading" can just as well be
spelled "writing to memory allocated by the program".

> |> Overall the security is still enhanced as for this extra code to run the
> |> plugin must be root already (and this will avoid much more code running
> |> as root).
> |>
> |
> | That depends. If the plugin and its libraries has (exploitable) bugs, the
> | extra code adds protection.
> | If not, then this extra layer adds nothing and if the extra code is
> | buggy in an exploitable way it even reduces security.
> 
> I'm sure we can handle such a small thing if we're careful enough.
> 
> Anyways I won't do anything before having more feedback, especially from
> the team; I'm more concerned about the user impact...
> 

I doubt anyone will use this option in the foreseeable future, so I think
the user impact is minimal. Otoh, I see very little reason for doing it
in the first place, so perhaps I'm just weird.

-- 
Andreas Ericsson                   andreas.ericsson at op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231