[openssl-commits] [openssl] master update

Richard Levitte levitte at openssl.org
Sat Jul 15 16:57:01 UTC 2017


The branch master has been updated
       via  1145995323a2a6b6e31602dbf2c546943a7db06f (commit)
       via  6eaebfaab503293f03c9d8cdd48e831d1f12f89b (commit)
       via  4c0669dc6f5863b4d79c45abe37f566ffc61af01 (commit)
       via  ae9c39d83aa0f993e382cf913cb4fd84fc25d03d (commit)
       via  ba476aa32c2de4c652f2fdb148981f7d1ea6cae1 (commit)
       via  346bf1a2382dd505e5fa4fd2ecff50e939a35f01 (commit)
       via  94437cebc4e6c638dbf1c089877d053fecbe3361 (commit)
      from  da8fc25a989cf4f4d26d626a85477e8a9282da12 (commit)


- Log -----------------------------------------------------------------
commit 1145995323a2a6b6e31602dbf2c546943a7db06f
Author: Richard Levitte <levitte at openssl.org>
Date:   Sat Jul 15 11:21:11 2017 +0200

    OSSL_STORE "file" scheme loader: check that a DOS device is correctly named
    
    Reviewed-by: Andy Polyakov <appro at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/3907)

commit 6eaebfaab503293f03c9d8cdd48e831d1f12f89b
Author: Richard Levitte <levitte at openssl.org>
Date:   Wed Jul 12 12:44:24 2017 +0200

    OSSL_STORE "file" scheme loader: check for absolute path in URI later
    
    If we have a local file with a name starting with 'file:', we don't
    want to check if the part after 'file:' is absolute.  Instead, mark
    each possibility for absolute check if needed, and perform the
    absolute check later on, when checking each actual path.
    
    Reviewed-by: Andy Polyakov <appro at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/3907)

commit 4c0669dc6f5863b4d79c45abe37f566ffc61af01
Author: Richard Levitte <levitte at openssl.org>
Date:   Wed Jul 12 12:42:16 2017 +0200

    test/recipes/90-test_store.t: Add a few cases with files starting with 'file:'
    
    These cases are performed on Linux only.  They check that files with
    names starting with 'file:' can be processed as well.
    
    Reviewed-by: Andy Polyakov <appro at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/3907)

commit ae9c39d83aa0f993e382cf913cb4fd84fc25d03d
Author: Richard Levitte <levitte at openssl.org>
Date:   Tue Jul 11 11:54:00 2017 +0200

    OSSL_STORE: Treat URIs as files first (with exceptions), then as full URIs
    
    To handle paths that contain devices (for example, C:/foo/bar.pem on
    Windows), try to "open" the URI using the file scheme loader first,
    and failing that, check if the device is really a scheme we know.
    
    The "file" scheme does the same kind of thing to pick out the path
    part of the URI.
    
    An exception to this special treatment is if the URI has an authority
    part (something that starts with "//" directly after what looks like a
    scheme).  Such URIs will never be treated as plain file paths.
    
    Reviewed-by: Andy Polyakov <appro at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/3907)

commit ba476aa32c2de4c652f2fdb148981f7d1ea6cae1
Author: Richard Levitte <levitte at openssl.org>
Date:   Tue Jul 11 11:46:14 2017 +0200

    OSSL_STORE: spell error reason correctly
    
    Reviewed-by: Andy Polyakov <appro at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/3907)

commit 346bf1a2382dd505e5fa4fd2ecff50e939a35f01
Author: Richard Levitte <levitte at openssl.org>
Date:   Tue Jul 11 09:51:04 2017 +0200

    test/recipes/90-test_store.t: Test absolute files
    
    We haven't tested plain absolute paths without making them URIs...
    
    Reviewed-by: Andy Polyakov <appro at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/3907)

