[PATCH] openssl-compat: Test for OpenSSL_add_all_algorithms before using.
Rosen Penev
rosenp at gmail.com
Thu Nov 22 09:38:39 AEDT 2018
On Mon, Nov 19, 2018 at 8:00 PM Damien Miller <djm at mindrot.org> wrote:
>
> On Mon, 19 Nov 2018, Rosen Penev wrote:
>
> > OpenSSL 1.1.0 has deprecated this function.
>
> I think we could consolidate a bit more. This folds the libcrypto init
> into the seed_rng() call that should happen early in each tool's main().
Looks good.
>
> -d
>
> diff --git a/configure.ac b/configure.ac
> index 3f7fe2cd..5a9b3ff1 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -2671,8 +2671,8 @@ if test "x$openssl" = "xyes" ; then
>
> AC_MSG_CHECKING([if programs using OpenSSL functions will link])
> AC_LINK_IFELSE(
> - [AC_LANG_PROGRAM([[ #include <openssl/evp.h> ]],
> - [[ OpenSSL_add_all_algorithms(); ]])],
> + [AC_LANG_PROGRAM([[ #include <openssl/err.h> ]],
> + [[ ERR_load_crypto_strings(); ]])],
> [
> AC_MSG_RESULT([yes])
> ],
> @@ -2682,8 +2682,8 @@ if test "x$openssl" = "xyes" ; then
> LIBS="$LIBS -ldl"
> AC_MSG_CHECKING([if programs using OpenSSL need -ldl])
> AC_LINK_IFELSE(
> - [AC_LANG_PROGRAM([[ #include <openssl/evp.h> ]],
> - [[ OpenSSL_add_all_algorithms(); ]])],
> + [AC_LANG_PROGRAM([[ #include <openssl/err.h> ]],
> + [[ ERR_load_crypto_strings(); ]])],
> [
> AC_MSG_RESULT([yes])
> ],
> @@ -2698,15 +2698,16 @@ if test "x$openssl" = "xyes" ; then
> AC_CHECK_FUNCS([ \
> BN_is_prime_ex \
> DSA_generate_parameters_ex \
> - EVP_DigestInit_ex \
> + EVP_CIPHER_CTX_ctrl \
> EVP_DigestFinal_ex \
> - EVP_MD_CTX_init \
> + EVP_DigestInit_ex \
> EVP_MD_CTX_cleanup \
> EVP_MD_CTX_copy_ex \
> + EVP_MD_CTX_init \
> HMAC_CTX_init \
> + OpenSSL_add_all_algorithms \
> RSA_generate_key_ex \
> RSA_get_default_method \
> - EVP_CIPHER_CTX_ctrl \
> ])
> # LibreSSL/OpenSSL 1.1x API
> AC_CHECK_FUNCS([ \
> diff --git a/entropy.c b/entropy.c
> index fc710ec2..97e83608 100644
> --- a/entropy.c
> +++ b/entropy.c
> @@ -56,6 +56,8 @@
> #include "sshbuf.h"
> #include "ssherr.h"
>
> +#define RANDOM_SEED_SIZE 48
> +
> /*
> * Portable OpenSSH PRNG seeding:
> * If OpenSSL has not "internally seeded" itself (e.g. pulled data from
> @@ -64,8 +66,6 @@
> */
> #ifndef OPENSSL_PRNG_ONLY
>
> -#define RANDOM_SEED_SIZE 48
> -
> /*
> * Collect 'len' bytes of entropy into 'buf' from PRNGD/EGD daemon
> * listening either on 'tcp_port', or via Unix domain socket at *
> @@ -216,9 +216,11 @@ rexec_recv_rng_seed(struct sshbuf *m)
> void
> seed_rng(void)
> {
> -#ifndef OPENSSL_PRNG_ONLY
> unsigned char buf[RANDOM_SEED_SIZE];
> -#endif
> +
> + /* Initialise libcrypto */
> + ssh_libcrypto_init();
> +
> if (!ssh_compatible_openssl(OPENSSL_VERSION_NUMBER,
> OpenSSL_version_num()))
> fatal("OpenSSL version mismatch. Built against %lx, you "
> @@ -226,27 +228,34 @@ seed_rng(void)
> OpenSSL_version_num());
>
> #ifndef OPENSSL_PRNG_ONLY
> - if (RAND_status() == 1) {
> + if (RAND_status() == 1)
> debug3("RNG is ready, skipping seeding");
> - return;
> + else {
> + if (seed_from_prngd(buf, sizeof(buf)) == -1)
> + fatal("Could not obtain seed from PRNGd");
> + RAND_add(buf, sizeof(buf), sizeof(buf));
> }
> -
> - if (seed_from_prngd(buf, sizeof(buf)) == -1)
> - fatal("Could not obtain seed from PRNGd");
> - RAND_add(buf, sizeof(buf), sizeof(buf));
> - memset(buf, '\0', sizeof(buf));
> -
> #endif /* OPENSSL_PRNG_ONLY */
> +
> if (RAND_status() != 1)
> fatal("PRNG is not seeded");
> +
> + /* Ensure arc4random() is primed */
> + arc4random_buf(buf, sizeof(buf));
> + explicit_bzero(buf, sizeof(buf));
> }
>
> #else /* WITH_OPENSSL */
>
> -/* Handled in arc4random() */
> +/* Acutal initialisation is handled in arc4random() */
> void
> seed_rng(void)
> {
> + unsigned char buf[RANDOM_SEED_SIZE];
> +
> + /* Ensure arc4random() is primed */
> + arc4random_buf(buf, sizeof(buf));
> + explicit_bzero(buf, sizeof(buf));
> }
>
> #endif /* WITH_OPENSSL */
> diff --git a/openbsd-compat/openssl-compat.c b/openbsd-compat/openssl-compat.c
> index 5ade8f0b..d8c00ebc 100644
> --- a/openbsd-compat/openssl-compat.c
> +++ b/openbsd-compat/openssl-compat.c
> @@ -66,26 +66,31 @@ ssh_compatible_openssl(long headerver, long libver)
> return 0;
> }
>
> -#ifdef USE_OPENSSL_ENGINE
> void
> -ssh_OpenSSL_add_all_algorithms(void)
> +ssh_libcrypto_init(void)
> {
> +#if defined(HAVE_OPENSSL_ADD_ALL_ALGORITHMS)
> OpenSSL_add_all_algorithms();
> +#elif defined(HAVE_OPENSSL_INIT_CRYPTO) && \
> + defined(OPENSSL_INIT_ADD_ALL_CIPHERS) && \
> + defined(OPENSSL_INIT_ADD_ALL_DIGESTS)
> + OPENSSL_init_crypto(OPENSSL_INIT_ADD_ALL_CIPHERS |
> + OPENSSL_INIT_ADD_ALL_DIGESTS, NULL);
> +#endif
>
> +#ifdef USE_OPENSSL_ENGINE
> /* Enable use of crypto hardware */
> ENGINE_load_builtin_engines();
> ENGINE_register_all_complete();
>
> -#if defined(HAVE_OPENSSL_INIT_CRYPTO) && \
> - defined(OPENSSL_INIT_ADD_ALL_CIPHERS) && \
> - defined(OPENSSL_INIT_ADD_ALL_DIGESTS) && \
> - defined(OPENSSL_INIT_LOAD_CONFIG)
> + /* Load the libcrypto config file to pick up engines defined there */
> +# if defined(HAVE_OPENSSL_INIT_CRYPTO) && defined(OPENSSL_INIT_LOAD_CONFIG)
> OPENSSL_init_crypto(OPENSSL_INIT_ADD_ALL_CIPHERS |
> OPENSSL_INIT_ADD_ALL_DIGESTS | OPENSSL_INIT_LOAD_CONFIG, NULL);
> -#else
> +# else
> OPENSSL_config(NULL);
> -#endif
> +# endif
> +#endif /* USE_OPENSSL_ENGINE */
> }
> -#endif
>
> #endif /* WITH_OPENSSL */
> diff --git a/openbsd-compat/openssl-compat.h b/openbsd-compat/openssl-compat.h
> index b87ce59e..917bc6f7 100644
> --- a/openbsd-compat/openssl-compat.h
> +++ b/openbsd-compat/openssl-compat.h
> @@ -31,6 +31,7 @@
> #include <openssl/dh.h>
>
> int ssh_compatible_openssl(long, long);
> +void ssh_libcrypto_init(void);
>
> #if (OPENSSL_VERSION_NUMBER < 0x1000100fL)
> # error OpenSSL 1.0.1 or greater is required
> @@ -92,27 +93,6 @@ void ssh_aes_ctr_iv(EVP_CIPHER_CTX *, int, u_char *, size_t);
> # endif
> #endif
>
> -/*
> - * We overload some of the OpenSSL crypto functions with ssh_* equivalents
> - * to automatically handle OpenSSL engine initialisation.
> - *
> - * In order for the compat library to call the real functions, it must
> - * define SSH_DONT_OVERLOAD_OPENSSL_FUNCS before including this file and
> - * implement the ssh_* equivalents.
> - */
> -#ifndef SSH_DONT_OVERLOAD_OPENSSL_FUNCS
> -
> -# ifdef USE_OPENSSL_ENGINE
> -# ifdef OpenSSL_add_all_algorithms
> -# undef OpenSSL_add_all_algorithms
> -# endif
> -# define OpenSSL_add_all_algorithms() ssh_OpenSSL_add_all_algorithms()
> -# endif
> -
> -void ssh_OpenSSL_add_all_algorithms(void);
> -
> -#endif /* SSH_DONT_OVERLOAD_OPENSSL_FUNCS */
> -
> /* LibreSSL/OpenSSL 1.1x API compat */
> #ifndef HAVE_DSA_GET0_PQG
> void DSA_get0_pqg(const DSA *d, const BIGNUM **p, const BIGNUM **q,
> diff --git a/regress/unittests/sshkey/tests.c b/regress/unittests/sshkey/tests.c
> index 13f265cd..78aa9223 100644
> --- a/regress/unittests/sshkey/tests.c
> +++ b/regress/unittests/sshkey/tests.c
> @@ -7,8 +7,6 @@
>
> #include "includes.h"
>
> -#include <openssl/evp.h>
> -
> #include "../test_helper/test_helper.h"
>
> void sshkey_tests(void);
> @@ -18,9 +16,6 @@ void sshkey_fuzz_tests(void);
> void
> tests(void)
> {
> - OpenSSL_add_all_algorithms();
> - ERR_load_CRYPTO_strings();
> -
> sshkey_tests();
> sshkey_file_tests();
> sshkey_fuzz_tests();
> diff --git a/regress/unittests/test_helper/test_helper.c b/regress/unittests/test_helper/test_helper.c
> index 4cc70852..11c3ee39 100644
> --- a/regress/unittests/test_helper/test_helper.c
> +++ b/regress/unittests/test_helper/test_helper.c
> @@ -35,11 +35,13 @@
> #include <signal.h>
>
> #include <openssl/bn.h>
> +#include <openssl/err.h>
>
> #if defined(HAVE_STRNVIS) && defined(HAVE_VIS_H) && !defined(BROKEN_STRNVIS)
> # include <vis.h>
> #endif
>
> +#include "entropy.h"
> #include "test_helper.h"
> #include "atomicio.h"
>
> @@ -121,6 +123,9 @@ main(int argc, char **argv)
> {
> int ch;
>
> + seed_rng();
> + ERR_load_CRYPTO_strings();
> +
> /* Handle systems without __progname */
> if (__progname == NULL) {
> __progname = strrchr(argv[0], '/');
> diff --git a/scp.c b/scp.c
> index 4f3fdcd3..eb17c341 100644
> --- a/scp.c
> +++ b/scp.c
> @@ -400,6 +400,8 @@ main(int argc, char **argv)
> /* Ensure that fds 0, 1 and 2 are open or directed to /dev/null */
> sanitise_stdfd();
>
> + seed_rng();
> +
> msetlocale();
>
> /* Copy argv, because we modify it */
> diff --git a/sftp-server-main.c b/sftp-server-main.c
> index c6ccd623..6230d897 100644
> --- a/sftp-server-main.c
> +++ b/sftp-server-main.c
> @@ -43,6 +43,8 @@ main(int argc, char **argv)
> /* Ensure that fds 0, 1 and 2 are open or directed to /dev/null */
> sanitise_stdfd();
>
> + seed_rng();
> +
> if ((user_pw = getpwuid(getuid())) == NULL) {
> fprintf(stderr, "No user found for uid %lu\n",
> (u_long)getuid());
> diff --git a/sftp.c b/sftp.c
> index ed95cf81..f886b330 100644
> --- a/sftp.c
> +++ b/sftp.c
> @@ -2367,6 +2367,8 @@ main(int argc, char **argv)
> sanitise_stdfd();
> msetlocale();
>
> + seed_rng();
> +
> __progname = ssh_get_progname(argv[0]);
> memset(&args, '\0', sizeof(args));
> args.list = NULL;
> diff --git a/ssh-add.c b/ssh-add.c
> index 627c0298..50165e7d 100644
> --- a/ssh-add.c
> +++ b/ssh-add.c
> @@ -544,10 +544,6 @@ main(int argc, char **argv)
> __progname = ssh_get_progname(argv[0]);
> seed_rng();
>
> -#ifdef WITH_OPENSSL
> - OpenSSL_add_all_algorithms();
> -#endif
> -
> setvbuf(stdout, NULL, _IOLBF, 0);
>
> /* First, get a connection to the authentication agent. */
> diff --git a/ssh-agent.c b/ssh-agent.c
> index cb552462..6baebc31 100644
> --- a/ssh-agent.c
> +++ b/ssh-agent.c
> @@ -1095,10 +1095,6 @@ main(int ac, char **av)
> if (getrlimit(RLIMIT_NOFILE, &rlim) == -1)
> fatal("%s: getrlimit: %s", __progname, strerror(errno));
>
> -#ifdef WITH_OPENSSL
> - OpenSSL_add_all_algorithms();
> -#endif
> -
> __progname = ssh_get_progname(av[0]);
> seed_rng();
>
> diff --git a/ssh-keygen.c b/ssh-keygen.c
> index 416d25be..a6773735 100644
> --- a/ssh-keygen.c
> +++ b/ssh-keygen.c
> @@ -2459,13 +2459,10 @@ main(int argc, char **argv)
>
> __progname = ssh_get_progname(argv[0]);
>
> -#ifdef WITH_OPENSSL
> - OpenSSL_add_all_algorithms();
> -#endif
> - log_init(argv[0], SYSLOG_LEVEL_INFO, SYSLOG_FACILITY_USER, 1);
> -
> seed_rng();
>
> + log_init(argv[0], SYSLOG_LEVEL_INFO, SYSLOG_FACILITY_USER, 1);
> +
> msetlocale();
>
> /* we need this for the home * directory. */
> diff --git a/ssh-keysign.c b/ssh-keysign.c
> index bcd1508c..8f487b8c 100644
> --- a/ssh-keysign.c
> +++ b/ssh-keysign.c
> @@ -174,9 +174,6 @@ main(int argc, char **argv)
> u_char *signature, *data, rver;
> char *host, *fp;
> size_t slen, dlen;
> -#ifdef WITH_OPENSSL
> - u_int32_t rnd[256];
> -#endif
>
> ssh_malloc_init(); /* must be called before any mallocs */
> if (pledge("stdio rpath getpw dns id", NULL) != 0)
> @@ -224,12 +221,6 @@ main(int argc, char **argv)
> if (found == 0)
> fatal("could not open any host key");
>
> -#ifdef WITH_OPENSSL
> - OpenSSL_add_all_algorithms();
> - arc4random_buf(rnd, sizeof(rnd));
> - RAND_seed(rnd, sizeof(rnd));
> -#endif
> -
> found = 0;
> for (i = 0; i < NUM_KEYTYPES; i++) {
> keys[i] = NULL;
> diff --git a/ssh.c b/ssh.c
> index 1e471f5c..1ac903d1 100644
> --- a/ssh.c
> +++ b/ssh.c
> @@ -610,6 +610,8 @@ main(int ac, char **av)
> av = saved_av;
> #endif
>
> + seed_rng();
> +
> /*
> * Discard other fds that are hanging around. These can cause problem
> * with backgrounded ssh processes started by ControlPersist.
> @@ -1036,11 +1038,6 @@ main(int ac, char **av)
>
> host_arg = xstrdup(host);
>
> -#ifdef WITH_OPENSSL
> - OpenSSL_add_all_algorithms();
> - ERR_load_crypto_strings();
> -#endif
> -
> /* Initialize the command to execute on remote host. */
> if ((command = sshbuf_new()) == NULL)
> fatal("sshbuf_new failed");
> @@ -1264,8 +1261,6 @@ main(int ac, char **av)
> tty_flag = 0;
> }
>
> - seed_rng();
> -
> if (options.user == NULL)
> options.user = xstrdup(pw->pw_name);
>
> diff --git a/ssh_api.c b/ssh_api.c
> index e727c0d6..53bbc9b4 100644
> --- a/ssh_api.c
> +++ b/ssh_api.c
> @@ -81,9 +81,7 @@ ssh_init(struct ssh **sshp, int is_server, struct kex_params *kex_params)
> int r;
>
> if (!called) {
> -#ifdef WITH_OPENSSL
> - OpenSSL_add_all_algorithms();
> -#endif /* WITH_OPENSSL */
> + seed_rng();
> called = 1;
> }
>
> diff --git a/sshd.c b/sshd.c
> index afd95932..fb9d9b60 100644
> --- a/sshd.c
> +++ b/sshd.c
> @@ -1510,6 +1510,8 @@ main(int ac, char **av)
> /* Ensure that fds 0, 1 and 2 are open or directed to /dev/null */
> sanitise_stdfd();
>
> + seed_rng();
> +
> /* Initialize configuration options to their default values. */
> initialize_server_options(&options);
>
> @@ -1631,10 +1633,6 @@ main(int ac, char **av)
> else
> closefrom(REEXEC_DEVCRYPTO_RESERVED_FD);
>
> -#ifdef WITH_OPENSSL
> - OpenSSL_add_all_algorithms();
> -#endif
> -
> /* If requested, redirect the logs to the specified logfile. */
> if (logfile != NULL)
> log_redirect_stderr_to(logfile);
> @@ -1677,8 +1675,6 @@ main(int ac, char **av)
> parse_server_config(&options, rexeced_flag ? "rexec" : config_file_name,
> cfg, NULL);
>
> - seed_rng();
> -
> /* Fill in default values for those options not explicitly set. */
> fill_default_server_options(&options);
>
More information about the openssh-unix-dev
mailing list