Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge branch 'openssl-sec-level' into 'master'
With --allow-insecure-crypto, set OpenSSL 1.1.0+ "security level" to 0, and attempt to disable system minimum crypto requirements

See merge request openconnect/openconnect!158
  • Loading branch information
dlenski committed Jan 23, 2021
2 parents dac3095 + 9781077 commit f07d798
Show file tree
Hide file tree
Showing 9 changed files with 110 additions and 22 deletions.
31 changes: 31 additions & 0 deletions compat.c
Expand Up @@ -19,6 +19,25 @@

#include <string.h>
#include <stdarg.h>
#ifdef _WIN32
#include <sec_api/stdlib_s.h> /* errno_t, size_t */
errno_t getenv_s(
size_t *ret_required_buf_size,
char *buf,
size_t buf_size_in_bytes,
const char *name
);
errno_t _putenv_s(
const char *varname,
const char *value_string
);

/* XX: needed to get _putenv_s, getenv_s from stdlib.h with MinGW,
* but only works on newer versions.
* https://stackoverflow.com/a/51977723
*/
/* #define MINGW_HAS_SECURE_API 1 */
#endif
#include <stdlib.h>
#include <errno.h>
#include <ctype.h>
Expand Down Expand Up @@ -196,6 +215,18 @@ int openconnect__inet_aton(const char *cp, struct in_addr *addr)
#endif

#ifdef _WIN32
int openconnect__win32_setenv(const char *name, const char *value, int overwrite)
{
/* https://stackoverflow.com/a/23616164 */
int errcode = 0;
if(!overwrite) {
size_t envsize = 0;
errcode = getenv_s(&envsize, NULL, 0, name);
if(errcode || envsize) return errcode;
}
return _putenv_s(name, value);
}

char *openconnect__win32_strerror(DWORD err)
{
wchar_t *msgw;
Expand Down
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 @@ -928,6 +928,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 @@ -1685,9 +1686,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
14 changes: 13 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 @@ -825,6 +834,9 @@ static inline int tun_is_up(struct openconnect_info *vpninfo)
#define pipe(fds) _pipe(fds, 4096, O_BINARY)
int openconnect__win32_sock_init();
char *openconnect__win32_strerror(DWORD err);
#undef setenv
#define setenv openconnect__win32_setenv
int openconnect__win32_setenv(const char *name, const char *value, int overwrite);
#undef inet_pton
#define inet_pton openconnect__win32_inet_pton
int openconnect__win32_inet_pton(int af, const char *src, void *dst);
Expand Down Expand Up @@ -1066,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
30 changes: 28 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 Expand Up @@ -1698,6 +1711,19 @@ int openconnect_open_https(struct openconnect_info *vpninfo)
SSL_CTX_set_options(vpninfo->https_ctx, SSL_OP_NO_TICKET);
#endif

#if OPENSSL_VERSION_NUMBER >= 0x010100000L
if (vpninfo->allow_insecure_crypto) {
/* OpenSSL versions after 1.1.0 added the notion of a "security level"
* that enforces checks on certificates and ciphers.
* These security levels overlap in functionality with the ciphersuite
* priority/allow-strings.
*
* For now we will set the security level to 0, thus reverting
* to the functionality seen in versions before 1.1.0. */
SSL_CTX_set_security_level(vpninfo->https_ctx, 0);
}
#endif

if (vpninfo->cert) {
err = load_certificate(vpninfo);
if (!err && !SSL_CTX_check_private_key(vpninfo->https_ctx)) {
Expand Down
18 changes: 10 additions & 8 deletions tests/obsolete-server-crypto
Expand Up @@ -25,13 +25,6 @@ top_builddir=${top_builddir:-..}

. `dirname $0`/common.sh

########################################
# Need to override mandatory system-wide crypto policy on Fedora 31+,
# for both ocserv and openconnect.
########################################

export GNUTLS_SYSTEM_PRIORITY_FILE=/dev/null

########################################
# Verify that we cannot connect to a server offering only obsolete, insecure
# crypto UNLESS --allow-insecure-crypto is specified.
Expand All @@ -43,7 +36,16 @@ echo "Testing against server with insecure crypto (3DES and RC4 only)... "
PORT=4568
TLS_PRIORITIES="LEGACY:%SERVER_PRECEDENCE:%COMPAT:-VERS-TLS-ALL:+VERS-TLS1.0:-CIPHER-ALL:+3DES-CBC:+ARCFOUR-128:+MD5:+SHA1"
update_config test-obsolete-server-crypto.config
launch_simple_sr_server -d 1 -f -c $CONFIG

########################################
# Need to override mandatory system-wide crypto policy on Fedora 31+, in
# order for ocserv to offer 3DES and RC4.
#
# However, we want to leave this policy in place for openconnect,
# in order to verify the client's ability to disable it on its own.
########################################
GNUTLS_SYSTEM_PRIORITY_FILE=/dev/null launch_simple_sr_server -d 1 -f -c $CONFIG

PID=$!
wait_server $PID

Expand Down
5 changes: 3 additions & 2 deletions www/changelog.xml
Expand Up @@ -18,8 +18,9 @@
<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>
<li><i>Add obsolete-server-crypto test (<a href="https://gitlab.com/openconnect/openconnect/-/merge_requests/114">!114</a>)</i></li>
<li>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>)</li>
<li>With <tt>--allow-insecure-crypto</tt>, additionally try to disable system-wide and library minimum crypto policies (<a href="https://gitlab.com/openconnect/openconnect/-/merge_requests/158">!158</a>, <a href="https://gitlab.com/openconnect/openconnect/-/issues/132">#132</a>)</li>
<li>Add obsolete-server-crypto test (<a href="https://gitlab.com/openconnect/openconnect/-/merge_requests/114">!114</a>)</li>
<li>Allow protocols to delay tunnel setup and shutdown (<a href="https://gitlab.com/openconnect/openconnect/-/merge_requests/117">!117</a>)</li>
<li>Incomplete, speculative support for GlobalProtect IPv6 (<a href="https://gitlab.com/openconnect/openconnect/-/merge_requests/155">!155</a>; previous work in <a href="https://gitlab.com/openconnect/openconnect/commit/d6db0ec03394234d41fbec7ffc794ceeb486a8f0">d6db0ec</a>)</li>
<li>SIGUSR1 causes OpenConnect to log detailed connection information and statistics (<a href="https://gitlab.com/openconnect/openconnect/-/merge_requests/154">!154</a>)</li>
Expand Down

0 comments on commit f07d798

Please sign in to comment.