Re: [Openvpn-devel] [PATCH 1/1] add more security features for systemd units

2016-12-09 Thread SviMik
> You can break this with something like:
> 
> status /etc/openvpn/client/status.log
> 
> in your configuration. Writing a status file
> to /run/openvpn-{client,server}/status.log works, though. So the default
> setups should be fine. Do we have any more cases where openvpn wants write
> access for whatever?

>From my configuration:
1) status
2) ifconfig-pool-persist
3) tmp-dir (for storing openvpn_pf_*.tmp files)
4) client-connect script may want to write something
5) a plugin may want to write something

For me even the read-only option will break nearly *everything*. And for user 
it will be completely not obvious why his scripts doesn't work, why his status 
file is not updated, and what's wrong with ifconfig-pool-persist.
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH 1/1] bind mount systemd notification socket into chroot

2016-12-09 Thread David Sommerseth
On 10/12/16 00:19, Christian Hesse wrote:
> From: Christian Hesse 
> 
> sd_notify() uses a socket to communicate with systemd. Communication
> fails if the socket is not available within the chroot. So bind mount
> the socket into the chroot when startet from systemd.
> 
> Unsharing namespace and mounting requires extra capability CAP_SYS_ADMIN.

I will pick up this one after 2.4.0 has been released.  This is a very
promising approach.  However, I'm not too happy about CAP_SYS_ADMIN
though, that grants quite some privileges.  Can we look at dropping this
capability once we know we won't need it any more?  Perhaps when we send
READY=1?

> +  char * chroot_notify = NULL;
> +
> +  if (sd_notify(0, "READY=0") > 0)
> +{
> +  asprintf(_notify, "%s/notify", 
> c->options.chroot_dir);

Here we should use the buffer/string functions, based on the gc_arena
implementation.  Unfortunately we do not have a direct equivalent to
asprintf().  A starting point would be to for example look at the string
handling in print_sockaddr_ex() [socket.c:2386] or x_msg_va()
[error.c:251] ... there might be better examples too, I'm just not able
to remember them now :)   buffer.[ch] keeps most of these functions.

The reason for this is basically to use the same well tested
infrastructure.  And with gc_arena, only a single gc_free() is required,
regardless of how many buffers you allocate to that arena.


-- 
kind regards,

David Sommerseth
OpenVPN Technologies, Inc




signature.asc
Description: OpenPGP digital signature
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH 1/1] Clean up plugin path handling

2016-12-09 Thread Christian Hesse
David Sommerseth  on Fri, 2016/12/09 23:40:
> On 09/12/16 22:54, Christian Hesse wrote:
> > David Sommerseth  on Fri, 2016/12/09
> > 22:37:  
> >> On 29/11/16 12:07, Christian Hesse wrote:  
> >>> From: Christian Hesse 
> >>>
> >>> Drop --with-plugindir, instead use an environment variable PLUGINDIR
> >>> to specify the plugin directory.
> >>>
> >>> This always defines PLUGIN_LIBDIR and enables plugin search path.
> >>>
> >>> Signed-off-by: Christian Hesse 
> >>> ---
> >>>  configure.ac| 14 ++
> >>>  src/openvpn/Makefile.am |  3 ++-
> >>>  2 files changed, 8 insertions(+), 9 deletions(-)
> >>>
> >>> diff --git a/configure.ac b/configure.ac
> >>> index f4073d0..5fe652e 100644
> >>> --- a/configure.ac
> >>> +++ b/configure.ac
> >>> @@ -301,13 +301,12 @@ AC_ARG_WITH(
> >>>   [with_crypto_library="openssl"]
> >>>  )
> >>>  
> >>> -AC_ARG_WITH(
> >>> - [plugindir],
> >>> - [AS_HELP_STRING([--with-plugindir], [plugin directory
> >>> @<:@default=LIBDIR/openvpn@:>@])],
> >>> - ,
> >>> - [with_plugindir="\$(libdir)/openvpn/plugins"]
> >>> -)
> >>> -
> >>> +AC_ARG_VAR([PLUGINDIR], [Path of plug-in directory
> >>> @<:@default=LIBDIR/openvpn/plugins@:>@]) +if test -n "${PLUGINDIR}";
> >>> then
> >>> + plugindir="${PLUGINDIR}"
> >>> +else
> >>> + plugindir="\${libdir}/openvpn/plugins"
> >>> +fi
> >>
> >> Finally had some time to dig into this one.  I like the idea here, I
> >> think it makes sense.  But I'm not sure ${libdir} is correct by default.
> >>  I think that should be /usr/local/lib64 by default on a 64bit system.
> >>
> >> My google-foo isn't helpful for me today ... This is somewhat related,
> >> especially towards the end of this doc page:
> >> 
> >>
> >> But I believe there are better ways to do this.
> >>
> >> I haven't checked this in detail what happens with 'make install'.  We
> >> should ensure that the plug-ins we ship (./src/plugins) which are built
> >> are installed as well into this directory.  
> > 
> > ${libdir} is where plugins are installed to... That's why I choose it. ;)
> > 
> > Installing anything to $prefix/lib64/ does not make sense imho. Never.  
> 
> Well, that is the default for Fedora/RHEL families.  32bit systems uses
> /usr/lib, 64bit uses /usr/lib64.
> 
> $ uname -m
> x86_64
> $ cat /etc/redhat-release
> Red Hat Enterprise Linux Server release 7.3 (Maipo)
> $ rpm --eval '%{_libdir}'
> /usr/lib64
> 
> I've tested this on Fedora 24 and Scientific Linux 6 and 7 as well, with
> the same result.
> 
> And you'll find the same in openSUSE too:
> 
> 
> Unless it has changed in Debian/Ubuntu, it is a similar policy there too.

Uh, interesting... Probably I did not look at these distributions for too
long... (And/or relied on the package manager.)

For Arch Linux we use /usr/lib/, though /usr/lib64/ is provided by a symlink
to lib nowadays.
-- 
main(a){char*c=/*Schoene Gruesse */"B?IJj;MEH"
"CX:;",b;for(a/*Best regards my address:*/=0;b=c[a++];)
putchar(b-1/(/*Chriscc -ox -xc - && ./x*/b/42*2-3)*42);}


pgpFNF7JacsGA.pgp
Description: OpenPGP digital signature
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH 1/1] bind mount systemd notification socket into chroot

2016-12-09 Thread Christian Hesse
From: Christian Hesse 

sd_notify() uses a socket to communicate with systemd. Communication
fails if the socket is not available within the chroot. So bind mount
the socket into the chroot when startet from systemd.

Unsharing namespace and mounting requires extra capability CAP_SYS_ADMIN.

Signed-off-by: Christian Hesse 
---
 distro/systemd/openvpn-client@.service |  2 +-
 distro/systemd/openvpn-server@.service |  2 +-
 src/openvpn/init.c | 37 +++---
 3 files changed, 23 insertions(+), 18 deletions(-)

diff --git a/distro/systemd/openvpn-client@.service 
b/distro/systemd/openvpn-client@.service
index 5618af3..3ebd1da 100644
--- a/distro/systemd/openvpn-client@.service
+++ b/distro/systemd/openvpn-client@.service
@@ -13,7 +13,7 @@ RuntimeDirectory=openvpn-client
 RuntimeDirectoryMode=0710
 WorkingDirectory=/etc/openvpn/client
 ExecStart=/usr/sbin/openvpn --suppress-timestamps --nobind --config %i.conf
-CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_RAW CAP_SETGID 
CAP_SETUID CAP_SYS_CHROOT CAP_DAC_OVERRIDE
+CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_RAW CAP_SETGID 
CAP_SETUID CAP_SYS_ADMIN CAP_SYS_CHROOT CAP_DAC_OVERRIDE
 LimitNPROC=10
 DeviceAllow=/dev/null rw
 DeviceAllow=/dev/net/tun rw
