Thread: [Openvpn-devel] [S] Change in openvpn[master]: Ensure that get_sigtype always return non-NULL

Robust and flexible VPN network tunnelling

Brought to you by: dazo, djpig, ericcrist, jimyonan, mattock

openvpn-devel

[Openvpn-devel] [S] Change in openvpn[master]: Ensure that get_sigtype always return non-NULL

From: plaisthos (C. Review) <ge...@op...> - 2025-10-29 11:30:59

Attention is currently required from: flichtenheld.

Hello flichtenheld,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1325?usp=email

to review the following change.


Change subject: Ensure that get_sigtype always return non-NULL
......................................................................

Ensure that get_sigtype always return non-NULL

There is a theoretical possibility that OpenSSL returns an NID that
OBJ_nid2sn cannot resolve and thus the function return NULL.

This is however extremely unlikely. But we still cover this case now
to make linters/code checker happy and avoid similar false positives
in the future.

Reported-by: Joshua Rogers <co...@jo...>
Found-by: ZeroPath (https://zeropath.com/)
Change-Id: I70e221ff5d9752fec17bad18fd41dcf188ae8fbc
Signed-off-by: Arne Schwabe <ar...@rf...>
---
M src/openvpn/ssl_openssl.c
1 file changed, 9 insertions(+), 1 deletion(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/25/1325/1

diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index d997141..1e1912e 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -2408,7 +2408,15 @@
             return "(error getting name)";
 
         default:
-            return OBJ_nid2sn(nid);
+            const char *type = OBJ_nid2sn(nid);
+            if (!type)
+            {
+                /* This is unlikely to ever happen as OpenSSL is unlikely to
+                 * return an NID it cannot resolve itself but we silence
+                 * linter/code checkers here */
+                type = "(error getting name, OBJ_nid2sn failed)";
+            }
+            return type;
     }
 }
 #endif /* ifndef LIBRESSL_VERSION_NUMBER */

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1325?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I70e221ff5d9752fec17bad18fd41dcf188ae8fbc
Gerrit-Change-Number: 1325
Gerrit-PatchSet: 1
Gerrit-Owner: plaisthos <arn...@rf...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: flichtenheld <fr...@li...>

[Openvpn-devel] [S] Change in openvpn[master]: Ensure that get_sigtype always return non-NULL

From: plaisthos (C. Review) <ge...@op...> - 2025-10-29 11:48:59

Attention is currently required from: flichtenheld.

Hello flichtenheld, 

I'd like you to reexamine a change. Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1325?usp=email

to look at the new patch set (#2).


Change subject: Ensure that get_sigtype always return non-NULL
......................................................................

Ensure that get_sigtype always return non-NULL

There is a theoretical possibility that OpenSSL returns an NID that
OBJ_nid2sn cannot resolve and thus the function return NULL.

This is however extremely unlikely. But we still cover this case now
to make linters/code checker happy and avoid similar false positives
in the future.

Reported-by: Joshua Rogers <co...@jo...>
Found-by: ZeroPath (https://zeropath.com/)
Change-Id: I70e221ff5d9752fec17bad18fd41dcf188ae8fbc
Signed-off-by: Arne Schwabe <ar...@rf...>
---
M src/openvpn/ssl_openssl.c
1 file changed, 11 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/25/1325/2

diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index d997141..a4a6863 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -2408,7 +2408,17 @@
             return "(error getting name)";
 
         default:
-            return OBJ_nid2sn(nid);
+        {
+            const char *type = OBJ_nid2sn(nid);
+            if (!type)
+            {
+                /* This is unlikely to ever happen as OpenSSL is unlikely to
+                 * return an NID it cannot resolve itself but we silence
+                 * linter/code checkers here */
+                type = "(error getting name, OBJ_nid2sn failed)";
+            }
+            return type;
+        }
     }
 }
 #endif /* ifndef LIBRESSL_VERSION_NUMBER */

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1325?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I70e221ff5d9752fec17bad18fd41dcf188ae8fbc
Gerrit-Change-Number: 1325
Gerrit-PatchSet: 2
Gerrit-Owner: plaisthos <arn...@rf...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: flichtenheld <fr...@li...>

[Openvpn-devel] [S] Change in openvpn[master]: Ensure that get_sigtype always return non-NULL

From: cron2 (C. Review) <ge...@op...> - 2025-10-30 19:29:58

Attention is currently required from: flichtenheld, plaisthos.

cron2 has posted comments on this change by plaisthos. ( http://gerrit.openvpn.net/c/openvpn/+/1325?usp=email )

Change subject: Ensure that get_sigtype always return non-NULL
......................................................................


Patch Set 2: Code-Review+2


-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1325?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: comment
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I70e221ff5d9752fec17bad18fd41dcf188ae8fbc
Gerrit-Change-Number: 1325
Gerrit-PatchSet: 2
Gerrit-Owner: plaisthos <arn...@rf...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Comment-Date: Thu, 30 Oct 2025 19:29:42 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes

[Openvpn-devel] [PATCH v2] Ensure that get_sigtype always return non-NULL

From: Gert D. <ge...@gr...> - 2025-10-30 19:30:12

From: Arne Schwabe <ar...@rf...>

There is a theoretical possibility that OpenSSL returns an NID that
OBJ_nid2sn cannot resolve and thus the function return NULL.

This is however extremely unlikely. But we still cover this case now
to make linters/code checker happy and avoid similar false positives
in the future.

Reported-by: Joshua Rogers <co...@jo...>
Found-by: ZeroPath (https://zeropath.com/)
Change-Id: I70e221ff5d9752fec17bad18fd41dcf188ae8fbc
Signed-off-by: Arne Schwabe <ar...@rf...>
Acked-by: Gert Doering <ge...@gr...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1325
---

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/+/1325
This mail reflects revision 2 of this Change.

Acked-by according to Gerrit (reflected above):
Gert Doering <ge...@gr...>

        
diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index d997141..a4a6863 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -2408,7 +2408,17 @@
             return "(error getting name)";
 
         default:
-            return OBJ_nid2sn(nid);
+        {
+            const char *type = OBJ_nid2sn(nid);
+            if (!type)
+            {
+                /* This is unlikely to ever happen as OpenSSL is unlikely to
+                 * return an NID it cannot resolve itself but we silence
+                 * linter/code checkers here */
+                type = "(error getting name, OBJ_nid2sn failed)";
+            }
+            return type;
+        }
     }
 }
 #endif /* ifndef LIBRESSL_VERSION_NUMBER */

