Skip to content

Commit

Permalink
with --allow-insecure-crypto, additionally attempt to disable insecur…
Browse files Browse the repository at this point in the history
…e 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 <dlenski@gmail.com>
  • Loading branch information
dlenski committed Jan 23, 2021
1 parent e5770db commit 4e07eec
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 12 deletions.
15 changes: 13 additions & 2 deletions gnutls.c
Expand Up @@ -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. */
Expand Down
4 changes: 1 addition & 3 deletions library.c
Expand Up @@ -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)
Expand Down
8 changes: 6 additions & 2 deletions main.c
Expand Up @@ -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");

Expand Down Expand Up @@ -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;
Expand Down
11 changes: 10 additions & 1 deletion openconnect-internal.h
Expand Up @@ -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 <openssl/ssl.h>
#include <openssl/err.h>
Expand Down Expand Up @@ -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 */
Expand Down
7 changes: 5 additions & 2 deletions openconnect.8.in
Expand Up @@ -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.
Expand Down
17 changes: 15 additions & 2 deletions openssl.c
Expand Up @@ -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)
Expand Down

0 comments on commit 4e07eec

Please sign in to comment.