In auth-pam plugin clear the password after use

From: Selva Nair <[email protected]...>

If static challenge is in use, the password passed to the plugin by openvpn is of the form "SCRV1:base64-pass:base64-response". Parse this string to separate it into password and response and use them to respond to queries in the pam conversation function.

On the plugin parameters line the substitution keyword for the static challenge response is "OTP". For example, for pam config named "test" that prompts for "user", "password" and "pin", use

plugin openvpn-auth-pam.so "test user USERNAME password PASSWORD pin OTP"

What is the status of this one? Does it need updating after 1/2 got changed to v2 and v3 are these independent enough that 2/2 is standalone?

From a cursory glance, it calls secure_memzero() which is now, I think, plugin_secure_memzero() - right?

I also seem to remember discussions between you and David regarding base64 function exporting - what's the state on this?

Hi,

wading through my heap of mails that did not get proper attention...

Quoted 3 lines · On Fri, May 05, 2017 at 02:24:02PM -0400, [email protected]... wrote:

From: Selva Nair <[email protected]...>

If static challenge is in use, the password passed to the plugin by

openvpn

Quoted 6 lines

is of the form "SCRV1:base64-pass:base64-response". Parse this string to separate it into password and response and use them to respond to queries in the pam conversation function.

On the plugin parameters line the substitution keyword for the static challenge response is "OTP". For example, for pam config named "test"

that

Quoted 3 lines

prompts for "user", "password" and "pin", use

plugin openvpn-auth-pam.so "test user USERNAME password PASSWORD pin OTP"

What is the status of this one? Does it need updating after 1/2 got changed to v2 and v3 are these independent enough that 2/2 is standalone?

From a cursory glance, it calls secure_memzero() which is now, I think, plugin_secure_memzero() - right?

I also seem to remember discussions between you and David regarding base64 function exporting - what's the state on this?

Quoted 26 lines · Gert Doering

Hi,

wading through my heap of mails that did not get proper attention...

Quoted 3 lines · On Fri, May 05, 2017 at 02:24:02PM -0400, [email protected]... wrote:

From: Selva Nair <[email protected]...>

If static challenge is in use, the password passed to the plugin by

openvpn

Quoted 1 line

is of the form "SCRV1:base64-pass:base64-response". Parse this string

to

Quoted 1 line

separate it into password and response and use them to respond to

queries

Quoted 4 lines

in the pam conversation function.

On the plugin parameters line the substitution keyword for the static challenge response is "OTP". For example, for pam config named "test"

that

Quoted 3 lines

prompts for "user", "password" and "pin", use

plugin openvpn-auth-pam.so "test user USERNAME password PASSWORD pin

OTP"

What is the status of this one? Does it need updating after 1/2 got changed to v2 and v3 are these independent enough that 2/2 is standalone?

Quoted 7 lines

From a cursory glance, it calls secure_memzero() which is now, I think, plugin_secure_memzero() - right?

I also seem to remember discussions between you and David regarding base64 function exporting - what's the state on this?

I have a version 2 that uses exported base64 but waiting on the following patch to get an NAK or ACK to finalize it.

   https://www.mail-archive.com/[email protected]...

rge.net/msg14655.html

That would also help David's base64 export patch revised, reviewed and merged. For background, see the thread

https://www.mail-archive.com/[email protected]...
.net/msg14577.html

From: Selva Nair <[email protected]...>

If static challenge is in use, the password passed to the plugin by openvpn is of the form "SCRV1:base64-pass:base64-response". Parse this string to separate it into password and response and use them to respond to queries in the pam conversation function.

On the plugin parameters line the substitution keyword for the static challenge response is "OTP". For example, for pam config named "test" that prompts for "user", "password" and "pin", use

plugin openvpn-auth-pam.so "test user USERNAME password PASSWORD pin OTP"

Signed-off-by Selva Nair <[email protected]...>

---
v2: Depends on the base64 export patch
v3: match password string with "SCRV1:" instead of "SCRV1" (pointed out by Joe Bell <[email protected]...>)

 src/plugins/auth-pam/README.auth-pam | 15 +++++---
 src/plugins/auth-pam/auth-pam.c      | 75 ++++++++++++++++++++++++++++++
+++++-
 2 files changed, 84 insertions(+), 6 deletions(-)

src/plugins/auth-pam/README.auth-pam +1 -1

