From mboxrd@z Thu Jan  1 00:00:00 1970
From: Michael Tremer <michael.tremer@ipfire.org>
To: development@lists.ipfire.org
Subject: Re: [PATCH] snort: Also monitor assigned alias addresses on red.
Date: Fri, 16 Oct 2015 17:39:22 +0100
Message-ID: <1445013562.18375.93.camel@ipfire.org>
In-Reply-To: <1445013436.2021.22.camel@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============2365972106316805647=="
List-Id: <development.lists.ipfire.org>

--===============2365972106316805647==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit

On Fri, 2015-10-16 at 18:37 +0200, Stefan Schantl wrote:
> > On Fri, 2015-10-16 at 12:09 +0200, Timo Eissler wrote:
> > > Reviewed-by: Timo Eissler <timo.eissler(a)ipfire.org>
> > 
> > I don't think that this patch is okay.
> > 
> > > Am 16.10.2015 um 11:41 schrieb Stefan Schantl:
> > > > These changes will allow snort to also inspect the traffic for
> > > > one or more configured alias addresses, which has not been done
> > > > in
> > > > the past.
> > 
> > What consequences did that have? What does this patch change? Is
> > anything of that user-visible or breaking backward-compatibility?
> 
> The current situation is, that snort if enabled on red, only inspects
> the traffic which is desired to the statically configured red
> address.
> 
> If some alias addresses have been assigned to the red interface the
> traffic to these addresses will not be checked by snort and
> completely
> bypasses the IDS.
> 
> There is no user interaction required, nor visible-effects or any
> backward-compatiblity required, only a restart of snort after the
> update process to protect all red addresses.

Please include that description in the new version of the patch.

> 
> 
> > 
> > There are some formatting inconsistencies in this patch.
> > 
> > > > 
> > > > diff --git a/src/initscripts/init.d/snort
> > > > b/src/initscripts/init.d/snort
> > > > index e03c80f..47e7998 100644
> > > > --- a/src/initscripts/init.d/snort
> > > > +++ b/src/initscripts/init.d/snort
> > > > @@ -20,6 +20,8 @@
> > > > PATH=/usr/local/sbin:/usr/local/bin:/bin:/usr/bin:/sbin:/usr/sb
> > > > in
> > > > ;
> > > > export PATH
> > > >  eval $(/usr/local/bin/readhash /var/ipfire/ethernet/settings)
> > > >  eval $(/usr/local/bin/readhash /var/ipfire/snort/settings)
> > > >  
> > > > +ALIASFILE="/var/ipfire/ethernet/aliases"
> > > > +
> > > >  case "$1" in
> > > >          start)
> > > >  		if [ "$BLUE_NETADDRESS" ]; then
> > > > @@ -59,6 +61,19 @@ case "$1" in
> > > >  			if [ "$LOCAL_IP" ]; then
> > > >  				HOMENET+="$LOCAL_IP,"
> > > >  			fi
> > > > +
> > > > +			# Check if the red device is set to
> > > > static
> > > > and
> > > > +			# any aliases have been configured.
> > > > +			if [ "$RED_TYPE" == "STATIC" ] && [ -s
> > > > "${ALIASFILE}" ]; then
> > 
> > RED_TYPE does not have curly braces, ALIASFILE has these.
> > 
> > Pick one based on the rest of the script and be consistent, please.
> 
> Thanks for the hint, I will upload a reworked patch soon.
> > 
> > > > +				# Read in aliases file.
> > > > +				while IFS="," read -r address
> > > > mode
> > > > remark; do
> > > > +					# Check if the alias
> > > > is
> > > > enabled.
> > > > +					[ "${mode}" = "on" ]
> > > > ||
> > > > continue
> > > > +
> > > > +					# Add alias to the
> > > > list
> > > > of
> > > > HOMENET addresses.
> > > > +					HOMENET+="${address},"
> > > > +				done < "${ALIASFILE}"
> > > > +			fi
> > > >  		fi
> > > >  		HOMENET+="127.0.0.1"
> > > >  		echo "ipvar HOME_NET [$HOMENET]" >
> > > > /etc/snort/vars
> > 

--===============2365972106316805647==
Content-Type: application/pgp-signature
Content-Transfer-Encoding: base64
Content-Disposition: attachment; filename="signature.asc"
MIME-Version: 1.0

LS0tLS1CRUdJTiBQR1AgU0lHTkFUVVJFLS0tLS0KVmVyc2lvbjogR251UEcgdjEKCmlRSWNCQUFC
Q2dBR0JRSldJU2c2QUFvSkVJQjU4UDl2a0FrSHBFVVFBSTNHNWhRa3NWMmRXaXVKOG84NlZ6ZGkK
NFg0bEVLV0gwYmJRdVI0eC90b0hJM2ZVRGNWaEIwMmMrakdJeVkyTzYyQmllUG8zZ2NFc1BEd1li
STd3K09nSApwWHZKSmN5Skt4eWRoZzJUa0VDc1JkOUNqSGZueEhQclNpRXFlMDRUVWlUSnV1RWth
QTYvekZFRU02L2JNdVpNCndaSnU3QUtaQ0JFTitzUUFZMzVnM3kwOGZBbXVCWDFEbXBESTFQc1ox
MTFNRndGYWd0UDNvWURKejUwa082b1UKUU0wU3ZaTWhNTjZJUmpmU2JIMm0xWGZ6ZnlxNXowZ29K
Vm82S3dKaXZpYzg3cTRqWjlCRzAyZnZSM3d6Q2ZkZwoyaTlJNnozZS83akowN0lsbklPQ2pLaTJu
OTVTdjdsNlFhSUNGcU1oS1M1Q3M0T01LZmFLWEhIT0xKN1lucmo5CmtsWGhObDk0WEE1ckRHbWtm
N2VPanJwZVArRytaczVFcGxqaDdiNUw5dFdTRElvQkFmeVdFdEoyYVY3dG1PVUcKZ1hucjcrV3Fm
cTR1VVhtWGg4Vk9WTkg5ZnlNMTJxTUc3WVo2NTh3bW9pQXVRUTNIOGQ3M2NuZGFHOS9sMVhxbQpD
RTQ1bC9hb2h2aXRhaUNMZFR6dDFzZWxyYkI2QWhOTkx4dDhUNUVaWHdYS1FndVJtc1JsRlBCT0pq
bkxJVXdTCk5tZU5WM1BrSS8xMUdLYUxoQlpZMi9NN1RKRUJVSFhpVVQ3KzBPL1dNdjFXYWhrbWN0
MTFJMVo5MmhIYVUwcWoKSVFOMHVBMWtvckFTaFpjU2dDNFl6alAyZ09ObTA1VHRjMWtHRG5uMnV4
VXhpeUhJcUF1V0Vvcmlpd2xBSVllVwpTV2dhTFFDeE1UWHVzeWxvVWsxRQo9U0dYYgotLS0tLUVO
RCBQR1AgU0lHTkFUVVJFLS0tLS0K

--===============2365972106316805647==--