commit 94437cebc4e6c638dbf1c089877d053fecbe3361
Author: Richard Levitte <levitte at openssl.org>
Date:   Tue Jul 11 09:48:08 2017 +0200

    test/recipes/90-test_store.t: Rename some functions
    
    to_rel_file_uri really treated all files appropriately, absolute and
    relative alike, and really just constructs a URI, so gets renamed to
    to_file_uri
    
    to_file_uri, on the other hand, forces the path into an absolute one,
    so gets renamed to to_abs_file_uri
    
    Reviewed-by: Andy Polyakov <appro at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/3907)

-----------------------------------------------------------------------

Summary of changes:
 crypto/err/openssl.txt       |  2 +-
 crypto/store/loader_file.c   | 85 +++++++++++++++++++++++++++++++-------------
 crypto/store/store_err.c     |  4 +--
 crypto/store/store_lib.c     | 43 +++++++++++++++++-----
 include/openssl/storeerr.h   |  2 +-
 test/recipes/90-test_store.t | 82 +++++++++++++++++++++++++++++++-----------
 6 files changed, 161 insertions(+), 57 deletions(-)

diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt
index 04f48a5..f842870 100644
--- a/crypto/err/openssl.txt
+++ b/crypto/err/openssl.txt
@@ -2004,7 +2004,7 @@ OSSL_STORE_R_UI_PROCESS_INTERRUPTED_OR_CANCELLED:109:\
 	ui process interrupted or cancelled
 OSSL_STORE_R_UNREGISTERED_SCHEME:105:unregistered scheme
 OSSL_STORE_R_UNSUPPORTED_CONTENT_TYPE:110:unsupported content type
-OSSL_STORE_R_URI_AUTHORITY_UNSUPPORED:111:uri authority unsuppored
+OSSL_STORE_R_URI_AUTHORITY_UNSUPPORTED:111:uri authority unsupported
 PEM_R_BAD_BASE64_DECODE:100:bad base64 decode
 PEM_R_BAD_DECRYPT:101:bad decrypt
 PEM_R_BAD_END_LINE:102:bad end line
