LinuxQuestions.org

LinuxQuestions.org (/questions/)
-   Programming (http://www.linuxquestions.org/questions/programming-9/)
-   -   Pitching a script to find improvements and suggestions: Firewall and Network Auditor (http://www.linuxquestions.org/questions/programming-9/pitching-a-script-to-find-improvements-and-suggestions-firewall-and-network-auditor-917592/)

sudhagud 12-07-2011 12:21 PM

Pitching a script to find improvements and suggestions: Firewall and Network Auditor
 
Hello all,

Recently I had to write a script to gather and audit some network info from Linux servers. I would appreciate suggestions and improvement tweaks and further inclusions to make the script more robust.

- Bellamkonda Sudhakar

The script itself:

Code:

#!/bin/bash
##################################################################
#
# Script to check / audit systems process using network resources
# and report a detailed info from various system file, process and
# outputs
#
##################################################################
#
# Author: Bellamkonda Sudhakar
# License: GNU GPL v3
##################################################################
VERSION=0.5
##################################################################
AUDITFL=`hostname -s`
CHKPID="/var/run/netaudchk-ppid"
FLWPID="/var/run/netaudchk-fpid"
TMPFD="/tmp/netadtchk"
TMPFL1="tmpfl1"
TMPFL2="tmpfl2"
TMPFL3="tmpfl3"
OPSYS=`uname -o`
OPKER=`uname -s`
OPDST=`uname -r`
HSTNM=$AUDITFL
FLTC="tcpflen"
FWTSTIN="fwtest.in"
FETSTOUT="fwtest.out"
HNIC=`lspci | grep -ic ethernet`
VNIC=`ip -d link | grep ^[0-9] | awk '{ print $2 }' | sed 's/://' | grep -vi "lo" | grep -vc "eth"`
LNIC=`ip -d link | grep ^[0-9] | awk '{ print $2 }' | sed 's/://' | grep -ic "lo"`
TNIC=`expr $HNIC + $VNIC + $LNIC`
IP4E=`ip -d addr | grep -ic inet`
IP6E=`ip -d addr | grep -ic inet6`
TTCP=`ss -t | wc -l`
TTCPL=`ss -t -l | wc -l`
TTCPC=`ss -t -a | wc -l`
IPFW=`cat /proc/sys/net/ipv4/ip_forward`
IPMQ=`cat /proc/sys/net/ipv4/ip_dynaddr`
IPPROC=""
MJR=${OPDST:0:1}
MIR=${OPDST:2:1}
FWILBIN="None"
FWON="Off"
FWIN="False"
FWOT="False"
FWFW="False"
FWMQ="False"
NMSR=`grep nameserver /etc/resolv.conf | grep -v ^# | head -1 | awk '{ print $2 } '`
DFGW=`ip route | grep default | sed 's/default\|proto\|static//g'`
###################################################################
# Functions
function lpnt {
for r in {1..80..1}
do
  printf "%c" $1
done
printf "\n"
}
#
###################################################################
function cleanup {
rm -f $CHKPID
rm -f $FLWPID
}
###################################################################
function create_workspace {
mkdir $TMPFD
}
###################################################################
function remove_workspace {
rm -Rf $TMPFD
}
###################################################################
function pause {
echo -n "Press enter to continue..."
read
}
###################################################################
function line {
echo >> $TMPFD/$AUDITFL
}
###################################################################
#Main Program
if [ $UID -ne 0 ]
then
    echo
    echo "No root previleges"
    exit 0
fi
if [ "$1" == "-c" ]
then
  cleanup
  echo "PID files removed"
  exit 0
fi
if [ "$1" == "-h" ]
then
    echo
    echo "Usage: "
    echo "netadtchk [option]"
    echo "          -a All"
    echo "          -h Help"
    echo "          -c Clear PID files"
    echo "          no option prints the summery information to the stdout and into the /tmp/netadtchk/$AUDITFL file."
    echo
    exit 0
fi
if [ -f $CHKPID ]
then
    echo " Net Auditor running with PID `cat $CHKPID`"
    echo " To clear the PID files use the -c option"   
    exit 0
else
    echo $$ > $CHKPID
fi
#
#
if [ -d $TMPFD ]
then
    remove_workspace
    create_workspace
else
    create_workspace
fi
#
case "$MJR" in
    "2" )
        case "$MIR" in
            "0" | "1" )
                FWILBIN="ipfwadm";
                IPPROC="ip_[mfp][ao][rs][wqt][qha]*";;
            "2" | "3" )
                FWILBIN="ipchains";
                IPPROC="ip_[fm][wa][cns]*";;
            * )
                FWILBIN="iptables";
                IPPROC="ip_tables*";;
        esac;;
    "3" )
      FWILBIN="iptables";
      IPPROC="ip_table*";;
    * )
      FWILBIN="Unknown";;
