Allow OPENSSLDIR/ENGINESDIR/MODULESDIR to be NULL

To prevent inadvertent use of insecure directories, we need to be able
to detect and react when our new registry keys aren't set, which implies
allowing the values for the dynamic representations of
OPENSSLDIR/ENGINESDIR/MODULESDIR to return NULL.  This in turn requires
that we detect and handle NULL string in several call sites that
previously assumed they would never be NULL.  This commit fixes those up

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

diff --git a/NOTES-WINDOWS.md b/NOTES-WINDOWS.md
index d39bb90918e863e3e3de318124f1cf3a1ef3cead..cd8fdfb3f0d1dbe03546066eaf2019f327efe50d 100644 (file)
--- a/NOTES-WINDOWS.md
+++ b/NOTES-WINDOWS.md
@@ -99,36 +99,12 @@ check the INSTALL.md file.
 Installation directories
 ------------------------
 
-The default installation directories are derived from environment
-variables.
+On most Unix platform installation directories are determined at build time via
+constant defines.  On Windows platforms however, installation directories are
+determined via registry keys, as it is common practice to build OpenSSL and
+install it to a variety of locations.
 
-For VC-WIN32, the following defaults are use:
-
-    PREFIX:      %ProgramFiles(x86)%\OpenSSL
-    OPENSSLDIR:  %CommonProgramFiles(x86)%\SSL
-
-For VC-WIN64, the following defaults are use:
-
-    PREFIX:      %ProgramW6432%\OpenSSL
-    OPENSSLDIR:  %CommonProgramW6432%\SSL
-
-Should those environment variables not exist (on a pure Win32
-installation for examples), these fallbacks are used:
-
-    PREFIX:      %ProgramFiles%\OpenSSL
-    OPENSSLDIR:  %CommonProgramFiles%\SSL
-
-ALSO NOTE that those directories are usually write protected, even if
-your account is in the Administrators group.  To work around that,
-start the command prompt by right-clicking on it and choosing "Run as
-Administrator" before running `nmake install`.  The other solution
-is, of course, to choose a different set of directories by using
-`--prefix` and `--openssldir` when configuring.
-
-Note that, on Windows platforms (both 32 and 64 bit), the above build-time
-defaults can be overridden by registry keys.  This is done because it is common
-practice for windows-based installers to allow users to place the installation
-tree at an arbitrary location not defined at build-time.  The following keys:
+The following keys:
 
     `\\HKEY_LOCAL_MACHINE\SOFTWARE\Wow6432Node\OpenSSL-<version>-<ctx>\OPENSSLDIR`
     `\\HKEY_LOCAL_MACHINE\SOFTWARE\Wow6432Node\OpenSSL-<version>-<ctx>\ENGINESDIR`
@@ -142,6 +118,7 @@ To enable the reading of registry keys from windows builds, add
 at build-time to construct library build specific registry key paths of the
 format:
 `\\HKEY_LOCAL_MACHINE\SOFTWARE\Wow6432node\OpenSSL-<version>-<ctx>`
+
 Where `<version>` is the semantic major.minor.patch version of the library being
 built, and `<ctx>` is the value specified by `-DOPENSSL_WINCTX`.  This allows
 for multiple openssl builds to be created and installed on a single system, in
@@ -150,6 +127,10 @@ which each library can use its own set of registry keys.
 Note the installer available at <https://github.com/openssl/installer> will set
 these keys when the installer is run.
 
+If the registry keys above do not exist on a given system, or if the
+`OSSL_WINCTX` variable is not defined at build time, OpenSSL makes no attempt to
+load configuration, engines of modules from disk.
+
 Special notes for Universal Windows Platform builds, aka `VC-*-UWP`
 -------------------------------------------------------------------
 

diff --git a/apps/info.c b/apps/info.c
index befc62dac10470e5b53455b494ead25b40c48e4e..c024ba446eb2bba11c18f95e90fc1cdc140d457a 100644 (file)
--- a/apps/info.c
+++ b/apps/info.c
@@ -40,6 +40,7 @@ int info_main(int argc, char **argv)
     int ret = 1, dirty = 0, type = 0;
     char *prog;
     OPTION_CHOICE o;
+    const char *typedata;
 
     prog = opt_init(argc, argv, info_options);
     while ((o = opt_next()) != OPT_EOF) {
@@ -97,7 +98,8 @@ opthelp:
         goto opthelp;
     }
 
-    BIO_printf(bio_out, "%s\n", OPENSSL_info(type));
+    typedata = OPENSSL_info(type);
+    BIO_printf(bio_out, "%s\n", typedata == NULL ? "Undefined" : typedata);
     ret = 0;
  end:
     return ret;

