Opened 3 years ago

Last modified 4 months ago

#225 new Bug / Defect

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

Reported by: ye_olde_iron Owned by:
Priority: minor Milestone: release 2.3.7
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 (9)

comment:1 Changed 3 years 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 3 years 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 2 years 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 2 years 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 23 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 23 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 16 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.

comment:8 Changed 12 months ago by cron2

  • Milestone set to release 2.3.5

Review before 3.2.5 release, decide if this should go in 3.2.5 or is too intrusive, then reclassify to "master". Sorry for the delay in working on this.

comment:9 Changed 4 months ago by cron2

  • Milestone changed from release 2.3.5 to release 2.3.7
Note: See TracTickets for help on using tickets.