diff --git a/distro/systemd/openvpn-server@.service 
b/distro/systemd/openvpn-server@.service
index b9b4dba..bcf1bcf 100644
--- a/distro/systemd/openvpn-server@.service
+++ b/distro/systemd/openvpn-server@.service
@@ -13,7 +13,7 @@ RuntimeDirectory=openvpn-server
 RuntimeDirectoryMode=0710
 WorkingDirectory=/etc/openvpn/server
 ExecStart=/usr/sbin/openvpn --status %t/openvpn-server/status-%i.log 
--status-version 2 --suppress-timestamps --config %i.conf
-CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_BIND_SERVICE 
CAP_NET_RAW CAP_SETGID CAP_SETUID CAP_SYS_CHROOT CAP_DAC_OVERRIDE
+CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_BIND_SERVICE 
CAP_NET_RAW CAP_SETGID CAP_SETUID CAP_SYS_ADMIN CAP_SYS_CHROOT CAP_DAC_OVERRIDE
 LimitNPROC=10
 DeviceAllow=/dev/null rw
 DeviceAllow=/dev/net/tun rw
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 26b236d..81bf9ab 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -31,6 +31,9 @@
 #include "syshead.h"
 
 #ifdef ENABLE_SYSTEMD
+#include 
+#include 
+#include 
 #include 
 #endif
 
@@ -969,22 +972,24 @@ do_uid_gid_chroot (struct context *c, bool no_delay)
  if (no_delay)
 {
 #ifdef ENABLE_SYSTEMD
-  /* If OpenVPN is started by systemd, the OpenVPN process needs
-   * to provide a preliminary status report to systemd.  This is
-   * needed as $NOTIFY_SOCKET will not be available inside the
-   * chroot, which sd_notify()/sd_notifyf() depends on.
-   *
-   * This approach is the simplest and the most non-intrusive
-   * solution right before the 2.4_rc2 release.
-   *
-   * TODO: Consider altnernative solutions - bind mount?
-   * systemd does not grok OpenVPN configuration files, thus cannot
-   * have a sane way to know if OpenVPN will chroot or not and to
-   * which subdirectory it will chroot into.
-   */
-  sd_notifyf(0, "READY=1\n"
-"STATUS=Entering chroot, most of the init completed 
successfully\n"
-"MAINPID=%lu", (unsigned long) getpid());
+  int fd;
+  char * chroot_notify = NULL;
+
+  if (sd_notify(0, "READY=0") > 0)
+{
+  asprintf(_notify, "%s/notify", c->options.chroot_dir);
+
+  if (unshare(CLONE_NEWNS) != 0)
+msg (M_ERR, "unshare failed");
+  if ((fd = open(chroot_notify, O_WRONLY | O_CREAT | O_TRUNC, 
0644)) < 0)
+msg (M_ERR, "touch failed");
+  close(fd);
+  if (mount(getenv("NOTIFY_SOCKET"), chroot_notify, NULL, 
MS_BIND, NULL) != 0)
+msg (M_ERR, "bind mounting notification socket failed");
+
+  setenv("NOTIFY_SOCKET", "/notify", 1);
+  free(chroot_notify);
+}
 #endif
   platform_chroot (c->options.chroot_dir);
 }
-- 
2.10.2


--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH 1/1] Clean up plugin path handling

2016-12-09 Thread David Sommerseth
On 09/12/16 22:54, Christian Hesse wrote:
> David Sommerseth  on Fri, 2016/12/09 22:37:
>> On 29/11/16 12:07, Christian Hesse wrote:
>>> From: Christian Hesse 
>>>
>>> Drop --with-plugindir, instead use an environment variable PLUGINDIR
>>> to specify the plugin directory.
>>>
>>> This always defines PLUGIN_LIBDIR and enables plugin search path.
>>>
>>> Signed-off-by: Christian Hesse 
>>> ---
>>>  configure.ac| 14 ++
>>>  src/openvpn/Makefile.am |  3 ++-
>>>  2 files changed, 8 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/configure.ac b/configure.ac
>>> index f4073d0..5fe652e 100644
>>> --- a/configure.ac
>>> +++ b/configure.ac
>>> @@ -301,13 +301,12 @@ AC_ARG_WITH(
>>> [with_crypto_library="openssl"]
>>>  )
>>>  
>>> -AC_ARG_WITH(
>>> -   [plugindir],
>>> -   [AS_HELP_STRING([--with-plugindir], [plugin directory
>>> @<:@default=LIBDIR/openvpn@:>@])],
>>> -   ,
>>> -   [with_plugindir="\$(libdir)/openvpn/plugins"]
>>> -)
>>> -
>>> +AC_ARG_VAR([PLUGINDIR], [Path of plug-in directory
>>> @<:@default=LIBDIR/openvpn/plugins@:>@]) +if test -n "${PLUGINDIR}"; then
>>> +   plugindir="${PLUGINDIR}"
>>> +else
>>> +   plugindir="\${libdir}/openvpn/plugins"
>>> +fi  
>>
>> Finally had some time to dig into this one.  I like the idea here, I
>> think it makes sense.  But I'm not sure ${libdir} is correct by default.
>>  I think that should be /usr/local/lib64 by default on a 64bit system.
>>
>> My google-foo isn't helpful for me today ... This is somewhat related,
>> especially towards the end of this doc page:
>> 
>>
>> But I believe there are better ways to do this.
>>
>> I haven't checked this in detail what happens with 'make install'.  We
>> should ensure that the plug-ins we ship (./src/plugins) which are built
>> are installed as well into this directory.
> 
> ${libdir} is where plugins are installed to... That's why I choose it. ;)
> 
> Installing anything to $prefix/lib64/ does not make sense imho. Never.

Well, that is the default for Fedora/RHEL families.  32bit systems uses
/usr/lib, 64bit uses /usr/lib64.

$ uname -m
x86_64
$ cat /etc/redhat-release
Red Hat Enterprise Linux Server release 7.3 (Maipo)
$ rpm --eval '%{_libdir}'
/usr/lib64

I've tested this on Fedora 24 and Scientific Linux 6 and 7 as well, with
the same result.

And you'll find the same in openSUSE too:


Unless it has changed in Debian/Ubuntu, it is a similar policy there too.


-- 
kind regards,

David Sommerseth
OpenVPN Technologies, Inc




signature.asc
Description: OpenPGP digital signature
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH 1/1] Clean up plugin path handling

2016-12-09 Thread Christian Hesse
David Sommerseth  on Fri, 2016/12/09 22:37:
> On 29/11/16 12:07, Christian Hesse wrote:
> > From: Christian Hesse 
> > 
> > Drop --with-plugindir, instead use an environment variable PLUGINDIR
> > to specify the plugin directory.
> > 
> > This always defines PLUGIN_LIBDIR and enables plugin search path.
> > 
> > Signed-off-by: Christian Hesse 
> > ---
> >  configure.ac| 14 ++
> >  src/openvpn/Makefile.am |  3 ++-
> >  2 files changed, 8 insertions(+), 9 deletions(-)
> > 
> > diff --git a/configure.ac b/configure.ac
> > index f4073d0..5fe652e 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -301,13 +301,12 @@ AC_ARG_WITH(
> > [with_crypto_library="openssl"]
> >  )
> >  
> > -AC_ARG_WITH(
> > -   [plugindir],
> > -   [AS_HELP_STRING([--with-plugindir], [plugin directory
> > @<:@default=LIBDIR/openvpn@:>@])],
> > -   ,
> > -   [with_plugindir="\$(libdir)/openvpn/plugins"]
> > -)
> > -
> > +AC_ARG_VAR([PLUGINDIR], [Path of plug-in directory
> > @<:@default=LIBDIR/openvpn/plugins@:>@]) +if test -n "${PLUGINDIR}"; then
> > +   plugindir="${PLUGINDIR}"
> > +else
> > +   plugindir="\${libdir}/openvpn/plugins"
> > +fi  
> 
> Finally had some time to dig into this one.  I like the idea here, I
> think it makes sense.  But I'm not sure ${libdir} is correct by default.
>  I think that should be /usr/local/lib64 by default on a 64bit system.
> 
> My google-foo isn't helpful for me today ... This is somewhat related,
> especially towards the end of this doc page:
> 
> 
> But I believe there are better ways to do this.
> 
> I haven't checked this in detail what happens with 'make install'.  We
> should ensure that the plug-ins we ship (./src/plugins) which are built
> are installed as well into this directory.