diff --git a/crypto/store/loader_file.c b/crypto/store/loader_file.c
index 06094bf..7cb1457 100644
--- a/crypto/store/loader_file.c
+++ b/crypto/store/loader_file.c
@@ -744,47 +744,84 @@ static OSSL_STORE_LOADER_CTX *file_open(const OSSL_STORE_LOADER *loader,
 {
     OSSL_STORE_LOADER_CTX *ctx = NULL;
     struct stat st;
-    const char *path = NULL;
+    struct {
+        const char *path;
+        unsigned int check_absolute:1;
+    } path_data[2];
+    size_t path_data_n = 0, i;
+    const char *path;
 
+    /*
+     * First step, just take the URI as is.
+     */
+    path_data[path_data_n].check_absolute = 0;
+    path_data[path_data_n++].path = uri;
+
+    /*
+     * Second step, if the URI appears to start with the 'file' scheme,
+     * extract the path and make that the second path to check.
+     * There's a special case if the URI also contains an authority, then
+     * the full URI shouldn't be used as a path anywhere.
+     */
     if (strncasecmp(uri, "file:", 5) == 0) {
-        if (strncasecmp(&uri[5], "//localhost/", 12) == 0) {
-            path = &uri[16];
-        } else if (strncmp(&uri[5], "///", 3) == 0) {
-            path = &uri[7];
-        } else if (strncmp(&uri[5], "//", 2) != 0) {
-            path = &uri[5];
-        } else {
-            OSSL_STOREerr(OSSL_STORE_F_FILE_OPEN,
-                          OSSL_STORE_R_URI_AUTHORITY_UNSUPPORED);
-            return NULL;
+        const char *p = &uri[5];
+
+        if (strncmp(&uri[5], "//", 2) == 0) {
+            path_data_n--;           /* Invalidate using the full URI */
+            if (strncasecmp(&uri[7], "localhost/", 10) == 0) {
+                p = &uri[16];
+            } else if (uri[7] == '/') {
+                p = &uri[7];
+            } else {
+                OSSL_STOREerr(OSSL_STORE_F_FILE_OPEN,
+                              OSSL_STORE_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(p[1]);
+
+            if (c >= 'a' && c <= 'z') {
+                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;
+    }
 
+
+    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[0] != '/') {
+        if (path_data[i].check_absolute && path_data[i].path[0] != '/') {
             OSSL_STOREerr(OSSL_STORE_F_FILE_OPEN,
                           OSSL_STORE_R_PATH_MUST_BE_ABSOLUTE);
+            ERR_add_error_data(1, path_data[i].path);
             return NULL;
         }
 
-#ifdef _WIN32
-        /* Windows file: URIs with a drive letter start with a / */
-        if (path[0] == '/' && path[2] == ':' && path[3] == '/')
-            path++;
-#endif
-    } else {
-        path = uri;
+        if (stat(path_data[i].path, &st) < 0) {
+            SYSerr(SYS_F_STAT, errno);
+            ERR_add_error_data(1, path_data[i].path);
+        } else {
+            path = path_data[i].path;
+        }
     }
-
-
-    if (stat(path, &st) < 0) {
-        SYSerr(SYS_F_STAT, errno);
-        ERR_add_error_data(1, path);
+    if (path == NULL) {
         return NULL;
     }
 
+    /* Successfully found a working path, clear possible collected errors */
+    ERR_clear_error();
+
     ctx = OPENSSL_zalloc(sizeof(*ctx));
     if (ctx == NULL) {
         OSSL_STOREerr(OSSL_STORE_F_FILE_OPEN, ERR_R_MALLOC_FAILURE);
diff --git a/crypto/store/store_err.c b/crypto/store/store_err.c
index 86a15c9..c78b3899 100644
--- a/crypto/store/store_err.c
+++ b/crypto/store/store_err.c
@@ -104,8 +104,8 @@ static const ERR_STRING_DATA OSSL_STORE_str_reasons[] = {
     "unregistered scheme"},
     {ERR_PACK(ERR_LIB_OSSL_STORE, 0, OSSL_STORE_R_UNSUPPORTED_CONTENT_TYPE),
     "unsupported content type"},
-    {ERR_PACK(ERR_LIB_OSSL_STORE, 0, OSSL_STORE_R_URI_AUTHORITY_UNSUPPORED),
-    "uri authority unsuppored"},
+    {ERR_PACK(ERR_LIB_OSSL_STORE, 0, OSSL_STORE_R_URI_AUTHORITY_UNSUPPORTED),
+    "uri authority unsupported"},
     {0, NULL}
 };
 
diff --git a/crypto/store/store_lib.c b/crypto/store/store_lib.c
index 91faae2..9dc3a70 100644
--- a/crypto/store/store_lib.c
+++ b/crypto/store/store_lib.c
@@ -10,6 +10,8 @@
 #include <stdlib.h>
 #include <string.h>
 
+#include "e_os.h"
+
 #include <openssl/crypto.h>
 #include <openssl/err.h>
 #include <openssl/store.h>
@@ -31,22 +33,45 @@ OSSL_STORE_CTX *OSSL_STORE_open(const char *uri, const UI_METHOD *ui_method,
                                 OSSL_STORE_post_process_info_fn post_process,
                                 void *post_process_data)
 {
-    const OSSL_STORE_LOADER *loader;
+    const OSSL_STORE_LOADER *loader = NULL;
     OSSL_STORE_LOADER_CTX *loader_ctx = NULL;
     OSSL_STORE_CTX *ctx = NULL;
-    char scheme_copy[256], *p;
-
+    char scheme_copy[256], *p, *schemes[2];
+    size_t schemes_n = 0;
+    size_t i;
+
+    /*
+     * Put the file scheme first.  If the uri does represent an existing file,
+     * possible device name and all, then it should be loaded.  Only a failed
+     * attempt at loading a local file should have us try something else.
+     */
+    schemes[schemes_n++] = "file";
+
+    /*
+     * Now, check if we have something that looks like a scheme, and add it
+     * as a second scheme.  However, also check if there's an authority start
+     * (://), because that will invalidate the previous file scheme.  Also,
+     * check that this isn't actually the file scheme, as there's no point
+     * going through that one twice!
+     */
     OPENSSL_strlcpy(scheme_copy, uri, sizeof(scheme_copy));
     if ((p = strchr(scheme_copy, ':')) != NULL) {
-        *p = '\0';
-        p = scheme_copy;
-    } else {
-        p = "file";
+        *p++ = '\0';
+        if (strcasecmp(scheme_copy, "file") != 0) {
+            if (strncmp(p, "//", 2) == 0)
+                schemes_n--;         /* Invalidate the file scheme */
+            schemes[schemes_n++] = scheme_copy;
+        }
     }
 
-    if ((loader = ossl_store_get0_loader_int(p)) == NULL
-        || (loader_ctx = loader->open(loader, uri, ui_method, ui_data)) == 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;
+
     if ((ctx = OPENSSL_zalloc(sizeof(*ctx))) == NULL) {
         OSSL_STOREerr(OSSL_STORE_F_OSSL_STORE_OPEN, ERR_R_MALLOC_FAILURE);
         goto done;
diff --git a/include/openssl/storeerr.h b/include/openssl/storeerr.h
index b1d23de..cffe8de 100644
--- a/include/openssl/storeerr.h
+++ b/include/openssl/storeerr.h
@@ -73,6 +73,6 @@ int ERR_load_OSSL_STORE_strings(void);
 # define OSSL_STORE_R_UI_PROCESS_INTERRUPTED_OR_CANCELLED 109
 # define OSSL_STORE_R_UNREGISTERED_SCHEME                 105
 # define OSSL_STORE_R_UNSUPPORTED_CONTENT_TYPE            110
-# define OSSL_STORE_R_URI_AUTHORITY_UNSUPPORED            111
+# define OSSL_STORE_R_URI_AUTHORITY_UNSUPPORTED           111
 
 #endif
diff --git a/test/recipes/90-test_store.t b/test/recipes/90-test_store.t
index 65cf9fb..699a67e 100644
--- a/test/recipes/90-test_store.t
+++ b/test/recipes/90-test_store.t
@@ -7,6 +7,7 @@
 # https://www.openssl.org/source/license.html
 
 use File::Spec;
+use File::Copy;
 use MIME::Base64;
 use OpenSSL::Test qw(:DEFAULT srctop_file srctop_dir bldtop_file data_file);
 
@@ -55,11 +56,25 @@ my @generated_files =
      "ec-key-pkcs8-pbes2-sha1.pem", "ec-key-pkcs8-pbes2-sha1.der",
      "ec-key-aes256-cbc-sha256.p12",
     );
+my %generated_file_files =
+    $^O eq 'linux'
+    ? ( "test/testx509.pem" => "file:testx509.pem",
+        "test/testrsa.pem" => "file:testrsa.pem",
+        "test/testrsapub.pem" => "file:testrsapub.pem",
+        "test/testcrl.pem" => "file:testcrl.pem",
+        "apps/server.pem" => "file:server.pem" )
+    : ();
+my @noexist_file_files =
+    ( "file:blahdiblah.pem",
+      "file:test/blahdibleh.der" );
 
-my $n = (2 * scalar @noexist_files)
-    + (5 * scalar @src_files)
-    + (3 * scalar @generated_files)
-    + 2;
+
+my $n = (3 * scalar @noexist_files)
+    + (6 * scalar @src_files)
+    + (4 * scalar @generated_files)
+    + (scalar keys %generated_file_files)
+    + (scalar @noexist_file_files)
+    + 3;
 
 plan tests => $n;
 
@@ -71,31 +86,42 @@ indir "store_$$" => sub {
         foreach (@noexist_files) {
             my $file = srctop_file($_);
             ok(!run(app(["openssl", "storeutl", $file])));
-            ok(!run(app(["openssl", "storeutl", to_file_uri($file)])));
+            ok(!run(app(["openssl", "storeutl", to_abs_file($file)])));
+            ok(!run(app(["openssl", "storeutl", to_abs_file_uri($file)])));
         }
         foreach (@src_files) {
             my $file = srctop_file($_);
             ok(run(app(["openssl", "storeutl", $file])));
-            ok(run(app(["openssl", "storeutl", to_file_uri($file)])));
-            ok(run(app(["openssl", "storeutl", to_file_uri($file, 0,
-                                                           "")])));
-            ok(run(app(["openssl", "storeutl", to_file_uri($file, 0,
-                                                           "localhost")])));
-            ok(!run(app(["openssl", "storeutl", to_file_uri($file, 0,
-                                                            "dummy")])));
+            ok(run(app(["openssl", "storeutl", to_abs_file($file)])));
+            ok(run(app(["openssl", "storeutl", to_abs_file_uri($file)])));
+            ok(run(app(["openssl", "storeutl", to_abs_file_uri($file, 0,
+                                                               "")])));
+            ok(run(app(["openssl", "storeutl", to_abs_file_uri($file, 0,
+                                                               "localhost")])));
+            ok(!run(app(["openssl", "storeutl", to_abs_file_uri($file, 0,
+                                                                "dummy")])));
         }
         foreach (@generated_files) {
             ok(run(app(["openssl", "storeutl", "-passin", "pass:password",
                         $_])));
             ok(run(app(["openssl", "storeutl", "-passin", "pass:password",
-                        to_file_uri($_)])));
+                        to_abs_file($_)])));
+            ok(run(app(["openssl", "storeutl", "-passin", "pass:password",
+                        to_abs_file_uri($_)])));
             ok(!run(app(["openssl", "storeutl", "-passin", "pass:password",
-                         to_rel_file_uri($_)])));
+                         to_file_uri($_)])));
+        }
+        foreach (values %generated_file_files) {
+            ok(run(app(["openssl", "storeutl", $_])));
+        }
+        foreach (@noexist_file_files) {
+            ok(!run(app(["openssl", "storeutl", $_])));
         }
         {
             my $dir = srctop_dir("test", "certs");
             ok(run(app(["openssl", "storeutl", $dir])));
-            ok(run(app(["openssl", "storeutl", to_file_uri($dir, 1)])));
+            ok(run(app(["openssl", "storeutl", to_abs_file($dir, 1)])));
+            ok(run(app(["openssl", "storeutl", to_abs_file_uri($dir, 1)])));
         }
     }
 }, create => 1, cleanup => 1;
