Re: [Openvpn-devel] [PATCH v3] Parse static challenge response in auth-pam plugin

2018-07-31 Thread David Sommerseth
On 30/07/18 16:58, Selva Nair wrote:
> Hi,
> 
> On Mon, Jul 30, 2018 at 10:31 AM, Antonio Quartulli  wrote:
>> Hi,
>>
>> On 30/07/18 04:16, Selva Nair wrote:
>>> Yes that's the base64 patch. What is stopping it is not the
>>> disagreement on that patch but an "error" [*] in the plugin header
>>> that I had discovered.  David wants to fix that before this one, but
>>> it seems he is too busy with other things.
>>>
>>> And there is a pending patch to fix that :
>>> https://patchwork.openvpn.net/patch/87/
>>>
>>
>> [CUT]
>>
>>
>>>
>>> [*] A function signature uses a pointer to an opaque handle (a void *)
>>> while it should be just the handle. It generates no warning as it is
>>> void * vs void ** and all existing codes out there must be correctly
>>> passing the pointer (handle) ignoring the signature in the header --
>>> else they wont work. I wanted the header to be fixed and David seems
>>> to agree with that.
>>>
>>
>> I remember discussing this patch with David as well and the general
>> feeling was that the patch was correct.
>>
>> Maybe David wanted to spend some more time on this patch, but it slipped
>> off the plate.
>>
>> As Selva said, if users of that function were following the header they
>> would see a lot of explosions, while this is not the case.
>> Therefore it should be happily applied with no risk.
> 
> Antonio, thanks for recalling your discussion and for the reassurance.
> 
> My description of this in the previous mail was not entirely correct:
> it has been a long time, and I was a bit rusty on the details -- the
> pointer in question is a member of "struct openvpn_plugin_args_open_return"
> where its wrongly declared.
> 


Hi all, and sorry for letting this one (with many others) fall through the
cracks.  And quickly responding from a holiday now (planning to be mostly
disconnected until mid-August; first real holiday in 2 years now).  I remember
patch was fine but never got around to fix the header file.

So if we get the header file fixed,  all should be good for getting the base64
stuff added; including this patch.  I have only glared at the code quickly of
this last v3 patch, so it just needs to be well tested before we add it to git
master.

Next cool thing to get added would be dynamic challenge, but that will require
quite some extensions in the plug-in API as well.  Use case: systems which
uses sssd to do the authentication via PAM, where OTP may or may not be
activated for some user accounts - or temporarily disabled if the backend sssd
depends on is unavailable (where sssd can do a local off-line password auth 
only).


-- 
kind regards,

David Sommerseth
OpenVPN Inc


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH] Clarify and expand management interface documentation

2018-07-31 Thread Jonathan K. Bullard via Openvpn-devel
Clarify and expand the documentation for the management interface:

* Add examples of static and dynamic challenge/response sequences in
the "COMMAND -- password and username" section.

* Expand the "Challenge/Response" section with more detail.

* Use "management interface client" throughout (instead of "management
client", which was used in several places previously).

* Clarify when both a username and password is needed, not just a
username or a password.

* Clarify that an exit with a fatal error for a dynamic C/R will occur
only if "--auth-retry none" (the default) is in effect.

* Update the list of UIs that support challenge/response.

* Fix a typo. ("posesses" => "possesses").

Signed-off-by: Jonathan K. Bullard 
---
 doc/management-notes.txt | 213 ---
 1 file changed, 147 insertions(+), 66 deletions(-)

diff --git a/doc/management-notes.txt b/doc/management-notes.txt
index 96470d0..174b3bf 100644
--- a/doc/management-notes.txt
+++ b/doc/management-notes.txt
@@ -12,7 +12,8 @@ as a client or server.

 The management interface is implemented using a client/server TCP
 connection or unix domain socket where OpenVPN will listen on a
-provided IP address and port for incoming management client connections.
+provided IP address and port for incoming management interface client
+connections.

 The management protocol is currently cleartext without an explicit
 security layer.  For this reason, it is recommended that the
@@ -104,7 +105,7 @@ be in the list, and it will cause the management interface
 to save the "forget-passwords" string in its list of echo
 parameters.

-The management client can use "echo all" to output the full
+The management interface client can use "echo all" to output the full
 list of echoed parameters, "echo on" to turn on real-time
 notification of echoed parameters via the ">ECHO:" prefix,
 or "echo off" to turn off real-time notification.
@@ -118,10 +119,10 @@ like this:

 Essentially the echo command allowed us to pass parameters from
 the OpenVPN server to the OpenVPN client, and then to the
-management client (such as a GUI).  The large integer is the
+management interface client (such as a GUI).  The large integer is the
 unix date/time when the echo parameter was received.

-If the management client had issued the command "echo on",
+If the management interface client had issued the command "echo on",
 it would have enabled real-time notifications of echo
 parameters.  In this case, our "forget-passwords" message
 would be output like this:
@@ -141,8 +142,8 @@ COMMAND -- exit, quit

 Close the managment session, and resume listening on the
 management port for connections from other clients. Currently,
-the OpenVPN daemon can at most support a single management client
-any one time.
+the OpenVPN daemon can at most support a single management interface
+client any one time.

 COMMAND -- help
 ---
@@ -167,7 +168,7 @@ The hold flag setting is persistent and will not
 be reset by restarts.

 OpenVPN will indicate that it is in a hold state by
-sending a real-time notification to the management
+sending a real-time notification to the management interface
 client, the parameter indicates how long OpenVPN would
 wait without UI (as influenced by connect-retry exponential
 backoff). The UI needs to wait for releasing the hold if it
@@ -275,7 +276,7 @@ COMMAND -- password and username
   OpenVPN is indicating that it needs a password of type
   "Private Key".

-  The management client should respond to this query as follows:
+  The management interface client should respond as follows:

 password "Private Key" foo

@@ -283,8 +284,8 @@ COMMAND -- password and username

 >PASSWORD:Need 'Auth' username/password

-  OpenVPN needs a --auth-user-pass password.  The management
-  client should respond:
+  OpenVPN needs a --auth-user-pass username and password.  The
+  management interface client should respond:

 username "Auth" foo
 password "Auth" bar
@@ -307,7 +308,8 @@ COMMAND -- password and username
 >PASSWORD:Verification Failed: 'Private Key'

   Example 4: The --auth-user-pass username/password failed,
-  and OpenVPN is exiting:
+  and OpenVPN will exit with a fatal error if '--auth-retry none'
+  (which is the default) is in effect:

 >PASSWORD:Verification Failed: 'Auth'

@@ -322,6 +324,34 @@ COMMAND -- password and username

 >PASSWORD:Auth-Token:foobar

+  Example 7: Static challenge/response:
+
+>PASSWORD:Need 'Auth' username/password SC:1,Please enter token PIN
+
+  OpenVPN needs an --auth-user-pass username and password and the
+  response to a challenge. The user's response to "Please enter
+  token PIN" should be obtained and included in the management
+  interface client's response along with the username and password.
+
+  Example 8: Dynamic challenge/response:
+
+>PASSWORD:Verification Failed: 
"['CRV1:R,E:Om01u7Fh4LrGBS7uh0SWmzwabUiGiW6l:Y3Ix:Please enter token PIN']"
+
+  Th

Re: [Openvpn-devel] [PATCH v3] Parse static challenge response in auth-pam plugin

2018-07-31 Thread Selva Nair
HI

On Tue, Jul 31, 2018 at 3:07 AM, David Sommerseth
 wrote:
> On 30/07/18 16:58, Selva Nair wrote:
>> Hi,
>>
>> On Mon, Jul 30, 2018 at 10:31 AM, Antonio Quartulli  wrote:
>>> Hi,
>>>
>>> On 30/07/18 04:16, Selva Nair wrote:
 Yes that's the base64 patch. What is stopping it is not the
 disagreement on that patch but an "error" [*] in the plugin header
 that I had discovered.  David wants to fix that before this one, but
 it seems he is too busy with other things.


snip

>>> I remember discussing this patch with David as well and the general
>>> feeling was that the patch was correct.
>>>
>>> Maybe David wanted to spend some more time on this patch, but it slipped
>>> off the plate.
>>>
>>> As Selva said, if users of that function were following the header they
>>> would see a lot of explosions, while this is not the case.
>>> Therefore it should be happily applied with no risk.
>>
>> Antonio, thanks for recalling your discussion and for the reassurance.
>>

snip

>
> Hi all, and sorry for letting this one (with many others) fall through the
> cracks.  And quickly responding from a holiday now (planning to be mostly
> disconnected until mid-August; first real holiday in 2 years now).  I remember
> patch was fine but never got around to fix the header file.

Agreed :)

The header patch is short and benign, and it seems more than a couple
of pairs of
eyes have viewed and "approved" it. So can we get an ACK?

Here is a link to the patch.
https://patchwork.openvpn.net/patch/87/

Once this is accepted, the rest should be straightforward.

> Next cool thing to get added would be dynamic challenge, but that will require
> quite some extensions in the plug-in API as well.

What is stopping this is really the same old problem with sending an AUTH_FAILED
reason back to the client -- once that is solved easy to support this
with minimal
changes to the plugin code.

I have some ideas on how to solve the former without too much
refactoring, but let's
get the static challenge support in first.

Thanks,

Selva

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH] Correct the declaration of handle in 'struct openvpn_plugin_args_open_return'

2018-07-31 Thread Antonio Quartulli
Hi,

On 21/11/17 09:43, selva.n...@gmail.com wrote:
> From: Selva Nair 
> 
> - This is an opaque pointer so the change should not affect
>   existing plugins. But it makes the code consistent and clears up
>   the documentation as the handle pointer is treated as of type
>   "openvpn_plugin_handle_t" in the rest of the code.
> 
> Signed-off-by: Selva Nair 

Will make some noise during application (git pw patch apply 87), such as:
"Falling back to patching base and 3-way merge..."

But it won't generate any conflict.

Acked-by: Antonio Quartulli 



-- 
Antonio Quartulli



signature.asc
Description: OpenPGP digital signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH] Correct the declaration of handle in 'struct openvpn_plugin_args_open_return'

2018-07-31 Thread Gert Doering
Hi,

On Wed, Aug 01, 2018 at 01:21:35AM +0800, Antonio Quartulli wrote:
> On 21/11/17 09:43, selva.n...@gmail.com wrote:
> > From: Selva Nair 
> > 
> > - This is an opaque pointer so the change should not affect
> >   existing plugins. But it makes the code consistent and clears up
> >   the documentation as the handle pointer is treated as of type
> >   "openvpn_plugin_handle_t" in the rest of the code.
> > 
> > Signed-off-by: Selva Nair 
> 
> Will make some noise during application (git pw patch apply 87), such as:
> "Falling back to patching base and 3-way merge..."
> 
> But it won't generate any conflict.
> 
> Acked-by: Antonio Quartulli 

Oh, we had the patch on the list already.  Thanks for finding it & ACK.

I'll apply tomorrow or thursday, on the road right now.

gert
-- 
"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
 Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany g...@greenie.muc.de


signature.asc
Description: PGP signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH 1/2] plugin: Export base64 encode and decode functions

2018-07-31 Thread Selva Nair
Hi,

Now that the minor "fix" for plugin header seems settled, back to the
base64 export patch from David.

On Fri, May 5, 2017 at 5:46 PM, David Sommerseth  wrote:
> This patch builds on the "Export secure_memzero() to plug-ins" patch and
> adds export of openvpn_base64_encode() and openvpn_base64_decode()
>
> This also ships with a very simple plug-in which demonstrates how to use
> the new exported functions.
>
> Signed-off-by: David Sommerseth 
> ---
>  include/openvpn-plugin.h.in   |  31 ++
>  sample/sample-plugins/simple/base64.c | 204 
> ++
>  src/openvpn/plugin.c  |   5 +-
>  3 files changed, 239 insertions(+), 1 deletion(-)
>  create mode 100644 sample/sample-plugins/simple/base64.c

The exports are very useful an work as expected. The sample code
would be appreciated by plugin authors though it could have been
a separate patch.

I have not tested the sample code but looks right and is well documented.

Only a two of typos to nitpick over (see below) -- hopefully those
could be fixed at merge time.

So ACK from me.

>
> diff --git a/include/openvpn-plugin.h.in b/include/openvpn-plugin.h.in
> index ec83f3a6..68180d62 100644
> --- a/include/openvpn-plugin.h.in
> +++ b/include/openvpn-plugin.h.in
> @@ -221,6 +221,8 @@ struct openvpn_plugin_string_list
>   *   OpenVPN to plug-ins.
>   *
>   *4  Exported secure_memzero() as plugin_secure_memzero()
> + *   Exported openvpn_base64_encode() as plugin_base64_encode()
> + *   Exported openvpn_base64_decode() as plugin_base64_decode()
>   */
>  #define OPENVPN_PLUGINv3_STRUCTVER 4
>
> @@ -269,6 +271,33 @@ typedef void (*plugin_vlog_t)(openvpn_plugin_log_flags_t 
> flags,
>   */
>  typedef void (*plugin_secure_memzero_t)(void *data, size_t len);
>
> +/**
> + *  Export of openvpn_base64_encode() to be used inside plug-ins
> + *
> + *  @param data   Pointer to data to BASE64 encode
> + *  @param size   Length of data, in bytes
> + *  @param *str   Pointer to the return buffer.  This needed memory is
> + *allocated by openvpn_base64_encode() and needs to be 
> free()d
> + *after use.
> + *
> + *  @return int   Returns the length of the buffer created, or -1 on error.
> + *
> + */
> +typedef int (*plugin_base64_encode_t)(const void *data, int size, char 
> **str);
> +
> +/**
> + *  Export of openvpn_base64_decode() to be used inside plug-ins
> + *
> + *  @param strPointer to the BASE64 encoded data
> + *  @param data   Pointer to the buffer where save the decoded data
> + *  @param size   Size of the destination buffer
> + *
> + *  @return int   Returns the length of the decoded data, or -1 on error or
> + *if the destination buffer is too small.
> + *
> + */
> +typedef int (*plugin_base64_decode_t)(const char *str, void *data, int size);
> +
>
>  /**
>   * Used by the openvpn_plugin_open_v3() function to pass callback
> @@ -291,6 +320,8 @@ struct openvpn_plugin_callbacks
>  plugin_log_t plugin_log;
>  plugin_vlog_t plugin_vlog;
>  plugin_secure_memzero_t plugin_secure_memzero;
> +plugin_base64_encode_t plugin_base64_encode;
> +plugin_base64_decode_t plugin_base64_decode;
>  };
>
>  /**
> diff --git a/sample/sample-plugins/simple/base64.c 
> b/sample/sample-plugins/simple/base64.c
> new file mode 100644
> index ..503b5ba0
> --- /dev/null
> +++ b/sample/sample-plugins/simple/base64.c
> @@ -0,0 +1,204 @@
> +/*
> + *  OpenVPN -- An application to securely tunnel IP networks
> + * over a single TCP/UDP port, with support for SSL/TLS-based
> + * session authentication and key exchange,
> + * packet encryption, packet authentication, and
> + * packet compression.
> + *
> + *  Copyright (C) 2017  David Sommerseth 
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2
> + *  as published by the Free Software Foundation.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program (see the file COPYING included with this
> + *  distribution); if not, write to the Free Software Foundation, Inc.,
> + *  59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> + */
> +
> +#include 
> +#include 
> +#include 
> +
> +#include "openvpn-plugin.h"
> +
> +#define PLUGIN_NAME "base64.c"
> +
> +/* Exported plug-in v3 API functions */
> +plugin_log_t ovpn_log = NULL;  /**< Pointer to the 
> OpenVPN log function.  See plugin_log() */
> +plugin_vlog_t ovpn_vlog = NULL;/**< Pointer to the 
> OpenVPN vlog function. See plugin