[Openvpn-devel,1/2] Remove x509-username-fields uppercasing

Message ID cb8317eb-bfb6-47e8-9bc3-ae5cc603ff21@gmx.de
State Accepted
Headers show
Series x509-username-fields improvements | expand

Commit Message

corubba Feb. 15, 2025, 7 p.m. UTC
The uppercasing was first introduced together with the
x509-username-field option in commit 935c62be, and first released with
v2.2.0 in 2011. The uppercasing was later deprecated with commit
f4e0ad82 and release v2.4.0 in 2016. It think it is time to finally
remove it.

This deprecated feature prevents you from using non-extension
all-lowercase fieldnames like `name`, because these are converted to
uppercase and then cause an error. The deprecation warning is also shown
in cases where there is no actual uppercasing happening, for example
with numerical forms (aka oids) like `2.5.4.41` (oid of `name`).

Signed-off-by: Corubba Smith <corubba@gmx.de>
---
 Changes.rst                      |  5 +++++
 doc/man-sections/tls-options.rst |  6 ------
 src/openvpn/options.c            | 27 +--------------------------
 3 files changed, 6 insertions(+), 32 deletions(-)

--
2.48.1

Comments

Gert Doering Feb. 20, 2025, 10:07 a.m. UTC | #1
Acked-by: Gert Doering <gert@greenie.muc.de>

Thanks for helping us get rid of our technical debt ;-) - and indeed,
if we declared it obsolete years ago, it should go now.  Someone will
complain, but this is not something which can't be fixed with a small
config change.

I have not tested this beyond a GHA test run (which is compiling on
all platforms but not actually excercising this code).

Your patch has been applied to the master branch.

commit 90d89cc4cc41da6678d244f03c842d1b745f106b
Author: Corubba Smith
Date:   Sat Feb 15 20:00:33 2025 +0100

     Remove x509-username-fields uppercasing

     Signed-off-by: Corubba Smith <corubba@gmx.de>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <cb8317eb-bfb6-47e8-9bc3-ae5cc603ff21@gmx.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg30915.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/Changes.rst b/Changes.rst
index e0118111..bcc64fca 100644
--- a/Changes.rst
+++ b/Changes.rst
@@ -92,6 +92,11 @@  Compression on send
     ``--allow-compression yes`` is now an alias for
     ``--allow-compression asym``.

+User-visible Changes
+--------------------
+- ``--x509-username-field`` will no longer automatically convert fieldnames to
+  uppercase. This is deprecated since OpenVPN 2.4, and has now been removed.
+
 Overview of changes in 2.6
 ==========================

diff --git a/doc/man-sections/tls-options.rst b/doc/man-sections/tls-options.rst
index cdb85716..7882e924 100644
--- a/doc/man-sections/tls-options.rst
+++ b/doc/man-sections/tls-options.rst
@@ -763,12 +763,6 @@  If the option is inlined, ``algo`` is always :code:`SHA256`.
   Only the :code:`subjectAltName` and :code:`issuerAltName` X.509
   extensions and :code:`serialNumber` X.509 attribute are supported.

-  **Please note:** This option has a feature which will convert an
-  all-lowercase ``fieldname`` to uppercase characters, e.g.,
-  :code:`ou` -> :code:`OU`. A mixed-case ``fieldname`` or one having the
-  :code:`ext:` prefix will be left as-is. This automatic upcasing feature is
-  deprecated and will be removed in a future release.
-
   Non-compliant symbols are being replaced with the :code:`_` symbol, same as
   the field separator, so concatenating multiple fields with such or :code:`_`
   symbols can potentially lead to username collisions.
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 3ae44dbe..6b2dfa58 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -9395,37 +9395,12 @@  add_option(struct options *options,
 #ifdef ENABLE_X509ALTUSERNAME
     else if (streq(p[0], "x509-username-field") && p[1])
     {
-        /* This option used to automatically upcase the fieldnames passed as the
-         * option arguments, e.g., "ou" became "OU". Now, this "helpfulness" is
-         * fine-tuned by only upcasing Subject field attribute names which consist
-         * of all lower-case characters. Mixed-case attributes such as
-         * "emailAddress" are left as-is. An option parameter having the "ext:"
-         * prefix for matching X.509v3 extended fields will also remain unchanged.
-         */
         VERIFY_PERMISSION(OPT_P_GENERAL);
         for (size_t j = 1; j < MAX_PARMS && p[j] != NULL; ++j)
         {
             char *s = p[j];

-            if (strncmp("ext:", s, 4) != 0)
-            {
-                size_t i = 0;
-                while (s[i] && !isupper(s[i]))
-                {
-                    i++;
-                }
-                if (strlen(s) == i)
-                {
-                    while ((*s = toupper(*s)) != '\0')
-                    {
-                        s++;
-                    }
-                    msg(M_WARN, "DEPRECATED FEATURE: automatically upcased the "
-                        "--x509-username-field parameter to '%s'; please update your "
-                        "configuration", p[j]);
-                }
-            }
-            else if (!x509_username_field_ext_supported(s+4))
+            if (strncmp("ext:", s, 4) == 0 && !x509_username_field_ext_supported(s+4))
             {
                 msg(msglevel, "Unsupported x509-username-field extension: %s", s);
             }