Hi, On Tue, Jun 23, 2020 at 3:22 AM Jan Just Keijser <janj...@nikhef.nl> wrote: > > Hi, > > On 21/06/20 17:14, Selva Nair wrote: > > On Sun, Jun 21, 2020 at 7:14 AM Gert Doering <g...@greenie.muc.de> wrote: > >> > >> going through OpenVPN threads that went stale - I think this is > >> actually a nice addition (read: other people have already asked > >> me if this can be done). > >> > >> On Thu, Mar 05, 2020 at 01:53:12PM +0100, Jan Just Keijser wrote: > >>> So, for what it's worth, I've dusted off the patch again and rebased it > >>> to the current openvpn master tree. See attached. Note that I did only > >>> rudimentary testing, as I don't use Windows 10 a lot and I was testing > >>> using a mingw cross-compile only. In wireshark I *do* see that the > >>> correct DHCP offer is sent to the tap-win adapter. > >>> > >>> Also note that I implemented multiple search domains by separating them > >>> using semi-colons, e.g. > >>> > >>> --dhcp-option SEARCH example.com;example.org;example.nl;example.de > >>> > >>> etc as that was easier to implement > >> The patch looks okay-ish on quick reading. > > Feature wise yes, but some improvements needed.. > > > > I do not like that option format > > > > "example.com; example.org" with quotes will parse right and fail later > > on because of the space, for example. And, we support multiple > > statements of dhcp options like DNS, so this is counter-intuitive. > > It's only a little more work to support a more forgiving format. > actually, the patch will be a bit more invasive if we add that; parsing > DNS/WINS etc is already there and it parses an option into an IP/IPv6 > address. changing the dns-search option to an array of strings will > require more work. I did look into that initially but decided not to do > it as the patch would be lengthier. > > Note that "multiple domain" option is also not support, really. The > domain is stored in the tuntap_options struct as > > 90 const char *domain; /* DOMAIN (15) */
Windows is probably the only platform where these options are "natively" supported through DHCP, and indeed DOMAIN (option 15) has to be a single domain. Android also seem to treat them internally. However, we export these as foreign_option in all other platforms, and do export anything that looks like "dhcp-option name value" with repetitions allowed. That said, I don't mind breaking scripts that use and misuse these... Anyway my point about multiple values was about DNS and WINS where it's allowed, and we parse them one entry at a time. I believe this option (search list) also should use the same format instead of inventing yet another format. Yes, it may need a bit "more work" to support one domain at a time, but that's no excuse not to use a sensible format. Concatenation of strings read one at a time with a delimiter added doesn't look that hard either. > > so it's "last option wins" ; the fact that some platforms abuse this > option to actually set multiple search domains is a different matter. > > >>> Also note that I did not fully implement the RFC3397 encoding of the > >>> search list, as that requires one to merge domain names that occur more > >>> than once - that would have made the code far more complicated. > >> Indeed. I haven't looked at what other DHCP implementations do, but > >> "correct" encoding definitely sounds like quite a bit of extra code just > >> to save a few bytes on the wire - might come handy if you have many > >> subdomains of a long internal DNS domain, though, but this can be > >> added "if needed". > > Same as my thoughts, the encoding part could be kept simple as in here > > and possibly improved later. But option format is hard to change or > > deprecate. > > > >> > >> More interesting is the question "which option to use" - it should > >> be synchronized between openvpn platform handlers. So if systemd-networkd > >> uses "SEARCH-DOMAIN" it would make sense to use that for windows > >> as well. > > I think we need both --- the current one retained as the connection > > specific suffix which would be just one entry and then this search > > list. As we allow multiple entries for DOMAIN right now, a user > > friendly approach would be to continue doing so but internally treat > > all but the first as a part of --dhcp-option DOMAIN-SEARCH. We could > > also deprecate the use of multiple entries in the dhcp-option DOMAIN > > keyword. > > > See above: like I said, multiple DOMAIN entries do not seem to be supported. > > So what option do we want? > > --dhcp-option SEARCH > --dhcp-option DOMAIN-SEARCH > --dhcp-option SEARCH-DOMAIN RFC 3397 calls it "Domain Search" so it has to be DOMAIN-SEARCH, in my view. Platform scripts accepting other forms in foreign_option is up to them. We don't have to officially support that. Selva _______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