${libdir} is where plugins are installed to... That's why I choose it. ;)

Installing anything to $prefix/lib64/ does not make sense imho. Never.

32bit systems install to $prefix/lib/, 64bit system install to $prefix/lib/.
The only valid extra is $prefix/lib32/ for multilib on 64bit systemd.
-- 
main(a){char*c=/*Schoene Gruesse */"B?IJj;MEH"
"CX:;",b;for(a/*Best regards my address:*/=0;b=c[a++];)
putchar(b-1/(/*Chriscc -ox -xc - && ./x*/b/42*2-3)*42);}


pgpYJbsIcTxRu.pgp
Description: OpenPGP digital signature
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH 1/1] add more security features for systemd units

2016-12-09 Thread Christian Hesse
David Sommerseth  on Fri, 2016/12/09 20:42:
> On 09/12/16 19:13, Christian Hesse wrote:
> > From: Christian Hesse 
> > 
> > ProtectSystem=strict mounts the entire file system hierarchy read-only,
> > except for the API file system subtrees /dev, /proc and /sys (which can
> > be protected using PrivateDevices=, ProtectKernelTunables=,
> > ProtectControlGroups=).
> > 
> > ProtectHome=true makes the directories /home, /root and /run/user
> > inaccessible and empty for the process.  
> 
> Currently I don't think we can use ProtectedHome=  as it is fully
> possible to save certificates and keys under $HOME/.cert on Fedora/RHEL
> (and clones).  There is even a specific SELinux label for files in that
> path, home_cert_t.

I know that NetworkManager and its openvpn plugin use $HOME/.cert/... But
openvpn is not started from systemd then. Do we have setups where openvpn
starts from systemd and reads certificates from $HOME?

ProtectHome=read-only could help here... But I would still prefer
ProtectHome=true.

BTW, setting can be overwritten with something like:

mkdir /etc/systemd/system/openvpn-client@example.service.d
cat > /etc/systemd/system/openvpn-client@example.service.d/protecthome.conf
< For the others, I think they are more reasonable ... But I need to dig
> into the more murky details to be 100% they are safe for us.  This is
> anyhow something we need to postpone until after 2.4.0 ... I don't dare
> adding more things which may backfire in rc2, as we're on a strict
> schedule to manage the next Debian release.
> 
> Once rc2 settles, I will start playing with this patch.

Agreed this is post-2.4.0 stuff. ;)

You can break this with something like:

status /etc/openvpn/client/status.log

in your configuration. Writing a status file
to /run/openvpn-{client,server}/status.log works, though. So the default
setups should be fine. Do we have any more cases where openvpn wants write
access for whatever?
-- 
main(a){char*c=/*Schoene Gruesse */"B?IJj;MEH"
"CX:;",b;for(a/*Best regards my address:*/=0;b=c[a++];)
putchar(b-1/(/*Chriscc -ox -xc - && ./x*/b/42*2-3)*42);}


pgpsreZ2srCdu.pgp
Description: OpenPGP digital signature
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] Coding style clean-up ... phase 1

2016-12-09 Thread Selva Nair
On Fri, Dec 9, 2016 at 4:39 PM, David Sommerseth <
open...@sf.lists.topphemmelig.net> wrote:

> On 09/12/16 22:27, Steffan Karger wrote:
> >
> > Sounds like we have a final config on the CodeStyle page now.  Are we
> > ready to run it on all code now, and publish a reformat branch?
> >
>
> Agreed.  I can do this later this night.



Wait a minute we may need cmt_cpp_to_c=true  -- I saw some C++ comments
(added by uncrustify) in header files against #endif etc..

Selva
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH applied] systemd: Intermediate --chroot fix with the new sd_notify() implementation

2016-12-09 Thread David Sommerseth
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Your patch has been applied to the master branch

commit 65140a3acfa42e5d42cdfcf8108f00a62d5767ff
Author: David Sommerseth
Date:   Wed Dec 7 03:51:52 2016 +0100

 systemd: Intermediate --chroot fix with the new sd_notify() implementation

 Signed-off-by: David Sommerseth 
 Acked-by: Christian Hesse 
 Message-Id: <1481079112-22990-1-git-send-email-dav...@openvpn.net>
 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13416.html


- --
kind regards,

David Sommerseth

-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJYSyTlAAoJEIbPlEyWcf3y/sMQAMaJhmd4WJjjr7am+6SqMyWg
pk7J4FzysRu8Dm/czDEULAriDO6YDMrVN/HNO3z/tLxUKnwvP1Qq9RbzM1HwDHbH
3vuVFu/Id4GGwiVqu35LsOq0dVwLra188L709AgTqepCXCuajUxSDtl8lcTCL2vN
gDTL0FiYQSTl/jAlMKAeFg8W8C5pFDPOUumBgcYNdqqwW7FUPsl5JYqZup9bqgPO
cuY5kSx8GUe7En1EPSbGIdX/cVwkHDtxs9Caqpr1Qb0d8iinchDCaz8YrYDjvpkG
j1NE32lNbWLpeiXP2hLN9YIg+V7TSuT1jCyUwKJUEt/y0dZxdlpfeyS52WAGVTZY
BaRmnzdGHmdYbSSv8ft1qKh6NI/3sRfmSzd2+eb9dTC2FsUUSoprU0yAISYR+dew
Jvp8jeE/F4xUGKp6ierMkzvmqtudQRrq3hkmtbpS1CaM2Pf+uugQ7Z3s7rk+GRwb
2E0YpzKQNheeTsbInQWJQoXLZx75anQUu8uVl9IA8uabLSNJdzCfik2Mjbzs7/Kc
hDJ4S0eJGIp2R7PWJUj0KdLyEP5853fGof6pfc1m8e0GCI/WCbPkEg7nRE6fqrXL
n92sFCzVq8z8Ad6Up0CEB3lxW9purT39QVtvd56N9D2M8i8OMydIOOoZA0iQeBuI
GcuJ6EC98Ul19QPdFCrk
=YYaa
-END PGP SIGNATURE-

--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] Coding style clean-up ... phase 1

2016-12-09 Thread David Sommerseth
On 09/12/16 22:27, Steffan Karger wrote:
> 
> Sounds like we have a final config on the CodeStyle page now.  Are we
> ready to run it on all code now, and publish a reformat branch?
> 

Agreed.  I can do this later this night.


-- 
kind regards,

David Sommerseth
OpenVPN Technologies, Inc




signature.asc
Description: OpenPGP digital signature
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH 1/1] Clean up plugin path handling

2016-12-09 Thread David Sommerseth
On 29/11/16 12:07, Christian Hesse wrote:
> From: Christian Hesse 
> 
> Drop --with-plugindir, instead use an environment variable PLUGINDIR
> to specify the plugin directory.
> 
> This always defines PLUGIN_LIBDIR and enables plugin search path.
> 
> Signed-off-by: Christian Hesse 
> ---
>  configure.ac| 14 ++
>  src/openvpn/Makefile.am |  3 ++-
>  2 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index f4073d0..5fe652e 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -301,13 +301,12 @@ AC_ARG_WITH(
>   [with_crypto_library="openssl"]
>  )
>  
> -AC_ARG_WITH(
> - [plugindir],
> - [AS_HELP_STRING([--with-plugindir], [plugin directory 
> @<:@default=LIBDIR/openvpn@:>@])],
> - ,
> - [with_plugindir="\$(libdir)/openvpn/plugins"]
> -)
> -
> +AC_ARG_VAR([PLUGINDIR], [Path of plug-in directory 
> @<:@default=LIBDIR/openvpn/plugins@:>@])
> +if test -n "${PLUGINDIR}"; then
> + plugindir="${PLUGINDIR}"
> +else
> + plugindir="\${libdir}/openvpn/plugins"
> +fi