index e123690..9081565 100644
@@ -36,19 +36,20 @@ pairs to answer PAM module queries.
 
 For example:
 
-  plugin openvpn-auth-pam.so "login login USERNAME password PASSWORD"
+  plugin openvpn-auth-pam.so "login login USERNAME password PASSWORD pin

OTP"

tells auth-pam to (a) use the "login" PAM module, (b) answer a
-"login" query with the username given by the OpenVPN client, and

src/plugins/auth-pam/README.auth-pam +4 -3

-(c) answer a "password" query with the password given by the
-OpenVPN client.  This provides flexibility in dealing with the different
+"login" query with the username given by the OpenVPN client,
+(c) answer a "password" query with the password, and (d) answer a
+"pin" query with the OTP given by the OpenVPN client.
+This provides flexibility in dealing with different
 types of query strings which different PAM modules might generate.
 For example, suppose you were using a PAM module called
 "test" which queried for "name" rather than "login":
 
   plugin openvpn-auth-pam.so "test name USERNAME password PASSWORD"
 
-While "USERNAME" "COMMONNAME" and "PASSWORD" are special strings which

substitute
+While "USERNAME" "COMMONNAME" "PASSWORD" and "OTP" are special strings which substitute
to client-supplied values, it is also possible to name literal values to use as PAM module query responses. For example, suppose that the login module queried for a third parameter, "domain" which

src/plugins/auth-pam/README.auth-pam +3 -0

@@ -61,6 +62,10 @@ the operation of this plugin:
 
   client-cert-not-required
   username-as-common-name
+  static-challenge
+
+Use of --static challenege is required to pass a pin (represented by

"OTP" in

src/plugins/auth-pam/README.auth-pam +1 -0

+parameter substituion) or a second password.
 
 Run OpenVPN with --verb 7 or higher to get debugging output from
 this plugin, including the list of queries presented by the

src/plugins/auth-pam/auth- renamed from src/plugins/auth-pam/auth-pam.c +0 -0

pam.c
index 26b0eeb..e22ce5f 100644

src/plugins/auth-pam/auth-pam.c +6 -0

@@ -6,6 +6,7 @@
  *             packet compression.
  *
  *  Copyright (C) 2002-2018 OpenVPN Inc <[email protected]...>
+ *  Copyright (C) 2016-2018 Selva Nair <[email protected]...>
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License version 2

@@ -64,6 +65,7 @@
 
 /* Pointers to functions exported from openvpn */
 static plugin_secure_memzero_t plugin_secure_memzero = NULL;
+static plugin_base64_decode_t plugin_base64_decode = NULL;
 
 /*
  * Plugin state, used by foreground

@@ -87,6 +89,7 @@ struct auth_pam_context
  *  "USERNAME" -- substitute client-supplied username
  *  "PASSWORD" -- substitute client-specified password
  *  "COMMONNAME" -- substitute client certificate common name
+ *  "OTP" -- substitute static challenge response if available
  */
 
 #define N_NAME_VALUE 16

@@ -111,6 +114,7 @@ struct user_pass {
     char username[128];
     char password[128];
     char common_name[128];
+    char response[128];
 
     const struct name_value_list *name_value_list;
 };

@@ -276,6 +280,66 @@ name_value_match(const char *query, const char *match)
     return strncasecmp(match, query, strlen(match)) == 0;
 }
 
+/*
+ * Split and decode up->password in the form SCRV1:base64_pass:base64_

response
+ * into pass and response and save in up->password and up->response.
+ * If the password is not in the expected format, input is not changed.
+ */

src/plugins/auth-pam/auth-pam.c +13 -0

