[PATCH] vpnc-script: add dnsmasq split-horizon DNS support

[Antonio Borneo]

Hi Joe,

On Tue, Jun 24, 2014 at 10:56 PM, Joe MacDonald
<joe_macdonald at mentor.com> wrote:
> Attempt to determine if dnsmasq is being used to manage name resolution
> and, if so, update the configuration with VPN-supplied DNS information.
>
> Signed-off-by: Joe MacDonald <joe_macdonald at mentor.com>
> ---
>
> Adding any level of dnsmasq interaction makes it pretty easy to set up
> best-effort split-horizon DNS.  This does depend on having resolvconf
> managing things as well, but the concept should apply equally well to a
> non-resolvconf scenario.  It just seemed easiest and least invasive to
> create / remove the dnsmasq.d configuration piece in the
> *_resolvconf_manager() functions.  I try to err on the side of asking the
> VPN DNS more often rather than less but still use the
> previously-configured resolver for what looks like non-VPN traffic.

I did not tested your patch since I'm not using dnsmasq

I suppose this will work when VPN sets a split network and dnsmasq can
still access DNS, inside and outside VPN channel.
What would happen if VPN setting forces all the traffic in the VPN
tunnel? Do you need to remove previous configuration from dnsmasq?

>
>
>  vpnc-script |   54 +++++++++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 47 insertions(+), 7 deletions(-)
>
> diff --git a/vpnc-script b/vpnc-script
> index 79cf6e2..bbde4cd 100755
> --- a/vpnc-script
> +++ b/vpnc-script
> @@ -360,6 +360,22 @@ fi
>
>  # =========== resolv.conf handling for any OS =========================
>
> +do_restart_dnsmasq() {
> +       SERVICE=`which service`

You need to redirect to /dev/null the stderr of "which".

> +       if [ -x $SERVICE ] ; then
> +               $SERVICE dnsmasq restart

If "service" is not installed, then "$SERVICE"==""
The condition would turn true and script will execute "dnsmasq
restart", probably rising error.
You need to write
    if [ -x "$SERVICE" ] ; then

> +       else
> +               if [ -x /etc/init.d/dnsmasq ] ; then

use "elif" to limit indentation

> +                       /etc/init.d/dnsmasq restart
> +               else
> +                       SYSTEMCTL=`which systemctl`
> +                       if [ -x $SYSTEMCTL ] ; then

Same as above, redirect stderr and "" around the variable

> +                               $SYSTEMCTL restart dnsmasq
> +                       fi
> +               fi

Embedded system could start dnsmasq by some custom script.

We are here because you have detected that dnsmasq is running, but you
cannot find a way to restart it.
Should we print an error message ?
Or return an error so caller will remove /etc/dnsmasq.d/$TUNDEV.conf ?

> +       fi
> +}
> +
>  modify_resolvconf_generic() {
>         grep '^#@VPNC_GENERATED@' /etc/resolv.conf > /dev/null 2>&1 || cp -- /etc/resolv.conf "$RESOLV_CONF_BACKUP"
>         NEW_RESOLVCONF="#@VPNC_GENERATED@ -- this file is generated by vpnc
> @@ -542,19 +558,43 @@ restore_resolvconf_openwrt() {
>
>  modify_resolvconf_manager() {

We are in this function because the script have detected that
"/sbin/resolvconf" is installed and it plans to use it.
But then you skip resolvconf and play with dnsmasq.
I think you should detect dnsmasq before and add dedicated
MODIFYRESOLVCONF / RESTORERESOLVCONF for dnsmasq.

>         NEW_RESOLVCONF=""
> -       for i in $INTERNAL_IP4_DNS; do
> -               NEW_RESOLVCONF="$NEW_RESOLVCONF
> +       if [[ -d /etc/dnsmasq.d/ && `pgrep dnsmasq` ]] ;
> +       then

Put "then" in the same line with "if"
"[[" is bash specific. Replace "[[" with "[" and "&&" with "-a"
"pgrep" will match any running application named "*dnsmasq*". Used
`pgrep '^dnsmasq$'`
But then, you have added a new dependency from pgrep. Any replacement
for system that does not have it installed?

> +               # the system has dnsmasq installed and appears to be using it so inform
> +               # dnsmasq about the new servers and domains
> +               if [ -n "$CISCO_DEF_DOMAIN" ]; then
> +                       #  limit searching the VPN servers for only VPN addresses, but it is
> +                       #  better to cast too wide a net on this than miss VPN address lookups.
> +                       VPN_DNS=`echo $CISCO_DEF_DOMAIN | awk -F. '{ print "/" $(NF-1) "." $NF "/" }'`
> +               fi
> +               for i in $INTERNAL_IP4_DNS; do
> +                       NEW_RESOLVCONF="$NEW_RESOLVCONF
> +server=$VPN_DNS$i"
> +               done
> +               echo "$NEW_RESOLVCONF" > /etc/dnsmasq.d/$TUNDEV.conf
> +               # inform dnsmasq that there is a new configuraiton fragment to consider.

s/configuraiton/configuration/

> +               do_restart_dnsmasq
> +       else
> +               for i in $INTERNAL_IP4_DNS; do
> +                       NEW_RESOLVCONF="$NEW_RESOLVCONF
>  nameserver $i"
> -       done
> -       if [ -n "$CISCO_DEF_DOMAIN" ]; then
> -               NEW_RESOLVCONF="$NEW_RESOLVCONF
> +               done
> +               if [ -n "$CISCO_DEF_DOMAIN" ]; then
> +                       NEW_RESOLVCONF="$NEW_RESOLVCONF
>  domain $CISCO_DEF_DOMAIN"
> +               fi
> +               echo "$NEW_RESOLVCONF" | /sbin/resolvconf -a $TUNDEV
>         fi
> -       echo "$NEW_RESOLVCONF" | /sbin/resolvconf -a $TUNDEV
>  }
>
>  restore_resolvconf_manager() {
> -       /sbin/resolvconf -d $TUNDEV
> +       if [[ -d /etc/dnsmasq.d/ && `pgrep dnsmasq` ]] ;
> +       then

As above; "then" in same line, replace "[[" and all said about "pgrep"

Best Regards,
Antonio

> +               rm -f /etc/dnsmasq.d/$TUNDEV.conf
> +               do_restart_dnsmasq
> +       else
> +               /sbin/resolvconf -d $TUNDEV
> +       fi
>  }
>
>  # ========= Toplevel state handling  =======================================
> --
> 1.7.10.4
>
>
> _______________________________________________
> openconnect-devel mailing list
> openconnect-devel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/openconnect-devel