diff --git a/apps/version.c b/apps/version.c
index 278afe8bf9344542d34e18cf5b873d8009bf976a..bfcb1f15f97c641e2024243c12e35e949d9d3176 100644 (file)
--- a/apps/version.c
+++ b/apps/version.c
@@ -48,6 +48,7 @@ int version_main(int argc, char **argv)
     int engdir = 0, moddir = 0, cpuinfo = 0, windows = 0;
     char *prog;
     OPTION_CHOICE o;
+    const char *tmp;
 
     prog = opt_init(argc, argv, version_options);
     while ((o = opt_next()) != OPT_EOF) {
@@ -123,12 +124,18 @@ opthelp:
     }
     if (cflags)
         printf("%s\n", OpenSSL_version(OPENSSL_CFLAGS));
-    if (dir)
-        printf("OPENSSLDIR: %s\n", OpenSSL_version(OPENSSL_DIR));
-    if (engdir)
-        printf("ENGINESDIR: %s\n", OpenSSL_version(OPENSSL_ENGINES_DIR));
-    if (moddir)
-        printf("MODULESDIR: %s\n", OpenSSL_version(OPENSSL_MODULES_DIR));
+    if (dir) {
+        tmp = OpenSSL_version(OPENSSL_DIR);
+        printf("OPENSSLDIR: %s\n", tmp == NULL ? "Undefined" : tmp);
+    }
+    if (engdir) {
+        tmp = OpenSSL_version(OPENSSL_ENGINES_DIR);
+        printf("ENGINESDIR: %s\n", tmp == NULL ? "Undefined" : tmp);
+    }
+    if (moddir) {
+        tmp = OpenSSL_version(OPENSSL_MODULES_DIR);
+        printf("MODULESDIR: %s\n", tmp == NULL ? "Undefined" : tmp);
+    }
     if (seed) {
         const char *src = OPENSSL_info(OPENSSL_INFO_SEED_SOURCE);
         printf("Seeding source: %s\n", src ? src : "N/A");
@@ -136,7 +143,7 @@ opthelp:
     if (cpuinfo)
         printf("%s\n", OpenSSL_version(OPENSSL_CPU_INFO));
     if (windows)
-       printf("OSSL_WINCTX: %s\n", OpenSSL_version(OPENSSL_WINCTX));
+        printf("OSSL_WINCTX: %s\n", OpenSSL_version(OPENSSL_WINCTX));
     ret = 0;
  end:
     return ret;

diff --git a/crypto/conf/conf_mod.c b/crypto/conf/conf_mod.c
index 1ac3d9f7b7573f887d4639718ea74841db195a27..5e0681e6db3157a7e6ae8b6119b71ca544a76b21 100644 (file)
--- a/crypto/conf/conf_mod.c
+++ b/crypto/conf/conf_mod.c
@@ -692,6 +692,8 @@ char *CONF_get1_default_config_file(void)
         return OPENSSL_strdup(file);
 
     t = X509_get_default_cert_area();
+    if (t == NULL)
+        return NULL;
 #ifndef OPENSSL_SYS_VMS
     sep = "/";
 #endif

diff --git a/crypto/cversion.c b/crypto/cversion.c
index 219479f19a3f5ac6e0cc70090ef6dbd5f069f49a..9c53041a69757ae34092354597c12cdeb64c8d41 100644 (file)
--- a/crypto/cversion.c
+++ b/crypto/cversion.c
@@ -71,7 +71,7 @@ const char *OpenSSL_version(int t)
         else
             return "CPUINFO: N/A";
     case OPENSSL_WINCTX:
-       return ossl_get_wininstallcontext();
+        return ossl_get_wininstallcontext();
     }
     return "not available";
 }

diff --git a/crypto/defaults.c b/crypto/defaults.c
index 8f60d182e36563cba42ffd64f5d6633a555399dd..2735efc63a94d1e53f8483ca66ecd976cf3d5584 100644 (file)
--- a/crypto/defaults.c
+++ b/crypto/defaults.c
@@ -20,8 +20,6 @@
 # define NOQUOTE(x) x
 #if defined(OSSL_WINCTX)
 # define REGISTRY_KEY "SOFTWARE\\WOW6432Node\\OpenSSL" ##"-"## NOQUOTE(OPENSSL_VERSION_STR) ##"-"## MAKESTR(OSSL_WINCTX)
-#else
-# define REGISTRY_KEY "NONE"
 #endif
 
 /**
@@ -29,17 +27,32 @@
  */
 static char openssldir[MAX_PATH + 1];
 
+/**
+ * @brief The pointer to the opennsldir buffer
+ */
+static char *openssldirptr = NULL;
+
 /**
  * @brief The directory where OpenSSL engines are located.
  */
 
 static char enginesdir[MAX_PATH + 1];
 
