From: deraadt Date: Sat, 11 Apr 2015 16:03:21 +0000 (+0000) Subject: Remove all getenv() calls, especially those wrapped by issetugid(). X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=920ccb623f2ff40701397bf8b898c6ebdf3a73a1;p=openbsd Remove all getenv() calls, especially those wrapped by issetugid(). 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 --- diff --git a/lib/libcrypto/conf/conf_api.c b/lib/libcrypto/conf/conf_api.c index efa4be9f6bc..f296e6a9629 100644 --- a/lib/libcrypto/conf/conf_api.c +++ b/lib/libcrypto/conf/conf_api.c @@ -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 diff --git a/lib/libcrypto/conf/conf_mod.c b/lib/libcrypto/conf/conf_mod.c index 4363f297c76..cb54cc2a87a 100644 --- a/lib/libcrypto/conf/conf_mod.c +++ b/lib/libcrypto/conf/conf_mod.c @@ -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); diff --git a/lib/libcrypto/engine/eng_list.c b/lib/libcrypto/engine/eng_list.c index 740db908525..939cc82b170 100644 --- a/lib/libcrypto/engine/eng_list.c +++ b/lib/libcrypto/engine/eng_list.c @@ -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 || diff --git a/lib/libcrypto/x509/by_dir.c b/lib/libcrypto/x509/by_dir.c index 032210424d9..7b7d14a9505 100644 --- a/lib/libcrypto/x509/by_dir.c +++ b/lib/libcrypto/x509/by_dir.c @@ -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); } diff --git a/lib/libcrypto/x509/by_file.c b/lib/libcrypto/x509/by_file.c index 91a8e781b2a..68920271fcd 100644 --- a/lib/libcrypto/x509/by_file.c +++ b/lib/libcrypto/x509/by_file.c @@ -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); diff --git a/lib/libcrypto/x509/x509_vfy.c b/lib/libcrypto/x509/x509_vfy.c index c383fda4f2d..442035625a8 100644 --- a/lib/libcrypto/x509/x509_vfy.c +++ b/lib/libcrypto/x509/x509_vfy.c @@ -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; } diff --git a/lib/libssl/src/crypto/conf/conf_api.c b/lib/libssl/src/crypto/conf/conf_api.c index efa4be9f6bc..f296e6a9629 100644 --- a/lib/libssl/src/crypto/conf/conf_api.c +++ b/lib/libssl/src/crypto/conf/conf_api.c @@ -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 diff --git a/lib/libssl/src/crypto/conf/conf_mod.c b/lib/libssl/src/crypto/conf/conf_mod.c index 4363f297c76..cb54cc2a87a 100644 --- a/lib/libssl/src/crypto/conf/conf_mod.c +++ b/lib/libssl/src/crypto/conf/conf_mod.c @@ -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); diff --git a/lib/libssl/src/crypto/engine/eng_list.c b/lib/libssl/src/crypto/engine/eng_list.c index 740db908525..939cc82b170 100644 --- a/lib/libssl/src/crypto/engine/eng_list.c +++ b/lib/libssl/src/crypto/engine/eng_list.c @@ -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 || diff --git a/lib/libssl/src/crypto/x509/by_dir.c b/lib/libssl/src/crypto/x509/by_dir.c index 032210424d9..7b7d14a9505 100644 --- a/lib/libssl/src/crypto/x509/by_dir.c +++ b/lib/libssl/src/crypto/x509/by_dir.c @@ -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); } diff --git a/lib/libssl/src/crypto/x509/by_file.c b/lib/libssl/src/crypto/x509/by_file.c index 91a8e781b2a..68920271fcd 100644 --- a/lib/libssl/src/crypto/x509/by_file.c +++ b/lib/libssl/src/crypto/x509/by_file.c @@ -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); diff --git a/lib/libssl/src/crypto/x509/x509_vfy.c b/lib/libssl/src/crypto/x509/x509_vfy.c index c383fda4f2d..442035625a8 100644 --- a/lib/libssl/src/crypto/x509/x509_vfy.c +++ b/lib/libssl/src/crypto/x509/x509_vfy.c @@ -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; } diff --git a/lib/libssl/src/doc/apps/config.pod b/lib/libssl/src/doc/apps/config.pod index d018dfce502..57ec54ec9e4 100644 --- a/lib/libssl/src/doc/apps/config.pod +++ b/lib/libssl/src/doc/apps/config.pod @@ -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, this will work -if the program looks up environment variables using the B library -instead of calling B 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<\>