On 12-02-23 10:53 AM, Teodor MICU wrote:
> 2012/2/22 Simon Deziel <[email protected]>:
>> On 12-02-22 08:38 AM, Teodor MICU wrote:
>>> I like this idea. However, I think you should change a few things:
>>> 1) default.send_redirects=0 and all.send_redirects=0 should be done
>>> only if necessary (is not 0) and the original value reverted back
>>> after the device was created
>>
>> Adding more logic would result in calling sysctl more times which is
>> suboptimal IMHO unless there are some advantages I'm not aware of ?
>
> This is not about "advantages" but for keeping the previous
> configuration (default or explicitly set by the user). I saw you have
> the logic for one but not for both.
I'm not sure what you imply by "both" here so I maybe missing your
point. In the patch, 2 sysctl keys are altered: the default and the all
ones.
The default is only altered to allow the tun to derive its own
send_redirects based on the default one (that is to have it =0). Once
the tun is up, the default is restored to its original value so in
effect the user's setting is retained.
Now for the all key, this has to be =0 as otherwise the tun one wouldn't
have any effect.
> That's why I though if you already have to get the current value why
> not check if =0 to avoid two sysctl call for each parameter. Thus,
> this would be more optimal.
OK, that would avoid uselessly calling sysctl -w to set/restore the
default key to its current value when it is already disabled.
You convinced me, your suggestion is cleaner. Here is the fixed patch
(which is now based on the Debian package as my previous ones were based
on the one from Lucid).
This was tested on OpenVPN 2.2.1-4 (Sid) and 2.1.0-1ubuntu1.1 (Lucid).
Thanks,
Simon
--- openvpn.orig 2012-02-08 08:32:48.000000000 -0500
+++ openvpn 2012-02-23 11:11:46.000000000 -0500
@@ -56,6 +56,25 @@
STATUSARG="--status /var/run/openvpn.$NAME.status $STATUSREFRESH"
fi
+ # tun using the "subnet" topology confuses the routing code that wrongly
+ # emits ICMP redirects for client to client communications
+ if grep -q '^[[:space:]]*dev[[:space:]]*tun' $CONFIG_DIR/$NAME.conf && \
+ grep -q '^[[:space:]]*topology[[:space:]]*subnet' $CONFIG_DIR/$NAME.conf ; then
+ # When using "client-to-client", OpenVPN routes the traffic itself without
+ # involving the TUN/TAP interface so no ICMP redirects are sent
+ if ! grep -q '^[[:space:]]*client-to-client' $CONFIG_DIR/$NAME.conf ; then
+ sysctl -w net.ipv4.conf.all.send_redirects=0 > /dev/null
+
+ # Save the default value for send_redirects before disabling it
+ # to make sure the tun device is created with send_redirects disabled
+ SAVED_DEFAULT_SEND_REDIRECTS=$(sysctl -n net.ipv4.conf.default.send_redirects)
+
+ if [ "$SAVED_DEFAULT_SEND_REDIRECTS" -ne 0 ]; then
+ sysctl -w net.ipv4.conf.default.send_redirects=0 > /dev/null
+ fi
+ fi
+ fi
+
log_progress_msg "$NAME"
STATUS=0
@@ -66,6 +85,11 @@
--config $CONFIG_DIR/$NAME.conf || STATUS=1
[ "$OMIT_SENDSIGS" -ne 1 ] || ln -s /var/run/openvpn.$NAME.pid /run/sendsigs.omit.d/openvpn.$NAME.pid
+
+ # Set the back the original default value of send_redirects if it was changed
+ if [ "$SAVED_DEFAULT_SEND_REDIRECTS" -ne 0 ]; then
+ sysctl -w net.ipv4.conf.default.send_redirects=$SAVED_DEFAULT_SEND_REDIRECTS > /dev/null
+ fi
}
stop_vpn () {
kill `cat $PIDFILE` || true
