[PATCH] write CISCO_SPLIT_INC in order

[Corey Hickey]

On 2017-08-14 03:20, David Woodhouse wrote:
> On Mon, 2017-07-10 at 15:44 -0700, Corey Hickey wrote:
>> The linked list implementation results in routes being queued in
>> reverse of the order in which they were received. So, when dequeuing
>> them, write to the buffer backwards.
>>
>> Signed-off-by: Corey Hickey <bugfood-ml at fatooh.org>
> 
> ...
>> -				*(p++) = ',';
>> +				*(--p) = ',';
>>   			}
>>   			script_setenv(vpninfo, "CISCO_SPLIT_DNS", list, 0);
> 
> That's CISCO_SPLIT_DNS that you're re-ordering, not CISCO_SPLIT_INC as
> it suggests in the subject.

Oh... Erm, yes. I got those mixed up in the subject.

> Arguably if order matters, then we ought to keep the list in the
> correct order in our internal data structures rather than deliberately
> keep it backwards and then *emitting* it in "reverse" order just
> because I was lazy about list-handling in cstp.c.
> 
> But does ordering matter?
> 
> For CISCO_SPLIT_INC it shouldn't. Those are an unordered list of
> networks which should be routed to this VPN connection; order really
> shouldn't matter. Even the overall routing table isn't ordered; it has
> explicit priorities (metric) for routes.
> 
> For CISCO_DEF_DOMAIN (the DNS search domain) it does matter. If a
> hostname lookup fails, we try appending the search domains in *order*
> until we get a match, and we might get different results for 'foo' if
> both 'foo.example.com' and 'foo.example.org' both exist and those are
> our search domains. But in both AnyConnect and Juniper code those are
> just single strings, so we aren't processing multiple entities.
> 
> For CISCO_SPLIT_DNS... I don't think order matters, again. That's an
> unordered list of "if doing a DNS lookup in one of <these> domains,
> then ask the DNS server on the VPN, else ask your local DNS servers".
> If you think order matters there, you're Doing It Wrong™.
> 
> We need to be careful to distinguish between 'search domain'
> (CISCO_DEF_DOMAIN) and 'domains to use this DNS server for'
> (CISCO_SPLIT_DNS). They are completely different things, and should not
> be conflated.

Ok, that's useful to know. It has been difficult for me to find 
documentation of the environment variables.

So, is your advice that we should continue to use CISCO_DEF_DOMAIN? The 
reason I originally shied away from that is that script.c handles 
CISCO_DEF_DOMAIN as a single string rather than a list--so I didn't even 
know if it was _supposed_ to be able to have multiple entries or if 
having space-separated entries in a single string worked by accident.

Thanks for your detailed response.

-Corey