Commit 6683c886f2 for openssl.org
commit 6683c886f27d1f21a3a893af994160b1b26fe2c1
Author: Richard Levitte <levitte@openssl.org>
Date: Wed Apr 23 20:14:38 2025 +0200
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 352c0345d3..5ce0e7f2ec 100644
--- 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 21e0ed25f8..cef313a137 100644
--- 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 134b8efefb..4860d40788 100644
--- 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 f0f9e4d94b..92fddefca2 100644
--- 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) {