On 2016-08-10 14:05, Raghavendra Prabhu wrote:
> Hi,
>
> Thanks for the patch.
>
> I have following questions regarding this patch:
>
> a) In the patch I see that it is handling only for
> modify_resolvconf_generic. What about other modify_*. This issue
> affects them as well. (In my case it uses modify_resolvconf_manager).
>
> b) Instead of choosing only the first domain, is it not possible to
> use 'search' followed by the list of domains. At least this is how I
> updated the script to do, and that works fine.
>
> i) At least, that is how the modify_resolvconf_generic
> seems to be operating when it parses the backup resolv.conf for
> 'search' entries first, if present, appends to them, if not uses the
> 'domain' keyword.
Hmm. Your comments lead me to realize this is more complicated than I
had thought.
1. According to my resolv.conf man page on Debian:
"The domain and search keywords are mutually exclusive. If more
than one instance of these keywords is present, the last instance
wins."
So, setting both "search" and "domain" is a bad idea. I don't know if
this reading is portable to other OSs though.
2. If I read the script correctly, it doesn't actually parse for search
first--it reads resolv.conf line by line, in order, and the presence of
"search" before "domain" in the case statement does not matter. What
does matter is that the script blanks out CISCO_DEF_DOMAIN when it
finds either "search" or "domain", so it only modifies the first one.
3. For "search", CISCO_DEF_DOMAIN is appended. For "domain"
CISCO_DEF_DOMAIN overrides whatever was present.
This all leads to some incorrect behaviors:
a. If "domain" is listed first in the original file, followed by a
later "search", then "domain" potentially gets multiple domains
appended (as noted), and "search" is unmodified. Since "search" is
still last in resolv.conf, this seems to be a no-op except that it
confuses some things that parse the file directly.
b. If "domain" is listed, but "search" is not, then the "domain" line
gets written incorrectly with no "search" line following. On my system,
this results in the first domain being recognized, and the rest ignored.
c. If "search" is first, followed by "domain", then "search" is
appended to but "domain" is ignored. Since "domain" comes later, this
is a syntactically valid no-op.
To be fair, many of these behaviors are the result of incorrect
situations in resolv.conf to begin with--"domain" and "search" should
not both be specified. We do want vpnc-script to do something correct
either way, though.
The following patch is a more intrusive rewrite, but should be much
better. Please let me know if I am missing something or making an
incorrect assumption.
commit cc3450ea460e59366ceeb6819eff1f698479eecc
Author: Corey Hickey <bugfood-ml at fatooh.org>
Date: Mon 2016-08-15 10:29:50
rewrite resolv.conf parsing
This patch simplifies parsing and changes behavior in two ways:
1. Domains for searching are now parsed from "search" and "domain"
lines. Only a "search" line is outputted, since "search" supports
multiple domains and is mutually exclusive with "domain". The motivation
for this is to make vpnc-script behave sanely when there are existing
resolv.conf files with only "domain" or with both "domain" and "search".
2. All original "nameserver" lines are discarded and replaced rather
than only the number of nameservers from $INTERNAL_IP4_DNS. The
rationale here is that vpnc-script should be consistent and either
retain all original nameservers or overwrite all of them. Retaining them
is problematic because there is a limit of three, and overwriting is
closer to the original behavior.
The Darwin changes are untested, but are a simple search/replace and
thus should work fine.
Signed-off-by: Corey Hickey <bugfood-ml at fatooh.org>
diff --git a/vpnc-script b/vpnc-script
index c3aafa4..9cf5e33 100755
--- a/vpnc-script
+++ b/vpnc-script
@@ -369,49 +369,31 @@ modify_resolvconf_generic() {
# and will be overwritten by vpnc
# as long as the above mark is intact"
- # If multiple domains are listed, prefer the first for "domain".
- DOMAIN="${CISCO_DEF_DOMAIN%% *}"
- # Remember the original value of CISCO_DEF_DOMAIN we need it later
- CISCO_DEF_DOMAIN_ORIG="$CISCO_DEF_DOMAIN"
- # Don't step on INTERNAL_IP4_DNS value, use a temporary variable
- INTERNAL_IP4_DNS_TEMP="$INTERNAL_IP4_DNS"
+ DOMAINS=$CISCO_DEF_DOMAIN
+
exec 6< "$RESOLV_CONF_BACKUP"
while read LINE <&6 ; do
case "$LINE" in
- nameserver*)
- if [ -n "$INTERNAL_IP4_DNS_TEMP" ]; then
- read ONE_NAMESERVER INTERNAL_IP4_DNS_TEMP <<-EOF
- $INTERNAL_IP4_DNS_TEMP
-EOF
- LINE="nameserver $ONE_NAMESERVER"
- else
- LINE=""
- fi
- ;;
- search*)
- if [ -n "$CISCO_DEF_DOMAIN" ]; then
- LINE="$LINE $CISCO_DEF_DOMAIN"
- CISCO_DEF_DOMAIN=""
- fi
- ;;
- domain*)
- if [ -n "$DOMAIN" ]; then
- LINE="domain $DOMAIN"
- fi
- ;;
+ # omit; we will overwrite these
+ nameserver*) ;;
+ # extract listed domains and prepend to list
+ domain* | search*) DOMAINS="${LINE#* } $DOMAINS" ;;
+ # retain other lines
+ *) NEW_RESOLVCONF="$NEW_RESOLVCONF
+$LINE" ;;
esac
- NEW_RESOLVCONF="$NEW_RESOLVCONF
-$LINE"
done
exec 6<&-
- for i in $INTERNAL_IP4_DNS_TEMP ; do
+ for i in $INTERNAL_IP4_DNS ; do
NEW_RESOLVCONF="$NEW_RESOLVCONF
nameserver $i"
done
- if [ -n "$CISCO_DEF_DOMAIN" ]; then
+ # note that "search" is mutually exclusive with "domain";
+ # "search" allows multiple domains to be listed, so use that
+ if [ -n "$DOMAINS" ]; then
NEW_RESOLVCONF="$NEW_RESOLVCONF
-search $CISCO_DEF_DOMAIN"
+search $DOMAINS"
fi
echo "$NEW_RESOLVCONF" > /etc/resolv.conf
@@ -453,7 +435,7 @@ search $CISCO_DEF_DOMAIN"
# DNS matching when available. When multiple DNS matching
# is present, anything reading the /etc/resolv.conf file
# directly will probably not work as intended.
- #if [ -z "$CISCO_DEF_DOMAIN_ORIG" ]; then
+ #if [ -z "$$CISCO_DEF_DOMAIN" ]; then
# Cannot use multiple DNS matching without a domain
OVERRIDE_PRIMARY='d.add OverridePrimary # 1'
#fi
@@ -471,13 +453,13 @@ search $CISCO_DEF_DOMAIN"
set State:/Network/Service/$TUNDEV/IPv4
close
EOF
- if [ -n "$CISCO_DEF_DOMAIN_ORIG" ]; then
+ if [ -n "$$CISCO_DEF_DOMAIN" ]; then
scutil >/dev/null 2>&1 <<-EOF
open
get State:/Network/Service/$TUNDEV/DNS
- d.add DomainName $CISCO_DEF_DOMAIN_ORIG
- d.add SearchDomains * $CISCO_DEF_DOMAIN_ORIG
- d.add SupplementalMatchDomains * $CISCO_DEF_DOMAIN_ORIG
+ d.add DomainName $$CISCO_DEF_DOMAIN
+ d.add SearchDomains * $$CISCO_DEF_DOMAIN
+ d.add SupplementalMatchDomains * $$CISCO_DEF_DOMAIN
set State:/Network/Service/$TUNDEV/DNS
close
EOF
Thanks,
Corey