+/**
+ * @brief The pointer to the enginesdir buffer
+ */
+static char *enginesdirptr = NULL;
+
 /**
  * @brief The directory where OpenSSL modules are located.
  */
 static char modulesdir[MAX_PATH + 1];
 
+/**
+ * @brief The pointer to the modulesdir buffer
+ */
+static char *modulesdirptr = NULL;
+
 /**
  * @brief Get the list of Windows registry directories.
  *
@@ -49,14 +62,15 @@ static char modulesdir[MAX_PATH + 1];
  */
 static char *get_windows_regdirs(char *dst, LPCTSTR valuename)
 {
+    char *retval = NULL;
+#ifdef REGISTY_KEY
     DWORD keysize;
     DWORD ktype;
     HKEY hkey;
     LSTATUS ret;
     DWORD index = 0;
     LPCTCH tempstr = NULL;
-    char *retval = NULL;
-
+   
     ret = RegOpenKeyEx(HKEY_LOCAL_MACHINE,
                        TEXT(REGISTRY_KEY), KEY_WOW64_32KEY,
                        KEY_QUERY_VALUE, &hkey);
@@ -90,6 +104,7 @@ static char *get_windows_regdirs(char *dst, LPCTSTR valuename)
 out:
     OPENSSL_free(tempstr);
     RegCloseKey(hkey);
+#endif
     return retval;
 }
 
@@ -105,6 +120,19 @@ DEFINE_RUN_ONCE_STATIC(do_defaults_setup)
     get_windows_regdirs(openssldir, TEXT("OPENSSLDIR"));
     get_windows_regdirs(enginesdir, TEXT("ENGINESDIR"));
     get_windows_regdirs(modulesdir, TEXT("MODULESDIR"));
+
+    /*
+     * Set our pointers only if the directories are fetched properly
+     */
+    if (strlen(openssldir) > 0)
+        openssldirptr = openssldir;
+
+    if (strlen(enginesdir) > 0)
+        enginesdirptr = enginesdir;
+
+    if (strlen(modulesdir) > 0)
+        modulesdirptr = modulesdir;
+
     return 1;
 }
 #endif
