Opened 21 months ago

Last modified 3 months ago

#225 new Bug / Defect

'--auth-user-pass FILE' and '--auth-nocache' problem

Reported by: ye_olde_iron Owned by:
Priority: minor Milestone:
Component: Configuration Version: 2.2.2
Severity: Not set (if unsure, select this one) Keywords: auth-user-pass auth-nocache
Cc:

Description

If you specify that username/password information should be read from a file ('--auth-user-pass FILE'), but that this information should NOT be cached ('--auth-nocache') then initially all works well, with the username and password being read from the file. However, when the renegotiation timeout occurs on the client ('--reneg-sec nnn'), an attempt is made to read the username and password FROM STDIN instead of from the specified file!

[The log gives the error "ERROR: could not read Auth username from stdin"]

If you have specified that the information should be read from a file, then the nocache should cause the information to be re-read from the file, not stdin.

(In this case I wish the file to contain a password that changes over time, so I do not want it to be cached).

The same problem may to have been reported under "Ticket #60 : --auth-user-pass <file> does not work after TLS soft reset disconnect", but '--auth-nocache' is not mentioned.

Change History (7)

comment:1 Changed 20 months ago by ye_olde_iron

Additional information ...

The problems appears to be caused by the fact that once the renegotiation timeout occurs auth_user_pass_setup() is being called with a 'NULL' auth_file pointer (and auth_user_pass.defined is false) - this causes get_user_pass_cr() to try to prompt from stdin.

I believe the initial sequence is as follows (very simplified) :-

  ssl_set_auth_nocache()
    {on exit: auth_user_pass.defined=0 , auth_user_pass.nocache=1}
  context_init_1( ... )
    init_query_passwords( ... )
      auth_user_pass_setup( auth_file=<somepath> )
        get_user_pass_cr( ... , auth_file=<somepath> , ... )
          ... from_stdin=0 ...
          {the username and password are read from <somepath>}
          {on exit: up->defined=1 , up->nocache=1}
  tls_process( ... )
    key_method_2_write( ... )
      auth_user_pass_setup( auth_file=NULL )  <-- second call to this routine, but now with a NULL auth_file
        {does not call get_user_pass_cr() as auth_user_pass.defined=1}
      purge_user_pass( auth_user_pas , false )
      {on exit: auth_user_pass.defined=0 , auth_user_pass.nocache=1}

At this stage the tunnel is running, but auth_user_pass.defined is now false so the program
will need to read the username/password data again.

The renegotiation timeout occurs and tls_process() gets underway again :-

  tls_process( ... )
    key_method_2_write( ... )
      auth_user_pass_setup( auth_file=NULL )  <-- third call to this routine, again with a NULL auth_file
        get_user_pass_cr( ... , '''auth_file=NULL''' , ... )
          ... from_stdin=1 ...
          "ERROR: could not read Auth username from stdin"

key_method_2_write() always makes the call "auth_user_pass_setup(NULL)" - this means that once purge_user_pass() purges the username and password from memory (as expected with nocache) the routine get_user_pass_cr() is not being given the name of the auth_file.

key_method_2_write() needs to pass the name of the auth_file if defined in the config, or perhaps the name could be saved in the auth_user_pass structure (and not cleared unless it changes).

comment:2 Changed 20 months ago by ye_olde_iron

Potential solution ... add a variable to "struct tls_options" that will save the auth-user-pass value for a connection and then use this variable rather than NULL when key_method_2_write() calls auth_user_pass_setup().

Changes to ssl.h to add in a new variable 'auth_user_pass_file' :-

--- ssl.h_ORIG	2011-12-14 05:58:56.000000000 +1300
+++ ssl.h	2012-08-21 08:00:11.000000000 +1200
@@ -477,6 +477,8 @@
   const char *auth_user_pass_verify_script;
   bool auth_user_pass_verify_script_via_file;
   const char *tmp_dir;
+  /* ... and at the client end ... */
+  const char *auth_user_pass_file;
 
   /* use the client-config-dir as a positive authenticator */
   const char *client_config_dir_exclusive;

Changes to init.c (in "do_init_crypto_tls()") to initialise the 'auth_user_pass_file' variable :-

--- init.c_ORIG	2011-12-14 05:58:56.000000000 +1300
+++ init.c	2012-08-21 08:08:38.000000000 +1200
@@ -2049,6 +2049,7 @@
   if (options->ccd_exclusive)
     to.client_config_dir_exclusive = options->client_config_dir;
 #endif
+  to.auth_user_pass_file = options->auth_user_pass_file;
 
   /* TLS handshake authentication (--tls-auth) */
   if (options->tls_auth_file)

Changes to ssl.c (in "key_method_2_write()") to use the new variable when calling "auth_user_pass_setup()" :-

--- ssl.c_ORIG	2011-12-14 05:58:56.000000000 +1300
+++ ssl.c	2012-08-21 09:20:37.000000000 +1200
@@ -3623,7 +3623,8 @@
   /* write username/password if specified */
   if (auth_user_pass_enabled)
     {
-      auth_user_pass_setup (NULL);
+      /* auth_user_pass_setup (NULL); */
+      auth_user_pass_setup (session->opt->auth_user_pass_file);
       if (!write_string (buf, auth_user_pass.username, -1))
 	goto error;
       if (!write_string (buf, auth_user_pass.password, -1))

comment:3 Changed 15 months ago by dazo

  • Component changed from Generic / unclassified to Configuration
  • Keywords stdin removed
  • Priority changed from major to minor

I'll grant you that this is a bug. But your fix is not the right fix, IMO.

The problem here is that mixing --auth-nocache with --auth-user-pass FILE is moot. That really doesn't make sense. If you don't mind having the password stored on a disk, why would you care if the password is temporarily saved in memory or not?

So the right fix is to make --auth-nocache and --auth-user-pass FILE mutually exclusive. Make the user choose one of them, not both.

comment:4 Changed 14 months ago by ye_olde_iron

... That really doesn't make sense. If you don't mind having the password stored on a disk, why would you care if the password is temporarily saved in memory or not?

Actually it does make sense in my situation. I am *updating* the password that is stored on disk every few hours to keep it fresh (with a current timestamp etc.) - this is done by an entirely separate program to OpenVPN. I want OpenVPN to pick up the updates over time, not use a previously cached one!

I would appreciate it if the patch could go through, there is a sensible use for it :-)

Many thanks

comment:5 Changed 10 months ago by lfarkas

i have to agree. eg. we change the on disk auth file each time when the network settings change (eg. dhcp gives a new ip). so we need both --auth-nocache AND --auth-user-pass.

is there any current version which fix this verion?

comment:6 Changed 10 months ago by cron2

Actually, I think from a first glance, the patch looks reasonable. The code surrounding this is a bit complicated, so it needs a more close look. Time...

comment:7 Changed 3 months ago by cron2

This seems to be a duplicate of #60.

Checked the patch from Davide Brini, and it does not fix the issue, but at least tell us more clearly about it...

+ msg (M_USAGE, "Cannot use --auth-nocache with credentials from file");

... so this needs independent observation, based on the patch and description here.

Note: See TracTickets for help on using tickets.