Re: [PATCH 6/9] hs20: Bind curl, including dns, to wlan interface.

On Fri, Feb 07, 2020 at 01:10:14AM -0800, [email protected] wrote:
> This lets things work better on multi-homed networks.

What is the issue on multi-homed networks? Isn't the local IP routing
configured properly to find the target IP address here? And what is that
DNS server configuration about? It seems to be doing more than just
binding "dns to wlan interface".. It would be good to split this into
three patches to address those apparently different parts: (1) bind HTTP
TCP/IP socket into a specific interface (from the hs20-osu-client -S
command line argument), (2) bind DNS UDP socket into a specific
interface, and (3) allow system DNS server IP addresses to be
overridden.

> diff --git a/hs20/client/osu_client.c b/hs20/client/osu_client.c
> +void check_dns_file(struct hs20_osu_client* ctx)
> +{
> +	/* Look for DNS servers in case user specified a place to look. */
> +	if (ctx->dns_file) {
> +		FILE *f;
> +		char buf[100];
> +
> +		f = fopen(ctx->dns_file, "r");
> +		if (f) {
> +			if (fgets(buf, sizeof(buf), f)) {
> +				wpa_printf(MSG_DEBUG, "Checking DNS file: %s contents: %s",
> +					   ctx->dns_file, buf);
> +				if (strncmp(buf, "DNS:", 4) == 0) {
..

Why would this need an external file? Wouldn't it be simpler to provide
the list of DNS server IP addresses on the command line with the new
command line argument -D that added here to point to a file?

> @@ -3241,6 +3277,7 @@ int main(int argc, char *argv[])
>  			break;
>  		case 'S':
>  			ctx.ifname = optarg;
> +			ctx.do_bind_iface = 1;
>  			break;

Couldn't this break some existing cases if do_bind_iface is hardcoded
for both HTTPS and DNS unconditionally? The current use of -S<ifname> is
to specify which wpa_supplicant control interface to connect to. I'm not
confident that it is the same as the interface used for all TCP/IP
packets needed in all testing scenarios.

-- 
Jouni Malinen                                            PGP id EFC895FA