On 10/10/2022 09:12, Gert Doering wrote:
We do not permit username changes on renegotiation (= username is
"locked" after successful initial authentication).

Unfortunately the way this is written this gets in the way of using
auth-user-pass-optional + pushing "auth-token-user" from client-connect
(and most likely also "from management") because we'll lock an empty
username, and on renegotiation, refuse the client with

    TLS Auth Error: username attempted to change from
             '' to 'MyTokenUser' -- tunnel disabled

Fix: extend "is username a valid pointer" to "... and points to a
      non-empty string" before locking.
  src/openvpn/ssl_verify.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
index 76cb9f19..4206cf9c 100644
--- a/src/openvpn/ssl_verify.c
+++ b/src/openvpn/ssl_verify.c
@@ -166,7 +166,7 @@ tls_lock_username(struct tls_multi *multi, const char 
-        if (username)
+        if (username && *username)

super uber nitpick (bike shadding level):

I think in other places we perform the very same check using the format: username[0] instead of *username.

That's because username is a sequence of chars, therefore it is a bit more logical for our brains to "check the first character in the sequence" rather than "dereference this pointer".

[or if we want to go the clean way, we should use strlen() == 0, but I understand that may be overkill]

my 3 cents.


              multi->locked_username = string_alloc(username, NULL);

Antonio Quartulli

Openvpn-devel mailing list

Reply via email to