#225 closed Bug / Defect (fixed)
'--auth-user-pass FILE' and '--auth-nocache' problem
Reported by: | ye_olde_iron | Owned by: | Steffan Karger |
---|---|---|---|
Priority: | minor | Milestone: | release 2.3.7 |
Component: | Configuration | Version: | OpenVPN 2.2.2 (Community Ed) |
Severity: | Not set (select this one, unless your'e a OpenVPN developer) | 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 (14)
comment:1 Changed 12 years ago by
comment:2 follow-up: 11 Changed 12 years ago by
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 12 years ago by
Component: | Generic / unclassified → Configuration |
---|---|
Keywords: | stdin removed |
Priority: | major → 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 12 years ago by
... 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 11 years ago by
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 11 years ago by
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 11 years ago by
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 10 years ago by
Milestone: | → 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 10 years ago by
Milestone: | release 2.3.5 → release 2.3.7 |
---|
comment:10 Changed 9 years ago by
Owner: | set to Steffan Karger |
---|---|
Status: | new → assigned |
Found a victim, uh, volunteer :-)
comment:11 Changed 9 years ago by
The changes proposed by ye_olde_iron indeed do look reasonable. There is however a problem: when used with --daemon or --cd, using relative paths will either fail during init (because the user/pass file is read once before any call to chdir() or daemon()) or during connection (because the file is re-read, but chdir()/daemon() will have changed the working directory).
I propose the following:
- For release/2.3, apply the changes as suggested by ye_olde_iron. It fixes this bug and introduces the above described (less severe) bug, but at least it is possible for users to have a correct functioning setup by using absolute paths. Since this particular setup would not work before anyway, it will not break any currently functioning setups.
- For master, only read the file during (re)connect, and make sure to add a notice to the change log that the relavite path of --auth-user-pass is now influenced by --cd and --daemon. This fixes both bugs but will probably break some user configs.
comment:12 Changed 9 years ago by
commit ac1cb5bfbb9e09e79fd737bc57999d968d77c5ad (master)
commit 6f789d2ec6b6aacb46ab27f1482222c6981faab6 (release/2.3)
Author: Steffan Karger
Date: Sat May 23 15:02:25 2015 +0200
Re-read auth-user-pass file on (re)connect if required
... which also includes a new text in openvpn.8 to explain that using --auth-user-pass file + --auth-nocache + --daemon will bite you regarding absolute/relative path names. Working around this (like, determine absolute path name and remember that one) would have been possible, but we did not consider this to be crucial for a somewhat niche application. "You know what you're getting yourself into" ;-)
comment:13 Changed 9 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
... which means 'fixed'!
comment:14 Changed 9 years ago by
#308 seems to be a duplicate of this one (using slighly different option combinations, but it smells very similar)
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) :-
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 :-
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).