esac
if [ $IP4E ]; then IPV="IPV4"; fi
if [ $IP6E ]; then IPV=$IPV"/IPV6"; fi
FWILPROC1=`ls /proc/net/$IPPROC 2>/dev/null | grep -c ip_tables`
FWILPROC2=`which $FWILBIN 2>/dev/null | grep -c $FWILBIN`
if [ $FWILPROC1 -ne 0 -a $FWILPROC2 -ne 0 ] ; then FWILSTAT="IP_Tables"; else FWILSTAT="None"; fi
#
if [ $FWILBIN == "iptables" ]
then
    $FWILBIN -L | grep -i chain > $TMPFD/$TMPFL1
    for C in `awk ' { print $2 } ' $TMPFD/$TMPFL1`
    do
      if [ $C == "INPUT" ] 
      then
          if [ `$FWILBIN -L $C | grep -wc 'all\|tcp\|udp'` -gt 0 ] ; then FWIN="True"; fi
      fi
      if [ $C == "OUTPUT" ]
      then
          if [ `$FWILBIN -L $C | grep -wc 'all\|tcp\|udp'` -gt 0 ] ; then FWOT="True"; fi
      fi
      if [ $C == "FORWARD" ]
      then
          if [ `$FWILBIN -L $C | grep -wc 'all\|tcp\|udp'` -gt 0 ] ; then FWFW="True"; fi
      fi
    done
fi
#
FWR=`$FWILBIN -L | sed '/^$/d' | grep -iv chain | grep -ivc "target\|source\|destination"`
if [ $FWR -gt 0 -a $FWIN == "True" ] ; then FWON="On"; fi
#
printf "%12s %12s %20s %20s %9s\n" "Hostname" "OS" "Kernel" "Release" "IPV" >> $TMPFD/$AUDITFL
printf "%12s %12s %20s %20s %9s\n\n" "$AUDITFL" "$OPSYS" "$OPKER" "$OPDST" "$IPV" >> $TMPFD/$AUDITFL
#
printf "%15s%12s%20s%20s%25s\n" "FW Binary" "Forwarding" "Masquerading" "1st Name Srv" "Default Gateway" >> $TMPFD/$AUDITFL
printf "%15s%12s%20s%20s%25s\n\n" "$FWILBIN" "$IPFW" "$IPMQ" "$NMSR" "$DFGW" >> $TMPFD/$AUDITFL
#
printf "%14s%14s%14s%14s%14s\n" "FW Status" "FW In" "FW Out" "FW Forward" "FW Masq" >> $TMPFD/$AUDITFL
printf "%14s%14s%14s%14s%14s\n" "$FWON" "$FWIN" "$FWOT" "$FWFW" "$FWMQ" >> $TMPFD/$AUDITFL
#
printf "\n%15s %15s %20s\n" "Type" "Name" "IP Details" >> $TMPFD/$AUDITFL
for N in `ip -d link | grep ^[0-9] | awk '{ print $2 }' | sed 's/://'`
do
  case "$N" in
        "lo" )
            TYPE="Loopback";;
        "eth0" | "eth1" | "eth2" | "eth3" | "eth4" | "eth5" )
            TYPE="Hardware";;
        * )
            TYPE="Virtual/Other"
  esac
  IPDET=`ifconfig $N | grep -w "inet"`
  printf "%15s %15s %s\n" "$TYPE" "$N" "$IPDET" >> $TMPFD/$AUDITFL