[Openvpn-devel] [PATCH applied] Re: Ensure that get_sigtype always return non-NULL

From: Gert D. <ge...@gr...> - 2025-10-30 21:06:07

As the commit says, this is very unlikely to happen - but then, with custom
OpenSSL providers, or some of the semi-compatible OpenSSL variants, who
knows.  So having a sanity check here is good, and not very costly.

Your patch has been applied to the master branch.

commit 9fa32e66c9ec66ca05704f8e8b2fd9db7c711c50
Author: Arne Schwabe
Date:   Thu Oct 30 20:29:57 2025 +0100

     Ensure that get_sigtype always return non-NULL

     Signed-off-by: Arne Schwabe <ar...@rf...>
     Acked-by: Gert Doering <ge...@gr...>
     Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1325
     Message-Id: <202...@gr...>
     URL: https://www.mail-archive.com/ope...@li.../msg34060.html
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

[Openvpn-devel] [S] Change in openvpn[master]: Ensure that get_sigtype always return non-NULL

From: cron2 (C. Review) <ge...@op...> - 2025-10-30 21:06:10

cron2 has uploaded a new patch set (#3) to the change originally created by plaisthos. ( http://gerrit.openvpn.net/c/openvpn/+/1325?usp=email )

The following approvals got outdated and were removed:
Code-Review+2 by cron2


Change subject: Ensure that get_sigtype always return non-NULL
......................................................................

Ensure that get_sigtype always return non-NULL

There is a theoretical possibility that OpenSSL returns an NID that
OBJ_nid2sn cannot resolve and thus the function return NULL.

This is however extremely unlikely. But we still cover this case now
to make linters/code checker happy and avoid similar false positives
in the future.

Reported-by: Joshua Rogers <co...@jo...>
Found-by: ZeroPath (https://zeropath.com/)
Change-Id: I70e221ff5d9752fec17bad18fd41dcf188ae8fbc
Signed-off-by: Arne Schwabe <ar...@rf...>
Acked-by: Gert Doering <ge...@gr...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1325
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg34060.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/ssl_openssl.c
1 file changed, 11 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/25/1325/3

diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index d997141..a4a6863 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -2408,7 +2408,17 @@
             return "(error getting name)";
 
         default:
-            return OBJ_nid2sn(nid);
+        {
+            const char *type = OBJ_nid2sn(nid);
+            if (!type)
+            {
+                /* This is unlikely to ever happen as OpenSSL is unlikely to
+                 * return an NID it cannot resolve itself but we silence
+                 * linter/code checkers here */
+                type = "(error getting name, OBJ_nid2sn failed)";
+            }
+            return type;
+        }
     }
 }
 #endif /* ifndef LIBRESSL_VERSION_NUMBER */

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1325?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I70e221ff5d9752fec17bad18fd41dcf188ae8fbc
Gerrit-Change-Number: 1325
Gerrit-PatchSet: 3
Gerrit-Owner: plaisthos <arn...@rf...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-CC: openvpn-devel <ope...@li...>

[Openvpn-devel] [S] Change in openvpn[master]: Ensure that get_sigtype always return non-NULL

From: cron2 (C. Review) <ge...@op...> - 2025-10-30 21:06:12

cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/1325?usp=email )

Change subject: Ensure that get_sigtype always return non-NULL
......................................................................

Ensure that get_sigtype always return non-NULL

There is a theoretical possibility that OpenSSL returns an NID that
OBJ_nid2sn cannot resolve and thus the function return NULL.

This is however extremely unlikely. But we still cover this case now
to make linters/code checker happy and avoid similar false positives
in the future.

Reported-by: Joshua Rogers <co...@jo...>
Found-by: ZeroPath (https://zeropath.com/)
Change-Id: I70e221ff5d9752fec17bad18fd41dcf188ae8fbc
Signed-off-by: Arne Schwabe <ar...@rf...>
Acked-by: Gert Doering <ge...@gr...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1325
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg34060.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/ssl_openssl.c
1 file changed, 11 insertions(+), 1 deletion(-)




diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index d997141..a4a6863 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -2408,7 +2408,17 @@
             return "(error getting name)";
 
         default:
-            return OBJ_nid2sn(nid);
+        {
+            const char *type = OBJ_nid2sn(nid);
+            if (!type)
+            {
+                /* This is unlikely to ever happen as OpenSSL is unlikely to
+                 * return an NID it cannot resolve itself but we silence
+                 * linter/code checkers here */
+                type = "(error getting name, OBJ_nid2sn failed)";
+            }
+            return type;
+        }
     }
 }
 #endif /* ifndef LIBRESSL_VERSION_NUMBER */

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1325?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: merged
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I70e221ff5d9752fec17bad18fd41dcf188ae8fbc
Gerrit-Change-Number: 1325
Gerrit-PatchSet: 3
Gerrit-Owner: plaisthos <arn...@rf...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-CC: openvpn-devel <ope...@li...>