Hi,
Here are some comments on the code -- there is one apparent memory leak
(see below).
.\"*********************************************************
> .TP
> +.B \-\-block\-outside\-dns
> +Block external DNS servers on other network adapters to prevent
>
the word "external" is not required nor appropriate as it blocks all, isn't
it?
> --- a/src/openvpn/init.c
> +++ b/src/openvpn/init.c
> @@ -1468,6 +1468,22 @@ do_open_tun (struct context *c)
> "up",
> c->c2.es);
> +#if _WIN32_WINNT >= 0x0600
> + if (c->options.block_outside_dns)
> + {
> + if (!win_wfp_init())
> + msg (M_NONFATAL, "Initialising WFP failed!");
> + else
> + {
> + dmsg (D_LOW, "Blocking outside DNS");
> + if (!win_wfp_block_dns(c->c1.tuntap->adapter_index))
> + msg (M_NONFATAL, "Blocking DNS failed!");
> + }
> + }
> + else
> + msg (M_NONFATAL, "Can't block outside DNS without configured
> DNS server");
>
This error/warning is unnecessary and confusing. If the user does not want
to block-outside-dns, why say "can't block"? And why the "without
configured DNS server" remark?
in win32.c
+
+ ret = ConvertInterfaceIndexToLuid(index, &tapluid);
+ if (ret == NO_ERROR)
+ dmsg (D_LOW, "Tap Luid: %I64d", tapluid.Value);
+
Is it safe to ignore the ret != NO_ERROR case? May be it is...
+
> + /* Get OpenVPN path. */
> + GetModuleFileNameW(NULL, openvpnpath, MAX_PATH);
> +
> + if (FwpmGetAppIdFromFileName0(openvpnpath, &openvpnblob) !=
> ERROR_SUCCESS)
>
openvpnblob is allocated here, but not freed anywhere. Need to free it
before return.
Finally, to nitpick,
There are a couple of unused variables
Some local vars could be eliminated (rpcStatus, dwFwAPIRetCode, ret)
FIREWALL_NAME defined as const, but passed to non-const
ZeroMemory --> CLEAR
CopyMemory --> memcpy
Regards,
Selva