done
line
echo "Routes:" >> $TMPFD/$AUDITFL
netstat -rn >> $TMPFD/$AUDITFL
line
ss -nl4 | awk ' { print $3 } ' | grep -iv local | sed 's/*/0.0.0.0/g' > $TMPFD/$TMPFL1
ss -nl6 | awk ' { print $3 } ' | grep -iv local | sed 's/\(.*\)\:/\1 /' > $TMPFD/$TMPFL2
sed -i 's/\:/ /g' $TMPFD/$TMPFL1
sort $TMPFD/$TMPFL1 > $TMPFD/$TMPFL3
sort $TMPFD/$TMPFL2 >> $TMPFD/$TMPFL3
echo "This system has `expr $TTCPL - 1` Listening Sockets (IPv4/IPv6) at TCP/Ports: " >> $TMPFD/$AUDITFL
cp $TMPFD/$TMPFL3 $TMPFD/$FLTC
#
#
for P in `awk ' { print $2 } ' $TMPFD/$FLTC`
do
  PP=$PP$P,
  lsof -i tcp:$P | grep LISTEN | awk -v P=$P -v TMPFL=$TMPFD/$TMPFL3 ' { printf " %12s %12s %20s\n", P, $1, $2 >> TMPFL } '
done
echo "$PP details" >> $TMPFD/$AUDITFL
sort -g -k3 $TMPFD/$TMPFL3 > $TMPFD/$TMPFL2
awk -v TMPFL3=$TMPFD/$TMPFL3 ' BEGIN { }
            { if(cf != $2)
              printf "%12s %12s %20s\n", $1, $2, $3 > TMPFL3
              cf = $2 }
      END { } ' $TMPFD/$TMPFL2
line
lsof -i tcp:$PP | grep LISTEN | sort -d > $TMPFD/$TMPFL2
awk -v TMPFL1=$TMPFD/$TMPFL1 ' BEGIN { }
            { if(cf != $1)
              print $1, $2, $9 > TMPFL1
              cf = $1 }
            END { } ' $TMPFD/$TMPFL2
printf "%5s %10s %8s %30s %20s %s\n" "PID" "Process" "Port" "As per /proc" "As per Which" "Working Dir" >> $TMPFD/$AUDITFL
for ID in `awk ' { print $2 } ' $TMPFD/$TMPFL1`
do
    PROCEXE=`ls -l --full-time /proc/$ID/exe | awk ' { print $11 } '`
    PROCPWS=`ls -l --full-time /proc/$ID/cwd | awk ' { print $11 } '`
    PROC=`awk -v P=$ID ' { if (P == $2) print $1 } ' $TMPFD/$TMPFL1`
    WPROC=`which 2>/dev/null $PROC`
    P=`awk -v ID=$ID ' BEGIN { } { if (ID == $3) print $1 } END { } ' $TMPFD/$TMPFL3`
    printf "%5s %10s %8s %30s %20s %s\n" "$ID" "$PROC" "$P" "-$PROCEXE" "-$WPROC" "-$PROCPWS" >> $TMPFD/$AUDITFL
done
line
netstat -tulnp | grep LISTEN | awk ' { print $4, $7 } ' | sed 's/\(.*\)\:/\1 /' | sed 's/\// /g' > $TMPFD/$TMPFL1
printf "%20s %8s %8s %20s\n" "IP" "PORT" "PID" "PROCESS" >> $TMPFD/$AUDITFL
awk ' {  printf "%20s %8s %8s %20s\n", $1, $2, $3, $4 } ' $TMPFD/$TMPFL1 >> $TMPFD/$AUDITFL
line
lpnt "#" >> $TMPFD/$AUDITFL
##################################################################
## Print details from Audit file
clear
cat $TMPFD/$AUDITFL
echo "Complete Information is stored at $TMPFD/$AUDITFL"
if [ "$1" == "-a" ]
then
    echo "All TCP Listening Process Details" >> $TMPFD/$AUDITFL
    netstat -tunlp | grep -v udp >> $TMPFD/$AUDITFL
    line
    echo "ALL UDP Listening Process Details" >> $TMPFD/$AUDITFL
    netstat -tunlp | grep -v tcp >> $TMPFD/$AUDITFL
    line
    echo "Current established connections - NETSTAT" >> $TMPFD/$AUDITFL
    netstat -tunap >> $TMPFD/$AUDITFL
    line
    echo "Current established connections - SS" >> $TMPFD/$AUDITFL
    ss -o state established >> $TMPFD/$AUDITFL
    line
    lpnt "#" >> $TMPFD/$AUDITFL
