Opened 7 years ago

Closed 2 years ago

#372 closed Bug / Defect (notabug)

auth-user-pass-verify passes an empty trailing arg

Reported by: alga Owned by:
Priority: minor Milestone:
Component: plug-ins / plug-in API Version: OpenVPN 2.3.2 (Community Ed)
Severity: Not set (select this one, unless your'e a OpenVPN developer) Keywords: auth-user-pass-verify
Cc:

Description

Then the server config contains a line like:

auth-user-pass-verify "/opt/vpnauth/bin/vpnauth /etc/vpnauth/config.cfg" via-env

The script in fact receives three arguments:

{"/opt/vpnauth/bin/vpnauth", "/etc/vpnauth/config.cfg", ""}

This upsets scripts with strict argument parsers.

Bug oserved on Ubuntu 13.10, openvpn-2.3.2-4ubuntu1.

Change History (3)

comment:1 Changed 6 years ago by David Sommerseth

Component: Generic / unclassifiedplug-ins / plug-in API
Keywords: auth-user-pass-verify added

Can you please try to test this patch, to see if this resolves the issue you see?

diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
index ccfa9d2..9f683b4 100644
--- a/src/openvpn/ssl_verify.c
+++ b/src/openvpn/ssl_verify.c
@@ -954,7 +954,7 @@ verify_user_pass_script (struct tls_session *session, const struct user_pass *up
 {
   struct gc_arena gc = gc_new ();
   struct argv argv = argv_new ();
-  const char *tmp_file = "";
+  const char *tmp_file = NULL;
   bool ret = false;
 
   /* Is username defined? */
@@ -996,7 +996,10 @@ verify_user_pass_script (struct tls_session *session, const struct user_pass *up
       setenv_untrusted (session);
 
       /* format command line */
-      argv_printf (&argv, "%sc %s", session->opt->auth_user_pass_verify_script, tmp_file);
+      if( tmp_file )
+        argv_printf (&argv, "%sc %s", session->opt->auth_user_pass_verify_script, tmp_file);
+      else
+        argv_printf (&argv, "%sc", session->opt->auth_user_pass_verify_script);
 
       /* call command */
       ret = openvpn_run_script (&argv, session->opt->es, 0,

comment:2 Changed 6 years ago by David Sommerseth

There has been no response to this ticket. April 21st I did e-mail the reporter directly and have not heard anything yet.

I propose to close this ticket as notabug unless people report this as a real issue within the end of May 2015.

The patch suggested in this ticket needs to be tested and reviewed to be considered a suitable fix.

comment:3 Changed 2 years ago by David Sommerseth

Resolution: notabug
Status: newclosed

This is most likely not a something we need to fix. I can acknowledge that the additional empty argument may very well be the result as it is now, but not convinced it's a severe enough issue to mandate "complicating" the code further. The patch here will anyhow not apply on git master nor release/2.4, so a new fix is needed.

If this ticket gets noticeable traction with good arguments why this needs to be fixed, please re-open it.

Note: See TracTickets for help on using tickets.