Re: [PATCH 4/4] [V2] zabbix_agentd: Add IPFire specific userparameters

[Robin Roevens <robin.roevens@disroot.org>]

[-- Attachment #1: Type: text/plain, Size: 23004 bytes --]

Hi Michael


Michael Tremer schreef op do 15-04-2021 om 12:21 [+0100]:
> Hello,
> 
> > On 12 Apr 2021, at 23:16, Robin Roevens <
> > robin.roevens(a)disroot.org
> > > wrote:
> > 
> > Hi Michael
> > 
> > 
> > Michael Tremer schreef op ma 12-04-2021 om 11:36 [+0100]:
> > > Hello,
> > > 
> > > > On 7 Apr 2021, at 21:44, Robin Roevens <
> > > > robin.roevens(a)disroot.org
> > > > >
> > > > wrote:
> > > > ...
> > > > ...
> > > > diff --git a/config/zabbix_agentd/ipfire_services.pl
> > > > b/config/zabbix_agentd/ipfire_services.pl
> > > > new file mode 100755
> > > > index 000000000..dbf8aec56
> > > > --- /dev/null
> > > > +++ b/config/zabbix_agentd/ipfire_services.pl
> > > > @@ -0,0 +1,221 @@
> > > > +#!/usr/bin/perl
> > > > +##############################################################
> > > > ######
> > > > ###########
> > > > +# ipfire_services.pl - Retrieves available IPFire services
> > > > information and 
> > > > +#                      return this as a JSON array suitable
> > > > for easy
> > > > processing 
> > > > +#                      by Zabbix server
> > > > +#
> > > > +# Author: robin.roevens (at) disroot.org
> > > > +# Version: 1.0
> > > > +#
> > > > +# Based on: services.cgi by IPFire Team
> > > > +# Copyright (C) 2007-2021  IPFire Team  <
> > > > info(a)ipfire.org
> > > > > 
> > > > +# 
> > > > +# This program is free software: you can redistribute it
> > > > and/or
> > > > modify
> > > > +# it under the terms of the GNU General Public License as
> > > > published
> > > > by
> > > > +# the Free Software Foundation, either version 3 of the
> > > > License, or
> > > > +# (at your option) any later version.
> > > > +# 
> > > > +# This program is distributed in the hope that it will be
> > > > useful,
> > > > +# but WITHOUT ANY WARRANTY; without even the implied warranty
> > > > of
> > > > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
> > > > the 
> > > > +# GNU General Public License for more details.
> > > > +# 
> > > > +# You should have received a copy of the GNU General Public
> > > > License 
> > > > +# along with this program.  If not, see <
> > > > http://www.gnu.org/licenses/
> > > > >.
> > > > +# 
> > > > +##############################################################
> > > > ######
> > > > ###########
> > > > +
> > > > +use strict;
> > > > +
> > > > +# enable only the following on debugging purpose
> > > > +# use warnings;
> > > > +
> > > > +# Maps a nice printable name to the changing part of the pid
> > > > file,
> > > > which
> > > > +# is also the name of the program
> > > > +my %servicenames =(
> > > > +        'DHCP Server' => 'dhcpd',
> > > > +        'Web Server' => 'httpd',
> > > > +        'CRON Server' => 'fcron',
> > > > +        'DNS Proxy Server' => 'unbound',
> > > > +        'Logging Server' => 'syslogd',
> > > > +        'Kernel Logging Server' => 'klogd',
> > > > +        'NTP Server' => 'ntpd',
> > > > +        'Secure Shell Server' => 'sshd',
> > > > +        'VPN' => 'charon',
> > > > +        'Web Proxy' => 'squid',
> > > > +        'Intrusion Detection System' => 'suricata',
> > > > +        'OpenVPN' => 'openvpn'
> > > > +);
> > > 
> > > You have a hard-coded list with process names here. It
> > > technically is
> > > missing add-ons but including everything would make this list
> > > very long
> > > and a lot of work to maintain...
> > > 
> > 
> > As explained in my previous mail, I don't currently know of a
> > method to
> > get a list of mandatory IPFire specific services. So this list is a
> > straight copy out of services.cgi minus the translation.
> > The add-on services are looked up below, exactly like it is done in
> > services.cgi.
> 
> What list are you looking for? Just all processes that are running?

The purpose of my extension to the default is that a user can easily
configure Zabbix to monitor "IPFire"-specific functionalities. Generic
Linux metrics can be monitored by default Zabbix templates and agent
built-in functionality eventually extended with user-defined scripts or
modules. 

The IPFire webgui has a nice overview of all vital IPFire services and
their state, as well as for addon-services. 
A user can browse the status pages of the WebGUI to have a view of how
their IPFire machine is running
This is exactly what I want to provide to the Zabbix user, hence my
copy of services.cgi-code.
Alternatively, I could 'parse' the output of the services-page of the
webserver. But that is rather something one would do when trying to
monitor a black-box. 


> > > > +
> > > > +# Hash to overwrite the process name of a process if it
> > > > differs from
> > > > the launch command.
> > > > +my %overwrite_exename_hash = (
> > > > +        "suricata" => "Suricata-Main"
> > > > +);
> > > > +
> > > > +my $first = 1;
> > > > +
> > > > +print "[";
> > > > +
> > > > +# Built-in services
> > > > +my $key = '';
> > > > +foreach $key (sort keys %servicenames){
> > > > +       print "," if not $first;
> > > > +       $first = 0;
> > > > +
> > > > +       print "{";
> > > > +       print "\"service\":\"$key\",";
> > > > +
> > > > +       my $shortname = $servicenames{$key};
> > > > +       print &servicestats($shortname);
> > > > +       
> > > > +       print "}";
> > > > +}
> > > > +
> > > > +# Generate list of installed addon pak's
> > > > +my @pak = `find /opt/pakfire/db/installed/meta-* 2>/dev/null |
> > > > cut -
> > > > d"-" -f2`;
> > > > +foreach (@pak){
> > > > +       chomp($_);
> > > > +
> > > > +       # Check which of the paks are services
> > > > +       my @svc = `find /etc/init.d/$_ 2>/dev/null | cut -d"/"
> > > > -f4`;
> > > > +       foreach (@svc){
> > > > +               # blacklist some packages
> > > > +               #
> > > > +               # alsa has trouble with the volume saving and
> > > > was not
> > > > really stopped
> > > > +               # mdadm should not stopped with webif because
> > > > this
> > > > could crash the system
> > > > +               #
> > > > +               chomp($_);
> > > > +               if ( $_ eq 'squid' ) {
> > > > +                       next;
> > > > +               }
> > > > +               if ( ($_ ne "alsa") && ($_ ne "mdadm") ) {
> > > 
> > > Another hardcoded list due to code duplication. Not your fault,
> > > but not
> > > pretty.
> > > 
> > 
> > Indeed, as is probably clear by now, I too would prefer a more
> > clean
> > way of generating a list of services.
> > 
> > Currently seeing this piece of code again.. I think this blacklist
> > in
> > services.cgi only exists so the user can't stop/start the service
> > using
> > the webgui.. But there is no such functionality in this script.. so
> > I
> > think I can skip this blacklist, and also provide service state of
> > alsa
> > and mdadm.. (not sure why squid is here, is there an addon that
> > also
> > provides squid, maybe squid-accounting?)
> 
> The code seemed to have some trouble with hyphenated package names,
> that is why those checks were introduced.
> 
> It is pretty bad code and we should rework it some time, but
> definitely not copy it if we can avoid it.
> 
> > > > +                       print ",";
> > > > +                       print "{";
> > > > +
> > > > +                       print "\"service\":\"Addon: $_\",";
> > > > +                       print "\"servicename\":\"$_\",";
> > > > +
> > > > +                       my $onboot = isautorun($_);
> > > > +                       print "\"onboot\":$onboot,";
> > > > +
> > > > +                       print &addonservicestats($_);
> > > > +
> > > > +                       print "}";
> > > > +               }
> > > > +       }
> > > > +}      
> > > > +
> > > > +print "]";
> > > > +
> > > > +sub servicestats{
> > > > +       my $cmd = $_[0];
> > > > +       my $status =
> > > > "\"servicename\":\"$cmd\",\"state\":\"0\"";
> > > > +       my $pid = '';
> > > > +       my $testcmd = '';
> > > > +        my $exename;
> > > > +        my $memory;
> > > > +
> > > > +
> > > > +       $cmd =~ /(^[a-z]+)/;
> > > > +       
> > > > +       # Check if the exename needs to be overwritten.
> > > > +        # This happens if the expected process name string
> > > > +        # differs from the real one. This may happened if
> > > > +        # a service uses multiple processes or threads.
> > > > +        if (exists($overwrite_exename_hash{$cmd})) {
> > > > +                # Grab the string which will be reported by
> > > > +                # the process from the corresponding hash.
> > > > +                $exename = $overwrite_exename_hash{$1};
> > > > +        } else {
> > > > +                # Directly expect the launched command as
> > > > +                # process name.
> > > > +                $exename = $1;
> > > > +        }
> > > > +       
> > > > +       if (open(FILE, "/var/run/${cmd}.pid")){
> > > > +                $pid = <FILE>; chomp $pid;
> > > > +                close FILE;
> > > > +                if (open(FILE, "/proc/${pid}/status")){
> > > > +                        while (<FILE>){
> > > > +                                if (/^Name:\W+(.*)/) {
> > > > +                                        $testcmd = $1;
> > > > +                                }
> > > > +                        }
> > > > +                        close FILE;
> > > > +                }
> > > > +                if (open(FILE, "/proc/${pid}/status")) {
> > > > +                        while (<FILE>) {
> > > > +                                my ($key, $val) = split(":",
> > > > $_, 2);
> > > > +                                if ($key eq 'VmRSS') {
> > > > +                                       $val =~ /\s*([0-
> > > > 9]*)\s+kB/;
> > > > +                                       # Convert kB to B
> > > > +                                        $memory = $1*1024;
> > > > +                                        last;
> > > > +                                }
> > > > +                        }
> > > > +                        close(FILE);
> > > > +                }
> > > > +                if ($testcmd =~ /$exename/){
> > > > +                       $status =
> > > > "\"servicename\":\"$cmd\",\"state\":1,\"pid\":$pid,\"memory\":$
> > > > memory
> > > > ";
> > > > +               }
> > > > +        }
> > > > +        return $status;
> > > > +}
> > > > +
> > > > +sub isautorun{
> > > > +        my $cmd = $_[0];
> > > > +        my $status = "0";
> > > > +        my $init = `find /etc/rc.d/rc3.d/S??${cmd}
> > > > 2>/dev/null`;
> > > > +        chomp ($init);
> > > > +        if ($init ne ''){
> > > > +                $status = "1";
> > > > +        }
> > > > +        $init = `find /etc/rc.d/rc3.d/off/S??${cmd}
> > > > 2>/dev/null`;
> > > > +        chomp ($init);
> > > > +        if ($init ne ''){
> > > > +                $status = "0";
> > > > +        }
> > > > +
> > > > +        return $status;
> > > > +}
> > > > +
> > > > +sub addonservicestats{
> > > > +        my $cmd = $_[0];
> > > > +        my $status = "0";
> > > > +        my $pid = '';
> > > > +        my $testcmd = '';
> > > > +        my $exename;
> > > > +        my @memory = (0);
> > > > +
> > > > +        $testcmd = `sudo /usr/local/bin/addonctrl $_ status
> > > > 2>/dev/null`;
> > > > +
> > > > +        if ( $testcmd =~ /is\ running/ && $testcmd !~ /is\
> > > > not\
> > > > running/){
> > > > +                $status = "\"state\":1";
> > > > +
> > > > +                $testcmd =~ s/.* //gi;
> > > > +                $testcmd =~ s/[a-z_]//gi;
> > > > +                $testcmd =~ s/\[[0-1]\;[0-9]+//gi;
> > > > +                $testcmd =~ s/[\(\)\.]//gi;
> > > > +                $testcmd =~ s/  //gi;
> > > > +                $testcmd =~ s/^[//gi;
> > > > +
> > > > +                my @pid = split(/\s/,$testcmd);
> > > > +                $status .=",\"pid\":\"$pid[0]\"";
> > > > +
> > > > +                my $memory = 0;
> > > > +
> > > > +                foreach (@pid){
> > > > +                        chomp($_);
> > > > +                        if (open(FILE, "/proc/$_/statm")){
> > > > +                                my $temp = <FILE>;
> > > > +                                @memory = split(/ /,$temp);
> > > > +                        }
> > > > +                        $memory+=$memory[0];
> > > > +                }
> > > > +               $memory*=1024;
> > > > +                $status .=",\"memory\":$memory";
> > > > +        }else{
> > > > +                $status = "\"state\":0";
> > > > +        }
> > > > +        return $status;
> > > > +} 
> > > > diff --git a/config/zabbix_agentd/sudoers
> > > > b/config/zabbix_agentd/sudoers
> > > > index 1b362a4fd..340bb8e66 100644
> > > > --- a/config/zabbix_agentd/sudoers
> > > > +++ b/config/zabbix_agentd/sudoers
> > > > @@ -14,4 +14,4 @@
> > > > # Append / edit the following list of commands to fit your
> > > > needs:
> > > > #
> > > > Defaults:zabbix !requiretty
> > > > -zabbix ALL=(ALL) NOPASSWD: /opt/pakfire/pakfire status
> > > > +zabbix ALL=(ALL) NOPASSWD: /opt/pakfire/pakfire status,
> > > > /usr/local/bin/addonctrl, /sbin/iptables, /usr/sbin/fping
> > > 
> > > This is absolutely impossible to give the zabbix user control to
> > > start/stop any add-on and to read and change the entire iptables
> > > ruleset.
> > > 
> > > Zabbix is listening on the network and does not have any kind of
> > > authentication. Any security vulnerabilities in Zabbix would
> > > allow to
> > > alter the firewall rules and DoS any add-ons. This is dangerous.
> > 
> > There is some level of security: 
> 
> I agree that it is *some*, but is it enough?
> 
> > - the agent will only respond on requests from the configured
> > Zabbix
> > server (which the user has to configure in the zabbix agentd conf-
> > file,
> > and defaults to 127.0.0.1 as we can't possibly know the ip of the
> > user's server, so by default the agent responds to no one)
> 
> Host-based ACLs are not really a thing. On the local network faking a
> source IP address is one of the easiest jobs.
> 
> > - By default remote commands (as in requests to the agent to
> > execute
> > any command given) are disabled so the only way to make the agent
> > call
> > those binaries is to request a predefined userparameter that in
> > turn
> > will execute such a command with predefined params (see the
> > additional
> > template_...conf-files)
> > and on top of that, the user can add additional security by
> > configuring
> > encryption in the agent main configfile
> 
> Encryption is not authentication.
> 
> And the predefined parameters might be a thing, but my view is that
> zabbix can execute quite serious commands if it is being exploited.
> 
> The Zabbix Agent had exactly one of these vulnerabilities in 2009
> where an attacker could execute arbitrary commands:
> 
>   
> https://www.cvedetails.com/cve/CVE-2009-4502/
> 
> 
> > But as you point out, a possible vulnerability of some sort may
> > indeed
> > still pose risks this way.
> 
> I just find this risk insanely large. I am not going to tell people
> what software they can use and what not, but I want to avoid that the
> default configuration is adding major security issues to the entire
> system. And adding those sudo rules by default does exactly this.

I totally agree, I was only pointing out the *some* security there is,
but that will indeed never be sufficient to allow full access to
iptables or addonctrl by default. 
A user could, on his own risk, do that to maybe have the agent execute
firewall-actions as a reaction to a detected network attack or so.. But
that is not something we should provide by default.

> 
> What would a monitoring tool do with a dump of the iptables ruleset?
> I do not even understand where the use of this is.

The same as IPFire webgui currently does on netother.cgi?fwhits?day
by calling /sbin/iptables -vnxL <chain> | grep "\/\* <rule> \*\/" | awk
'{ print $2 }';: keep history of and eventually react on the number of
firewall-hits.

But you are completely right that access to iptables should be
restricted to just that what we want to get from it, which can be
achieved with a wrapper script as explained below in my previous mail.

Regards

Robin

> 
> > > Are there any alternative ways to realise this functionality?
> > 
> > As sudo is not able to allow commands with variable parameters
> > together
> > with fixed parameters, a popular solution is using wrapper scripts,
> > not
> > writable by the user, denying the user direct access to the
> > commands in
> > question.
> > 
> > /usr/local/bin/addonctrl is only called from within the
> > ipfire_services.pl script I provide, in the form of "addonctrl
> > <service> status". 
> > So I could add /etc/zabbix_agentd/scripts/ipfire_services.pl to the
> > sudo list instead so that the zabbix user won't be able to call
> > addonctrl directly. (making sure zabbix user has no write
> > permission on
> > that script)
> > The same goes for iptables. This is only called by the predefined
> > userparameter "ipfire.net.fw.hits[*]" defined in
> > template_module_ipfire_network_stats.conf in the form
> > "/sbin/iptables -
> > vnxL $1 | grep "\/\* $2 \*\/" | awk '{ print $$2 }';" where $1
> > (chain)
> > and $2 (rule) are parameters that have to be supplied to the agent
> > when
> > requesting that item. 
> > Here I can also supply a wrapper-script to be called  by sudo
> > /etc/zabbix_agentd/scripts/firewall_hits.sh that accepts those
> > parameters, and checks them for validity and against abuse.
> > 
> > Regards
> > Robin
> 
> -Michael
> 
> > > > diff --git
> > > > a/config/zabbix_agentd/template_module_ipfire_network_stats.con
> > > > f
> > > > b/config/zabbix_agentd/template_module_ipfire_network_stats.con
> > > > f
> > > > new file mode 100644
> > > > index 000000000..f1658ed07
> > > > --- /dev/null
> > > > +++
> > > > b/config/zabbix_agentd/template_module_ipfire_network_stats.con
> > > > f
> > > > @@ -0,0 +1,4 @@
> > > > +### Parameters for monitoring IPFire network statistics 
> > > > +UserParameter=ipfire.net.gateway.pingtime,sudo /usr/sbin/fping
> > > > -c 3
> > > > gateway 2>&1 | tail -n 1 | awk '{print $NF}' | cut -d '/' -f2
> > > > +UserParameter=ipfire.net.gateway.ping,sudo /usr/sbin/fping -q
> > > > -r 3
> > > > gateway; [ ! $? ]; echo $?
> > > > +UserParameter=ipfire.net.fw.hits[*],sudo /sbin/iptables -vnxL
> > > > $1 |
> > > > grep "\/\* $2 \*\/" | awk '{ print $$2 }';
> > > > diff --git
> > > > a/config/zabbix_agentd/template_module_ipfire_services.conf
> > > > b/config/zabbix_agentd/template_module_ipfire_services.conf
> > > > new file mode 100644
> > > > index 000000000..5f95218e3
> > > > --- /dev/null
> > > > +++ b/config/zabbix_agentd/template_module_ipfire_services.conf
> > > > @@ -0,0 +1,2 @@
> > > > +### Parameter for monitoring IPFire services
> > > > +UserParameter=ipfire.services,/etc/zabbix_agentd/scripts/ipfir
> > > > e_serv
> > > > ices.pl
> > > > diff --git a/lfs/zabbix_agentd b/lfs/zabbix_agentd
> > > > index 73e08d20a..c0d28d51f 100644
> > > > --- a/lfs/zabbix_agentd
> > > > +++ b/lfs/zabbix_agentd
> > > > @@ -33,7 +33,7 @@ DIR_APP    = $(DIR_SRC)/$(THISAPP)
> > > > TARGET     = $(DIR_INFO)/$(THISAPP)
> > > > PROG       = zabbix_agentd
> > > > PAK_VER    = 5
> > > > -DEPS       =
> > > > +DEPS       = "fping"
> > > > 
> > > > ###############################################################
> > > > ######
> > > > ##########
> > > > # Top-level Rules
> > > > @@ -97,6 +97,12 @@ $(TARGET) : $(patsubst
> > > > %,$(DIR_DL)/%,$(objects))
> > > >                 /etc/zabbix_agentd/zabbix_agentd.conf.ipfirenew
> > > >         install -v -m 644
> > > > $(DIR_SRC)/config/zabbix_agentd/template_app_pakfire.conf \
> > > >                 /etc/zabbix_agentd/zabbix_agentd.d/template_app
> > > > _pakfi
> > > > re.conf.ipfirenew
> > > > +       install -v -m 644
> > > > $(DIR_SRC)/config/zabbix_agentd/template_module_ipfire_network_
> > > > stats.
> > > > conf \
> > > > +               /etc/zabbix_agentd/zabbix_agentd.d/template_mod
> > > > ule_ip
> > > > fire_network_stats.conf.ipfirenew
> > > > +       install -v -m 644
> > > > $(DIR_SRC)/config/zabbix_agentd/template_module_ipfire_services
> > > > .conf
> > > > \
> > > > +               /etc/zabbix_agentd/zabbix_agentd.d/template_mod
> > > > ule_ip
> > > > fire_services.conf.ipfirenew
> > > > +       install -v -m 755
> > > > $(DIR_SRC)/config/zabbix_agentd/ipfire_services.pl \
> > > > +               /etc/zabbix_agentd/scripts/ipfire_services.pl.i
> > > > pfiren
> > > > ew
> > > > 
> > > >         # Create directory for additional agent modules
> > > >         -mkdir -pv /usr/lib/zabbix
> > > > diff --git a/src/paks/zabbix_agentd/install.sh
> > > > b/src/paks/zabbix_agentd/install.sh
> > > > index 4248a7ec1..ced915c81 100644
> > > > --- a/src/paks/zabbix_agentd/install.sh
> > > > +++ b/src/paks/zabbix_agentd/install.sh
> > > > @@ -66,8 +66,13 @@ restore_backup ${NAME}
> > > > # Put zabbix configfiles in place
> > > > setup_configfile /etc/zabbix_agentd/zabbix_agentd.conf
> > > > setup_configfile
> > > > /etc/zabbix_agentd/zabbix_agentd.d/template_app_pakfire.conf
> > > > +setup_configfile
> > > > /etc/zabbix_agentd/zabbix_agentd.d/template_module_ipfire_netwo
> > > > rk_sta
> > > > ts.conf
> > > > +setup_configfile
> > > > /etc/zabbix_agentd/zabbix_agentd.d/template_module_ipfire_servi
> > > > ces.co
> > > > nf
> > > > setup_configfile /etc/sudoers.d/zabbix
> > > > 
> > > > +# Overwrite script if it exists as user should not modify it
> > > > but it
> > > > is included in backup
> > > > +mv /etc/zabbix_agentd/scripts/ipfire_services.pl.ipfirenew
> > > > /etc/zabbix_agentd/scripts/ipfire_services.pl
> > > > +
> > > > if $review_required; then
> > > >         echo "WARNING: New versions of some configfile(s) where
> > > > provided as .ipfirenew-files."
> > > >         echo "         They may need manual review in order to
> > > > take
> > > > advantage of new features"
> > > > diff --git a/src/paks/zabbix_agentd/uninstall.sh
> > > > b/src/paks/zabbix_agentd/uninstall.sh
> > > > index 7a13880c5..ccbc8f7cf 100644
> > > > --- a/src/paks/zabbix_agentd/uninstall.sh
> > > > +++ b/src/paks/zabbix_agentd/uninstall.sh
> > > > @@ -26,6 +26,8 @@ stop_service ${NAME}
> > > > 
> > > > # Remove .ipfirenew files in advance so they won't be included
> > > > in
> > > > backup
> > > > rm -rfv /etc/zabbix_agentd/*.ipfirenew
> > > > /etc/zabbix_agentd/*/*.ipfirenew
> > > > +# Remove script-file as it should not have been modified by
> > > > user
> > > > +rm -fv /etc/zabbix_agentd/scripts/ipfire_services.pl
> > > > 
> > > > make_backup ${NAME}
> > > > remove_files
> > > > -- 
> > > > 2.30.2
> > > > 
> > > > 
> > > > -- 
> > > > Dit bericht is gescanned op virussen en andere gevaarlijke
> > > > inhoud door MailScanner en lijkt schoon te zijn.
> > > > 
> > > 
> > > -Michael
> > > 
> > > 
> > 
> > 
> > -- 
> > Dit bericht is gescanned op virussen en andere gevaarlijke
> > inhoud door MailScanner en lijkt schoon te zijn.
> 
> 


-- 
Dit bericht is gescanned op virussen en andere gevaarlijke
inhoud door MailScanner en lijkt schoon te zijn.