Finally had some time to dig into this one.  I like the idea here, I
think it makes sense.  But I'm not sure ${libdir} is correct by default.
 I think that should be /usr/local/lib64 by default on a 64bit system.

My google-foo isn't helpful for me today ... This is somewhat related,
especially towards the end of this doc page:


But I believe there are better ways to do this.

I haven't checked this in detail what happens with 'make install'.  We
should ensure that the plug-ins we ship (./src/plugins) which are built
are installed as well into this directory.


-- 
kind regards,

David Sommerseth
OpenVPN Technologies, Inc




signature.asc
Description: OpenPGP digital signature
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] Coding style clean-up ... phase 1

2016-12-09 Thread Steffan Karger
On 9 December 2016 at 21:43, Selva Nair  wrote:
> On Fri, Dec 9, 2016 at 8:41 AM, Steffan Karger  wrote:
>> On 9 December 2016 at 00:14, David Sommerseth
>>  wrote:
>> > I just spotted in ssl.c that we need sp_assign=add.
>> >
>> > [ ssl.c, tls1_PRF() ]
>> > len = slen/2;
>> > S1 = sec;
>> > S2 = &(sec[len]);
>> > len += (slen&1); /* add for odd, make longer */
>> >
>> > I believe we've agreed on spaces around assignments.
>>
>> Hm, I'd be fine with that, but I'm afraid there are too many such
>> details to bikeshed about.  Once we've fixed the curious newlines
>> Selva mentionded (sorry, not sure when I can look at that), I think we
>> should stop the discussion and move forward.  Just 5 more days until
>> rc2 (where the reformatting should be done).
>
> Agreed.  I'll make no more noise :)
>
> To troubleshoot the extra newlines in places like line 130 of the file you
> uploaded
> https://paste.fedoraproject.org/502063/36953148
> I ran uncrustify using the options listed at our CodeStyle page and all look
> good and nice. No such extra newlines. By the way sp_between_byref is not a
> valid option.

Hm, indeed.  Must have been an artifact of running uncrustify multiple
times or something like that.  I removed sp_between_byref.

Sounds like we have a final config on the CodeStyle page now.  Are we
ready to run it on all code now, and publish a reformat branch?

-Steffan

--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] Coding style clean-up ... phase 1

2016-12-09 Thread Selva Nair
On Fri, Dec 9, 2016 at 8:41 AM, Steffan Karger  wrote:

>
> On 9 December 2016 at 00:14, David Sommerseth
>  wrote:
> > I just spotted in ssl.c that we need sp_assign=add.
> >
> > [ ssl.c, tls1_PRF() ]
> > len = slen/2;
> > S1 = sec;
> > S2 = &(sec[len]);
> > len += (slen&1); /* add for odd, make longer */
> >
> > I believe we've agreed on spaces around assignments.
>
> Hm, I'd be fine with that, but I'm afraid there are too many such
> details to bikeshed about.  Once we've fixed the curious newlines
> Selva mentionded (sorry, not sure when I can look at that), I think we
> should stop the discussion and move forward.  Just 5 more days until
> rc2 (where the reformatting should be done).



Agreed.  I'll make no more noise :)

To troubleshoot the extra newlines in places like line 130 of the file you
uploaded
https://paste.fedoraproject.org/502063/36953148
I ran uncrustify using the options listed at our CodeStyle page and all
look good and nice. No such extra newlines. By the way sp_between_byref is
not a valid option.

Selva
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH] systemd: Intermediate --chroot fix with the new sd_notify() implementation

2016-12-09 Thread David Sommerseth
On 09/12/16 18:59, Christian Hesse wrote:
> Christian Hesse  on Fri, 2016/12/09 18:37:
>> David Sommerseth  on Wed, 2016/12/07 03:51:
>>> Commit c5931897ae8d663e7e introduced support for talking directly
>>> to the systemd service manager about the situation for the OpenVPN
>>> tunnel. This approach makes a lot of sense and is mostly the proper
>>> way to do it.  But it was discovered that it breaks OpenVPN
>>> configurations using --chroot.
>>>
>>> The reason sd_notify() calls fails when using chroot() is that
>>> sd_notify() expects to have access to a file as declared in the
>>> $NOTIFY_SOCKET environment variable.  It is the main systemd
>>> instance which is responsible to provide both the environment variable
>>> as well as the socket file sd_nodify() should use.  When --chroot
>>> comes into play, the $NOTIFY_SOCKET file will not be available
>>> for OpenVPN any more.
>>>
>>> As things are getting close to the 2.4_rc2 release we will not dare
>>> to bring a too invasive fix.  As well we need some time to discuss
>>> an approrpriate solution.  So this intermediate fix will only
>>> provide a "successful start" message to the systemd service manager
>>> right before chroot() happens.  This will at least resolve the issue
>>> in a safe and non-intrusive way.
>>>
>>> Signed-off-by: David Sommerseth 
>>> ---
>>>  src/openvpn/init.c | 22 +-
>>>  1 file changed, 21 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/openvpn/init.c b/src/openvpn/init.c
>>> index 74f1139..e47f0d4 100644
>>> --- a/src/openvpn/init.c
>>> +++ b/src/openvpn/init.c
>>> @@ -967,7 +967,27 @@ do_uid_gid_chroot (struct context *c, bool no_delay)
>>>if (c->options.chroot_dir)
>>> {
>>>   if (no_delay)
>>> -   platform_chroot (c->options.chroot_dir);
>>> +{
>>> +#ifdef ENABLE_SYSTEMD
>>> +  /* If OpenVPN is started by systemd, the OpenVPN process
>>> needs
>>> +   * to provide a preliminary status report to systemd.  This
>>> is
>>> +   * needed as $NOTIFY_SOCKET will not be available inside
>>> the
>>> +   * chroot, which sd_notify()/sd_notifyf() depends on.
>>> +   *
>>> +   * This approach is the simplest and the most non-intrusive
>>> +   * solution right before the 2.4_rc2 release.
>>> +   *
>>> +   * TODO: Consider altnernative solutions - bind mount?
>>> +   * systemd does not grok OpenVPN configuration files, thus
>>> cannot
>>> +   * have a sane way to know if OpenVPN will chroot or not
>>> and to
>>> +   * which subdirectory it will chroot into.
>>> +   */
>>> +  sd_notifyf(0, "READY=1\n"
>>> +"STATUS=Entering chroot, most of the init completed
>>> successfully\n"
>>> +"MAINPID=%lu", (unsigned long) getpid());
>>> +#endif
>>> +  platform_chroot (c->options.chroot_dir);
>>> +}
>>>   else if (c->first_time)
>>> msg (M_INFO, "NOTE: chroot %s", why_not);
>>> }  
>>
>> Looks good to me, so: ACK

Thanks!

>> In long term we should think about a proper solution. Notification socket
>> is /run/systemd/notify, so we would have to make that available from within
>> the chroot.
> 
> BTW, systemd used to use abstract sockets for notifications, which worked with
> processes that chroot themselves. This was changed [0] in favor to
> PrivateNetwork setting. Private network (in terms of systemd) is nothing we
> can make use of, though. Obviously... :-p

:)

> Does it make sense to deny to chroot when running from systemd and instead
> add some more security features to the systemd unit? Things like
> ProtectSystem=strict, ProtectHome=true and friends. See systemd.exec(5) [1]
> for details.

Yes, I think this can make sense.  I would also like to see more where
things are headed in regards to namespaces (or app-container, if you
want).  If we can also isolate the openvpn process from as much as
possible from the rest of the system (and find a solution for
$HOME/.cert).   But still that does provide some other challenges,
though - like: What to do with script hooks and --plugin files?  And
especially other dependencies of shells/interpretors and shared objects
they may depend on.

Perhaps a discussion we could open up with the systemd team?


-- 
kind regards,

David Sommerseth
OpenVPN Technologies, Inc




signature.asc
Description: OpenPGP digital signature
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net

Re: [Openvpn-devel] [PATCH 1/1] add more security features for systemd units