@@ -282,6 +308,16 @@ sub init {
                           close $outfh;
                           return 1;
                       }, grep(/\.der$/, @generated_files))
+            && runall(sub {
+                          my $srcfile = shift;
+                          my $dstfile = $generated_file_files{$srcfile};
+
+                          unless (copy srctop_file($srcfile), $dstfile) {
+                              warn "$!\n";
+                              return 0;
+                          }
+                          return 1;
+                      }, keys %generated_file_files)
            );
 }
 
@@ -296,12 +332,12 @@ sub runall {
 
 # According to RFC8089, a relative file: path is invalid.  We still produce
 # them for testing purposes.
-sub to_rel_file_uri {
+sub to_file_uri {
     my ($file, $isdir, $authority) = @_;
     my $vol;
     my $dir;
 
-    die "to_rel_file_uri: No file given\n" if !defined($file) || $file eq '';
+    die "to_file_uri: No file given\n" if !defined($file) || $file eq '';
 
     ($vol, $dir, $file) = File::Spec->splitpath($file, $isdir // 0);
 
@@ -341,9 +377,15 @@ sub to_rel_file_uri {
     return "file:$file";
 }
 
-sub to_file_uri {
+sub to_abs_file {
+    my ($file) = @_;
+
+    return File::Spec->rel2abs($file);
+}
+
+sub to_abs_file_uri {
     my ($file, $isdir, $authority) = @_;
 
-    die "to_file_uri: No file given\n" if !defined($file) || $file eq '';
-    return to_rel_file_uri(File::Spec->rel2abs($file), $isdir, $authority);
+    die "to_abs_file_uri: No file given\n" if !defined($file) || $file eq '';
+    return to_file_uri(to_abs_file($file), $isdir, $authority);
 }


More information about the openssl-commits mailing list