8
\$\begingroup\$

This is a program written in Python 3.x and Bash to check and find any users who have logged in as root via sudo su, sudo -i, and sudo bash, and report the username(s) and how many times they logged in as root. I am wondering if there is anything about my code that can be changed to make the program more efficient. One thing I hope to get rid of is the need for the bash script. I used bash because it was easier to find the specific info I needed, every know user on the system, and place it into a separate file fast and easily. I honestly couldn’t find out how to do that with Python.


root_login_search.py (main file/script)

import re
from datetime import datetime, timedelta
from itertools import islice
import subprocess
import os

N = 7 # determines number of days ago, -1, that will be scanned through in auth.log (Current day: March 29, variable: 8, will look at the logs taken on March 22 through March 29)

def root_users():
    N_days_ago = datetime.now() - timedelta(days=N) # determines what days in the auth.log will be scanned, starting from 7 to 0 days prior to the current day
    # the two lines below changes date to unix/linux format(e.g. March  1 "or" March 20)
    date2 = N_days_ago.strftime("%b  %-d")
    date1 = N_days_ago.strftime("%b %d")
    tmp = open("tmp.txt", "w+")

    with open("/var/log/auth.log", "r") as txt: 
        for line in txt:
            if re.match("^%s.+" % date1, line) or re.match("^%s.+" % date2, line): # 1. takes all lines in /var/log/auth.log that were made N days ago
                if "Successful su for root by root" in line: # this will identify users who used "sudo su" # 2. then takes all lines that say "Successful su..."
                    lines_gen = islice(txt, 2) # 3. and takes the 2 lines below it...
                    tmp.writelines(lines_gen) # 4. then writes them into the tmp.txt file
                if re.match("^.+COMMAND=/bin/bash$", line): # this will identify users who use "sudo bash" and "sudo -i"
                    tmp.writelines(line)
    tmp.close()

    subprocess.call("./users.sh") # calls to and executes users.sh

    users = [] # a list/array of all known users on the system
    with open("users", "r") as txt: # places all users that are in the users file into the users array
        for u in txt:
            users.append(u)

    login = False # at this moment, no one has been detected as a user who has logged in as root
    root_users = []
    # checks the tmp.txt file to see if any known users are named within it
    with open("tmp.txt", "r") as txt:
        for line in txt:
            for word in re.findall(r"\w+", line):
                if word != "root": # this makes sure that if root is in a line, which it will always be, it won't add root to the root_users array
                    if word + "\n" in users:
                        root_users.append(word)
                        login = True # someone has been detected as someone who logged in as root
                        break # this break is placed here to prevent accidental miscount of times a user logged into the root account. it also prevents users who did not log in as root to be falsely tagged.

    print("On " + date1 + ":")
    for u in users: # goes through the users array where u = a single known user
        x = 0 # tallies the total amount of times a single users logged into the root account
        t = 0 # keeps track of where variable r is in the root_users array and once it become the length of root_users, it starts back at the top for loop, moving onto the next user in users
        for r in root_users: # goes through the root_users array where r = a single user
            t += 1
            if u == r + "\n":
                x += 1
            if t == len(root_users):
                if x >= 1:
                    print("    " + u + "     became root " + str(x) + " times.")

    if login == False:
        print("    No one became root")

    os.remove("users")
    os.remove("tmp.txt")

for i in range(N):
    N -= 1 # every time root_users() has gone through its course, one date earlier in auth.log will be scanned (May 7th will be scanned, next May 8th will be scanned, etc.)
    root_users()

users.sh (sub file/script)

#!/bin/bash

USERS=`awk -F: '{ print $1 }' < /etc/passwd`

touch users
for u in $USERS; do
    echo $u >> users
done

Here is a picture of the program in action: program in action

\$\endgroup\$
5
  • \$\begingroup\$ Can you give some sample data? \$\endgroup\$
    – Azsgy
    Apr 2 '18 at 21:57
  • \$\begingroup\$ @Azsgy Like the outcome of the program? \$\endgroup\$ Apr 2 '18 at 22:00
  • \$\begingroup\$ No, the input data. That way it's clearer what you are doing and why. \$\endgroup\$
    – Azsgy
    Apr 2 '18 at 23:13
  • \$\begingroup\$ @Azsgy there is no inicial input data. You just call to the script sudo python3 root_login_search.py then it runs its course. Do you want me to explain step by step what happens when the code is executed? \$\endgroup\$ Apr 2 '18 at 23:32
  • \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, as doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. \$\endgroup\$ Apr 4 '18 at 20:07
3
\$\begingroup\$

The code assumes that we have write permissions to the current working directory. You seem to be under a misapprehension that the program can only be run from that directory. As an obvious counterexample, when I run /bin/bash, my working directory doesn't change to /bin. So it's not safe to assume that a program in $HOME will write to $HOME - nor that it's safe to modify a file called users that you find there.

It would be better to create the temporary files in the directory specified by environment variable TMPDIR is the de-facto standard environment variable to indicate where temporary files should be created. It's usually chosen to be somewhere that's writeable (obviously), reasonably fast, and not necessarily persistent (quite often it's a tmpfs or otherwise cleaned at boot, and some systems have a daemon to remove unused files).


The Bash script is strange - it could simply be a plain shell one-liner:

#!/bin/sh
exec awk -F: '{ print $1 }' </etc/passwd >>users

To be honest, I think that's better kept within the python, rather than mixing languages. Luckily, the Python standard library provides facilities to parse the passwd file for us:

import pwd
users = [u.pw_name for u in pwd.getpwall()]

I haven't fully inspected the Python code, but things like this stand out:

if login == False:

Most would write that simply as

if not login:

Also, why do we re-read the users list for each day we're looking at, rather than just once, outside the loop?


I don't see any need at all to read the full user list. Instead, just report the user names we've seen in the auth file. The advantage is that we no longer need to review the logs on the same (or equivalent, e.g. using NIS) system as generated them, and users that have been deleted, or never actually existed (obviously suspicious) can be displayed.

We only need to read the file once, if we keep a record of which users have become root on which days as we go through it. This is how I would do that:

import collections

from datetime import datetime, timedelta

N = 7   # how many days

def root_users():
    today = datetime.now().date()
    start_date = today - timedelta(days=N)
    this_year = datetime.now().year
    last_year = this_year - 1
    days = collections.defaultdict(collections.Counter)

    with open("/var/log/auth.log", "r") as txt: 
        for line in txt:
            fields = line.split()
            date_str = " ".join(fields[0:2]) + " "
            try:
                date = datetime.strptime(date_str + str(this_year), "%b %d %Y").date()
                if date > today: raise ValueError
            except ValueError:
                date = datetime.strptime(date_str + str(last_year), "%b %d %Y").date()

            if (date < start_date):
                # too old for interest
                continue
            # "user : TTY=tty/1 ; PWD=/home/user ; USER=root ; COMMAND=/bin/su
            if fields[4] == "sudo:":
                user=fields[5]
                if user != "root" and fields[-3] == "USER=root" and fields[-1] in ("COMMAND=/bin/bash", "COMMAND=/bin/sh", "COMMAND=/bin/su"):
                    days[date][user] += 1
            # "Successful su for root by user"
            if fields[4].startswith("su[") and fields[5] == "Successful" and fields[-3] == "root":
                user=fields[-1]
                if user != "root":
                    days[date][user] += 1


    while start_date <= today:
        print(start_date.strftime("On %b %d:"))
        users = days[start_date]
        if users:
            for user,count in users.items():
                print("    ", user, "became root", {1: 'once.', 2: 'twice.'}.get(count, str(count) + " times."))
        else:
            print("    No one became root")
        start_date += timedelta(days=1)

root_users()

You might need to tune the matchers for your system's date format and particular su and sudo log formats - check that the comments agree with what you see in your logs. It may well be worth extracting the matchers to functions of their own, which return either a username or None for each log line.

One thing you might consider to get consistent log lines is to stack a PAM module to generate exactly the logging you want (i.e. use that to filter instead of doing that here in the Python world). That's beyond the scope of this answer, but may be worth investigating.


Finally, we might need to look at old auth.log files (if logrotate has been at work, for example). Consider using fileinput.FileInput to iterate over some/all log files (you could consider only those whose last-modified time is at least start_date). If old files are compressed, you'll want to supply an appropriate openhook argument when creating the FileInput object.


Answering some specific questions you had about my version:

We never count "Successful su for root by root", because that's not a user transition (we already count users who sudo su, because su is one of the matched COMMAND strings for a sudo log line). We only count su when it's run by a non-root user.

days = collections.defaultdict(collections.Counter) is a defaultdict which maps objects (in our case dates) to counters. When we look up a date, we get back a Counter object for that date, a new one being created if necessary. The Counter is also a kind of dictionary, this time mapping user names to the number of times they have been seen in the log file for that date.

This block of code is to find the correct year for the date:

        try:
            date = datetime.strptime(date_str + str(this_year), "%b %d %Y").date()
            if date > today: raise ValueError
        except ValueError:
            date = datetime.strptime(date_str + str(last_year), "%b %d %Y").date()

We first guess that the date is this year. If the date is 29 February and this year is not a leap year, datetime.strptime() will throw a ValueError; we can catch that, and revise our guess to be last year. If the date is in the future, we also want to revise our guess to last year - it's a bit of a hack, but the shortest way to achieve this is to raise ValueError ourselves so we don't have an extra path through the logic.

When we iterate through the users for a particular day, we're reading from a Counter. Remember that this is a dictionary that maps user name to count of occurrences, so the key is the name, and the value is the count:

            for user,count in users.items():

And when we print the results, we could use + to concatenate the substrings into a single bigger string for printing, but by passing them as separate arguments, we save memory and processing time. (I then steal back some of the benefit by constructing the dictionary of once/twice/n_times just to pick one of its elements - that could be better moved to a function, really).

\$\endgroup\$
1
  • \$\begingroup\$ Comments are not for extended discussion; this conversation has been moved to chat. \$\endgroup\$ Apr 5 '18 at 14:27
4
\$\begingroup\$

Users list

I think you should completely bypass the shell script and use Python instead. We're in luck, the second example in the csv module documentation is about parsing a passwd file.

So you could use something like:

import csv
with open('passwd', newline='') as f:
    reader = csv.reader(f, delimiter=':', quoting=csv.QUOTE_NONE)
    users = [row[0] for row in reader]

That way you directly have the users list without resorting to an other file or script.

Temporary files

Use the tempfile module to handle temporary files cleanly; that allows you to ensure they're deleted. But in your case, you probably don't even need them and should store user lists and so in plain Python lists instead of external files.

Users count

Counting users could be simplified a lot:

# Suppose we have a list of known users:
users = ["abe", "barb", "charles"]
# And a list of users that became root:
root_users = ["abe", "abe", "barb", "abe", "barb"]
# The following then prints how many times each user became root
for u in users:
    count = root_users.count(u)
    if count:
        print(u, count)

I think this snippet should give you the idea.

General stuff

You should run a linter (i.e. flake8 or so), that would give you tips to help you make your code more readable. Nitpicking example: comments should be placed on a separate line when they're long enough.

Global variables are generally discouraged: you should redefine root_users() to take N as a parameter instead. While we're at it, you could read the passwd file only once before calling root_users(), and pass it the users array too !

There are lots of small improvements that will reduce your script's complexity. I probably haven't them all covered but I hope to get you started. Don't hesitate to ask for clarification, cheers !

\$\endgroup\$
9
  • \$\begingroup\$ I’ll try the user list and tempfile stuff, but the user account part does not work. First of all, it prints out ever single user, not only the ones who logged in as root. Second, it doesn’t correctly say how many times a single user logged in as root. \$\endgroup\$ Apr 4 '18 at 10:02
  • \$\begingroup\$ For the user list, rather follow @toby 's advice, I didn't even remember the pwd module he mentions in his answer, and that's a better solution. As for the user account part, that was meant as an example; if you want to filter out users that didn't log in, you can filter the results. I'll update my answer. If it doesn't show the correct count though, I might have made a wrong assumption on your root_users list. \$\endgroup\$
    – etene
    Apr 4 '18 at 10:21
  • \$\begingroup\$ Just like @toby it works but it does not. Recognizes that that someone logged in as root but it won’t give their username or the amount of times they became root. \$\endgroup\$ Apr 4 '18 at 10:34
  • \$\begingroup\$ I'm not entirely sure I understand, but is it possible that users don't match because of newline problems ? I saw some newline concatenation in your comparisons (r + "\n"), which made me think you could be comparing usernames with a trailing newline to usernames without it. \$\endgroup\$
    – etene
    Apr 4 '18 at 10:39
  • \$\begingroup\$ to get a better idea on how that part works, look at the last comments on @toby’s answer. I explained how they work together to get me the answer/correct output. \$\endgroup\$ Apr 4 '18 at 10:40

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.