diff mbox series

[Openvpn-devel,v1] Do not stop reading from file/uri when OPENSSL_STORE_load() returns error

Message ID 20240911104941.19429-1-gert@greenie.muc.de
State Accepted
Headers show
Series [Openvpn-devel,v1] Do not stop reading from file/uri when OPENSSL_STORE_load() returns error | expand

Commit Message

Gert Doering Sept. 11, 2024, 10:49 a.m. UTC 
From: Selva Nair <selva.nair@gmail.com>

OPENSSL_STORE_load() can error and return NULL even when the file or URI
still has readable objects left.

Fix by iterating until OPENSSL_STORE_eof(). Also clear such errors to avoid
misleading messages printed at the end by crypto_print_openssl_errors().

Change-Id: I2bfa9ffbd17d0599014d38b2a2fd319766cdb1e3
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
---

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

Acked-by according to Gerrit (reflected above):
Arne Schwabe <arne-openvpn@rfc2549.org>

Comments

Gert Doering Sept. 11, 2024, 12:25 p.m. UTC | #1 
Thanks for fixing this so quickly.  Now this seems to be an... interesting 
API to work with... :-)

I do not have something really interesting set up in terms of cert files
and storage, so I just did basic client side testing with OSSL 3.x (works).

Your patch has been applied to the master branch
(bugfix, but offending code not in 2.6).

commit e9ad1b31a04799de98f15220eb39225c3d9eaa64
Author: Selva Nair
Date:   Wed Sep 11 12:49:41 2024 +0200

     Do not stop reading from file/uri when OPENSSL_STORE_load() returns error

     Signed-off-by: Selva Nair <selva.nair@gmail.com>
     Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
     Message-Id: <20240911104941.19429-1-gert@greenie.muc.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29187.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index 0d845f4..5fd6572 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -813,6 +813,15 @@ 
     }
     return 0;
 }
+
+static void
+clear_ossl_store_error(OSSL_STORE_CTX *store_ctx)
+{
+    if (OSSL_STORE_error(store_ctx))
+    {
+        ERR_clear_error();
+    }
+}
 #endif /* defined(HAVE_OPENSSL_STORE_API) */
 
 /**
@@ -864,7 +873,19 @@ 
     {
         goto end;
     }
-    info = OSSL_STORE_load(store_ctx);
+    while (1)
+    {
+        info = OSSL_STORE_load(store_ctx);
+        if (info || OSSL_STORE_eof(store_ctx))
+        {
+            break;
+        }
+        /* OPENSSL_STORE_load can return error and still have usable objects to follow.
+         * ref: man OPENSSL_STORE_open
+         * Clear error and recurse through the file if info = NULL and eof not reached
+         */
+        clear_ossl_store_error(store_ctx);
+    }
     if (!info)
     {
         goto end;
@@ -1099,7 +1120,19 @@ 
         goto end;
     }
 
-    info = OSSL_STORE_load(store_ctx);
+    while (1)
+    {
+        info = OSSL_STORE_load(store_ctx);
+        if (info || OSSL_STORE_eof(store_ctx))
+        {
+            break;
+        }
+        /* OPENSSL_STORE_load can return error and still have usable objects to follow.
+         * ref: man OPENSSL_STORE_open
+         * Clear error and recurse through the file if info = NULL and eof not reached.
+         */
+        clear_ossl_store_error(store_ctx);
+    }
     if (!info)
     {
         goto end;
@@ -1120,9 +1153,14 @@ 
     OSSL_STORE_INFO_free(info);
 
     /* iterate through the store and add extra certificates if any to the chain */
-    info = OSSL_STORE_load(store_ctx);
-    while (info && !OSSL_STORE_eof(store_ctx))
+    while (!OSSL_STORE_eof(store_ctx))
     {
+        info = OSSL_STORE_load(store_ctx);
+        if (!info)
+        {
+            clear_ossl_store_error(store_ctx);
+            continue;
+        }
         x = OSSL_STORE_INFO_get1_CERT(info);
         if (x && SSL_CTX_add_extra_chain_cert(tls_ctx->ctx, x) != 1)
         {
@@ -1131,7 +1169,6 @@ 
             break;
         }
         OSSL_STORE_INFO_free(info);
-        info = OSSL_STORE_load(store_ctx);
     }
 
 end: