[Openvpn-devel,v1] Remove comparing username to NULL in tls_lock_username

Message ID 20250121161247.37883-1-frank@lichtenheld.com
State Accepted
Headers show
Series [Openvpn-devel,v1] Remove comparing username to NULL in tls_lock_username | expand

Commit Message

Frank Lichtenheld Jan. 21, 2025, 4:12 p.m. UTC
From: Arne Schwabe <arne@rfc2549.org>

tls_lock_username is only called in a single place and that place
calls this is function with up->username, which is always defined.

Change-Id: Ib8adf7b31cae02e2de3d45da23b76a2d79f13e20
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/871
This mail reflects revision 1 of this Change.

Acked-by according to Gerrit (reflected above):
Frank Lichtenheld <frank@lichtenheld.com>

Comments

Gert Doering Jan. 27, 2025, 11:39 a.m. UTC | #1
JFTR, v2 in gerrit is "the same as v1, just rebased", so taking what is
already on the list here.

And indeed, in the only call chain to this, we have

    string_mod_remap_name(up->username);

.. so this should better be non-NULL :-)  (ASSERT(str) in string_mod()).

Your patch has been applied to the master branch.

commit d9af13e8c222cba41000202908663a6d1e2cd028
Author: Arne Schwabe
Date:   Tue Jan 21 17:12:47 2025 +0100

     Remove comparing username to NULL in tls_lock_username

     Signed-off-by: Arne Schwabe <arne@rfc2549.org>
     Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
     Message-Id: <20250121161247.37883-1-frank@lichtenheld.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg30520.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
index 4c4b58d..e7d7ed6 100644
--- a/src/openvpn/ssl_verify.c
+++ b/src/openvpn/ssl_verify.c
@@ -153,11 +153,11 @@ 
 {
     if (multi->locked_username)
     {
-        if (!username || strcmp(username, multi->locked_username))
+        if (strcmp(username, multi->locked_username) != 0)
         {
             msg(D_TLS_ERRORS, "TLS Auth Error: username attempted to change from '%s' to '%s' -- tunnel disabled",
                 multi->locked_username,
-                np(username));
+                username);
 
             /* disable the tunnel */
             tls_deauthenticate(multi);
@@ -166,10 +166,7 @@ 
     }
     else
     {
-        if (username)
-        {
-            multi->locked_username = string_alloc(username, NULL);
-        }
+        multi->locked_username = string_alloc(username, NULL);
     }
     return true;
 }