fi
#
cleanup
exit 0
#
###################################################################


catkin 12-10-2011 11:42 AM

That's a lot of code to comment on and it looks pretty tidy; two reasons why nobody has commented?

As said, it's tidy code but some things you might like to consider:
  • $( ... ) is recommended over ` ... ` for reasons explained here.
  • [[ ... ]] is recommended over [ ... ] for reasons explained here.
  • A common but not universal convention is to use upper case variable names for environment variables and lower case names for the rest.
  • Some of the command output string manipulation looks as if it could be simplified. For example
    Code:

    ip -d link | grep ^[0-9] | awk '{ print $2 }' | sed 's/://' | grep -vi "lo" | grep -vc "eth"
    ... can be simplified to
    Code:

    ip -d link | grep -c '^[0-9]*: eth'
  • Errors conventionally result in a message to stderr and a non-zero return code. If you regard it as an error, this code
    Code:

    if [ $UID -ne 0 ]
    then
        echo
        echo "No root previleges"
        exit 0
    fi

    could be changed to
    Code:

    if [ $UID -ne 0 ]
    then
        echo "No root privileges" >&2
        exit 1
    fi

  • The single command line option could more conveniently be handled using a case statement, here including an error trap for an invalid option and assuming usage is moved to a function which gives verbose output if called with -v (and otherwise could suggest using the -h option):
    Code:

    case $1 in
        -a )
            echo 'Unserviced option -a' >&2
            exit 1
            ;;
        -c )
          cleanup
          echo "PID files removed"
          exit 0
          ;;
        -h )
          usage -v
          ;;
        * )
          echo "Invalid option $1" >&2
          usage
          exit 1
    esac

    More complex command line parsing, facilitating trapping invalid extra arguments is best handled by getopts.
  • Purely a matter of style, no right and no wrong way; it is more compact to use
    Code:

    if [[ <test expression> ]];then
        <commands>
    fi

  • Again purely a matter of style, no right and no wrong way; you may prefer, instead of
    Code:

    if [ $IP4E ]; then IPV="IPV4"; fi
    to use
    Code:

    [[ $IP4E ]] && IPV="IPV4"
    ... but beware; [[ <test expression> ]] && <commands 1> || <commands 2> can behave unexpectedly (if <commands 1> set a non zero return code, <commands 2> is executed).
  • If strings do not contain anything to be substituted by bash, they may be in single quotes, not double (for example "IPV4" above).
  • Aliases can make what are intended to be external commands behave unexpectedly. ls is often aliased and so troublesome. Safest for scripts to unset all aliases with unalias -a
  • In case the script's name is changed, the usage message could use ${0##*/} for its name.
  • The usage message could be done with a single echo (but is not then so easy to indent prettily):
    Code:

    echo '
    Usage: netadtchk [-a | -h | -c]
        -a All
        -h Help. Prints this usage message and exits
        -c Clear PID files"
        If no option is given, prints the summary information to the stdout and into the /tmp/netadtchk/$AUDITFL file
    '

    Note: the -a option is not serviced in the script.
  • Spelling: privilege and summary

asimba 12-10-2011 02:26 PM

I apologize - I need to learn quite q few things. Bear with me .

Still reading script - and catkin's insight was good. I am unsure about command

Code:

uname -o
I mean script if script is already meant for *nix why would you need that - that too if you intend to use it just once. I think a static value would have been good enough.


Following variable seems unused
Code:

TTCPC=`ss -t -a | wc -l`

unSpawn 12-11-2011 08:18 AM

Quote:

Originally Posted by sudhagud (Post 4544417)
I would appreciate suggestions and improvement tweaks and further inclusions to make the script more robust.

