[Openswan dev] [PATCH] Incorrect automatic route via ipsec0

Harald Jenny harald at a-little-linux-box.at
Mon Oct 25 12:47:27 EDT 2010


Hi @all,

could we please make a list of test scenarios which lead to all these changes
and do some testing before the new code gets into production? Namely I would
suggest that Roel tests it with the advanced routing patch (maybe you have a
chance to do this in a lab?), Bart checks if Ubuntu 10.04 still plays nicely,
Davids tests the IPv6 functionality and somebody with a ppp link may also check
that this works ok (did I miss anything that this code is currently supposed to
handle?).

Kind regards
Harald

On Mon, Oct 25, 2010 at 11:49:00AM -0400, Bart Trojanowski wrote:
> * Roel van Meer <rolek at bokxing.nl> [101025 08:10]:
> > Note: in order to avoid adding confusion to an already long and
> > confusing thread: I think your comments are based on a version of a
> > patch I submitted but which has been replaced by a different
> > version. In that light most of it is no longer relevant, since the
> > new patch removes the metric bumping code altogether.
> 
> I've been caught red handed, and guilty of not reading the entire
> thread.  Now that I've reviewed the rest, I agree with your findings.
> 
> I am not sure why we ever created the virtual interface with a mask
> other than /32.  Maybe just to avoid confusion.  It would make sense to
> me to just assign it the address, but like Harald I worry that we might
> create other problems.
> 
> I've reworked your patch a bit to remove the assignment of the
> broadcast, and peer addresses from the virtual interface.  That's
> attached.
> 
> I think that should still work for you, and I need to do some more
> testing.
> 
> Sorry about the confusion, and thanks for taking time to explain it
> (again).
> 
> -Bart
> 
> -- 
> 				WebSig: http://www.jukie.net/~bart/sig/

> From c50381d8756e476efc8db024706df9a7ef78839c Mon Sep 17 00:00:00 2001
> From: Bart Trojanowski <bart at jukie.net>
> Date: Mon, 25 Oct 2010 11:24:24 -0400
> Subject: [PATCH 1/2] fix interface parsing in getinterfaceinfo()
> 
> - otheraddr needs to be returned even if empty (thanks Roel van Meer),
> - return type as before IPv6 port
> ---
>  programs/_startklips/_startklips.in |   20 ++++++++++++--------
>  1 files changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/programs/_startklips/_startklips.in b/programs/_startklips/_startklips.in
> index b841510..689c395 100755
> --- a/programs/_startklips/_startklips.in
> +++ b/programs/_startklips/_startklips.in
> @@ -143,21 +143,24 @@ getinterfaceinfo() {
>  	ip addr show dev $1 | awk '
>  	BEGIN {
>  		MTU=""
> +		TYPE="unknown"
>  	}
> +	/BROADCAST/   { TYPE="broadcast" }
> +	/POINTOPOINT/ { TYPE="pointtopoint" }
>  	/mtu/ {
>  			sub("^.*mtu ", "", $0)
>  			MTU=$1
>  		}
>  	$1 == "inet" || $1 == "inet6" {
> -			addr=$2
> -			sub("/.*$","",addr)
> -			sub("^.*/","",$2)
> -			print "'$2'addr=" addr
> -			print "'$2'mask=" $2
> -			if (MTU != "")
> -				print "'$2'mtu=" MTU
> +			split($2,addr,"/")
> +			other=""
>  			if ($3 == "peer")
> -				print "'$2'otheraddr=" $4
> +				other=$4
> +			print "'$2'type=" TYPE
> +			print "'$2'addr=" addr[1]
> +			print "'$2'mask=" addr[2]
> +			print "'$2'otheraddr=" other
> +			print "'$2'mtu=" MTU
>  			exit 0
>  		}'
>  }
> @@ -408,4 +411,5 @@ do
>  	esac
>  done
>  
> +# vim: set noet sw=8 ts=8
>  exit 0
> -- 
> 1.7.0.2.g2b0b7e7
> 

