]> www.infradead.org Git - users/dwmw2/openconnect.git/commitdiff
add --allow-insecure-crypto, and corresponding API functions, to explicitly enable...
authorDaniel Lenski <dlenski@gmail.com>
Mon, 18 May 2020 17:54:03 +0000 (10:54 -0700)
committerDaniel Lenski <dlenski@gmail.com>
Wed, 4 Nov 2020 22:04:22 +0000 (14:04 -0800)
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>
gnutls.c
java/src/org/infradead/libopenconnect/LibOpenConnect.java
jni.c
libopenconnect.map.in
library.c
main.c
openconnect-internal.h
openconnect.8.in
openconnect.h
openssl.c
www/changelog.xml

index 255b6942a4e14f444eeb18088fa26628db3e9f88..9c91abf11cf1a9a45615767f1d15d4b18b78ae8e 100644 (file)
--- a/gnutls.c
+++ b/gnutls.c
@@ -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)
 {
@@ -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,
index 56fe45543e9d3d269167c1b8684dd4c13369dc49..2f60e1078174787833aee6bce4a3a362be8bd914 100644 (file)
@@ -147,6 +147,7 @@ public abstract class LibOpenConnect {
        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);
 
diff --git a/jni.c b/jni.c
index d900eea879f480bdaefc0e505a60809ffcddbbcc..8f5b982a03a070376b6bd9d11a91222bb959446a 100644 (file)
--- a/jni.c
+++ b/jni.c
@@ -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)
 {
index 1f0a27608e41d7fbcf451bf033bbcbb56866392e..a45b25fd7c33ca7f47a68c514e0344c0c63d08da 100644 (file)
@@ -111,6 +111,7 @@ OPENCONNECT_5_6 {
 OPENCONNECT_5_7 {
  global:
        openconnect_set_cookie;
+       openconnect_set_allow_insecure_crypto;
 } OPENCONNECT_5_6;
 
 OPENCONNECT_PRIVATE {
index e4bb056757862d93aa6b24bf56cc1bd06569c53d..f54afb96d120722b9fc0aaddbafabe8e5db4e49d 100644 (file)
--- a/library.c
+++ b/library.c
@@ -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;
diff --git a/main.c b/main.c
index 1211288e62bab12ec5bae2a3cdafcac54e935b6d..becfb1c090e3117cff4744b7740bf7dc6105b9ae 100644 (file)
--- a/main.c
+++ b/main.c
@@ -188,6 +188,7 @@ enum {
        OPT_OS,
        OPT_TIMESTAMP,
        OPT_PFS,
+       OPT_ALLOW_INSECURE_CRYPTO,
        OPT_PROXY_AUTH,
        OPT_HTTP_AUTH,
        OPT_LOCAL_HOSTNAME,
@@ -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'),
@@ -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");
 
@@ -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);
index 9ff851506ea857a6c200cfe44239b3fd1a081b39..1f30140e19e0341c58e47e61d3469b4b71c35535 100644 (file)
@@ -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;
@@ -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);
index 350428c32aa5fd64411df67125272c4cf5a20eae..741f42399d7b7017f3e56a801977563c602399c2 100644 (file)
@@ -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
@@ -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 }
index af4eebc7cac41abd145a3a396af1fc2703d526a6..8595908ecbd9c00db6799e4c976cba8026f7cefe 100644 (file)
@@ -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()
@@ -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
index b15f2f6f25c794964b36d8b1d3b238d03ce43a7a..4c889815b5f05765aa87f33e0348833118eef064 100644 (file)
--- a/openssl.c
+++ b/openssl.c
@@ -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();
@@ -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)) {
index 28ac2c39fcc53fdb53c71451aad2b074a6a43ecb..083f37fec7b7c06bfaf48198d499fb375a57edcc 100644 (file)
@@ -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>