Remove all getenv() calls, especially those wrapped by issetugid().
authorderaadt <deraadt@openbsd.org>
Sat, 11 Apr 2015 16:03:21 +0000 (16:03 +0000)
committerderaadt <deraadt@openbsd.org>
Sat, 11 Apr 2015 16:03:21 +0000 (16:03 +0000)
getenv()'s wrapped by issetugid() are safe, but issetugid() is correct
difficult to impliment on many operating systems.  By accident, a grand
experiment was run over the last year, where issetugid() returned 1 (the
safe value) on a few operating systems.  Noone noticed & complained that
certain environment variables were not working.......
ok doug beck jsing, discussion with others

13 files changed:
lib/libcrypto/conf/conf_api.c
lib/libcrypto/conf/conf_mod.c
lib/libcrypto/engine/eng_list.c
lib/libcrypto/x509/by_dir.c
lib/libcrypto/x509/by_file.c
lib/libcrypto/x509/x509_vfy.c
lib/libssl/src/crypto/conf/conf_api.c
lib/libssl/src/crypto/conf/conf_mod.c
lib/libssl/src/crypto/engine/eng_list.c
lib/libssl/src/crypto/x509/by_dir.c
lib/libssl/src/crypto/x509/by_file.c
lib/libssl/src/crypto/x509/x509_vfy.c
lib/libssl/src/doc/apps/config.pod

index efa4be9..f296e6a 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: conf_api.c,v 1.14 2015/02/10 11:22:21 jsing Exp $ */
+/* $OpenBSD: conf_api.c,v 1.15 2015/04/11 16:03:21 deraadt Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -130,7 +130,6 @@ char *
 _CONF_get_string(const CONF *conf, const char *section, const char *name)
 {
        CONF_VALUE *v, vv;
-       char *p;
 
        if (name == NULL)
                return (NULL);
@@ -141,14 +140,6 @@ _CONF_get_string(const CONF *conf, const char *section, const char *name)
                        v = lh_CONF_VALUE_retrieve(conf->data, &vv);
                        if (v != NULL)
                                return (v->value);
-                       if (strcmp(section, "ENV") == 0) {
-                               if (issetugid() == 0)
-                                       p = getenv(name);
-                               else
-                                       p = NULL;
-                               if (p != NULL)
-                                       return (p);
-                       }
                }
                vv.section = "default";
                vv.name = (char *)name;
@@ -157,11 +148,8 @@ _CONF_get_string(const CONF *conf, const char *section, const char *name)
                        return (v->value);
                else
                        return (NULL);
-       } else {
-               if (issetugid())
-                       return (NULL);
-               return (getenv(name));
-       }
+       } else
+               return (NULL);
 }
 
 static unsigned long
index 4363f29..cb54cc2 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: conf_mod.c,v 1.25 2014/07/22 02:21:20 beck Exp $ */
+/* $OpenBSD: conf_mod.c,v 1.26 2015/04/11 16:03:21 deraadt Exp $ */
 /* Written by Stephen Henson (steve@openssl.org) for the OpenSSL
  * project 2001.
  */
