Relax absolut path checking in our 'file' scheme implementation

So far, we strictly obeyed [RFC 8089], which only allows absolute paths
in a 'file:' URI.  However, this seems to give a confusing user
experience, where something like 'file:foo.pem' wouldn't open foo.pem,
even though it's there in the current directory, but 'file:$(pwd)/foo.pem'
would.

To be less surprising for such use cases, we relax our implementation
visavi [RFC 8089] to allow relative paths.

[RFC 8089]: https://datatracker.ietf.org/doc/html/rfc8089

Fixes #27461

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/27482)

diff --git a/CHANGES.md b/CHANGES.md
index 352c0345d36c0e7efeee58165431cfb6a93163a5..5ce0e7f2ec74e3ab3516bafa1c6931d28fce5a45 100644 (file)
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -31,6 +31,14 @@ OpenSSL 3.6
 
 ### Changes between 3.5 and 3.6 [xx XXX xxxx]
 
+ * Relax the path check in OpenSSL's 'file:' scheme implementation for
+   OSSL_STORE.  Previously, when the 'file:' scheme is an explicit part
+   of the URI, our implementation required an absolute path, such as
+   'file:/path/to/file.pem'.  This requirement is now relaxed, allowing
+   'file:path/to/file.pem', as well as 'file:file.pem'.
+
+   *Richard Levitte*
+
  * Changed openssl-pkey(1) to match the documentation when private keys
    are output in DER format (`-outform DER`) by producing the `PKCS#8` form by
    default.  Previously this would output the *traditional* form for those

diff --git a/engines/e_loader_attic.c b/engines/e_loader_attic.c
index 21e0ed25f8b3ba1a2e50e4228a0a3a7c7a39c640..cef313a137bc609d9d65c1de8e2633c8ccdc7dcc 100644 (file)
--- a/engines/e_loader_attic.c
+++ b/engines/e_loader_attic.c
@@ -947,18 +947,14 @@ static OSSL_STORE_LOADER_CTX *file_open_ex
 {
     OSSL_STORE_LOADER_CTX *ctx = NULL;
     struct stat st;
-    struct {
-        const char *path;
-        unsigned int check_absolute:1;
-    } path_data[2];
+    const char *path_data[2];
     size_t path_data_n = 0, i;
     const char *path, *p = uri, *q;
 
     /*
      * First step, just take the URI as is.
      */
-    path_data[path_data_n].check_absolute = 0;
-    path_data[path_data_n++].path = uri;
+    path_data[path_data_n++] = uri;
 
     /*
      * Second step, if the URI appears to start with the "file" scheme,
@@ -971,48 +967,39 @@ static OSSL_STORE_LOADER_CTX *file_open_ex
         if (CHECK_AND_SKIP_PREFIX(q, "//")) {
             path_data_n--;           /* Invalidate using the full URI */
             if (CHECK_AND_SKIP_CASE_PREFIX(q, "localhost/")
-                    || CHECK_AND_SKIP_PREFIX(q, "/")) {
+                || CHECK_AND_SKIP_PREFIX(q, "/")) {
+                /*
+                 * In this case, we step back on char to ensure that the
+                 * first slash is preserved, making the path always absolute
+                 */
                 p = q - 1;
             } else {
                 ATTICerr(0, ATTIC_R_URI_AUTHORITY_UNSUPPORTED);
                 return NULL;
             }
         }
-
-        path_data[path_data_n].check_absolute = 1;
 #ifdef _WIN32
         /* Windows "file:" URIs with a drive letter start with a '/' */
         if (p[0] == '/' && p[2] == ':' && p[3] == '/') {
             char c = tolower((unsigned char)p[1]);
 
             if (c >= 'a' && c <= 'z') {
+                /* Skip past the slash, making the path a normal Windows path */
                 p++;
-                /* We know it's absolute, so no need to check */
-                path_data[path_data_n].check_absolute = 0;
             }
         }
 #endif
-        path_data[path_data_n++].path = p;
+        path_data[path_data_n++] = p;
     }
 
 
     for (i = 0, path = NULL; path == NULL && i < path_data_n; i++) {
-        /*
-         * If the scheme "file" was an explicit part of the URI, the path must
-         * be absolute.  So says RFC 8089
-         */
-        if (path_data[i].check_absolute && path_data[i].path[0] != '/') {
-            ATTICerr(0, ATTIC_R_PATH_MUST_BE_ABSOLUTE);
-            ERR_add_error_data(1, path_data[i].path);
-            return NULL;
-        }
-
-        if (stat(path_data[i].path, &st) < 0) {
+        if (stat(path_data[i], &st) < 0) {
             ERR_raise_data(ERR_LIB_SYS, errno,
                            "calling stat(%s)",
-                           path_data[i].path);
+                           path_data[i]);
         } else {
-            path = path_data[i].path;
+            path = path_data[i];
         }
     }
     if (path == NULL) {

diff --git a/providers/implementations/storemgmt/file_store.c b/providers/implementations/storemgmt/file_store.c
index 134b8efefbb2a6218132abbe5ca6ed2f70c5bf0f..4860d40788ed0324663931edb5f1b1630147b696 100644 (file)
--- a/providers/implementations/storemgmt/file_store.c
+++ b/providers/implementations/storemgmt/file_store.c
@@ -195,10 +195,7 @@ static void *file_open(void *provctx, const char *uri)
 {
     struct file_ctx_st *ctx = NULL;
     struct stat st;
-    struct {
-        const char *path;
-        unsigned int check_absolute:1;
-    } path_data[2];
+    const char *path_data[2];
     size_t path_data_n = 0, i;
     const char *path, *p = uri, *q;
     BIO *bio;
@@ -208,8 +205,7 @@ static void *file_open(void *provctx, const char *uri)
     /*
      * First step, just take the URI as is.
      */
-    path_data[path_data_n].check_absolute = 0;
-    path_data[path_data_n++].path = uri;
+    path_data[path_data_n++] = uri;
 
     /*
      * Second step, if the URI appears to start with the "file" scheme,
@@ -222,7 +218,11 @@ static void *file_open(void *provctx, const char *uri)
         if (CHECK_AND_SKIP_CASE_PREFIX(q, "//")) {
             path_data_n--;           /* Invalidate using the full URI */
             if (CHECK_AND_SKIP_CASE_PREFIX(q, "localhost/")
-                    || CHECK_AND_SKIP_CASE_PREFIX(q, "/")) {
+                || CHECK_AND_SKIP_CASE_PREFIX(q, "/")) {
+                /*
+                 * In this case, we step back on char to ensure that the
+                 * first slash is preserved, making the path always absolute
+                 */
                 p = q - 1;
             } else {
                 ERR_clear_last_mark();
@@ -230,42 +230,28 @@ static void *file_open(void *provctx, const char *uri)
                 return NULL;
             }
         }
-
-        path_data[path_data_n].check_absolute = 1;
 #ifdef _WIN32
         /* Windows "file:" URIs with a drive letter start with a '/' */
         if (p[0] == '/' && p[2] == ':' && p[3] == '/') {
             char c = tolower((unsigned char)p[1]);
 
             if (c >= 'a' && c <= 'z') {
+                /* Skip past the slash, making the path a normal Windows path */
                 p++;
-                /* We know it's absolute, so no need to check */
-                path_data[path_data_n].check_absolute = 0;
             }
         }
 #endif
-        path_data[path_data_n++].path = p;
+        path_data[path_data_n++] = p;
     }
 
 
     for (i = 0, path = NULL; path == NULL && i < path_data_n; i++) {
-        /*
-         * If the scheme "file" was an explicit part of the URI, the path must
-         * be absolute.  So says RFC 8089
-         */
-        if (path_data[i].check_absolute && path_data[i].path[0] != '/') {
-            ERR_clear_last_mark();
-            ERR_raise_data(ERR_LIB_PROV, PROV_R_PATH_MUST_BE_ABSOLUTE,
-                           "Given path=%s", path_data[i].path);
-            return NULL;
-        }
-
-        if (stat(path_data[i].path, &st) < 0) {
+        if (stat(path_data[i], &st) < 0) {
             ERR_raise_data(ERR_LIB_SYS, errno,
                            "calling stat(%s)",
-                           path_data[i].path);
+                           path_data[i]);
         } else {
-            path = path_data[i].path;
+            path = path_data[i];
         }
     }
     if (path == NULL) {

diff --git a/test/recipes/90-test_store.t b/test/recipes/90-test_store.t
index f0f9e4d94b1d5e6363cf7b1cd27eac4bb9e7ec81..92fddefca27613e8103e0fe07e811e0f0f18d11f 100644 (file)
--- a/test/recipes/90-test_store.t
+++ b/test/recipes/90-test_store.t
@@ -233,8 +233,9 @@ indir "store_$$" => sub {
 
                     ok(run(app([@storeutl, "-noout", "-passin",
                                 "pass:password", to_abs_file_uri($_)])));
-                    ok(!run(app([@storeutl, "-noout", "-passin",
-                                 "pass:password", to_file_uri($_)])));
+                    # Check relaxed 'file' scheme implementation
+                    ok(run(app([@storeutl, "-noout", "-passin",
+                                "pass:password", to_file_uri($_)])));
                 }
             }
             foreach (values %generated_file_files) {