Hi, On Tue, Jul 07, 2020 at 06:14:25PM +0200, Jan Just Keijser wrote: > > This one works(!), so generally, Win10 accepts this DHCP option - but > > it seems to want "all domains in one". > > > > Can you send a v3? > > > not sure if all went well , but here's V3.
Unfortunately not, that one seems to be based on your V1 patch, so
we have "remove 'SEARCH', add 'DOMAIN-SEARCH'" hunks...
> diff --git a/src/openvpn/options.c b/src/openvpn/options.c
> index e59b22b..85f1d8a 100644
> --- a/src/openvpn/options.c
> +++ b/src/openvpn/options.c
> @@ -726,7 +726,7 @@ static const char usage_message[] =
> " which allow multiple addresses,\n"
> " --dhcp-option must be repeated.\n"
> " DOMAIN name : Set DNS suffix\n"
> - " SEARCH name : Set DNS domain search list\n"
> + " DOMAIN-SEARCH entry : Add entry to DNS domain
> search list\n"
This is ok for me to have a look, but to actually merge I need something
that applies "as it is" on top of master...
> diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
> index eed9ae6..60a149c 100644
> --- a/src/openvpn/tun.c
> +++ b/src/openvpn/tun.c
> @@ -5567,46 +5567,70 @@ write_dhcp_str(struct buffer *buf, const int type,
> const char *str, bool *error)
> buf_write(buf, str, len);
> }
>
> +/*
> + * RFC3397 states that multiple searchdomains are encoded as follows:
> + * - at start the length of the entire option is given
> + * - each subdomain is preceded by its length
> + * - each searchdomain is separated by a NUL character
> + * e.g. if you want "openvpn.net" and "duckduckgo.com" then you end up with
> + * 0x13 0x7 openvpn 0x3 net 0x00 0x0A duckduckgo 0x3 com 0x00
> + */
Richard commented on IRC that the "0x13" does not seem to be right here
- adding up all of it (1+7+1+3+1+1+10+1+3+1 = 29). Can you double-check?
It's just a comment, but if that is wrong, it's not helpful in trying to
understand the code.
> static void
> -write_dhcp_search_str(struct buffer *buf, const int type, const char *str,
> bool *error)
> +write_dhcp_search_str(struct buffer *buf, const int type, const char * const
> *str_array,
> + int array_len, bool *error)
> {
> - const char *ptr = str, *dotptr = str;
> - int i, j;
> + char tmp_buf[256];
> + int i;
> + int len = 0;
> +
> + for (i=0; i < array_len; i++)
> + {
> + const char *ptr = str_array[i], *dotptr = str_array[i];
> + int j, k;
> +
> + msg(M_INFO, "Processing '%s'", ptr);
> +
> + if (strlen(ptr) + len + 1 > sizeof(tmp_buf))
> + {
> + *error = true;
> + msg(M_WARN, "write_dhcp_search_str: temp buffer overflow
> building DHCP options");
> + return;
> + }
> + /* Loop over all subdomains separated by a dot and replace the dot
> + with the length of the subdomain */
> + while ((dotptr = strchr(ptr, '.')) != NULL)
> + {
> + j = dotptr - ptr;
> + tmp_buf[len++] = j;
> + for (k=0; k < j; k++) tmp_buf[len++] = ptr[k];
> + ptr = dotptr + 1;
> + }
> +
> + /* Now do the remainder after the last dot */
> + j = strlen(ptr);
> + tmp_buf[len++] = j;
> + for (k=0; k < j; k++) tmp_buf[len++] = ptr[k];
This should work. It might be possible to do this with slightly less
code with strsep() (which will give you nicely null-terminated chunks,
including "the remainder", but will require a writeable copy of the string).
It's overflow safe, so "good enough".
The restriction to max. 10 search domains sounds reasonable to me - since
the total string is limited, and we do not implement multiple options
(*and* windows 10 is, by observation, ignoring more than one anyway) this
is "good enough for all reasonable deployments".
Can I have a v4, please? :-)
gert
--
"If was one thing all people took for granted, was conviction that if you
feed honest figures into a computer, honest figures come out. Never doubted
it myself till I met a computer with a sense of humor."
Robert A. Heinlein, The Moon is a Harsh Mistress
Gert Doering - Munich, Germany g...@greenie.muc.de
signature.asc
Description: PGP signature
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