@@ -546,10 +546,6 @@ CONF_get1_default_config_file(void)
 {
        char *file = NULL;
 
-       if (issetugid() == 0)
-               file = getenv("OPENSSL_CONF");
-       if (file)
-               return strdup(file);
        if (asprintf(&file, "%s/openssl.cnf",
            X509_get_default_cert_area()) == -1)
                return (NULL);
index 740db90..939cc82 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: eng_list.c,v 1.17 2015/02/11 03:19:37 doug Exp $ */
+/* $OpenBSD: eng_list.c,v 1.18 2015/04/11 16:03:21 deraadt Exp $ */
 /* Written by Geoff Thorpe (geoff@geoffthorpe.net) for the OpenSSL
  * project 2000.
  */
@@ -386,12 +386,7 @@ ENGINE_by_id(const char *id)
                return iterator;
        /* Prevent infinite recusrion if we're looking for the dynamic engine. */
        if (strcmp(id, "dynamic")) {
-               if (issetugid() == 0) {
-                       load_dir = getenv("OPENSSL_ENGINES");
-                       if (load_dir == NULL)
-                               load_dir = ENGINESDIR;
-               } else
-                       load_dir = ENGINESDIR;
+               load_dir = ENGINESDIR;
 
                iterator = ENGINE_by_id("dynamic");
                if (!iterator ||
index 0322104..7b7d14a 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: by_dir.c,v 1.36 2015/02/12 03:54:07 jsing Exp $ */
+/* $OpenBSD: by_dir.c,v 1.37 2015/04/11 16:03:21 deraadt Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -124,20 +124,14 @@ dir_ctrl(X509_LOOKUP *ctx, int cmd, const char *argp, long argl,
 {
        int ret = 0;
        BY_DIR *ld;
-       char *dir = NULL;
 
        ld = (BY_DIR *)ctx->method_data;
 
        switch (cmd) {
        case X509_L_ADD_DIR:
                if (argl == X509_FILETYPE_DEFAULT) {
-                       if (issetugid() == 0)
-                               dir = getenv(X509_get_default_cert_dir_env());
-                       if (dir)
-                               ret = add_cert_dir(ld, dir, X509_FILETYPE_PEM);
-                       else
-                               ret = add_cert_dir(ld, X509_get_default_cert_dir(),
-                                   X509_FILETYPE_PEM);
+                       ret = add_cert_dir(ld, X509_get_default_cert_dir(),
+                           X509_FILETYPE_PEM);
                        if (!ret) {
                                X509err(X509_F_DIR_CTRL, X509_R_LOADING_CERT_DIR);
                        }
index 91a8e78..6892027 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: by_file.c,v 1.18 2015/02/05 01:33:22 reyk Exp $ */
+/* $OpenBSD: by_file.c,v 1.19 2015/04/11 16:03:21 deraadt Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -94,21 +94,13 @@ by_file_ctrl(X509_LOOKUP *ctx, int cmd, const char *argp, long argl,
     char **ret)
 {
        int ok = 0;
-       char *file = NULL;
 
        switch (cmd) {
        case X509_L_FILE_LOAD:
                if (argl == X509_FILETYPE_DEFAULT) {
-                       if (issetugid() == 0)
-                               file = getenv(X509_get_default_cert_file_env());
-                       if (file)
-                               ok = (X509_load_cert_crl_file(ctx, file,
-                                   X509_FILETYPE_PEM) != 0);
-                       else
-                               ok = (X509_load_cert_crl_file(ctx,
-                                   X509_get_default_cert_file(),
-                                   X509_FILETYPE_PEM) != 0);
-
+                       ok = (X509_load_cert_crl_file(ctx,
+                           X509_get_default_cert_file(),
+                           X509_FILETYPE_PEM) != 0);
                        if (!ok) {
                                X509err(X509_F_BY_FILE_CTRL,
                                    X509_R_LOADING_DEFAULTS);
index c383fda..4420356 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: x509_vfy.c,v 1.40 2015/02/11 02:17:59 jsing Exp $ */
+/* $OpenBSD: x509_vfy.c,v 1.41 2015/04/11 16:03:21 deraadt Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -483,12 +483,6 @@ check_chain_extensions(X509_STORE_CTX *ctx)
        } else {
                allow_proxy_certs =
                    !!(ctx->param->flags & X509_V_FLAG_ALLOW_PROXY_CERTS);
-#if 0
-               /* A hack to keep people who don't want to modify their
-                  software happy */
-               if (issetugid() == 0 && getenv("OPENSSL_ALLOW_PROXY_CERTS"))
-                       allow_proxy_certs = 1;
-#endif
                purpose = ctx->param->purpose;
        }
 
index efa4be9..f296e6a 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: conf_api.c,v 1.14 2015/02/10 11:22:21 jsing Exp $ */
+/* $OpenBSD: conf_api.c,v 1.15 2015/04/11 16:03:21 deraadt Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -130,7 +130,6 @@ char *
 _CONF_get_string(const CONF *conf, const char *section, const char *name)
 {
        CONF_VALUE *v, vv;
-       char *p;
 
        if (name == NULL)
                return (NULL);
@@ -141,14 +140,6 @@ _CONF_get_string(const CONF *conf, const char *section, const char *name)
                        v = lh_CONF_VALUE_retrieve(conf->data, &vv);
                        if (v != NULL)
                                return (v->value);
-                       if (strcmp(section, "ENV") == 0) {
-                               if (issetugid() == 0)
-                                       p = getenv(name);
-                               else
-                                       p = NULL;
-                               if (p != NULL)
-                                       return (p);
-                       }
                }
                vv.section = "default";
                vv.name = (char *)name;
@@ -157,11 +148,8 @@ _CONF_get_string(const CONF *conf, const char *section, const char *name)
                        return (v->value);
                else
                        return (NULL);
-       } else {
-               if (issetugid())
-                       return (NULL);
-               return (getenv(name));
-       }
+       } else
+               return (NULL);
 }
 
 static unsigned long
index 4363f29..cb54cc2 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: conf_mod.c,v 1.25 2014/07/22 02:21:20 beck Exp $ */
+/* $OpenBSD: conf_mod.c,v 1.26 2015/04/11 16:03:21 deraadt Exp $ */
 /* Written by Stephen Henson (steve@openssl.org) for the OpenSSL
  * project 2001.
  */
@@ -546,10 +546,6 @@ CONF_get1_default_config_file(void)
 {
        char *file = NULL;
 
-       if (issetugid() == 0)
-               file = getenv("OPENSSL_CONF");
-       if (file)
-               return strdup(file);
        if (asprintf(&file, "%s/openssl.cnf",
            X509_get_default_cert_area()) == -1)
                return (NULL);
index 740db90..939cc82 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: eng_list.c,v 1.17 2015/02/11 03:19:37 doug Exp $ */
+/* $OpenBSD: eng_list.c,v 1.18 2015/04/11 16:03:21 deraadt Exp $ */
 /* Written by Geoff Thorpe (geoff@geoffthorpe.net) for the OpenSSL
  * project 2000.
  */
@@ -386,12 +386,7 @@ ENGINE_by_id(const char *id)
                return iterator;
        /* Prevent infinite recusrion if we're looking for the dynamic engine. */
        if (strcmp(id, "dynamic")) {
-               if (issetugid() == 0) {
-                       load_dir = getenv("OPENSSL_ENGINES");
-                       if (load_dir == NULL)
-                               load_dir = ENGINESDIR;
-               } else
-                       load_dir = ENGINESDIR;
+               load_dir = ENGINESDIR;
 
                iterator = ENGINE_by_id("dynamic");
                if (!iterator ||
index 0322104..7b7d14a 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: by_dir.c,v 1.36 2015/02/12 03:54:07 jsing Exp $ */
+/* $OpenBSD: by_dir.c,v 1.37 2015/04/11 16:03:21 deraadt Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -124,20 +124,14 @@ dir_ctrl(X509_LOOKUP *ctx, int cmd, const char *argp, long argl,
 {
        int ret = 0;
        BY_DIR *ld;
-       char *dir = NULL;
 
        ld = (BY_DIR *)ctx->method_data;
 
        switch (cmd) {
        case X509_L_ADD_DIR:
                if (argl == X509_FILETYPE_DEFAULT) {
-                       if (issetugid() == 0)
-                               dir = getenv(X509_get_default_cert_dir_env());
-                       if (dir)
-                               ret = add_cert_dir(ld, dir, X509_FILETYPE_PEM);
-                       else
-                               ret = add_cert_dir(ld, X509_get_default_cert_dir(),
-                                   X509_FILETYPE_PEM);
+                       ret = add_cert_dir(ld, X509_get_default_cert_dir(),
+                           X509_FILETYPE_PEM);
                        if (!ret) {
                                X509err(X509_F_DIR_CTRL, X509_R_LOADING_CERT_DIR);
                        }
index 91a8e78..6892027 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: by_file.c,v 1.18 2015/02/05 01:33:22 reyk Exp $ */
+/* $OpenBSD: by_file.c,v 1.19 2015/04/11 16:03:21 deraadt Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -94,21 +94,13 @@ by_file_ctrl(X509_LOOKUP *ctx, int cmd, const char *argp, long argl,
     char **ret)
 {
        int ok = 0;
-       char *file = NULL;
 
        switch (cmd) {
        case X509_L_FILE_LOAD:
                if (argl == X509_FILETYPE_DEFAULT) {
-                       if (issetugid() == 0)
-                               file = getenv(X509_get_default_cert_file_env());
-                       if (file)
-                               ok = (X509_load_cert_crl_file(ctx, file,
-                                   X509_FILETYPE_PEM) != 0);
-                       else
-                               ok = (X509_load_cert_crl_file(ctx,
-                                   X509_get_default_cert_file(),
-                                   X509_FILETYPE_PEM) != 0);
-
+                       ok = (X509_load_cert_crl_file(ctx,
+                           X509_get_default_cert_file(),
+                           X509_FILETYPE_PEM) != 0);
                        if (!ok) {
                                X509err(X509_F_BY_FILE_CTRL,
                                    X509_R_LOADING_DEFAULTS);
index c383fda..4420356 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: x509_vfy.c,v 1.40 2015/02/11 02:17:59 jsing Exp $ */
+/* $OpenBSD: x509_vfy.c,v 1.41 2015/04/11 16:03:21 deraadt Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -483,12 +483,6 @@ check_chain_extensions(X509_STORE_CTX *ctx)
        } else {
                allow_proxy_certs =
                    !!(ctx->param->flags & X509_V_FLAG_ALLOW_PROXY_CERTS);
-#if 0
-               /* A hack to keep people who don't want to modify their
-                  software happy */
-               if (issetugid() == 0 && getenv("OPENSSL_ALLOW_PROXY_CERTS"))
-                       allow_proxy_certs = 1;
-#endif
                purpose = ctx->param->purpose;
        }
 
index d018dfc..57ec54e 100644 (file)
@@ -43,11 +43,8 @@ The value string undergoes variable expansion. This can be done by
 including the form B<$var> or B<${var}>: this will substitute the value
 of the named variable in the current section. It is also possible to
 substitute a value from another section using the syntax B<$section::name>
-or B<${section::name}>. By using the form B<$ENV::name> environment
-variables can be substituted. It is also possible to assign values to
-environment variables by using the name B<ENV::name>, this will work
-if the program looks up environment variables using the B<CONF> library
-instead of calling B<getenv()> directly.
+or B<${section::name}>. An old form using B<$ENV::name> has been deprecated
+because it is unsafe.
 
 It is possible to escape certain characters by using any kind of quote
 or the B<\> character. By making the last character of a line a B<\>