2016-12-09 Thread David Sommerseth
On 09/12/16 19:13, Christian Hesse wrote:
> From: Christian Hesse 
> 
> ProtectSystem=strict mounts the entire file system hierarchy read-only,
> except for the API file system subtrees /dev, /proc and /sys (which can
> be protected using PrivateDevices=, ProtectKernelTunables=,
> ProtectControlGroups=).
> 
> ProtectHome=true makes the directories /home, /root and /run/user
> inaccessible and empty for the process.

Currently I don't think we can use ProtectedHome=  as it is fully
possible to save certificates and keys under $HOME/.cert on Fedora/RHEL
(and clones).  There is even a specific SELinux label for files in that
path, home_cert_t.

For the others, I think they are more reasonable ... But I need to dig
into the more murky details to be 100% they are safe for us.  This is
anyhow something we need to postpone until after 2.4.0 ... I don't dare
adding more things which may backfire in rc2, as we're on a strict
schedule to manage the next Debian release.

Once rc2 settles, I will start playing with this patch.


-- 
kind regards,

David Sommerseth
OpenVPN Technologies, Inc




signature.asc
Description: OpenPGP digital signature
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] On saving passwords

2016-12-09 Thread Илья Шипицин
9 дек. 2016 г. 22:40 пользователь "Selva Nair" 
написал:

Hi,

A comment  on the GUI github page said:

"For ISO27001 certification, we are not allowed to let users save their VPN
passwords locally. Is there a way to remove or disable the 'save password'
box upon authentication ?"

Although I suggested to use an up script to delete the saved password, the
GUI displaying a checkbox to save password may not be acceptable to some
setups. Any idea how widespread a concern is this? Note that the GUI saves
it encrypted. Personally I believe not saving passwords encourages users to
choose weak passwords, but we could make the GUI respect any --auth-nocache
in the config.



There might be some interest for FIPS or ISO27001 variants, we may run some
questionary to investigate that.

It also makes sense to make default openvpn software to satisfy those
standards. I've idea is it possible or not.


More info here (https://github.com/OpenVPN/openvpn-gui/issues/105)

Thanks,

Selva


--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH 1/1] add more security features for systemd units

2016-12-09 Thread Christian Hesse
From: Christian Hesse 

ProtectSystem=strict mounts the entire file system hierarchy read-only,
except for the API file system subtrees /dev, /proc and /sys (which can
be protected using PrivateDevices=, ProtectKernelTunables=,
ProtectControlGroups=).

ProtectHome=true makes the directories /home, /root and /run/user
inaccessible and empty for the process.

See systemd.exec(5) [0] for details.

[0] https://www.freedesktop.org/software/systemd/man/systemd.exec.html

Signed-off-by: Christian Hesse 
---
 distro/systemd/openvpn-client@.service | 2 ++
 distro/systemd/openvpn-server@.service | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/distro/systemd/openvpn-client@.service 
b/distro/systemd/openvpn-client@.service
index 5618af3..3a9b7e2 100644
--- a/distro/systemd/openvpn-client@.service
+++ b/distro/systemd/openvpn-client@.service
@@ -17,6 +17,8 @@ CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_RAW 
CAP_SETGID CAP_SETU
 LimitNPROC=10
 DeviceAllow=/dev/null rw
 DeviceAllow=/dev/net/tun rw
+ProtectSystem=strict
+ProtectHome=true
 
 [Install]
 WantedBy=multi-user.target
diff --git a/distro/systemd/openvpn-server@.service 
b/distro/systemd/openvpn-server@.service
index b9b4dba..a9e57b2 100644
--- a/distro/systemd/openvpn-server@.service
+++ b/distro/systemd/openvpn-server@.service
@@ -17,6 +17,8 @@ CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN 
CAP_NET_BIND_SERVICE CAP_NET_RA
 LimitNPROC=10
 DeviceAllow=/dev/null rw
 DeviceAllow=/dev/net/tun rw
+ProtectSystem=strict
+ProtectHome=true
 
 [Install]
 WantedBy=multi-user.target
-- 
2.10.2


--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH] systemd: Intermediate --chroot fix with the new sd_notify() implementation

2016-12-09 Thread Christian Hesse
Christian Hesse  on Fri, 2016/12/09 18:37:
> David Sommerseth  on Wed, 2016/12/07 03:51:
> > Commit c5931897ae8d663e7e introduced support for talking directly
> > to the systemd service manager about the situation for the OpenVPN
> > tunnel. This approach makes a lot of sense and is mostly the proper
> > way to do it.  But it was discovered that it breaks OpenVPN
> > configurations using --chroot.
> > 
> > The reason sd_notify() calls fails when using chroot() is that
> > sd_notify() expects to have access to a file as declared in the
> > $NOTIFY_SOCKET environment variable.  It is the main systemd
> > instance which is responsible to provide both the environment variable
> > as well as the socket file sd_nodify() should use.  When --chroot
> > comes into play, the $NOTIFY_SOCKET file will not be available
> > for OpenVPN any more.
> > 
> > As things are getting close to the 2.4_rc2 release we will not dare
> > to bring a too invasive fix.  As well we need some time to discuss
> > an approrpriate solution.  So this intermediate fix will only
> > provide a "successful start" message to the systemd service manager
> > right before chroot() happens.  This will at least resolve the issue
> > in a safe and non-intrusive way.
> > 
> > Signed-off-by: David Sommerseth 
> > ---
> >  src/openvpn/init.c | 22 +-
> >  1 file changed, 21 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/openvpn/init.c b/src/openvpn/init.c
> > index 74f1139..e47f0d4 100644
> > --- a/src/openvpn/init.c
> > +++ b/src/openvpn/init.c
> > @@ -967,7 +967,27 @@ do_uid_gid_chroot (struct context *c, bool no_delay)
> >if (c->options.chroot_dir)
> > {
> >   if (no_delay)
> > -   platform_chroot (c->options.chroot_dir);
> > +{
> > +#ifdef ENABLE_SYSTEMD
> > +  /* If OpenVPN is started by systemd, the OpenVPN process
> > needs
> > +   * to provide a preliminary status report to systemd.  This
> > is
> > +   * needed as $NOTIFY_SOCKET will not be available inside
> > the
> > +   * chroot, which sd_notify()/sd_notifyf() depends on.
> > +   *
> > +   * This approach is the simplest and the most non-intrusive
> > +   * solution right before the 2.4_rc2 release.
> > +   *
> > +   * TODO: Consider altnernative solutions - bind mount?
> > +   * systemd does not grok OpenVPN configuration files, thus
> > cannot
> > +   * have a sane way to know if OpenVPN will chroot or not
> > and to
> > +   * which subdirectory it will chroot into.
> > +   */
> > +  sd_notifyf(0, "READY=1\n"
> > +"STATUS=Entering chroot, most of the init completed
> > successfully\n"
> > +"MAINPID=%lu", (unsigned long) getpid());
> > +#endif
> > +  platform_chroot (c->options.chroot_dir);
> > +}
> >   else if (c->first_time)
> > msg (M_INFO, "NOTE: chroot %s", why_not);
> > }  
> 
> Looks good to me, so: ACK
> 
> In long term we should think about a proper solution. Notification socket
> is /run/systemd/notify, so we would have to make that available from within
> the chroot.

BTW, systemd used to use abstract sockets for notifications, which worked with
processes that chroot themselves. This was changed [0] in favor to
PrivateNetwork setting. Private network (in terms of systemd) is nothing we
can make use of, though. Obviously... :-p

Does it make sense to deny to chroot when running from systemd and instead
add some more security features to the systemd unit? Things like
ProtectSystem=strict, ProtectHome=true and friends. See systemd.exec(5) [1]
for details.

[0]
https://github.com/systemd/systemd/commit/7181dbdb2e3112858d62bdaea4f0ad2ed685ccba
[1] https://www.freedesktop.org/software/systemd/man/systemd.exec.html
-- 
main(a){char*c=/*Schoene Gruesse */"B?IJj;MEH"
"CX:;",b;for(a/*Best regards my address:*/=0;b=c[a++];)
putchar(b-1/(/*Chriscc -ox -xc - && ./x*/b/42*2-3)*42);}