+static void
+split_scrv1_password(struct user_pass *up)
+{
+    const int skip = strlen("SCRV1:");
+    if (strncmp(up->password, "SCRV1:", skip) != 0)
+    {
+        return;
+    }
+
+    char *tmp = strdup(up->password);
+    if (!tmp)
+    {
+        fprintf(stderr, "AUTH-PAM: out of memory parsing static challenge

password\n");

src/plugins/auth-pam/auth-pam.c +12 -0

+        goto out;
+    }
+
+    char *pass = tmp + skip;
+    char *resp = strchr(pass, ':');
+    if (!resp) /* string not in SCRV1:xx:yy format */
+    {
+        goto out;
+    }
+    *resp++ = '\0';
+
+    int n = plugin_base64_decode(pass, up->password,

sizeof(up->password)-1);

src/plugins/auth-pam/auth-pam.c +4 -0

+    if (n > 0)
+    {
+        up->password[n] = '\0';
+        n = plugin_base64_decode(resp, up->response,

sizeof(up->response)-1);

src/plugins/auth-pam/auth-pam.c +6 -0

+        if (n > 0)
+        {
+            up->response[n] = '\0';
+            if (DEBUG(up->verb))
+            {
+                fprintf(stderr, "AUTH-PAM: BACKGROUND: parsed static

challenge password\n");

src/plugins/auth-pam/auth-pam.c +6 -0

+            }
+            goto out;
+        }
+    }
+
+    /* decode error: reinstate original value of up->password and return

*/

src/plugins/auth-pam/auth-pam.c +3 -0

+    plugin_secure_memzero(up->password, sizeof(up->password));
+    plugin_secure_memzero(up->response, sizeof(up->response));
+    strcpy(up->password, tmp); /* tmp is guaranteed to fit in

up->password */

src/plugins/auth-pam/auth-pam.c +2 -0

+
+    fprintf(stderr, "AUTH-PAM: base64 decode error while parsing static

challenge password\n");

src/plugins/auth-pam/auth-pam.c +14 -0

+
+out:
+    if (tmp)
+    {
+        plugin_secure_memzero(tmp, strlen(tmp));
+        free(tmp);
+    }
+}
+
 OPENVPN_EXPORT int
 openvpn_plugin_open_v3(const int v3structver,
                        struct openvpn_plugin_args_open_in const *args,

@@ -316,6 +380,7 @@ openvpn_plugin_open_v3(const int v3structver,
 
     /* Save global pointers to functions exported from openvpn */
     plugin_secure_memzero = args->callbacks->plugin_secure_memzero;
+    plugin_base64_decode = args->callbacks->plugin_base64_decode;
 
     /*
      * Make sure we have two string arguments: the first is the .so name,

@@ -599,6 +664,10 @@ my_conv(int n, const struct pam_message **msg_array,
                     {
                         aresp[i].resp = searchandreplace(match_value,
 "COMMONNAME", up->common_name);
                     }
+                    else if (strstr(match_value, "OTP"))
+                    {
+                        aresp[i].resp = searchandreplace(match_value,
 "OTP", up->response);
+                    }
                     else

                     {
                         aresp[i].resp = strdup(match_value);

src/plugins/auth-pam/auth-pam.c +0 -0

@@ -787,6 +856,9 @@ pam_server(int fd, const char *service, int verb,

const struct name_value_list *

 #endif
                 }

+ /* If password is of the form SCRV1:base64:base64 split
it up */

src/plugins/auth-pam/auth-pam.c +2 -0

+                split_scrv1_password(&up);
+
                 if (pam_auth(service, &up)) /* Succeeded */
                 {
                     if (send_control(fd, RESPONSE_VERIFY_SUCCEEDED) == -1)

@@ -818,10 +890,11 @@ pam_server(int fd, const char *service, int verb,

const struct name_value_list *

                         command);
                 goto done;
         }

src/plugins/auth-pam/auth-pam.c +2 -2

+        plugin_secure_memzero(up.response, sizeof(up.response));
     }
 done:
-
     plugin_secure_memzero(up.password, sizeof(up.password));
+    plugin_secure_memzero(up.response, sizeof(up.response));
 #ifdef USE_PAM_DLOPEN
     dlclose_pam();
 #endif
--

2.1.4

------------------------------------------------------------
------------------
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 [email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

I don't know if it is appropriate to reply to this post in this manner,

If there is anything I can do to provide a list of testcases, write-up on its configuration and usage, etc., in aide of getting the patch merged and released, please let me know.

Hi,

Quoted 1 line · Joe Bell

I don't know if it is appropriate to reply to this post in this manner,

It is and it helps :-)

We want test reports, as in "I am using this, because it is a useful feature for me, and it works fine!" - not as strong as a full code review, but an indication that something is beneficial at least.

[..]

Quoted 2 lines

If there is anything I can do to provide a list of testcases, write-up on its configuration and usage, etc., in aide of getting the patch merged

and

Quoted 1 line

released, please let me know.

If you could find a few spare weeks for me, that would help quite a bit :-)

Jokes aside, a bit of writeup how this looks in the client config, how the client prompts will then look, and how you configure the server would be nice - just here on the list, so google can find it.

From: Selva Nair <[email protected]...>

If static challenge is in use, the password passed to the plugin by openvpn is of the form "SCRV1:base64-pass:base64-response". Parse this string to separate it into password and response and use them to respond to queries in the pam conversation function.

On the plugin parameters line the substitution keyword for the static challenge response is "OTP". For example, for pam config named "test" that prompts for "user", "password" and "pin", use

plugin openvpn-auth-pam.so "test user USERNAME password PASSWORD pin OTP"

Signed-off-by Selva Nair <[email protected]...>

---
v2: Depends on the base64 export patch
v3: match password string with "SCRV1:" instead of "SCRV1" (pointed out by Joe Bell <[email protected]...>)

Hi,

Quoted 11 lines · On Tue, Jul 24, 2018 at 10:34:53PM -0400, [email protected]... wrote:

From: Selva Nair <[email protected]...>

If static challenge is in use, the password passed to the plugin by openvpn is of the form "SCRV1:base64-pass:base64-response". Parse this string to separate it into password and response and use them to respond to queries in the pam conversation function.

On the plugin parameters line the substitution keyword for the static challenge response is "OTP". For example, for pam config named "test" that prompts for "user", "password" and "pin", use

Quoted 8 lines

plugin openvpn-auth-pam.so "test user USERNAME password PASSWORD pin OTP"

Signed-off-by Selva Nair <[email protected]...>

---
v2: Depends on the base64 export patch
v3: match password string with "SCRV1:" instead of "SCRV1" (pch ointed out by Joe Bell <[email protected]...>)

I'm a bit confused about the prerequisites for this - if I found the right "base64 export patch", this is still pending some reworking because you and David disagreed on details, and then David got sucked into "Client for OpenVPN 3 on Linux" and had no time working on the plugin API for v2.x anymore.

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/

[*] 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.

Hi,

Quoted 8 lines · Selva Nair

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]

Quoted 8 lines

[*] 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.

Hi,

Quoted 27 lines · Antonio Quartulli

Hi,

Quoted 8 lines · Selva Nair

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]

Quoted 8 lines

[*] 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.

Quoted 6 lines

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.

Quoted 11 lines · Selva Nair

Hi,

Quoted 8 lines · Antonio Quartulli

Hi,

Quoted 5 lines · Selva Nair

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.

Quoted 12 lines

Quoted 9 lines

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.

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.

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

From: Selva Nair <[email protected]...>

If static challenge is in use, the password passed to the plugin by openvpn is of the form "SCRV1:base64-pass:base64-response". Parse this string to separate it into password and response and use them to respond to queries in the pam conversation function.

On the plugin parameters line the substitution keyword for the static challenge response is "OTP". For example, for pam config named "test" that prompts for "user", "password" and "pin", use

plugin openvpn-auth-pam.so "test user USERNAME password PASSWORD pin OTP"

Signed-off-by Selva Nair <[email protected]...>

---
v2: Depends on the base64 export patch
v3: match password string with "SCRV1:" instead of "SCRV1" (pointed out by Joe Bell <[email protected]...>)

Quoted 3 lines

v2: Depends on the base64 export patch
v3: match password string with "SCRV1:" instead of "SCRV1" (pointed out by Joe Bell <[email protected]...>)

Nicely works and does what it says on the lid. So...

Hi,

Quoted 5 lines · Gert Doering

Quoted 3 lines

v2: Depends on the base64 export patch
v3: match password string with "SCRV1:" instead of "SCRV1" (pointed out by Joe Bell <[email protected]...>)

Nicely works and does what it says on the lid. So...

Talking to myself a lot, lately...

Found an interesting caveat which should be addressed, I think.

Our system (LinOTP) knows "PIN+OTP" or "PIN" as valid input, the latter leading to "send me a token by SMS/e-mail/...".

If I press return at the challenge prompt, it seems the SCRV1: string is not formed the way the plugin wants it, and I end up with

pass=SCRV1%3AMTE5NQ%3D%3D

in the LinOTP URL - so, it didn't decode it, because the second ':' was missing (if I put a blank in there, I get pass=mypin%20).

Is this intentional? Should it be that way?

Quoted 9 lines

Found an interesting caveat which should be addressed, I think.

Our system (LinOTP) knows "PIN+OTP" or "PIN" as valid input, the latter leading to "send me a token by SMS/e-mail/...".

If I press return at the challenge prompt, it seems the SCRV1: string is not formed the way the plugin wants it, and I end up with

pass=SCRV1%3AMTE5NQ%3D%3D

How to format this if response is empty is not clearly documented but my impression was that the second ':' (%3A) is required.

management-notes.txt specifies the format as

password "Auth" "SCRV1:<BASE64_PASSWORD>:<BASE64_RESPONSE>"

The Windows GUI prints it using a template "SCRV1:%s:%s", so the second colon will be present if response is empty -- if that's not happening I would consider that a bug in the GUI.

When password is read from stdin, its formatted as (from misc.c line 358)

buf_printf(&packed_resp, "SCRV1:%s:%s", pw64, resp64);

So that should also contain the second colon.

Quoted 4 lines

If I press return at the challenge prompt, it seems the SCRV1: string is not formed the way the plugin wants it, and I end up with

pass=SCRV1%3AMTE5NQ%3D%3D

How to format this if response is empty is not clearly documented but my impression was that the second ':' (%3A) is required.

management-notes.txt specifies the format as

password "Auth" "SCRV1:<BASE64_PASSWORD>:<BASE64_RESPONSE>"

When password is read from stdin, its formatted as (from misc.c line 358)

buf_printf(&packed_resp, "SCRV1:%s:%s", pw64, resp64);

So that should also contain the second colon.

Quoted 4 lines

in the LinOTP URL - so, it didn't decode it, because the second ':' was missing (if I put a blank in there, I get pass=mypin%20).

Is this intentional? Should it be that way?

If you are constructing the SCRV1: line using a custom UI,
I would suggest to add the second colon. If using Windows-GUI or running OpenVPN from command line we'll need to fix this one place or the other.

Quoted 10 lines

Quoted 5 lines

in the LinOTP URL - so, it didn't decode it, because the second ':' was missing (if I put a blank in there, I get pass=mypin%20).

Is this intentional? Should it be that way?

If you are constructing the SCRV1: line using a custom UI,
I would suggest to add the second colon. If using Windows-GUI or running OpenVPN from command line we'll need to fix this one place or the other.

Command line client (git:master/5961250e776194a4, what I happened to have lying around), run with a config file that has

auth-user-pass
auth-nocache
auth-retry interact
static-challenge "token value: " 1

in it, and pressing <return> at the

CHALLENGE: token value: _

prompt.

There's another catch which we might want to at least document: if you build this plugin and run it from a slightly older openvpn binary which doesn't export the base64 functions, it will core dump most ungraciously

Aug  7 20:17:19 openvpn-tcp kernel: pid 49459 (openvpn), uid 0: exited on signal 11 (core dumped)

(unsurprisingly, jumping over an uninitialized function pointer)

So, shall we bump the plugin API version "with base64" to 5, and change this plugin to actually check it?

(Applying it anyway, because this can all be addressed in a subsequent patch)

Your patch has been applied to the master branch.

(I'm a bit undecided about release/2.4 - this is in "new feature!" land, and all the challenge stuff is "master" territory. OTOH, it's not openvpn main code, and the code is sane enough - so if folks think it should be in release/2.4, tell me)

commit 7369d01bf360bcfa02f26c05b86dde5496d120f6 (master)

Quoted 8 lines · Gert Doering

Your patch has been applied to the master branch.

(I'm a bit undecided about release/2.4 - this is in "new feature!" land, and all the challenge stuff is "master" territory. OTOH, it's not openvpn main code, and the code is sane enough - so if folks think it should be in release/2.4, tell me)

commit 7369d01bf360bcfa02f26c05b86dde5496d120f6 (master)

I failed to respond to this that time.

Although this is a new feature, I too think its safe and useful to include this in 2.4.
If we do, we'll need this one (commit 7369d01bf360bcfa02f26c05b86dde5496d120f6) and the followup one 7a8109023f4c345fe12f23421c5fa7e88e1ea85b

Both should cherry-pick without conflicts.

See also Trac #1275 https://community.openvpn.net/openvpn/ticket/1275

From: Selva Nair <[email protected]...>

This adds a minimal secure_memzero()

Signed-off-by Selva Nair <[email protected]...>

Hi,

Quoted 5 lines · On Fri, May 05, 2017 at 02:24:01PM -0400, [email protected]... wrote:

From: Selva Nair <[email protected]...>

This adds a minimal secure_memzero()

Signed-off-by Selva Nair <[email protected]...>

Feature-ACK, Code-NAK, but just because David is planning to export secure_memzero() to plugins from OpenVPN proper - mentioned just today on IRC

17:09 <@dazo> syzzer: just thinking aloud (remembered our discussion on wiping
passwords securely in plug-ins) ... what do you think about exposing secure_memzero() to plug-ins, similar to what we do with plugin_{log,vlog}()?
17:09 <@dazo> to avoid each plug-in needing to re-implement this 17:09 <@syzzer> yeah, sounds useful
17:10 <@dazo> I'll send a patch doing that too (should be quick to solve)

So I'd postpone this until David's patch plus instructions for plugin authors show up.

Good timing :-)

