Re: [Openvpn-devel] [PATCH] Make ValdikSS's DNS leak fix platform agnostic
Hi, On Thu, Dec 10, 2015 at 7:46 PM, Fish wrote: > Based on release/2.3 branch and ValdikSS's v9 patch, this patch is > cross-compiled on Linux and tested on Windows XP/10. The VC project file is > left untouched - you might want to add rpcrt4.lib to compile and link it > under > MSVC. > Builds on 2.3 out of the box (i.e., target WINXP). The implementation looks fine, and works on win7 and 10, but I've no XP machines for the crucial test. A minor suggestion: All those prototypes taken from mingw (32 or -w64?) may be better placed in new header file to be included only from win32.c -- -- preferably with a license matching the source (PD or ZPL?). Makes win32.c less cluttered as well. I'm not familiar with the policy of OpenVPN on license-related matters, though. Selva
Re: [Openvpn-devel] [PATCH applied] Re: Add Windows DNS Leak fix using WFP ('block-outside-dns')
On Sun, Dec 13, 2015 at 4:00 PM, Gert Doering wrote: > ACK. Lots of people have reviewed this, and it made the patch better :-) > - so I list them all, and take the final blame (ACK) on me... > Thanks (for assuming the blame :). And, ValdikSS, thanks for putting up with all the opinionated comments. Cheers, Selva
[Openvpn-devel] [PATCH applied] Re: Add Windows DNS Leak fix using WFP ('block-outside-dns')
ACK. I verified that this patch is identical to the 2.3 patch except for the option check in options.c, and the #ifdef for VISTA+ - so same ACK here, and same list of reviewers. Thanks. Your patch has been applied to the master branch. commit 38c8565810f892a41a2ea0d18a707676119f1af0 (master) Author: ValdikSS List-Post: openvpn-devel@lists.sourceforge.net Date: Thu Dec 10 23:51:55 2015 +0300 Add Windows DNS Leak fix using WFP ('block-outside-dns') Reviewed-by: Selva Nair Reviewed-by: Lev Stipakov Reviewed-by: James Yonan Acked-by: Gert Doering Message-Id: <1449780715-4027-1-git-send-email-...@valdikss.org.ru> URL: http://article.gmane.org/gmane.network.openvpn.devel/10744 Signed-off-by: Gert Doering -- kind regards, Gert Doering
[Openvpn-devel] [PATCH applied] Re: Add Windows DNS Leak fix using WFP ('block-outside-dns')
ACK. Lots of people have reviewed this, and it made the patch better :-) - so I list them all, and take the final blame (ACK) on me... Your patch has been applied to the release/2.3 branch. commit dd628d2e0d786e478fd99d54000dceaa42d53855 (release/2.3) Author: ValdikSS List-Post: openvpn-devel@lists.sourceforge.net Date: Thu Dec 10 23:51:35 2015 +0300 Add Windows DNS Leak fix using WFP ('block-outside-dns') Reviewed-by: Selva Nair Reviewed-by: Lev Stipakov Reviewed-by: James Yonan Acked-by: Gert Doering Message-Id: <1449780695-3879-1-git-send-email-...@valdikss.org.ru> URL: http://article.gmane.org/gmane.network.openvpn.devel/10743 Signed-off-by: Gert Doering -- kind regards, Gert Doering
Re: [Openvpn-devel] [PATCH v2] Use adapter index instead of name
Hi, On Fri, Dec 11, 2015 at 10:29:33PM +0200, Lev Stipakov wrote: > Thanks, I think (I think!) I got it now. > > 1) Since we have tt->adapter_index (which temporarily disappeared from > my perception of reality), no need to add new member to tuntap or > tuntap_options. > > 2) tt->adapter_index has nothing to do with rgi->adapter_index, first > one is windows adapter index (which we pass to netsh), last one is for > routing (which we pass to netsh too in special case). > > 3) In add/del_route* we (try to) use tt->adapter_index. Right :-) > Regarding master's "special case" code. Should it be something like this: > >if ( r6->adapter_index ) /* vpn server special route */ > { >struct buffer out = alloc_buf_gc (64, &gc); >buf_printf (&out, "interface=%d", r6->adapter_index ); >device = buf_bptr(&out); >gateway_needed = true; > } >else > { >/* device = interface=tt->adapter_index */ > } Already committed ;-) - but yes, exactly like this. So if we need to have route that points elsewhere (/128 to existing default gateway to enable overlapping v6-in-v6), r6->adapter_index is set. If not, the tap index is used. ... and it works (except that my win7 vm started to really misbehave regarding IPv6 today, so my tests took much longer than planned... grrr) gert -- USENET is *not* the non-clickable part of WWW! //www.muc.de/~gert/ Gert Doering - Munich, Germany g...@greenie.muc.de fax: +49-89-35655025g...@net.informatik.tu-muenchen.de signature.asc Description: PGP signature
[Openvpn-devel] [PATCH applied] Re: Use adapter index for add/delete_route_ipv6
ACK, thanks. I have not actually built or tested a 2.3 binary, but since the code is nearly the same as for master and that one works nicely, I assume this works as well. Your patch has been applied to the release/2.3 branch. commit ca8cead8fd8c00154f35b90593442e2bfa8f735d (release/2.3) Author: Lev Stipakov List-Post: openvpn-devel@lists.sourceforge.net Date: Sat Dec 12 01:10:30 2015 +0200 Use adapter index for add/delete_route_ipv6 Acked-by: Gert Doering Message-Id: <1449875430-15579-1-git-send-email-lstipa...@gmail.com> URL: http://article.gmane.org/gmane.network.openvpn.devel/10760 Signed-off-by: Gert Doering -- kind regards, Gert Doering
Re: [Openvpn-devel] [PATCH applied] Re: Use adapter index for add/delete_route_ipv6
Hi, On Sun, Dec 13, 2015 at 08:00:26PM +0100, Gert Doering wrote: > ACK, and thanks. Code looks good and I'm reasonably sure that it will > do the right thing - waiting for buildbot now to produce a windows binary > to actually test it :-) - then applying the 2.3 version to 2.3 Works! "ifconfig" and routes are both nicely added to "interface=16", which is "tap adapter v9" (and with overlapping routes, I see the /128 installed on "interface=10", so that is still working as well :) ). Good :-) gert -- USENET is *not* the non-clickable part of WWW! //www.muc.de/~gert/ Gert Doering - Munich, Germany g...@greenie.muc.de fax: +49-89-35655025g...@net.informatik.tu-muenchen.de signature.asc Description: PGP signature
Re: [Openvpn-devel] Options that are "safe" for users to modify?
forgot to copy the list.. Hi, Sorry, I missed the point that only referenced files are being replaced. On Sat, Dec 12, 2015 at 9:31 PM, Jonathan K. Bullard wrote: > I'm not clear at all about --crl-verify. Would it ever be used in a > client? Would there be a security risk if a client erased the contents > of the file? (Would that allow a client to connect to a server that > has a revoked certificate which would otherwise not be allowed? > The risk appears to be even less than that of the user replacing ca (i.e very minor). With a new ca, the user can connect to a server presenting a cert from that ca. With crl replaced or removed, the client can connect to a server using a revoked cert. Both these cases also need dns or IP spoofing as you do not allow --remote to be modified by the user. Unless admins start misusing crl as an easy way of disabling a remote (among multiple remotes) listed in .. Selva
[Openvpn-devel] [PATCH applied] Re: Use adapter index for add/delete_route_ipv6
ACK, and thanks. Code looks good and I'm reasonably sure that it will do the right thing - waiting for buildbot now to produce a windows binary to actually test it :-) - then applying the 2.3 version to 2.3 Chances are good that this will break compilation with older MSVC versions, though (as the "struct buffer out = ..." part is now right in the middle of a block which wasn't allowed in earlier versions). Your patch has been applied to the master branch. commit 6417a6f8a01c702e7c8f19f01b696c3b0d2dc1f1 (master) Author: Lev Stipakov List-Post: openvpn-devel@lists.sourceforge.net Date: Sat Dec 12 00:37:52 2015 +0200 Use adapter index for add/delete_route_ipv6 Signed-off-by: Lev Stipakov Acked-by: Gert Doering Message-Id: <1449873472-14954-1-git-send-email-lstipa...@gmail.com> URL: http://article.gmane.org/gmane.network.openvpn.devel/10759 Signed-off-by: Gert Doering -- kind regards, Gert Doering
Re: [Openvpn-devel] Options that are "safe" for users to modify?
Am 12.12.15 um 23:37 schrieb Jonathan K. Bullard: > Hi. > > On Sat, Dec 12, 2015 at 5:23 PM, Arne Schwabe wrote: >> Might not really be related to this but have looked into the work that >> provides the certificates and keys via the managment console? We have >> even have a contrib program that gets certificates from the Mac OS X >> keychain and provides them to OpenVPN. > > Thanks, but I think that should be a separate discussion. That would > work for some situations, but the keychain (actually, OS X has several > keychains) may not be accessible to OpenVPN; files are always > accessible, even before a user is logged in. And I don't think (I'm > doing this from memory, so I might be wrong) that the Keychain patch > allows **all** of the encryption info to be taken from the keychain: I > don't think it allows --secret, --ta, etc. Yes. But the only reason that --secrect, --ta? etc. is not implmented yet is that nobody needed it so far. Arne
Re: [Openvpn-devel] Options that are "safe" for users to modify?
Thanks, Selva. On Sat, Dec 12, 2015 at 5:43 PM, Selva Nair wrote: > I suppose, not just adding but also removing options will be allowed. There > could be more options that are ok (i.e not unsafe) to remove but not change. What I'm proposing isn't to allow "add/remove/modify" options in the OpenVPN configuration file, but to allow the replacement of the contents of files that are referred to in options in it. I admit that is a much less general approach. >> --pkcs1 Sorry, should have been --pkcs12 >> --static Sorry, should have been --secret >> --ta > > --ta ? Oops. No, sorry, that was just plain wrong. The corrected list is: --askpass --auth-user-pass --ca --cert --dh --extra-certs --key --pkcs12 --secret --tls-auth I'm not clear at all about --crl-verify. Would it ever be used in a client? Would there be a security risk if a client erased the contents of the file? (Would that allow a client to connect to a server that has a revoked certificate which would otherwise not be allowed? **That** would be a security problem.) > As remote cant change, several more options may be safe, though note > necessarily very useful. Here are a couple of options that could help when > the server is updated, for example > > --topology t (mainly to remove from client so that a new setting at the > server can take effect through push -- say moving from net30 to subnet) > --comp-lzo > --secret (for non-tls) > --auth > --cipher I'm not (at this point, anyway) talking about changing options in the configuration file, I'm talking about changing the contents of files **referred to** in the configuration file. (So yes to --secret, which I mistyped as --static, but no to the others. I think they are configuration changes that warrant an admin doing them.)