> From 9e107f01c9d2e58f5ea32adac5c642aaf2128555 Mon Sep 17 00:00:00 2001
> From: Bart Trojanowski <bart at jukie.net>
> Date: Mon, 25 Oct 2010 11:26:25 -0400
> Subject: [PATCH 2/2] [KLIPS] avoid routes towards virtual ipsecN interface
> 
> The issue was discovered by Roel van Meer, who provided the original patch.
> 
> * Roel van Meer writes:
> > When openswan is used with klips, it creates a virtual device at
> > startup. The virtual device is associated with a physical device and
> > the ip addresses present on the physical device are also assigned to
> > the virtual device. By assigning these ip addresses to the virtual
> > device with the same netmasks as they have on the physical device,
> > routes for locally connected networks are created through the virtual
> > device. In most setups, these routes are never used, since the route
> > through the physical device takes precedence because it was installed
> > earlier.
> >
> > However, in some setups, the route through the virtual device would
> > take precedence, breaking connectivity to these networks. This happens
> > with Ubuntu 10.04, which has non-zero-metric routes and when using
> > Julian Anastasov's routing patches.
> >
> > Avoid creating these routes by assigning ip addresses to the virtual device
> > with a netmask of /32 (for ipv4) or /128 (for ipv6).
> >
> > This also means the ubuntu route metric fix is no longer necessary.
> 
> Signed-off-by: Bart Trojanowski <bart at jukie.net>
> ---
>  programs/_startklips/_startklips.in |   23 +++++++++--------------
>  1 files changed, 9 insertions(+), 14 deletions(-)
> 
> diff --git a/programs/_startklips/_startklips.in b/programs/_startklips/_startklips.in
> index 689c395..bc468a5 100755
> --- a/programs/_startklips/_startklips.in
> +++ b/programs/_startklips/_startklips.in
> @@ -217,10 +217,16 @@ klipsinterface() {
>  	then
>  		ipsec tncfg --attach --virtual $virt --physical $phys
>  		# configure all the IPv4/IPv6 addresses (including point-to-point)
> -		ip addr show dev $phys | awk '$1 == "inet" || $1 == "inet6" {
> +		ip addr show dev $phys \
> +		| awk '$1 == "inet" || ($1 == "inet6" && !/ dynamic/) {
>  				cmd = "ip addr add"
> -		        	for (i = 2; i < NF; i++)
> -					cmd = cmd " " $i
> +				sub("/.*","",$2)
> +				for (i = 2; i < NF; i++) {
> +					if ($i == "brd" || $i == "peer")
> +						i++
> +					else
> +						cmd = cmd " " $i
> +				}
>  				if ($NF != phys)
>  					cmd = cmd " " $NF
>  				cmd = cmd " dev " virt "> /dev/null"
> @@ -229,17 +235,6 @@ klipsinterface() {
>  		ip link set up dev $virt
>          fi
>  
> -        # do we have to fix the route metric for this device?
> -        maxmetric=$(ip route show $phys_otheraddr/$phys_mask 2> /dev/null | sed -n -e 's/.*metric \([0-9]\+\)/\1/p' | sort -n | tail -n1)
> -        if [ -n "$maxmetric" ] ; then
> -                # If there are other none-zero-metric routes that match this spec
> -                # (the case on Ubuntu 10.04, Lucid) then move the route to the end.
> -                newmetric=$(( $maxmetric + 1))
> -                oldroute=$(ip route show $phys_otheraddr/$phys_mask dev $virt)
> -                ip route del $oldroute dev $virt
> -                ip route add $oldroute dev $virt metric $newmetric
> -        fi
> -
>          # Double check the mtu is not 0 - if it is set it to a saner default
>  	ip link show dev $virt | grep -q 'mtu 0 '
>          RETVAL=$?
> -- 
> 1.7.0.2.g2b0b7e7
> 

> _______________________________________________
> Dev mailing list
> Dev at openswan.org
> http://lists.openswan.org/mailman/listinfo/dev



More information about the Dev mailing list