Re: [Openvpn-devel] [PATCH] Make ValdikSS's DNS leak fix platform agnostic

2015-12-13 Thread Selva Nair
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')

2015-12-13 Thread Selva Nair
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')

2015-12-13 Thread Gert Doering
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')

2015-12-13 Thread Gert Doering
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

2015-12-13 Thread Gert Doering
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

2015-12-13 Thread Gert Doering
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

2015-12-13 Thread Gert Doering
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?

2015-12-13 Thread Selva Nair
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

2015-12-13 Thread Gert Doering
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?

2015-12-13 Thread Arne Schwabe
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?

2015-12-13 Thread Jonathan K. Bullard
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.)