diff mbox series

[Openvpn-devel,S] Change in openvpn[master]: Do not stop reading from file/uri when OPENSSL_STORE_load() returns e...

Message ID 3ba839f6d29460d1c132a0beee5f11cfaa93aac8-HTML@gerrit.openvpn.net
State Superseded
Headers show
Series [Openvpn-devel,S] Change in openvpn[master]: Do not stop reading from file/uri when OPENSSL_STORE_load() returns e... | expand

Commit Message

cron2 (Code Review) Sept. 11, 2024, 3:19 a.m. UTC 
Attention is currently required from: flichtenheld, plaisthos.

Hello plaisthos, flichtenheld,

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

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

to review the following change.


Change subject: Do not stop reading from file/uri when OPENSSL_STORE_load() returns error
......................................................................

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

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>
---
M src/openvpn/ssl_openssl.c
1 file changed, 42 insertions(+), 5 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/42/742/1
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: