HiveBrain v1.2.0
Get Started
← Back to all entries
patternbashMinor

Script to check for failed logins and then trawl users' .bash_history for keywords

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
scripttrawlloginsusersforfailedthenandkeywordscheck

Problem

I have created a script to check for failed logins and then trawl User .bash_history for keywords. There is also processing on the .bash_history files to convert epoch to human-readable timestamps.
I've run my script through shellcheck.net and the only real thing that it's complaining about are my For loops. It's saying they should be while loops instead, and I'm struggling to convert them.

Here's the script in its entirety:

#! /bin/bash
#

printf "\n"
echo -e "\e[93m*** Failed Login Checker ***\e[0m"
sleep 2
grep -E 'Failed password|invalid' /var/log/secure* | grep "sshd\[" | uniq
read -r -p "Press [Enter] key to continue..."

printf "\n"
echo -e "\e[93m*** BASH History Checker - script initiated ***\e[0m"
sleep 2

HOLDINGDIR=/root/check_history

if [ ! -d $HOLDINGDIR ];
then
        mkdir $HOLDINGDIR
fi

for i in $( find /home -name .bash_history -type f );
do
        cp "$i" $HOLDINGDIR/$( ls -la "$i" | awk '{print$3}' )_history
done

echo -e "\e[93mAll BASH History files copied - now processing date format...\e[0m"

for k in $( find $HOLDINGDIR -name '*_history' -type f );
do
        echo "Processing file: " "$k"
        sed -i -E 's/^#([0-9]+).*$/date -d @\1/e' "$k"
        chmod 755 "$k"
done

printf "\n"
echo -e "\e[93mAll BASH History files timestamps converted\e[0m"

printf "\n"
read -r -e -p "Enter search term: " -i "sudo su" search
echo "You typed: " "$search"
sleep 3
printf "\n"

for m in $( find $HOLDINGDIR -name '*_history' -type f );
do
        printf "\n"
        FILENAME=$m
        echo "-----------------------------------------------------------"
        echo "History file for: " "$FILENAME" | awk -F/ '{print$4}'
        echo "-----------------------------------------------------------"
        grep -F -B1 -i "$search" "$m"
        read -r -p "Press [Enter] key for next file..."
done

Solution

Don't embed terminal escapes in your strings

You don't know what terminal will be displaying your output - or even whether output will be going to a terminal. The standard tool to get the appropriate strings (if they exist) is tput. You'll want something like:

sgr0="$(tput sgr0)"
sgr93="$(tput sgr 93)"


which you can then use in your strings

echo -e isn't portable

Since you're using printf, you can conveniently use that when you don't want a newline:

printf "\n%s" "$sgr93*** Failed Login Checker ***$sgr0"


Pointless sleeping

sleep 2
read -r -p "Press [Enter] key to continue..."


These server only two purposes - annoy the user, and make it harder to use in a pipeline.

Unquoted variable expansion

Although the value you've given $HOLDINGDIR is currently safe to expand without quotes, I recommend you quote its expansion anyway, to give you the freedom to change it later (for example, you might want its value to incorporate $HOME instead of hard-coding /root).

No check that mkdir succeeded

If $HOLDINGDIR already exists as a regular file, or its parent directory doesn't exist or can't be written, then it won't be created, and we don't check that it succeeded. The easy way to deal with this is to make any failed command abort the script:

set -x


Unbounded find

Do you really mean to search recursively for .bash_history files? It probably makes more sense to look in each user's home directory only. And not all home directories are necessarily under /home. I'd be more inclined to iterate over awk -F: '{ print $6 "/.bash_history" }' /etc/passwd instead:

awk -F: '{ print $1, $6 "/.bash_history" }' /etc/passwd \
 | while read u f
   do if test -e "$f"
      then cp -v "$f" "$HOLDINGDIR/${u}_history"
      fi
   done


Replace copy+edit with a pipeline

You copy files and then process them with sed. It's much better to simply have sed take its input from the original file and send its output to the new file, so instead of the cp above we can just:

sed -e 's/^#([0-9]+).*$/date -d @\1/e' "$f" >"$HOLDINGDIR/${u}_history"


and remove the following for loop.

And drop the chmod 755: you don't really want to make the transformed history files executable and readable by everybody.

Interactive reading of search term

read -r -e -p "Enter search term: " -i "sudo su" search


Again, this makes it hard to run from anything other than a terminal. And if you mistype it, you have to start right from the beginning, seeking out all the history files again. Instead, you could provide the terms as positional arguments, then you can

for search in "$@"
do grep -FiH -B1 "$search" "$HOLDINGDIR"/*_history
done


Even better, you could search for all the search terms together, remembering that grep -F accepts a newline-separated list of search terms:

grep -FiH -B1 "$(printf '%s\n' "$@")" "$HOLDINGDIR"/*_history


Modified program

#!/bin/bash

set -e

if [ "$#" = 0 ]
then
    echo "No search terms specified!" >&2
    exit 1
fi

HOLDINGDIR=/root/check_history
test -d "$HOLDINGDIR" || mkdir "$HOLDINGDIR"

awk -F: '{ print $1, $6 "/.bash_history" }' /etc/passwd \
 | while read u f
   do if test -e "$f"
      then sed -e 's/^#([0-9]+).*$/date -d @\1 "+#%c"/e' "$f" >"$HOLDINGDIR/${u}_history"
      fi
   done

grep -FiH -B1 "$(printf '%s\n' "$@")" "$HOLDINGDIR"/*_history

Code Snippets

sgr0="$(tput sgr0)"
sgr93="$(tput sgr 93)"
printf "\n%s" "$sgr93*** Failed Login Checker ***$sgr0"
sleep 2
read -r -p "Press [Enter] key to continue..."
awk -F: '{ print $1, $6 "/.bash_history" }' /etc/passwd \
 | while read u f
   do if test -e "$f"
      then cp -v "$f" "$HOLDINGDIR/${u}_history"
      fi
   done
sed -e 's/^#([0-9]+).*$/date -d @\1/e' "$f" >"$HOLDINGDIR/${u}_history"

Context

StackExchange Code Review Q#156088, answer score: 5

Revisions (0)

No revisions yet.