From: Richard Levitte Date: Wed, 23 Apr 2025 18:14:38 +0000 (+0200) Subject: Relax absolut path checking in our 'file' scheme implementation X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6683c886f27d1f21a3a893af994160b1b26fe2c1;p=thirdparty%2Fopenssl.git 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 Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/27482) --- diff --git a/CHANGES.md b/CHANGES.md index 352c0345d36..5ce0e7f2ec7 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 21e0ed25f8b..cef313a137b 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 134b8efefbb..4860d40788e 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 f0f9e4d94b1..92fddefca27 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) {