Quoted 7 lines · Gert Doering

Hi,

Quoted 4 lines · On Fri, May 05, 2017 at 02:24:01PM -0400, [email protected]... wrote:

From: Selva Nair <[email protected]...>

This adds a minimal secure_memzero()

Quoted 7 lines

Quoted 1 line

Signed-off-by Selva Nair <[email protected]...>

Feature-ACK, Code-NAK, but just because David is planning to export secure_memzero() to plugins from OpenVPN proper - mentioned just today on IRC

17:09 <@dazo> syzzer: just thinking aloud (remembered our discussion on

wiping

Quoted 2 lines

passwords securely in plug-ins) ... what do you think about exposing secure_memzero() to plug-ins, similar to what we

do with

Quoted 6 lines

              plugin_{log,vlog}()?

17:09 <@dazo> to avoid each plug-in needing to re-implement this 17:09 <@syzzer> yeah, sounds useful
17:10 <@dazo> I'll send a patch doing that too (should be quick to solve)

Quoted 4 lines

So I'd postpone this until David's patch plus instructions for plugin authors show up.

Good timing :-)

Indeed :)

So the patch exporting secure_memzero() is on the way to the mailing list now.

To use this, you need to switch the openvpn_plugin_open_v1() to openvpn_plugin_open_v3(). The API on this function is quite different, but you shouldn't need to tweak too much. All the pointers are available in the new struct pointers the _v3 function uses.

Then you basically can declare a global variable like this:

  plugin_secure_memzero_t ovpn_secure_memzero = NULL;

From: Selva Nair <[email protected]...>

v2: Change the plugin open to use v3 API so that
openvpn_secure_memzero() exported from OpenVPN can be used.

Note: context is cast as (openvpn_plugin_handle_t *) for consistency with the current plugin header. If/when the header is fixed, change this cast as well.

Signed-off-by Selva Nair <[email protected]...>

---
 src/plugins/auth-pam/auth-pam.c       | 31 ++++++++++++++++++++++++++-----
 src/plugins/auth-pam/auth-pam.exports |  2 +-
 2 files changed, 27 insertions(+), 6 deletions(-)

src/plugins/auth-pam/auth-pam.c +7 -2

index d3e2c89..f3566c7 100644
@@ -63,6 +63,9 @@
 #define RESPONSE_VERIFY_SUCCEEDED 12
 #define RESPONSE_VERIFY_FAILED    13
 
+/* Pointers to functions exported from openvpn */
+static plugin_secure_memzero_t plugin_secure_memzero = NULL;
+
 /*
  * Plugin state, used by foreground
  */

@@ -274,8 +277,10 @@ name_value_match(const char *query, const char *match)
     return strncasecmp(match, query, strlen(match)) == 0;
 }
 
-OPENVPN_EXPORT openvpn_plugin_handle_t
-openvpn_plugin_open_v1(unsigned int *type_mask, const char *argv[], const char *envp[])
+OPENVPN_EXPORT int
+openvpn_plugin_open_v3(const int v3structver,
+                       struct openvpn_plugin_args_open_in const *args,
+                       struct openvpn_plugin_args_open_return *ret)
 {
     pid_t pid;
     int fd[2];

If the plug-in built and packaged separately and that build is not tied to OpenVPN itself, this can make this plug-in fail without any particular real reason if the OpenVPN binary gets updated independently.

Even though not explicitly documented, we should never remove any information from the plug-in v3 structs, only append to and only append at the end of the existing structs.

The plug-ins in src/plugins should use, which ensures forward compatibility against the OpenVPN binary:

if (v3structver < OPENVPN_PLUGINv3_STRUCTVER)