Msec bugs

I have found the following bugs in Mandrake but am unable to report them now because I don’t have access to Bugzilla.

 

Bug one

 

Component: msec

 

The problem is this: the pattern matching in allow_root_login(arg) in /usr/share/msec/libmsec.py doesn’t work if the PermitRootLogin statement is not already in the /etc/ssh/sshd_config file.

 

Suggested solution: Update the pattern matching to recognize a commented out PermitRootLogin line and update it. Then, if no replacement is made, add the new configuration line to the bottom of the file.

 

Proof:

 

[root@web1 root]# msec 5

[root@web1 root]#

[root@web1 root]# grep -i /etc/ssh/sshd_config

 

[root@web1 root]# grep -i root /etc/ssh/sshd_config

#PermitRootLogin yes

[root@web1 root]# msec 2

[root@web1 root]#

[root@web1 root]# grep -i root /etc/ssh/sshd_config

#PermitRootLogin yes

[root@web1 root]#

[root@web1 root]# msec 5

[root@web1 root]#

[root@web1 root]# grep -i root /etc/ssh/sshd_config

#PermitRootLogin yes

[root@web1 root]#

[root@web1 root]#

[root@web1 root]# perl -i -pe ' s|#PermitRootLogin yes|PermitRootLogin no|'

 

[root@web1 root]# perl -i -pe ' s|#PermitRootLogin yes|PermitRootLogin no|' /etc/ssh/sshd_config

[root@web1 root]#

[root@web1 root]# grep -i root /etc/ssh/sshd_config

PermitRootLogin no

[root@web1 root]#

[root@web1 root]#

[root@web1 root]# msec 2

[root@web1 root]# grep -i root /etc/ssh/sshd_config

PermitRootLogin yes

[root@web1 root]#

[root@web1 root]# ssh root@localhost

root@localhost's password:

Last login: Wed Jun 26 12:32:04 2002 from localhost.localdomain

[root@web1 root]# exit

logout

 

Connection to localhost closed.

[root@web1 root]#

[root@web1 root]#

[root@web1 root]# msec 5

[root@web1 root]# grep -i root /etc/ssh/sshd_config

PermitRootLogin no

[root@web1 root]# ssh root@localhost

root@localhost's password:

Permission denied, please try again.

root@localhost's password:

Permission denied, please try again.

root@localhost's password:

 

[root@web1 root]#

[root@web1 root]#

 

 

Bug two

 

Component: msec

 

The problem is this: msec accept_icmp_echo(arg) in /usr/share/msec/libmsec.py only sets /etc/sysctl.conf and doesn’t make sure that /etc/sysctl.conf (or preferably just these values) is loaded into the actual kernel sysctl’s.

 

Suggested resolution: Set the actual sysctls in addition to updating /etc/sysctl.conf. I.e, perform “echo 1 > /proc/sys/net/ipv4/icmp_echo_ignore_all” and “echo 1 > /proc/sys/net/ipv4/icmp_echo_ignore_broadcasts”

 

Proof:

 

[root@web1 root]# msec 5

[root@web1 root]# grep echo /etc/sysctl.conf

net.ipv4.icmp_echo_ignore_all=1

net.ipv4.icmp_echo_ignore_broadcasts=1

[root@web1 root]# cat /proc/sys/net/ipv4/icmp_echo_ignore_all

0

[root@web1 root]# ping localhost

PING localhost.localdomain (127.0.0.1) from 127.0.0.1 : 56(84) bytes of data.

64 bytes from localhost.localdomain (127.0.0.1): icmp_seq=0 ttl=255 time=51 usec

64 bytes from localhost.localdomain (127.0.0.1): icmp_seq=1 ttl=255 time=27 usec

 

--- localhost.localdomain ping statistics ---

2 packets transmitted, 2 packets received, 0% packet loss

round-trip min/avg/max/mdev = 0.027/0.039/0.051/0.012 ms

[root@web1 root]#

[root@web1 root]# msec 2

[root@web1 root]# grep echo /etc/sysctl.conf

net.ipv4.icmp_echo_ignore_all=0

net.ipv4.icmp_echo_ignore_broadcasts=0

[root@web1 root]# cat /proc/sys/net/ipv4/icmp_echo_ignore_all

0

[root@web1 root]#

[root@web1 root]# msec 5

[root@web1 root]# grep echo /etc/sysctl.conf

net.ipv4.icmp_echo_ignore_all=1

net.ipv4.icmp_echo_ignore_broadcasts=1

[root@web1 root]# cat /proc/sys/net/ipv4/icmp_echo_ignore_all

0

[root@web1 root]# cat /proc/sys/net/ipv4/icmp_echo_ignore_broadcasts

0

[root@web1 root]#

[root@web1 root]# ping localhost

PING localhost.localdomain (127.0.0.1) from 127.0.0.1 : 56(84) bytes of data.

64 bytes from localhost.localdomain (127.0.0.1): icmp_seq=0 ttl=255 time=57 usec

64 bytes from localhost.localdomain (127.0.0.1): icmp_seq=1 ttl=255 time=28 usec

64 bytes from localhost.localdomain (127.0.0.1): icmp_seq=2 ttl=255 time=25 usec

 

--- localhost.localdomain ping statistics ---

3 packets transmitted, 3 packets received, 0% packet loss

round-trip min/avg/max/mdev = 0.025/0.036/0.057/0.016 ms

[root@web1 root]#

[root@web1 root]# grep /etc/sysctl.conf /etc/rc.d/init.d/network

        action "Setting network parameters: " sysctl -e -p /etc/sysctl.conf

[root@web1 root]# sysctl -e -p /etc/sysctl.conf

[root@web1 root]#

[root@web1 root]# cat /proc/sys/net/ipv4/icmp_echo_ignore_all

1

[root@web1 root]# cat /proc/sys/net/ipv4/icmp_echo_ignore_broadcasts

1

[root@web1 root]# ping localhost

PING localhost.localdomain (127.0.0.1) from 127.0.0.1 : 56(84) bytes of data.

 

--- localhost.localdomain ping statistics ---

2 packets transmitted, 0 packets received, 100% packet loss

[root@web1 root]#

 

Note: this bug also applies to accept_bogus_error_responses(arg) and allow_reboot(arg) and enable_ip_spoofing_protection() and enable_log_strange_packets(arg).

 

Note: I found the appropriate code commented out in /usr/share/msec/libmsec.py:

 

#ConfigFile.add_config_assoc(SYSCTLCONF, '/sbin/sysctl -e -p /etc/sysctl.conf; service network restart')

 

I don’t think the “service network restart” is required.

 

I wonder if not updating the sysctl is the desired behavior for some reason. If so, you might want to document this.

 

Bug three

 

Component: msec

 

In /usr/share/msec/promisc_check.sh logging is never done to a TTY because the configuration variable TTYLOG_WARN is used instead of the correct TTY_WARN.

 

Patch:

 

I’ve updated Ttylog to use the same syntax for grepping the root TTY list as is used in Ttylog() from /usr/share/msec/security.sh.

 

--- /usr/share/msec/promisc_check.sh    Fri Feb 22 08:23:18 2002

+++ /tmp/test_promisc_check.sh  Thu Jun 27 06:16:23 2002

