Config sync should only sync icinga config files

[Mikesch-mp]

Describe the bug

Icinga still syncs every file, even it is not a `.conf`` from master to other master/satellites & agents if it inside a zone directory of the configuration master.

To Reproduce

1. Create a file or copy evil.sh to /etc/icinga2/zones.d/global-config
2. reload icinga2
3. check /var/lib/icinga2/api/zones/global-config/_etc

Expected behavior

Icinga2 should only sync files with .conf extension or even better .conf files that contains only valid configuration, otherwise you can have a lot of fun on a lot of host if you are evil like me

[Al2Klimov]

@N-o-X What's your opinion?

[N-o-X]

@Mikesch-mp is right, it's not necessary to sync every file. We explicitly say in our docs, that you should not use that mechanism for anything else. My only concern is to break stuff users have build with this "unintended feature".

[Al2Klimov]

Right, I've heard someone is even syncing check plugins. Not that I consider that good practice, but we shall not break anything.

[Mikesch-mp]

What do you break if it is written in the docs that trhere should only be config. To roll out plugins other stuff there are other tools like ansible. For me this is a big security hole, because i can sync scripts to agents, satellites etc that do real bad stuff that should not be possible. Think about installations that monitors servers for other customers that think its only config that can be snyced and not scripts, binaries that can then be called by icinga creating a command/service configuration to execute them

[Al2Klimov]

I can also sync check commands that run bash -c ... by themselves w/o any additional script.

[Mikesch-mp]

Yeah but as customer you have it under control what the icinga deamon can run, if it is possible to put scripts and binaries without knowledge of the owner to the agents its a security risk

[Al2Klimov]

No, it's not.

[lippserd]

I think we should rather make this a promoted feature. It is cool stuff to be able to sync plugins, tickets, certs, whatever between nodes. Though this may be restricted on nodes with a new option like allow_files instead of just syncing everything right away. What do you think?

This has the same security risk as syncing check commands with the director and any other stuff. If one is able to gain root privileges with the icinga user, you're lost.

[Al2Klimov]

I think we should rather make this a promoted feature. It is cool stuff to be able to sync plugins, tickets, certs, whatever between nodes. Though this may be restricted on nodes with a new option like allow_files instead of just syncing everything right away. What do you think?

I disagree with you in all of these points.

[gethash]

I think we should rather make this a promoted feature. It is cool stuff to be able to sync plugins, tickets, certs, whatever between nodes. Though this may be restricted on nodes with a new option like allow_files instead of just syncing everything right away. What do you think?

This has the same security risk as syncing check commands with the director and any other stuff. If one is able to gain root privileges with the icinga user, you're lost.

Like the idea as well..

[Mikesch-mp]

lol

I think we should rather make this a promoted feature. It is cool stuff to be able to sync plugins, tickets, certs, whatever between nodes. Though this may be restricted on nodes with a new option like allow_files instead of just syncing everything right away. What do you think?

This has the same security risk as syncing check commands with the director and any other stuff. If one is able to gain root privileges with the icinga user, you're lost.

Its a good idea to start with, only make sure that the daemon cannot overwrite this settings with a command. maybe create a special (global) zone for such files ... there are many ways to make icinga better and more secure

[lippserd]

I don't see what makes Icinga less secure here. You either need access to the Director, the Icinga API or the icinga user in order to deploy and sync config (and other files). Everything you can do here, is also possible with the other methods as well. If you've managed to really exploit this, I'm eager to learn. Syncing arbitrary files is not an exploit per se. Plus, you have to be privileged anyway in order to deploy config (and other files).

[Mikesch-mp]

@lippserd just think about icinga as a service or in environemts that dont allow this behavior on agents of any kind (financial etc.)

[davekempe]

A least privilege model would suggest that config sync only syncs config, ideally as parsed objects. I agree this would make Icinga as a Service (IaaS) more palatable where you don't have full control of the agent endpoint. Customer wants monitoring using plugins they have installed, but not arbitrary binaries installed. accept_config and accept_commands should really only do just that. I am also in favour of a fully managed mode where accept_files would be implemented as a feature. Lowering the barrier to entry in complicated environments is a good thing for adoption, and I think Icinga should provide choice.

[Mikesch-mp]

Maybe reopen helps that this issue will not be forgotten, as the option is still named accept_configand not accept_everything

[dnsmichi]

@Mikesch-mp Here's a reply on the forum which can shed more light into the zero trust design of these config options. Specifically:

What is an example of a scenario where one would set accept_config = false ?

Security rationale, same as accept_commands. By default, no-one should be trusted, even with the endpoint/zone relationship is established. A more concrete scenario would be e.g. when an agent shouldn’t receive synced check commands or alike. Users don’t use that much, but enabling these flags by default would cause trouble since this was the default behavior since 2.0 and everyone knows about it.

[Mikesch-mp]

If it will be a feature you should also add #8182 and reenable sync of binaries

[netphantm]

I, for one, would welcome this feature, also w/ an addition. I have some scripts in the configuration because we have one staging server and two master servers in a cluster, so it's simpler to sync them between the hosts when they get modified. The other problem that I encountered is, that when icinga is synchronizing the configs with the following commands:

information/ApiListener: Updating configuration file: /var/lib/icinga2/api/zones/master//_etc/foo/scripts/mail-host-notification.sh
information/ApiListener: Updating configuration file: /var/lib/icinga2/api/zones/master//_etc/foo/scripts/tracker-host-notification.sh

the scripts lose the x-bit, so I cannot really use them from there w/o doing some other steps to get it set again.

[Zoe-openi]

This is still an issue to an extent, the binary sync is now optional, but syncing non configuration text files is still present. This would allow syncing malicious scripts and then executing them on clients. As already mentioned a parse based instead of file based sync would improve this, but malicious oneliners using basic *nix/windows system tools would still be possible.

To prevent this sort of exploit one possible solution would be to split the icinga/nagios user into a service user and a check user, with the check user being limited to a specific list of binaries/folders. These binaries and folders should not be changeable by the check user. Disallowing the check user Python and Bash would also be a good step to improve security in environments requiring this.

I do agree with the point that an attacker would need access to a master node and as such would likely not be the biggest worry at that point