From 4e07eecaf04a48c3253a5dfd69d817673194e154 Mon Sep 17 00:00:00 2001 From: Daniel Lenski Date: Thu, 21 Jan 2021 16:27:23 -0800 Subject: [PATCH] with --allow-insecure-crypto, additionally attempt to disable insecure systemwide minimum crypto settings Because openconnect_set_allow_insecure_crypto() now does more than just attempt to reenable 3DES and ARC4, its failure to enable those ciphers should not be treated as fatal, but merely a warning. Setting the appropriate environment variable (GNUTLS_SYSTEM_PRIORITY_FILE or OPENSSL_CONF) to `/dev/null` *before* crypto library initialization should ensure that a systemwide crypto configuration file doesn't set a minimum crypto requirement which would override the user choice. See https://gitlab.com/openconnect/openconnect/-/issues/211#note_482161646 for discussion of GnuTLS settings, and https://www.openssl.org/docs/man1.1.1/man5/config.html for OpenSSL. FIXME: OpenSSL implementation needs library reinitialization. Signed-off-by: Daniel Lenski --- gnutls.c | 15 +++++++++++++-- library.c | 4 +--- main.c | 8 ++++++-- openconnect-internal.h | 11 ++++++++++- openconnect.8.in | 7 +++++-- openssl.c | 17 +++++++++++++++-- 6 files changed, 50 insertions(+), 12 deletions(-) diff --git a/gnutls.c b/gnutls.c index 7d5b9924..bab38e00 100644 --- a/gnutls.c +++ b/gnutls.c @@ -78,12 +78,23 @@ const char *openconnect_get_tls_library_version() int can_enable_insecure_crypto() { + int ret = 0; + + if (setenv("GNUTLS_SYSTEM_PRIORITY_FILE", DEVNULL, 1) < 0) + return -errno; + + gnutls_global_deinit(); + ret = openconnect_init_ssl(); + if (ret) + return ret; + /* XX: As of GnuTLS 3.6.13, no released version has (yet) removed 3DES/RC4 from default builds, * but like OpenSSL (removed in 1.1.0) it may happen. */ if (gnutls_cipher_get_id("3DES-CBC") == GNUTLS_CIPHER_UNKNOWN || gnutls_cipher_get_id("ARCFOUR-128") == GNUTLS_CIPHER_UNKNOWN) - return -ENOENT; - return 0; + ret = -ENOENT; + + return ret; } /* Helper functions for reading/writing lines over TLS/DTLS. */ diff --git a/library.c b/library.c index 3a8d6cd6..4db0d368 100644 --- a/library.c +++ b/library.c @@ -757,10 +757,8 @@ void openconnect_set_pfs(struct openconnect_info *vpninfo, unsigned val) int openconnect_set_allow_insecure_crypto(struct openconnect_info *vpninfo, unsigned val) { int ret = can_enable_insecure_crypto(); - if (ret) - return ret; vpninfo->allow_insecure_crypto = val; - return 0; + return ret; } void openconnect_set_cancel_fd(struct openconnect_info *vpninfo, int fd) diff --git a/main.c b/main.c index 58a7fc0f..9780e44d 100644 --- a/main.c +++ b/main.c @@ -924,6 +924,7 @@ static void usage(void) printf(" --no-http-keepalive %s\n", _("Disable HTTP connection re-use")); printf(" --no-xmlpost %s\n", _("Do not attempt XML POST authentication")); printf(" --allow-insecure-crypto %s\n", _("Allow use of the ancient, insecure 3DES and RC4 ciphers")); + printf(" %s\n", _("(and attempt to override OS crypto policies)")); printf("\n"); @@ -1639,9 +1640,12 @@ int main(int argc, char **argv) openconnect_set_pfs(vpninfo, 1); break; case OPT_ALLOW_INSECURE_CRYPTO: - if (openconnect_set_allow_insecure_crypto(vpninfo, 1)) { - fprintf(stderr, _("Cannot enable insecure 3DES or RC4 ciphers, because the library\n" + ret = openconnect_set_allow_insecure_crypto(vpninfo, 1); + if (ret == -ENOENT) + fprintf(stderr, _("WARNING: cannot enable insecure 3DES and/or RC4 ciphers, because the library\n" "%s no longer supports them.\n"), openconnect_get_tls_library_version()); + else if (ret < 0) { + fprintf(stderr, _("Unknown error while enabling insecure crypto.\n")); exit(1); } break; diff --git a/openconnect-internal.h b/openconnect-internal.h index d35bf445..429aa6f8 100644 --- a/openconnect-internal.h +++ b/openconnect-internal.h @@ -41,6 +41,15 @@ #include "openconnect.h" +/* Equivalent of "/dev/null" on Windows. + * See https://stackoverflow.com/a/44163934 + */ +#ifdef _WIN32 +#define DEVNULL "NUL" +#else +#define DEVNULL "/dev/null" +#endif + #if defined(OPENCONNECT_OPENSSL) #include #include @@ -1069,7 +1078,7 @@ int do_gen_hotp_code(struct openconnect_info *vpninfo, struct oc_auth_form *form, struct oc_form_opt *opt); -int set_oidc_token(struct openconnect_info *vpninfo, +int set_oidc_token(struct openconnect_info *vpninfo, const char *token_str); /* stoken.c */ diff --git a/openconnect.8.in b/openconnect.8.in index 741f4239..73edd838 100644 --- a/openconnect.8.in +++ b/openconnect.8.in @@ -469,8 +469,11 @@ The ancient, broken 3DES and RC4 ciphers are insecure; we explicitly disable them by default. However, some still-in-use VPN servers can't do any better. -This option enables use of these insecure ciphers, as well as the use -of SHA1 for server certificate validation. +This option +.B attempts +to enable use of these insecure ciphers, as well as +the use of SHA1 for server certificate validation, and to override any +other system policies regarding minimum crypto requirements. .TP .B \-\-non\-inter Do not expect user input; exit if it is required. diff --git a/openssl.c b/openssl.c index 26ac750e..8fb03b92 100644 --- a/openssl.c +++ b/openssl.c @@ -63,10 +63,23 @@ const char *openconnect_get_tls_library_version() int can_enable_insecure_crypto() { + int ret = 0; + + if (setenv("OPENSSL_CONF", DEVNULL, 1) < 0) + return -errno; + + /* FIXME: deinitialize and reinitialize library, as is done for GnuTLS, + * to ensure that updated value is used. + * + * Cleaning up and reinitalizing OpenSSL appears to be complex: + * https://wiki.openssl.org/index.php/Library_Initialization#Cleanup + */ + if (EVP_des_ede3_cbc() == NULL || EVP_rc4() == NULL) - return -ENOENT; - return 0; + ret = -ENOENT; + + return ret; } int openconnect_sha1(unsigned char *result, void *data, int len)