@@ -10,8 +10,8 @@

 }

 

 Ttylog() {

-    if [[ ${TTYLOG_WARN} == yes ]]; then

-       w | grep -v "load\|TTY" | grep '^root' | awk '{print $2}' | while read line; do

+    if [[ ${TTY_WARN} == yes ]]; then

+    for i in `w | grep -v "load\|TTY" | grep '^root' | awk '{print $2}'` ; do

             echo -e "${1}" > /dev/$i

         done

     fi

 

Bug four

 

Component: msec documentation

 

The feature of writing data to /var/security.log is presented as a configuration option. It is not a configuration option. The scripts that write to /var/security.log write to the location regardless of any configuration. It only matters if these scripts are running.

 

These are the scripts that write to /var/security.log:

 

/usr/share/msec/promisc_check.sh

/usr/share/msec/diff_check.sh

/usr/share/msec/security_check.sh

 

If you look closely you will notice that there is no branching to make it optional.

 

Here is how the documentation seems misleading:

 

In http://www.mandrakesecure.net/en/docs/msec.php, “Warnings in security.log” is listed along with all the other configuration options.

 

In /usr/share/doc/msec-0.19/security.txt the feature, “Warning in /var/log/security.log” is listed along with all the other features.

 

I consider this a bug, because I spent some time trying to hunt down where I could customize this behavior of syslog logging in “man mseclib” only to find that it wasn’t settable (like everything else listed along with it).

 

Notice that in the table listed under “Doc of the rewritting in python:” in /usr/share/doc/msec-0.19/README, the feature “Warnings in security.log” is not listed, but for some reason it shows up in the same table in http://www.mandrakesecure.net/en/docs/msec.php.

 

Bug five

 

Component: msec documentation

 

In http://www.mandrakesecure.net/en/docs/msec.php “Warnings in syslog” has been added to the first table. The feature is really the same as SYSLOG_WARN in the second table. It shouldn’t be listed twice.

 

Also note that “Warnings in syslog” is not listed in the table under “Doc of the rewritting in python:” in /usr/share/doc/msec-0.19/README (which is what the first table in http://www.mandrakesecure.net/en/docs/msec.php was derived from). For some reason it got added.

 

Bug six

 

Component: msec

 

In enable_ip_spoofing_protection() in /usr/share/msec/libmsec.py, the net.ipv4.conf.all.rp_filter setting is only set to 1 when enabling spoofing protection and never set back to 0 when disabling spoofing protection.

 

Proof:

 

[root@web1 root]# msec 2

[root@web1 root]# grep rp_filter /etc/sysctl.conf

net.ipv4.conf.default.rp_filter = 1

net.ipv4.conf.all.rp_filter=1

[root@web1 root]#

[root@web1 root]# msec 5

[root@web1 root]# grep rp_filter /etc/sysctl.conf

net.ipv4.conf.default.rp_filter = 1

net.ipv4.conf.all.rp_filter=1

[root@web1 root]#

[root@web1 root]# msec 2

[root@web1 root]# grep rp_filter /etc/sysctl.conf

net.ipv4.conf.default.rp_filter = 1

net.ipv4.conf.all.rp_filter=1

 

Patch (untested):

 

--- /usr/share/msec/libmsec.py  Fri Mar  8 13:41:21 2002

+++ /usr/share/msec/libmsec.py.fix_bug_six      Thu Jun 27 07:31:01 2002

@@ -427,6 +427,8 @@

         _interactive and log(_('Disabling ip spoofing protection'))

         hostconf.remove_line_matching('nospoof')

         hostconf.remove_line_matching('spoofalert')

+        sysctlconf = ConfigFile.get_config_file(SYSCTLCONF)

+        sysctlconf.set_shell_variable('net.ipv4.conf.all.rp_filter', 0)

 

 def accept_icmp_echo(arg):

     '''   Accept/Refuse icmp echo.'''

 

 Bug seven (or perhaps “enhancement request”)

 

Component: msec

 

The configuration enable_ip_spoofing_protection(arg, alert=1) sets two different “anti-spoofing measures” that may be needed separately in some cases.

 

It sets “nospoof on” in /etc/host.conf which relates to reverse DNS lookups.

 

It also sets net.ipv4.conf.all.rp_filter in /etc/sysctl.conf.

 

About this setting /usr/src/linux/Documentation/networking/ip-sysctl.txt says: “do source validation by reversed path, as specified in RFC1812. Recommended option for single homed hosts and stub network routers. Could cause troubles for complicated (not loop free) networks running a slow unreliable protocol (sort of RIP), or using static routes.

 

(Note: /usr/src/linux-2.4.18-6mdk/freeswan-1.95/INSTALL and /usr/src/linux-2.4.18-6mdk/freeswan-1.95/utils/TODO seem to indicate that there can be some interaction between FreeS/Wan and the rp_filter sysctl.)

 

Because the sysctl can potentially cause problems for non-cut-and-dry networks, I’d suggest it be a separate option so that it can be disabled separately from the reverse-DNS anti-spoofing.

 

Remember, msec has an important responsibility. In higher security levels it stomps on any changes manually made to the underlying files. (For example, if a sysadmin disables the sysctl in /etc/sysctl.conf to solve their routing problem, msec will gladly set it back to the original settting.) This configuration must now be done through msec, or msec must be disabled. Therefore, IMO, msec has a responsibility to be fully configurable, otherwise it takes power away from the admin.

 

Bug eight

 

Component: msec

 

Overriding the default configuration for promisc_check.sh is a real pain and requires calling two independent functions to set one setting. Each function is useless without the other. They should be one function.

 

First, enable_promisc_check(arg) has to be run so that /etc/cron.d/msec is updated to call the /usr/share/msec/promisc_check.sh script every minute.

 

Then, in /var/lib/msec/security.conf the config CHECK_PROMISC has to be adjusted.

 

So, in /etc/security/msec/level.local someone would have to write to commands to effect this one setting like below.

 

To enable:

 

enable_promisc_check(1)

set_security_conf('CHECK_PROMISC', 'yes')

 

To disable:

 

enable_promisc_check(0)

set_security_conf('CHECK_PROMISC', 'no')

 

(Of course, depending on the existing msec only one of these commands will do the trick. But then you change base msec level and it’s anybody’s guess. This is a real trap.)

 

Proposed solution:

 

I suggest removing the CHECK_PROMISC configuration option from /var/lib/msec/security.conf. Have enable_promisc_check(arg) be the one place to set this configuration. (Remember to update the documentation!)

 

Bug nine

 

Component: msec

 

The documentation for the default settings for allow_x_connections() is incorrect.

 

The code in /usr/share/msec/msec.py says:

 

if level != 0:

    enable_security_check(1)

    enable_password(1)

    if level < 3:

        allow_x_connections(LOCAL, 1)

    else:

        if level == 3:

            allow_x_connections(NONE, 1)

        else:

            allow_x_connections(NONE, 0)

else:

    enable_security_check(0)

    enable_password(0)

    allow_x_connections(ALL, 1)

 

From that code I get this chart of the arguments for allow_x_connections():

 

 

Level 0

Level 1

Level 2

Level 3

Level 4

Level 5

allow_x_connections(arg, listen_tcp)

 

(all, 1)

(local, 1)

(local, 1)

(none, 1)

(none, 0)

(none, 0)

 

 

The source code agrees with /usr/share/doc/msec-0.19/README:

 

                                0       1       2       3       4       5

allow X connections             yes     local   local   no      no      no

 

The source code disagrees with /usr/share/doc/msec-0.19/security.txt:

 

# perl -ne ' print if /security level|x display|x server/i ' /usr/share/doc/msec-0.19/security.txt

Security level 0 :

- everybody authorized to connect to X display.

Security level 1 :

- localhost authorized to connect to X display.

Security level 2 ( Aka normal system ) :

- localhost authorized to connect to X display.

Security level 3  ( Aka more secure system ) :

- localhost authorized to connect to X display.

Security level 4 ( Aka Secured system ) :

- localhost authorized to connect to X display.

- X server doesn't listen for tcp connections

Security level 5 ( Aka Paranoid system ) :

- X server doesn't listen for tcp connections

 

The source code also disagrees with http://www.mandrakesecure.net/en/docs/msec.php:

 

 

Level 0

Level 1

Level 2

Level 3

Level 4

Level 5

Allow X TCP Connections

yes

local

local

local

no

no

 

Connect to X Display

all

localhost

localhost

localhost

localhost

no

 

I presume that “Allow X TCP Connections” is the first argument to allow_x_connections() and that “Connect to X Display” is the second argument to Connect to X Display(). The exact meaning of these terms is vague and should be clarified.

 

I suggest the following:

 

 

Level 0

Level 1

Level 2

Level 3

Level 4

Level 5

 

Hosts allowed to make connections to the X server (think xhost)

Sets: first argument of allow_x_connections

 

yes

local

local

none

no

no

 

X server listens for tcp connections

Sets: second argument of allow_x_connections

1

1

1

1

0

0

 

Bug 10

 

Component: msec

 

The setting for what msec levels allow “.” in $PATH is hard coded into /etc/profile.d/msec.sh and /etc/profile.d/msec.csh, which get the msec security level from the SECURE_LEVEL variable in /etc/sysconfig/msec. This means this setting can’t be overridden. The creation of /etc/issue and /etc/issue.net is also hard coded in /etc/rc.d/rc.local and also can’t be overridden.

 

Here is a complete list of hard coded settings (where SECURE_LEVEL directly effects a setting without opportunity for override) that I was able to find:

 

/etc/profile.d/msec.csh

Controls “.” In $PATH

/etc/profile.d/msec.sh

Controls “.” In $PATH

/etc/rc.d/rc.local

Controls creation of /etc/issue and /etc/issue.net.

(This means that overriding allow_issues(arg) may have problems.)

/usr/lib/libDrakX/any.pm

Uses SECURE_LEVEL but I’m not sure for what. This file belongs to package drakxtools-newt-1.1.7-97mdk.

 

Bug 11

 

Component: msec

 

There is a race condition with the /var/run/msec.pid lockfile in this code from /usr/sbin/msec:

 

LCK=/var/run/msec.pid

 

function cleanup() {

    rm -f $LCK

}

 

if [ -f $LCK ]; then

    if [ -d /proc/`cat $LCK` ]; then

        exit 0

    else

        rm -f $LCK

    fi

fi

 

echo -n $$ > $LCK

 

trap cleanup 0

 

Suggested solution:

 

Change /usr/sbin/msec to python code so that you can use fock and a real lockfile

 

Note: the same race condition exists in /usr/share/msec/security.sh for /var/run/msec-security.pid.

 

Bug 12

 

Component: msec

 

Many of the checks will not be performed unless enable_security_check(1) is set and CHECK_SECURITY is true. For those who are considering overriding the default levels, this should be pointed out in the documentation.

 

The CHECK_SECURITY option seems somewhat pointless as it is duplicated by enable_security_check().

 

(Although, when enable_security_check(1) is set and CHECK_SECURITY is false /usr/share/msec/security.sh still gathers much of the information used in the checks and the diffs, but no reporting is done. This may be a feature. I’m not sure how to use it. If this is a feature, documenting this would be good.)

 

Here are the dependencies as I have found them:

 

 

Requires enable_security_check(1)

Requires CHECK_SECURITY

CHECK_SECURITY

1

n/a

CHECK_PERMS

1

1

CHECK_SUID_ROOT

1

1

CHECK_SUID_MD5

1

1

CHECK_SUID_GROUP

1

1

CHECK_WRITEABLE

1

1

CHECK_UNOWNED

1

1

CHECK_PROMISC

0

0

CHECK_OPEN_PORT

1

1

CHECK_PASSWD

1

1

CHECK_SHADOW

1

1

TTY_WARN

n/a

n/a

MAIL_WARN

n/a

n/a

SYSLOG_WARN

n/a

n/a

RPM_CHECK

1

1

CHKROOTKIT_CHECK

1

1

 

 

Contribution 1

 

Component: msec

 

msec_pem_to_table.pl – A script that creates an HTML summary report of the file permission settings for different security levels so that the settings are easy to compare. Currently, this information is stored by msec in a different file for each level (without any summary documentation!), so it is hard to compare the levels and know which one does what.

 

Here is the source code:

 

#!/usr/bin/perl -w

 

use strict;

 

use Data::Dumper;

 

 

my $file_prefix = "/usr/share/msec/perm.";

 

my @levels = 1 .. 5;

 

my $data = { map { ($_ => get_data("$file_prefix$_") ) } @levels };

 

#print Dumper($data);

#exit 1;

 

my @files = sort keys %{{ map { ($_ => 1) } map { keys %$_ } values %$data }};

 

print "<table>\n";

 

print "<tr>\n";

print map { "<th>$_</th>\n" } ("filename", @levels, "rpm");

print "</tr>\n";

 

my $even = 1;

foreach my $file ( @files ) {

 

        print "<tr>\n";

        print "<td>$file</td>\n";

 

        my $rpm_data = get_rpm_data($file);

 

        my $color = 1;

        my $last_point;

        foreach my $level ( @levels ) {

                my $this_point = $data->{$level}{$file};

                my $change =

                        defined($last_point) &&

                        fileattr_to_html($this_point) ne fileattr_to_html($last_point);

                $color = ! $color if ( $change );

                my $color_code =

                        $even

                        ? ( $color ? "ddcccc" : "ffeeee" )

                        : ( $color ? "ccccdd" : "eeeeff" );

                print

                        "<td nowrap bgcolor='#$color_code'>" .

                        fileattr_to_html($this_point) .

                        "</td>\n";

                $last_point = $this_point;

        }

 

        print

                "<td nowrap>" .

                fileattr_to_html($rpm_data) .

                "</td>\n";

 

        $even = not $even;

        print "</tr>\n";

}

 

print "</table>\n";

 

sub get_data

{

        my $filename = shift;

 

        open(FH, "<$filename") or die "unable to open file $filename: $!";

        my $data = {

                map {

                        chomp;

                        my @ii = split(/\s+/, $_);

                        ( $ii[0] => [ $ii[1], $ii[2] ] );

                }

                grep { not /^\s*#/ }

                <FH>

        };

        close(FH);

        return $data;

}

 

sub get_rpm_data

{

        my $filename = shift;

        $filename =~ s|/\.$||;

        $filename =~ s|/+$|| if ( $filename ne "/" );

        open (FH, "rpm -qlf $filename --dump |") or die "unable to open pipe to rpm: $!";

        my @aa = grep { $_->[0] eq $filename } map { [ split(/ /, $_) ] } <FH>;

        close(FH);

        return [ undef, undef ] if ( not @aa );

        return [ $aa[0][5] . "." . $aa[0][6], sprintf("%lo", oct($aa[0][4]) & 07777) ];

}

 

sub fileattr_to_html

{

        my $this_point = shift;

        return

                defined($this_point->[0])

                ? "$this_point->[0]<br>$this_point->[1]"

                : "<i>nodata</i>";

 

}

 

Bug 13

 

In /usr/share/msec/libmsec.py in function password_aging(), the code that parses the output from the chage command dose not accept negative “Maximum” settings. Somehow (I have no idea!) I got a user on my system, “bob”, that had -1 for the Maximum setting and msec started throwing fatal errors.

 

You can see that msec gives an error here and is unable to set the password expiration details for that user. When I change the maximum setting for that user manually to a non-negative, msec is then able to run without error.

 

[root@fermi root]# chage -l bob

Minimum:        -1

Maximum:        -1

Warning:        -1

Inactive:       -1

Last Change:            Feb 13, 2002

Password Expires:       Never

Password Inactive:      Never

Account Expires:        Never

[root@fermi root]#

[root@fermi root]# grep bob /etc/shadow /etc/passwd

/etc/shadow:bob:$1$13614916$AAAAAAAAAA.AAAAAAAAAA0:11731::::::

/etc/passwd:bob:x:506:506:Bob Harris:/home/bob:/bin/sh

[root@fermi root]#

[root@fermi root]#          

[root@fermi root]# msec

msec: unable to parse chage output

[root@fermi root]# chage -M 99999 bob

[root@fermi root]#

[root@fermi root]# chage -l bob

Minimum:        -1

Maximum:        99999

Warning:        -1

Inactive:       -1

Last Change:            Feb 13, 2002

Password Expires:       Never

Password Inactive:      Never

Account Expires:        Never

[root@fermi root]# grep bob /etc/shadow /etc/passwd

/etc/shadow:bob:$1$13614916$AAAAAAAAAA.AAAAAAAAAA0:11731::99999::::

/etc/passwd:bob:x:506:506:Bob Harris:/home/bob:/bin/sh

[root@fermi root]#

[root@fermi root]# msec

[root@fermi root]#

 

Here is an untested patch:

 

--- /usr/share/msec/libmsec.py  Fri Mar  8 13:41:21 2002

+++ /usr/share/msec/libmsec.py.fixchange        Fri Jul 12 08:06:38 2002

@@ -543,7 +543,7 @@

         cronallow.replace_line_matching('root', 'root', 1)

         atallow.replace_line_matching('root', 'root', 1)

 

-maximum_regex = re.compile('^Maximum:\s*([0-9]+)', re.MULTILINE)

+maximum_regex = re.compile('^Maximum:\s*(-?[0-9]+)', re.MULTILINE)

 inactive_regex = re.compile('^Inactive:\s*(-?[0-9]+)', re.MULTILINE)

 

 # TODO FL Sat Dec 29 20:18:20 2001