Clear error stack on successful OSSL_STORE_open()
authorRichard Levitte <levitte@openssl.org>
Sat, 5 Aug 2017 12:56:13 +0000 (14:56 +0200)
committerRichard Levitte <levitte@openssl.org>
Tue, 15 Aug 2017 12:28:23 +0000 (14:28 +0200)
Since OSSL_STORE_open() tries with the 'file' scheme loader first, and
then on the loader implied by the URI if the former fails, the former
leaves an error on the error stack.  This is confusing, so let's clear
the error stack on success.  The implementation uses ERR_set_mark,
ERR_pop_to_mark and ERR_clear_last_mark to make sure caller errors are
preserved as much as possible.

Fixes #4089

Reviewed-by: Andy Polyakov <appro@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4094)

crypto/store/store_lib.c

index b982e9c7532bce3c459b496b0dd86d66c580ae33..6f789eb79c9226d68b1f837b840ab4a26120af60 100644 (file)
@@ -64,28 +64,39 @@ OSSL_STORE_CTX *OSSL_STORE_open(const char *uri, const UI_METHOD *ui_method,
         }
     }
 
         }
     }
 
+    ERR_set_mark();
+
     /* Try each scheme until we find one that could open the URI */
     for (i = 0; loader_ctx == NULL && i < schemes_n; i++) {
         if ((loader = ossl_store_get0_loader_int(schemes[i])) != NULL)
             loader_ctx = loader->open(loader, uri, ui_method, ui_data);
     }
     if (loader_ctx == NULL)
     /* Try each scheme until we find one that could open the URI */
     for (i = 0; loader_ctx == NULL && i < schemes_n; i++) {
         if ((loader = ossl_store_get0_loader_int(schemes[i])) != NULL)
             loader_ctx = loader->open(loader, uri, ui_method, ui_data);
     }
     if (loader_ctx == NULL)
-        goto done;
+        goto err;
 
     if ((ctx = OPENSSL_zalloc(sizeof(*ctx))) == NULL) {
         OSSL_STOREerr(OSSL_STORE_F_OSSL_STORE_OPEN, ERR_R_MALLOC_FAILURE);
 
     if ((ctx = OPENSSL_zalloc(sizeof(*ctx))) == NULL) {
         OSSL_STOREerr(OSSL_STORE_F_OSSL_STORE_OPEN, ERR_R_MALLOC_FAILURE);
-        goto done;
+        goto err;
     }
 
     ctx->loader = loader;
     ctx->loader_ctx = loader_ctx;
     }
 
     ctx->loader = loader;
     ctx->loader_ctx = loader_ctx;
-    loader_ctx = NULL;
     ctx->ui_method = ui_method;
     ctx->ui_data = ui_data;
     ctx->post_process = post_process;
     ctx->post_process_data = post_process_data;
 
     ctx->ui_method = ui_method;
     ctx->ui_data = ui_data;
     ctx->post_process = post_process;
     ctx->post_process_data = post_process_data;
 
- done:
+    /*
+     * If the attempt to open with the 'file' scheme loader failed and the
+     * other scheme loader succeeded, the failure to open with the 'file'
+     * scheme loader leaves an error on the error stack.  Let's remove it.
+     */
+    ERR_pop_to_mark();
+
+    return ctx;
+
+ err:
+    ERR_clear_last_mark();
     if (loader_ctx != NULL) {
         /*
          * We ignore a returned error because we will return NULL anyway in
     if (loader_ctx != NULL) {
         /*
          * We ignore a returned error because we will return NULL anyway in
@@ -94,7 +105,7 @@ OSSL_STORE_CTX *OSSL_STORE_open(const char *uri, const UI_METHOD *ui_method,
          */
         (void)loader->close(loader_ctx);
     }
          */
         (void)loader->close(loader_ctx);
     }
-    return ctx;
+    return NULL;
 }
 
 int OSSL_STORE_ctrl(OSSL_STORE_CTX *ctx, int cmd, ...)
 }
 
 int OSSL_STORE_ctrl(OSSL_STORE_CTX *ctx, int cmd, ...)