Figure out a way to protect command_runner.yml

[Deedasmi]

Trivial code execution/privilege escalation depending on the environment. Might use https://doc.rust-lang.org/std/fs/struct.Permissions.html, haven't tested it.

Also, could send a hash of the config file with the results. That way the recepter should be warned any time the config was changed.

[George3d6]

I'd rather no have this, similar to a mandatory or default SSL layer for log transport, it's a security measure that should be implemented by the server administrator[s] if desired.

This tool isn't meant to babysit.

A change like this would add complexity to the code, make Inquisitor code harder to modify and update, adds useless complexity for the end user and makes further changes to the plugin (e.g. runtime config reload) harder or impossible.

As a side note, in the future, plugins shouldn't be able to crash the agent altogether and instead will send errors to the agent via the return value.

[Deedasmi]

Plugins already crash the agent if their config is missing or invalid. This just expands what is considered invalid.

In an environment with hundreds if not thousands of machines and your bosses nephew as your "awesome new intern", a silently insecure code execution path is a disaster in the making.

[Deedasmi]

Gave it some more thought, and this isn't an acceptable solution. It's reasonable to want some type of runtime --config option in the future, which breaks down any runtime readonly validation.

[George3d6]

The whole plugin crashing the agent will be fixed this week (plugins will just return a result with the error), it's just that I hadn't had the time to implement it yet due to some recent events. But the command runner plugin could return an error in the result (e.g.: "Wrong permission on the config file please chmod to XYZ for user Blax)

My initial reluctance do these sort of changes is that they "pollute" the code with stuff that most people won't probably use and is outside the scope of monitroing. This could be bad since it heads towards the same direction as other monitoring tools (which are horrible to modify, partially, due to the bloat needed to support all types of theoretical or never-meet usecases and architectures).

What I'm thinking is:

a) An option in the command_runner.yml

check_file_permission_is: xyz Which basically checks, whenever the config is reloaded, that the permissions to the file had stayed the same as the previous check_file_permission_is parameter value.

Still, this only works for hot reload, since the file could be edited, the agent restarted and a change to the parameter wouldn't be noted.

b) An option in the inquisitor_agent config, (maybe a command line option when running it) that checks the permission of all config files. E.g:

./inuqisitor_agent --max_permissions: 750 Would check x <= 7, y <= 5, z <= 0

As such, someone would at least need permission to run the inqusitor_agent and change that parameter in order to change the config files. As long as said flag defaults to 777, the default behavior would remain the same and not complexity would be added to the end user (but it would be added in the code).

Overall, as I said before, I think bosses nephew as your "awesome new intern" is not a usecase for this tool as much as Friendly to non-technical people (e.g. having option to edit agent plugin configs via a web-ui) is not in the scope of this tool.

But I can see that privilege escalation may be an issue on a large scaledeployments and I can see some edge cases where people would want an option to stop that build into the tool.

If I end up implementing it I'd prefer to add it in once this is somewhat ready for a v1.x.y release, I think that whilst it's still in very active development it would just be cumbersome to have something like option (b) and option (a) would be to easy to work around.

Making permission checking the default is something I wouldn't do, since it seems like annoying the many for the benefits of the few.

[Deedasmi]

As long as command_runner and other plugins are NOT in the default build (you have to re-compile the agent binary to allow them), it should be alright I guess.

Another possibility (with #6) would be to have a receptor plugin that tracks which agents/plugins are being used and sends an alert e-mail the first time agents use a certain plugin. Basically a "Hey, a new agent connected with us and is using command_runner! This plugin has been flagged as sensitive and security considerations should be taken into account. See XXXX for details".

[George3d6]

Hmh, receptor plugin that "tracks" what plugins are being used is a nice idea in general (though an endpoint/plugin for email alerts would have to be a separate entity).

So that would be quite fine.

I'd also think once we reach 1.0.0 a 'prod' release and a 'dev' release could be made and the former would lack the command_runner (as a default) and any other plugins that can be easily used to change the state of the machine.

Again, the problem here is that complexity is being added at the cost of preventing an imagined security breach due to stupidity and/or carelessness and if you try making software childproof you can go down the rabbit-hole as much as you want and still not get a perfect solution.

Overall, again, I think it's a discussion to be finished later once there's some people interested in using it with their infrastructure (currently there's only 3, one being myself and one possibly being yourself), because security in general can be a rabbit-hole, so I'd rather make the package as insecure as possible (assume the user is able to use it correctly) and only add the security features that are really necessary for it's users and can't be easily obtained otherwise.

As it stands, the command_runner plugin is easily fixed by following some good-practices and running "read only" programs (e.g. the agent_runner) on a "read only" user (e.g. that can't modify anything outside of his home dir). The people that wouldn't run it this way are exposing themselves to bigger security risks anyway.