@@ -116,12 +144,10 @@ DEFINE_RUN_ONCE_STATIC(do_defaults_setup)
  */
 const char *ossl_get_openssldir(void)
 {
-#if defined(_WIN32)
-# if defined(OSSL_WINCTX)
+#if defined(_WIN32) && defined (OSSL_WINCTX)
     if (!RUN_ONCE(&defaults_setup_init, do_defaults_setup))
         return NULL;
-    return (const char *)openssldir;
-# endif
+    return (const char *)openssldirptr;
 # else
     return OPENSSLDIR;
 #endif
@@ -134,14 +160,10 @@ const char *ossl_get_openssldir(void)
  */
 const char *ossl_get_enginesdir(void)
 {
-#if defined(_WIN32)
-# if defined(OSSL_WINCTX)
+#if defined(_WIN32) && defined (OSSL_WINCTX)
     if (!RUN_ONCE(&defaults_setup_init, do_defaults_setup))
         return NULL;
-    return (const char *)enginesdir;
-# else
-    return "UNDEFINED";
-# endif
+    return (const char *)enginesdirptr;
 #else
     return ENGINESDIR;
 #endif
@@ -154,14 +176,10 @@ const char *ossl_get_enginesdir(void)
  */
 const char *ossl_get_modulesdir(void)
 {
-#if defined(_WIN32)
-# if defined (OSSL_WINCTX)
+#if defined(_WIN32) && defined(OSSL_WINCTX)
     if (!RUN_ONCE(&defaults_setup_init, do_defaults_setup))
         return NULL;
-    return (const char *)modulesdir;
-# else
-    return "UNDEFINED";
-# endif
+    return (const char *)modulesdirptr;
 #else
     return MODULESDIR;
 #endif
@@ -175,8 +193,8 @@ const char *ossl_get_modulesdir(void)
 const char *ossl_get_wininstallcontext(void)
 {
 #if defined(_WIN32) && defined (OSSL_WINCTX)
-       return MAKESTR(OSSL_WINCTX);
+    return MAKESTR(OSSL_WINCTX);
 #else
-       return "UNDEFINED";
+    return "Undefined";
 #endif
 }

diff --git a/crypto/o_fopen.c b/crypto/o_fopen.c
index 09c28e0bf7574a8c821803a7f83be098a645ba9e..ed73e95578e0571a397bba890bacb6a888589783 100644 (file)
--- a/crypto/o_fopen.c
+++ b/crypto/o_fopen.c
@@ -38,6 +38,9 @@
 FILE *openssl_fopen(const char *filename, const char *mode)
 {
     FILE *file = NULL;
+
+    if (filename == NULL)
+        return NULL;
 # if defined(_WIN32) && defined(CP_UTF8)
     int sz, len_0 = (int)strlen(filename) + 1;
     DWORD flags;

diff --git a/crypto/x509/x509_def.c b/crypto/x509/x509_def.c
index 874c61d7f13b92255aea20f9350704b2269bb658..25fb014fcf79c68207a145cde4767916eb7027ba 100644 (file)
--- a/crypto/x509/x509_def.c
+++ b/crypto/x509/x509_def.c
@@ -17,9 +17,16 @@
 #if defined(_WIN32)
 
 static char x509_private_dir[MAX_PATH + 1]; 
+static char *x509_private_dirptr = NULL;
+
 static char x509_cert_area[MAX_PATH + 1];
+static char *x509_cert_areaptr = NULL;
+
 static char x509_cert_dir[MAX_PATH + 1];
+static char *x509_cert_dirptr = NULL;
+
 static char x509_cert_file[MAX_PATH + 1];
+static char *x509_cert_fileptr = NULL;
 
 static void get_windows_default_path(char *pathname, const char *suffix)
 {
@@ -27,6 +34,9 @@ static void get_windows_default_path(char *pathname, const char *suffix)
 
     ossldir = ossl_get_openssldir();
 
+    if (ossldir == NULL)
+        return;
+
     OPENSSL_strlcpy(pathname, ossldir, MAX_PATH - 1);
     if (MAX_PATH - strlen(pathname) > strlen(suffix))
         strcat(pathname, suffix);
@@ -36,9 +46,21 @@ static CRYPTO_ONCE openssldir_setup_init = CRYPTO_ONCE_STATIC_INIT;
 DEFINE_RUN_ONCE_STATIC(do_openssldir_setup)
 {
     get_windows_default_path(x509_private_dir, "\\private");
+    if (strlen(x509_private_dir) > 0)
+        x509_private_dirptr = x509_private_dir;
+
     get_windows_default_path(x509_cert_area, "\\");
+    if (strlen(x509_cert_area) > 0)
+        x509_cert_areaptr = x509_cert_area;
+
     get_windows_default_path(x509_cert_dir, "\\certs");
+    if (strlen(x509_cert_dir) > 0)
+        x509_cert_dirptr = x509_cert_dir;
+
     get_windows_default_path(x509_cert_file, "\\cert.pem");
+    if (strlen(x509_cert_file) > 0)
+        x509_cert_fileptr = x509_cert_file;
+
     return 1;
 }
 #endif
@@ -47,7 +69,7 @@ const char *X509_get_default_private_dir(void)
 {
 #if defined (_WIN32)
     RUN_ONCE(&openssldir_setup_init, do_openssldir_setup);
-    return x509_private_dir;
+    return x509_private_dirptr;
 #else
     return X509_PRIVATE_DIR;
 #endif
@@ -57,7 +79,7 @@ const char *X509_get_default_cert_area(void)
 {
 #if defined (_WIN32)
     RUN_ONCE(&openssldir_setup_init, do_openssldir_setup);
-    return x509_cert_area;
+    return x509_cert_areaptr;
 #else
     return X509_CERT_AREA;
 #endif
@@ -67,7 +89,7 @@ const char *X509_get_default_cert_dir(void)
 {
 #if defined (_WIN32)
     RUN_ONCE(&openssldir_setup_init, do_openssldir_setup);
-    return x509_cert_dir;
+    return x509_cert_dirptr;
 #else
     return X509_CERT_DIR;
 #endif
@@ -77,7 +99,7 @@ const char *X509_get_default_cert_file(void)
 {
 #if defined (_WIN32)
     RUN_ONCE(&openssldir_setup_init, do_openssldir_setup);
-    return x509_cert_file;
+    return x509_cert_fileptr;
 #else
     return X509_CERT_FILE;
 #endif

diff --git a/test/recipes/02-test_windows_registry.t b/test/recipes/02-test_windows_registry.t
index cee93a4c006f9cbce5e35bf03ef1fe09b69c8b50..ceb6d99694592f5d3ccb4ca15238f2dceb50272d 100644 (file)
--- a/test/recipes/02-test_windows_registry.t
+++ b/test/recipes/02-test_windows_registry.t
@@ -81,5 +81,3 @@ $expect =~ s/.*REG_EXPAND_SZ *//;
 $expect =~ s/ .*$//;
 $actual =~ s/MODULESSDIR: *//;
 ok(grep(/$expect/, $actual) == 1, "Confirming version output for modulesdir from registry");
-
-