Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
add --allow-insecure-crypto, and corresponding API functions, to expl…
…icitly enable 3DES/RC4/SHA1

This closes #145, and adds tests intended to prevent similar situations from recurring.

Allowing the ancient, broken 3DES and RC4 ciphers is insecure; we do not
want to (re-)enable them by default.  (See discussion:
https://gitlab.com/openconnect/openconnect/-/issues/145#note_344687335)

However, some still-in-use VPN servers can't do any better. So instead, we
explicitly disable them, unless explicitly enabled with the
`--allow-insecure-crypto` option, or corresponding API functions.

Also attempts to future-proof --allow-obsolete-crypto a bit, by setting
`%VERIFY_ALLOW_SIGN_WITH_SHA1` (per nmav:
https://gitlab.com/openconnect/openconnect/-/merge_requests/114#note_346496796),
and explicitly enabling SHA1 (which was moved to GnuTLS “bad hashes list” in
1d75e116b1681d0e6b140d7530e7f0403088da88)

Signed-off-by: Daniel Lenski <dlenski@gmail.com>
  • Loading branch information
dlenski committed Nov 4, 2020
1 parent 7b42041 commit 685d880
Show file tree
Hide file tree
Showing 11 changed files with 83 additions and 8 deletions.
32 changes: 27 additions & 5 deletions gnutls.c
Expand Up @@ -76,6 +76,16 @@ const char *openconnect_get_tls_library_version()
return tls_library_version;
}

int can_enable_insecure_crypto()
{
/* 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;
}

/* Helper functions for reading/writing lines over SSL. */
static int _openconnect_gnutls_write(gnutls_session_t ses, int fd, struct openconnect_info *vpninfo, char *buf, size_t len)
{
Expand Down Expand Up @@ -2227,14 +2237,26 @@ int openconnect_open_https(struct openconnect_info *vpninfo)
#ifdef DEFAULT_PRIO
default_prio = DEFAULT_PRIO ":%COMPAT";
#else
/* GnuTLS 3.5.19 and onward refuse to negotiate AES-CBC-HMAC-SHA256
* by default but some Cisco servers can't do anything better, so
* explicitly add '+SHA256' to allow it. Yay Cisco. */
/* GnuTLS 3.5.19 and onward remove AES-CBC-HMAC-SHA256 from NORMAL,
* but some Cisco servers can't do anything better, so
* explicitly add '+SHA256' to allow it. Yay Cisco.
* - GnuTLS commit that removed: c433cdf92349afae66c703bdacedf987f423605e
* - Old server requiring SHA256: https://gitlab.com/openconnect/openconnect/-/issues/21
*
* Likewise, GnuTLS 3.6.0 and onward remove 3DES-CBC from NORMAL,
* but some ancient servers can't do anything better. This (and ARCFOUR-128)
* should not be reenabled by default due to serious security flaws, so adding as an
* option, --allow-insecure-crypto. Yay ancient, unpatched servers.
* - GnuTLS commit that removed: 66f2a0a271bcc10e8fb68771f9349a3d3ecf6dda
* - Old server requiring 3DES-CBC: https://gitlab.com/openconnect/openconnect/-/issues/145
*/
default_prio = "NORMAL:-VERS-SSL3.0:+SHA256:%COMPAT";
#endif

snprintf(vpninfo->ciphersuite_config, sizeof(vpninfo->ciphersuite_config), "%s%s%s",
default_prio, vpninfo->pfs?":-RSA":"", vpninfo->no_tls13?":-VERS-TLS1.3":"");
snprintf(vpninfo->ciphersuite_config, sizeof(vpninfo->ciphersuite_config), "%s%s%s%s%s",
default_prio, vpninfo->pfs?":-RSA":"", vpninfo->no_tls13?":-VERS-TLS1.3":"",
vpninfo->allow_insecure_crypto?":+3DES-CBC:+ARCFOUR-128:+SHA1":":-3DES-CBC:-ARCFOUR-128",
vpninfo->allow_insecure_crypto && gnutls_check_version_numeric(3,6,0) ? ":%VERIFY_ALLOW_SIGN_WITH_SHA1" : "");
}

err = gnutls_priority_set_direct(vpninfo->https_sess,
Expand Down
1 change: 1 addition & 0 deletions java/src/org/infradead/libopenconnect/LibOpenConnect.java
Expand Up @@ -147,6 +147,7 @@ public synchronized native void setMobileInfo(String mobilePlatformVersion,
public synchronized native void setClientCert(String cert, String sslKey);
public synchronized native void setReqMTU(int mtu);
public synchronized native void setPFS(boolean isEnabled);
public synchronized native int setAllowInsecureCrypto(boolean isEnabled);
public synchronized native void setSystemTrust(boolean isEnabled);
public synchronized native int setProtocol(String protocol);

Expand Down
10 changes: 10 additions & 0 deletions jni.c
Expand Up @@ -1071,6 +1071,16 @@ JNIEXPORT void JNICALL Java_org_infradead_libopenconnect_LibOpenConnect_setPFS(
openconnect_set_pfs(ctx->vpninfo, arg);
}

JNIEXPORT jint JNICALL Java_org_infradead_libopenconnect_LibOpenConnect_setAllowInsecureCrypto(
JNIEnv *jenv, jobject jobj, jboolean arg)
{
struct libctx *ctx = getctx(jenv, jobj);

if (!ctx)
return -EINVAL;
return openconnect_set_allow_insecure_crypto(ctx->vpninfo, arg);
}

JNIEXPORT void JNICALL Java_org_infradead_libopenconnect_LibOpenConnect_setSystemTrust(
JNIEnv *jenv, jobject jobj, jboolean arg)
{
Expand Down
1 change: 1 addition & 0 deletions libopenconnect.map.in
Expand Up @@ -111,6 +111,7 @@ OPENCONNECT_5_6 {
OPENCONNECT_5_7 {
global:
openconnect_set_cookie;
openconnect_set_allow_insecure_crypto;
} OPENCONNECT_5_6;

OPENCONNECT_PRIVATE {
Expand Down
9 changes: 9 additions & 0 deletions library.c
Expand Up @@ -746,6 +746,15 @@ void openconnect_set_pfs(struct openconnect_info *vpninfo, unsigned val)
vpninfo->pfs = 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;
}

void openconnect_set_cancel_fd(struct openconnect_info *vpninfo, int fd)
{
vpninfo->cmd_fd = fd;
Expand Down
10 changes: 10 additions & 0 deletions main.c
Expand Up @@ -188,6 +188,7 @@ enum {
OPT_OS,
OPT_TIMESTAMP,
OPT_PFS,
OPT_ALLOW_INSECURE_CRYPTO,
OPT_PROXY_AUTH,
OPT_HTTP_AUTH,
OPT_LOCAL_HOSTNAME,
Expand Down Expand Up @@ -217,6 +218,7 @@ static const struct option long_options[] = {
OPTION("csd-wrapper", 1, OPT_CSD_WRAPPER),
#endif
OPTION("pfs", 0, OPT_PFS),
OPTION("allow-insecure-crypto", 0, OPT_ALLOW_INSECURE_CRYPTO),
OPTION("certificate", 1, 'c'),
OPTION("sslkey", 1, 'k'),
OPTION("cookie", 1, 'C'),
Expand Down Expand Up @@ -912,6 +914,7 @@ static void usage(void)
printf("\n%s:\n", _("Server bugs"));
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("\n");

Expand Down Expand Up @@ -1537,6 +1540,13 @@ int main(int argc, char **argv)
case OPT_PFS:
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"
"%s no longer supports them.\n"), openconnect_get_tls_library_version());
exit(1);
}
break;
case OPT_SERVERCERT:
server_cert = keep_config_arg();
openconnect_set_system_trust(vpninfo, 0);
Expand Down
2 changes: 2 additions & 0 deletions openconnect-internal.h
Expand Up @@ -506,6 +506,7 @@ struct openconnect_info {

unsigned pfs;
unsigned no_tls13;
unsigned allow_insecure_crypto; /* Allow 3DES and RC4 (known-insecure, but the best that some ancient servers can do) */
#if defined(OPENCONNECT_OPENSSL)
#ifdef HAVE_LIBP11
PKCS11_CTX *pkcs11_ctx;
Expand Down Expand Up @@ -1004,6 +1005,7 @@ int encrypt_esp_packet(struct openconnect_info *vpninfo, struct pkt *pkt, int cr

/* {gnutls,openssl}.c */
const char *openconnect_get_tls_library_version();
int can_enable_insecure_crypto();
int ssl_nonblock_read(struct openconnect_info *vpninfo, void *buf, int maxlen);
int ssl_nonblock_write(struct openconnect_info *vpninfo, void *buf, int buflen);
int openconnect_open_https(struct openconnect_info *vpninfo);
Expand Down
10 changes: 9 additions & 1 deletion openconnect.8.in
Expand Up @@ -464,6 +464,14 @@ option, then you have found a bug in OpenConnect. Please see
http://www.infradead.org/openconnect/mail.html and report this to the
developers.
.TP
.B \-\-allow\-insecure\-crypto
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.
.TP
.B \-\-non\-inter
Do not expect user input; exit if it is required.
.TP
Expand Down Expand Up @@ -503,7 +511,7 @@ will call liboath to generate an RFC 6238 time-based password, and
.B \-\-token\-mode=hotp
will call liboath to generate an RFC 4226 HMAC-based password. Yubikey
tokens which generate OATH codes in hardware are supported with
.B \-\-token\-mode=yubioath. \-\-token\-mode=oidc will use the provided
.B \-\-token\-mode=yubioath. \-\-token\-mode=oidc will use the provided
OpenIDConnect token as an RFC 6750 bearer token.
.TP
.B \-\-token\-secret={ SECRET[,COUNTER] | @FILENAME }
Expand Down
2 changes: 2 additions & 0 deletions openconnect.h
Expand Up @@ -38,6 +38,7 @@ extern "C" {
/*
* API version 5.7:
* - Add openconnect_set_cookie()
* - Add openconnect_set_allow_insecure_crypto()
*
* API version 5.6 (v8.06; 2020-03-31):
* - Add openconnect_set_trojan_interval()
Expand Down Expand Up @@ -552,6 +553,7 @@ int openconnect_parse_url(struct openconnect_info *vpninfo, const char *url);
void openconnect_set_cert_expiry_warning(struct openconnect_info *vpninfo,
int seconds);
void openconnect_set_pfs(struct openconnect_info *vpninfo, unsigned val);
int openconnect_set_allow_insecure_crypto(struct openconnect_info *vpninfo, unsigned val);

/* If this is set, then openconnect_obtain_cookie() will abort and return
failure if the file descriptor is readable. Typically a user may create
Expand Down
13 changes: 11 additions & 2 deletions openssl.c
Expand Up @@ -61,6 +61,14 @@ const char *openconnect_get_tls_library_version()
return tls_library_version;
}

int can_enable_insecure_crypto()
{
if (EVP_des_ede3_cbc() == NULL ||
EVP_rc4() == NULL)
return -ENOENT;
return 0;
}

int openconnect_sha1(unsigned char *result, void *data, int len)
{
EVP_MD_CTX *c = EVP_MD_CTX_new();
Expand Down Expand Up @@ -1718,8 +1726,9 @@ int openconnect_open_https(struct openconnect_info *vpninfo)
SSL_CTX_set_default_verify_paths(vpninfo->https_ctx);

if (!strlen(vpninfo->ciphersuite_config)) {
strncpy(vpninfo->ciphersuite_config, vpninfo->pfs ? "HIGH:!aNULL:!eNULL:-RSA" : "DEFAULT",
sizeof(vpninfo->ciphersuite_config)-1);
snprintf(vpninfo->ciphersuite_config, sizeof(vpninfo->ciphersuite_config), "%s%s",
vpninfo->pfs ? "HIGH:!aNULL:!eNULL:-RSA" : "DEFAULT",
vpninfo->allow_insecure_crypto?":+3DES:+RC4":":-3DES:-RC4");
}

if (!SSL_CTX_set_cipher_list(vpninfo->https_ctx, vpninfo->ciphersuite_config)) {
Expand Down
1 change: 1 addition & 0 deletions www/changelog.xml
Expand Up @@ -18,6 +18,7 @@
<li>Make <tt>tncc-emulate.py</tt> work with Python 3.7+. (<a href="https://gitlab.com/openconnect/openconnect/-/issues/152">!152</a>, <a href="https://gitlab.com/openconnect/openconnect/merge_requests/120">!120</a>)</li>
<li>Emulated a newer version of GlobalProtect official clients, 5.1.5-8; was 4.0.2-19 (<a href="https://gitlab.com/openconnect/openconnect/merge_requests/131">!131</a>)</li>
<li>Support Juniper login forms containing both password and 2FA token (<a href="https://gitlab.com/openconnect/openconnect/-/merge_requests/121">!121</a>)</li>
<li><i>Explicitly disable 3DES and RC4, unless enabled with <tt>--allow-insecure-crypto</tt> (<a href="https://gitlab.com/openconnect/openconnect/-/merge_requests/114">!114</a>)</i></li>
</ul><br/>
</li>
<li><b><a href="ftp://ftp.infradead.org/pub/openconnect/openconnect-8.10.tar.gz">OpenConnect v8.10</a></b>
Expand Down

0 comments on commit 685d880

Please sign in to comment.