pgpllYZqjGzAH.pgp
Description: OpenPGP digital signature
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] On saving passwords

2016-12-09 Thread Selva Nair
Hi,

A comment  on the GUI github page said:

"For ISO27001 certification, we are not allowed to let users save their VPN
passwords locally. Is there a way to remove or disable the 'save password'
box upon authentication ?"

Although I suggested to use an up script to delete the saved password, the
GUI displaying a checkbox to save password may not be acceptable to some
setups. Any idea how widespread a concern is this? Note that the GUI saves
it encrypted. Personally I believe not saving passwords encourages users to
choose weak passwords, but we could make the GUI respect any --auth-nocache
in the config.

More info here (https://github.com/OpenVPN/openvpn-gui/issues/105)

Thanks,

Selva
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH] systemd: Intermediate --chroot fix with the new sd_notify() implementation

2016-12-09 Thread Christian Hesse
David Sommerseth  on Wed, 2016/12/07 03:51:
> Commit c5931897ae8d663e7e introduced support for talking directly
> to the systemd service manager about the situation for the OpenVPN
> tunnel. This approach makes a lot of sense and is mostly the proper
> way to do it.  But it was discovered that it breaks OpenVPN
> configurations using --chroot.
> 
> The reason sd_notify() calls fails when using chroot() is that
> sd_notify() expects to have access to a file as declared in the
> $NOTIFY_SOCKET environment variable.  It is the main systemd
> instance which is responsible to provide both the environment variable
> as well as the socket file sd_nodify() should use.  When --chroot
> comes into play, the $NOTIFY_SOCKET file will not be available
> for OpenVPN any more.
> 
> As things are getting close to the 2.4_rc2 release we will not dare
> to bring a too invasive fix.  As well we need some time to discuss
> an approrpriate solution.  So this intermediate fix will only
> provide a "successful start" message to the systemd service manager
> right before chroot() happens.  This will at least resolve the issue
> in a safe and non-intrusive way.
> 
> Signed-off-by: David Sommerseth 
> ---
>  src/openvpn/init.c | 22 +-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/src/openvpn/init.c b/src/openvpn/init.c
> index 74f1139..e47f0d4 100644
> --- a/src/openvpn/init.c
> +++ b/src/openvpn/init.c
> @@ -967,7 +967,27 @@ do_uid_gid_chroot (struct context *c, bool no_delay)
>if (c->options.chroot_dir)
>   {
> if (no_delay)
> - platform_chroot (c->options.chroot_dir);
> +{
> +#ifdef ENABLE_SYSTEMD
> +  /* If OpenVPN is started by systemd, the OpenVPN process
> needs
> +   * to provide a preliminary status report to systemd.  This
> is
> +   * needed as $NOTIFY_SOCKET will not be available inside the
> +   * chroot, which sd_notify()/sd_notifyf() depends on.
> +   *
> +   * This approach is the simplest and the most non-intrusive
> +   * solution right before the 2.4_rc2 release.
> +   *
> +   * TODO: Consider altnernative solutions - bind mount?
> +   * systemd does not grok OpenVPN configuration files, thus
> cannot
> +   * have a sane way to know if OpenVPN will chroot or not and
> to
> +   * which subdirectory it will chroot into.
> +   */
> +  sd_notifyf(0, "READY=1\n"
> +"STATUS=Entering chroot, most of the init completed
> successfully\n"
> +"MAINPID=%lu", (unsigned long) getpid());
> +#endif
> +  platform_chroot (c->options.chroot_dir);
> +}
> else if (c->first_time)
>   msg (M_INFO, "NOTE: chroot %s", why_not);
>   }

Looks good to me, so: ACK

In long term we should think about a proper solution. Notification socket
is /run/systemd/notify, so we would have to make that available from within
the chroot.

BTW, feel free to CC me on systemd related topics.
-- 
main(a){char*c=/*Schoene Gruesse */"B?IJj;MEH"
"CX:;",b;for(a/*Best regards my address:*/=0;b=c[a++];)
putchar(b-1/(/*Chriscc -ox -xc - && ./x*/b/42*2-3)*42);}


pgpzdl_cITWjn.pgp
Description: OpenPGP digital signature
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH applied] Re: mbedtls: include correct net/net_sockets header according to version

2016-12-09 Thread Gert Doering
Your patch has been applied to the master branch.  Thanks.

I have not changed the #include formatting, as The Great Reformatting
will catch it anyway.

commit c00919e8bd6a4e36d9fa009f3b1a93b262a59fc6
Author: Magnus Kroken
Date:   Fri Dec 9 10:07:35 2016 +0100

 mbedtls: include correct net/net_sockets header according to version

 Signed-off-by: Magnus Kroken 
 Acked-by: Steffan Karger 
 Message-Id: <1481274455-657-1-git-send-email-mkro...@gmail.com>
 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13451.html
 Signed-off-by: Gert Doering 


--
kind regards,

Gert Doering


--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH 2/2 v3] push: Provide a warning if --ifconfig-push have --topology mismatch

2016-12-09 Thread David Sommerseth
This adds a warning to the log file if --topology is configured to use
subnet or net30 and the 'subnet mask' argument of an --ifconfig-push option
is not an subnet mask.

v2 - Make use of ifconfig_sanity_check() in tun.c instead of doing the exact
 same check and warning in prepare_push_reply().  Also improve documentation
 of ifconfig_sanity_check() while at it.

v3 - Adopted to the new ifconfig_sanity_check() API

Trac: #755
Signed-off-by: David Sommerseth 
---
 src/openvpn/push.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index 9953079..5292b06 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -333,6 +333,14 @@ prepare_push_reply (struct context *c, struct gc_arena *gc,
   print_in_addr_t (ifconfig_local, 0, gc),
   print_in_addr_t (c->c2.push_ifconfig_remote_netmask,
0, gc));
+
+  /* Warn if ifconfig_remote_netmask contains an unexpected value
+   * when checking configuration up against TUN/TAP device and
+   * network topology
+   */
+  ifconfig_sanity_check(c->c1.tuntap->type == DEV_TYPE_TUN,
+c->c2.push_ifconfig_remote_netmask,
+c->options.topology, true);
 }
 
   /* Send peer-id if client supports it */
-- 
1.8.3.1


--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH 0/2] Improve --ifconfig and --ifconfig-push sanity check

2016-12-09 Thread David Sommerseth
This patch set is combining two separate mail threads [1] [2] as they are 
related.

These patches have also been rearragned, where the first patch adds the generic
improvements and prepares for the push.c update which is in the second patch.

These patches combined will resolve the issue reported in Trac ticket #755


[1] 
,
Message-Id: <1480545687-19365-1-git-send-email-dav...@openvpn.net>
[2] 
,
Message-Id: <1481251952-30704-1-git-send-email-dav...@openvpn.net>

David Sommerseth (2):
  Improve ifconfig_sanity_check()
  push: Provide a warning if --ifconfig-push have --topology mismatch

 src/openvpn/push.c |  8 
 src/openvpn/tun.c  | 46 ++
 src/openvpn/tun.h  |  2 ++
 3 files changed, 44 insertions(+), 12 deletions(-)

-- 
1.8.3.1


--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] Coding style clean-up ... phase 1

2016-12-09 Thread Steffan Karger
Hi,

On 9 December 2016 at 00:14, David Sommerseth
 wrote:
> I just spotted in ssl.c that we need sp_assign=add.
>
> [ ssl.c, tls1_PRF() ]
> len = slen/2;
> S1 = sec;
> S2 = &(sec[len]);
> len += (slen&1); /* add for odd, make longer */
>
> I believe we've agreed on spaces around assignments.

Hm, I'd be fine with that, but I'm afraid there are too many such
details to bikeshed about.  Once we've fixed the curious newlines
Selva mentionded (sorry, not sure when I can look at that), I think we
should stop the discussion and move forward.  Just 5 more days until
rc2 (where the reformatting should be done).

-Steffan