Moved: This thread is more suitable in the Programming forum since you're only interested in robustness (I would first look at existing functionality tools like GNU Tiger, LSAT or OpenVAS provide) and has been moved accordingly to help your thread/question get the exposure it deserves.

David the H. 12-11-2011 02:38 PM

  • You have a lot of unquoted variables in your script. While this is ok as long as their contents don't contain spaces or other reserved characters, if for some reason this should change, then you're going to have trouble.

    You should get into the habit of always quoting variable expansions by default, then you only have to worry about those few times when they need to be unquoted.

  • You make four individual calls to "ip". It might be more efficient if you call "id -d addr" only once and store its entire output into a variable. Then you could simply parse that for all the values you want.

  • As catkin pointed out, you have lots of useless uses of grep and related tools. If your command uses sed or awk, then you already have grep-like ability built in and generally shouldn't need it. Also, don't forget that grep and sed have -e options allowing you to apply multiple expressions at once.

    Finally, always quote your expressions to avoid conflicts with shell grammar.

    Code:

    #NMSR=`grep nameserver /etc/resolv.conf | grep -v ^# | head -1 | awk '{ print $2 } '`
    NMSR=$( awk '/nameserver/ { print $2 ; exit }' /etc/resolv.conf )

    #DFGW=`ip route | grep default | sed 's/default\|proto\|static//g'`
    DFGW=$( sed ' /default/ s/default\|proto\|static//g' < <( ip route ) )

  • BTW, the -1 syntax for head/tail is deprecated; you should use -n 1 instead.

  • Notice too in the above that process substitution can be used instead of a pipe.

  • As the guys at Greg's Wiki say: "expr is a relic of ancient Rome. Do not wield it". Bash has integer math ability built in.

    Code:

    #TNIC=`expr $HNIC + $VNIC + $LNIC`
    TNIC=$(( $HNIC + $VNIC + $LNIC ))

  • The < redirector works stand-alone inside a command substitution, so bash can import a file into a variable without using cat. And please use $(), as catkin mentioned. It's so much more readable and flexible.

    Code:

    #IPFW=`cat /proc/sys/net/ipv4/ip_forward`
    #IPMQ=`cat /proc/sys/net/ipv4/ip_dynaddr`
    IPFW=$( < /proc/sys/net/ipv4/ip_forward )
    IPMQ=$( < /proc/sys/net/ipv4/ip_dynaddr )

  • catkin stated that [[..]] is better than [..], but also, ((..)) is better than either when doing numerical evaluations.

    Code:

    #if [ $UID -ne 0 ]
    if (( UID != 0 ))
    #or even just...
    if (( UID ))


I could go on, but this post is getting to be a bit tiring to work on, so I think I'll stop here. ;)

grail 12-11-2011 08:51 PM

I believe the above is plenty enough for you to digest at the moment, so my only addition is a personal view.
The use of a function is generally tied to reducing the need to repeat the same lines of code several times.
You have functions like create_workspace which ultimately are only required at one point in the entire script,
so I probably would not have worried about creating a function for this example.

Look forward to your next revision :)

catkin 12-11-2011 11:28 PM

Quote:

Originally Posted by grail (Post 4547562)
The use of a function is generally tied to reducing the need to repeat the same lines of code several times.

Another reason for a function is to make the code more comprehensible; it has been suggested that no section of code should be more than X lines where X is what is visible on a screen (a lot bigger than it used to be!). Like all such "rules" it is a judgement call.

asimba 12-11-2011 11:44 PM

Its just me - I usually get impatient when OP forgets to respond/acknowledge.

sudhagud 12-12-2011 02:57 PM

that's a big WOW, Thanks a lot, in fact you can say that my BASH scripting knowledge is a bit of an ancient relic :) and I have used some of the suggestions in other scripts that i have been writing. I guess that I will rewrite the script and revise it with all there in mind.

I appreciate all you guys and especially Catkin and David the H.

@asimba : so that i can use this script in different *nix other than linux, i.e. freebsd etc. hopefully.

Thanks a lot.
Sudhagud
(Till the revised script :) )


All times are GMT -5. The time now is 02:21 AM.