[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