--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH] mbedtls: include correct net/net_sockets header according to version

2016-12-09 Thread Steffan Karger
Hi,

On 9 December 2016 at 10:07, Magnus Kroken  wrote:
>  is deprecated as of mbedTLS 2.4.0, it is renamed
> . OpenVPN will fail to build with
> mbedTLS 2.4.0 with MBEDTLS_DEPRECATED_REMOVED defined.
>
> Check MBEDTLS_VERSION_NUMBER, and include net.h for < 2.4.0 and
> net_sockets.h for >= 2.4.0.
>
> Signed-off-by: Magnus Kroken 
> ---
> Tested, builing with both mbedTLS 2.3.0 and 2.4.0 with
> MBEDTLS_DEPRECATED_REMOVED is successful.
>
> This patch is as much to document the issue as a suggested patch.
> I don't know if MBEDTLS_DEPRECATED_REMOVED is considered a
> distro/package maintainers problem or something to fix with
> an #if, so I won't be sad if it isn't accepted.
>
>  src/openvpn/ssl_mbedtls.c | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
> index 11ee65b..985a39f 100644
> --- a/src/openvpn/ssl_mbedtls.c
> +++ b/src/openvpn/ssl_mbedtls.c
> @@ -51,11 +51,17 @@
>  #include "ssl_verify_mbedtls.h"
>  #include 
>  #include 
> -#include 
> +#include 
> +
> +#if MBEDTLS_VERSION_NUMBER >= 0x0204
> +#include 
> +#else
> +#include 
> +#endif
> +
>  #include 
>  #include 
>  #include 
> -#include 
>
>  void
>  tls_init_lib()
> --
> 2.1.4

Looks good, passes tests. ACK.

We might remove the indenting of the preprocessor statements, but if
not, this will be fixed soon by The Great Reformatting anyway.

-Steffan

--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH 1/1] Handle SIGHUP during initialisation phase

2016-12-09 Thread Hervé CODINA
Without this commit, if remote host cannot be reached, we get stuck in loop 
trying to
connect and we cannot reload a new configuration using SIGHUP.
In this case, the only way to reload the configuration is to kill and relaunch 
the daemon.

With this commit, following use case can be done:
- Set a good config file
- Launch vpn client daemon -> Connect OK
- Change config file (configure a wrong remote host)
- SIGHUP
- client daemon reload conf -> Connect failed and loop retrying connection
- Change config file (configure a good remote host)
- SIGHUP
- client daemon reload conf -> Connect OK

Signed-off-by: Herve Codina 
---
 src/openvpn/sig.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/openvpn/sig.c b/src/openvpn/sig.c
index b3ae645..073a285 100644
--- a/src/openvpn/sig.c
+++ b/src/openvpn/sig.c
@@ -226,7 +226,7 @@ pre_init_signal_catch (void)
   signal_mode = SM_PRE_INIT;
   signal (SIGINT, signal_handler);
   signal (SIGTERM, signal_handler);
-  signal (SIGHUP, SIG_IGN);
+  signal (SIGHUP, signal_handler);
   signal (SIGUSR1, SIG_IGN);
   signal (SIGUSR2, SIG_IGN);
   signal (SIGPIPE, SIG_IGN);
-- 
1.7.9.5

--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] Coding style clean-up ... phase 1

2016-12-09 Thread Gert Doering
Hi,

On Fri, Dec 09, 2016 at 08:24:24AM -0500, Selva Nair wrote:
> On Fri, Dec 9, 2016 at 2:42 AM, Gert Doering  wrote:
> 
> >   if (a>0)
> >  { do_this(); }
> >   else
> >  { do_that(); }
> >
> 
> In such cases I would normally skip all braces, in spite of all the
> arguments against it... But that's just me.

We've decided that we always want braces, "because apple goto bug",
but also because there's a slippery slope towards surprising bugs.


if (foo)
   return 3;
else
   if (bar)
   do_something(); return 2;
   else
   return 4;

(maybe a non-convincing example because "obviously buggy")

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
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] Coding style clean-up ... phase 1

2016-12-09 Thread Selva Nair
On Fri, Dec 9, 2016 at 2:42 AM, Gert Doering  wrote:

>   if (a>0)
>  { do_this(); }
>   else
>  { do_that(); }
>

In such cases I would normally skip all braces, in spite of all the
arguments against it... But that's just me.

That said the proposed re-formatting looks super good to me and uncrustify
is doing an amazing job. Kudos to all who narrowed down the options.

Selva
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH 1/1] Handle SIGHUP during initialisation phase

2016-12-09 Thread Herve Codina
Without this commit, if remote host cannot be reached, we get stuck in loop 
trying to
connect and we cannot reload a new configuration using SIGHUP.
In this case, the only way to reload the configuration is to kill and relaunch 
the daemon.

With this commit, following use case can be done:
- Set a good config file
- Launch vpn client daemon -> Connect OK
- Change config file (configure a wrong remote host)
- SIGHUP
- client daemon reload conf -> Connect failed and loop retrying connection
- Change config file (configure a good remote host)
- SIGHUP
- client daemon reload conf -> Connect OK

Signed-off-by: Herve Codina 
---
 src/openvpn/sig.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/openvpn/sig.c b/src/openvpn/sig.c
index b3ae645..073a285 100644
--- a/src/openvpn/sig.c
+++ b/src/openvpn/sig.c
@@ -226,7 +226,7 @@ pre_init_signal_catch (void)
   signal_mode = SM_PRE_INIT;
   signal (SIGINT, signal_handler);
   signal (SIGTERM, signal_handler);
-  signal (SIGHUP, SIG_IGN);
+  signal (SIGHUP, signal_handler);
   signal (SIGUSR1, SIG_IGN);
   signal (SIGUSR2, SIG_IGN);
   signal (SIGPIPE, SIG_IGN);
-- 
1.7.9.5


--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH] Improve ifconfig_sanity_check()

2016-12-09 Thread David Sommerseth
On 09/12/16 09:15, Gert Doering wrote:
> Hi,
> 
> On Fri, Dec 09, 2016 at 03:52:32AM +0100, David Sommerseth wrote:
>> - Instead of checking the complete in_addr_t (which lacked proper htonl()),
>>   just do a simple peek at the last byte which contains the first octet
>>   of an IP address or subnet mask.
> 
> Have you *tested* this on a non-intel platform?
> 
> I know that I said on IRC that we might do it that way, but looking
> at stuff like print_in_addr_t() I'm not sure our usage of in_addr_t is
> consistant with "normal usage of it" (namely, always in network byte
> order).

I don't have access to any big-endian machines.  I might still have
access to a virtual POWER8 machine, but that is configured in
little-endian mode - so no difference from the Intel platform.

After I sent this patch, I started to ponder on if there were any
performance reasons why to change the in_addr_t check.  And I've come to
the conclusion I doubt it makes much of a difference.

First of all, an IPv4 address is 32-bits, which is the smallest CPU
variants we do support.  So 0xff00 or 0xff will just be copied into
a 32-bit register anyway.  Secondly, checking for just 0xff will remove
one AND operation on the CPU and the mov(e) operation will just be
slightly different as it only loads 8 bits from a memory region vs 32
bits.  So we might end up with or without saving just a couple of CPU
cycles.  And when considering that this is code paths which is not
called very often, it starts to smell very much like pointless
micro-optimization.


I suggest we leave the in_addr_t parsing unchanged, and fix the message
handling to differentiate between --ifconfig and --ifconfig-push.  And
after 2.4.0, we can look at the in_addr_t tackling to see if it behaves
correctly on both big- and little-endian.  If it doesn't behave well
then we'll fix it.

Thoughts?


-- 
kind regards,

David Sommerseth
OpenVPN Technologies, Inc




signature.asc
Description: OpenPGP digital signature
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] help wanted: OpenSolaris building

