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
> >