Re: [Openvpn-devel] [PATCH v3] Parse static challenge response in auth-pam plugin
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
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
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'
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'
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
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