2016-12-09 Thread Alexander Pyhalov
On 09/11/16 09:51 PM, Gert Doering wrote:
> Hi,
>
> as you might know, we try to build everything we commit to git on all
> supported platforms (using buildbot).  This works quite well and has
> helped us keep things consistently working across all platforms, at least
> as far as we have tests for each feature...
>
> One of my problem childs is (Open)Solaris, though.  I have a VM that is
> fairly old and has the GNU toolchain on it, but we have no access to
> a current system, using the Sun/Oracle C compiler.
>
> So, I'm looking for pointers :-)
>
>  - is there an (easy to install and maintain) OpenSource variant of
>"Solaris" still around?
>
>  - is there a developer program at Oracle, where one can get access to
>a current Solaris version plus Oracle C compiler, free of charge
>(a hosted build environment is only partially usable, because we
>run VM tests from buildbot, and those need root access...)

Hi.

I was looking through openvpn-devel archives and found this message.
Do you still need any help with illumos build bot? Like dedicated zone? 
Or do you have some technical issues?
-- 
Best regards,
Alexander Pyhalov,
system administrator of Southern Federal University IT department

--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH] Use getpassphrase on Solaris/illumos

2016-12-09 Thread Alexander Pyhalov


--
С уважением,
Александр Пыхалов,
программист отдела телекоммуникационной инфраструктуры
управления информационно-коммуникационной инфраструктуры ЮФУ
>From 971d1d5e66ba714fc8f74b8da0672e7da47dc557 Mon Sep 17 00:00:00 2001
From: Alexander Pyhalov 
Date: Fri, 9 Dec 2016 13:16:01 +0300
Subject: [PATCH] Use getpassphrase on Solaris/illumos

---
 src/openvpn/console_builtin.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/openvpn/console_builtin.c b/src/openvpn/console_builtin.c
index 06994fd..98257b6 100644
--- a/src/openvpn/console_builtin.c
+++ b/src/openvpn/console_builtin.c
@@ -214,7 +214,12 @@ static bool get_console_input (const char *prompt, const bool echo, char *input,
 }
 close_tty (fp);
 } else {
+#ifdef __sun
+/* On Solaris/illumos getpass() returns up to 8 symbols */
+char *gp = getpassphrase (prompt);
+#else
 char *gp = getpass (prompt);
+#endif
 if (gp)
 {
 strncpynt (input, gp, capacity);
-- 
2.9.2

--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH] mbedtls: include correct net/net_sockets header according to version

2016-12-09 Thread Magnus Kroken
 is deprecated as of mbedTLS 2.4.0, it is renamed
. OpenVPN will fail to build with
mbedTLS 2.4.0 with MBEDTLS_DEPRECATED_REMOVED defined.

Check MBEDTLS_VERSION_NUMBER, and include net.h for < 2.4.0 and
net_sockets.h for >= 2.4.0.

Signed-off-by: Magnus Kroken 
---
Tested, builing with both mbedTLS 2.3.0 and 2.4.0 with 
MBEDTLS_DEPRECATED_REMOVED is successful. 

This patch is as much to document the issue as a suggested patch.
I don't know if MBEDTLS_DEPRECATED_REMOVED is considered a
distro/package maintainers problem or something to fix with 
an #if, so I won't be sad if it isn't accepted.

 src/openvpn/ssl_mbedtls.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
index 11ee65b..985a39f 100644
--- a/src/openvpn/ssl_mbedtls.c
+++ b/src/openvpn/ssl_mbedtls.c
@@ -51,11 +51,17 @@
 #include "ssl_verify_mbedtls.h"
 #include 
 #include 
-#include 
+#include 
+
+#if MBEDTLS_VERSION_NUMBER >= 0x0204
+#include 
+#else
+#include 
+#endif
+
 #include 
 #include 
 #include 
-#include 
 
 void
 tls_init_lib()
-- 
2.1.4


--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH] push: Provide a warning if --ifconfig-push have argument mismatch with --topology

2016-12-09 Thread Gert Doering
Hi,

On Fri, Dec 09, 2016 at 09:13:19AM +0100, Gert Doering wrote:
> ... ifconfig_sanity_check() does *nothing* for TOP_SUBNET 

Overlooked the second patch (since it wasn't threaded).  So with the other
patch, that argument is no longer valid, of course.  Apologies.

[..]
> Also we might to re-think the warning message printed - if called from
> an ifconfig-push context, the text "the second argument to --ifconfig must
> be an IP address" is less than clear ("my --ifconfig settings are just 
> fine, what is openvpn warning about?").

That one still holds...  technically it's more "for the other patch",
but since *this* patch is calling ifconfig_sanity_check() from a new
context, it should tackle the now-misleading warning text, I think.

(So maybe apply the other one first, after verifying endian correctness 
on sparc or mips/be architecture, and this one then removes the static,
adds a flag for "ifconfig or ifconfig-push context?" and adapts the
messages accordingly)

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
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH] Improve ifconfig_sanity_check()

2016-12-09 Thread Gert Doering
Hi,

On Fri, Dec 09, 2016 at 03:52:32AM +0100, David Sommerseth wrote:
> - Instead of checking the complete in_addr_t (which lacked proper htonl()),
>   just do a simple peek at the last byte which contains the first octet
>   of an IP address or subnet mask.

Have you *tested* this on a non-intel platform?

I know that I said on IRC that we might do it that way, but looking
at stuff like print_in_addr_t() I'm not sure our usage of in_addr_t is
consistant with "normal usage of it" (namely, always in network byte
order).

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
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH] push: Provide a warning if --ifconfig-push have argument mismatch with --topology

2016-12-09 Thread Gert Doering
Hi,

On Fri, Dec 09, 2016 at 03:50:48AM +0100, David Sommerseth wrote:
> This adds a warning to the log file if --topology is configured to use
> subnet or net30 and the 'subnet mask' argument of an --ifconfig-push option
> is not an subnet mask.
> 
> v2 - Make use of ifconfig_sanity_check() in tun.c instead of doing the exact
>  same check and warning in prepare_push_reply().  Also improve 
> documentation
>  of ifconfig_sanity_check() while at it.

Not being the one to complain about your code all the time... but...
since trac #755 is about "topology subnet"...

static void
ifconfig_sanity_check (bool tun, in_addr_t addr, int topology)
{
  struct gc_arena gc = gc_new ();
  const bool looks_like_netmask = ((addr & 0xFF00) == 0xFF00);
  if (tun)
{
  if (looks_like_netmask && (topology == TOP_NET30 || topology == TOP_P2P))
msg (M_WARN, "WARNING: Since you are using --dev tun with a point-to-poi
nt topology, the second argument to --ifconfig must be an IP address.  You are u
sing something (%s) that looks more like a netmask. %s",
 print_in_addr_t (addr, 0, ),
 ifconfig_warn_how_to_silence);
}
  else /* tap */

... ifconfig_sanity_check() does *nothing* for TOP_SUBNET (and the code
disagrees with your commit message for "net30").

So the v3 version of that patch would need to add the (missing today,
but actually important diagnostic aid - and the point of #755) clause

if (!looks_like_netmask && topology == TOP_SUBNET)
{
msg (M_WARN, "topology subnet needs a netmask as second argument bla 
bla");
}


Also we might to re-think the warning message printed - if called from
an ifconfig-push context, the text "the second argument to --ifconfig must
be an IP address" is less than clear ("my --ifconfig settings are just 
fine, what is openvpn warning about?").


Plus, it needs to be actually tested on a non-intel architecture - the
code has been like that forever, but I would not trust it to be endian-
safe (though our use of "in_addr_t" is not actually consistent with
what *should* be in one - parts of the code treat it as "network byte
order", other parts as "host byte order", and the point of using
"in_addr_t" as well defined *type* and not "a bag of 32bits, unsigned"
should have been "as defined by the socket API", which wants network
byte order).


But since this is an enhancement and not a bug, it should not be rushed -
it does not have to be in 2.4.0.  Let's do it properly, test it, then
see if we want to have it in 2.4.1  (and while at it, we should teach
check_addr_clash() that networks do have a netmask for a reason, and
"local/remote and --ifconfig are in the same /24" could be perfectly fine 
if the pushed netmask is *not* a /24).

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
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel