Fix up string handling for ciphersuite_config

Sure it isn't C++ and std::string, but we *have* a method for appending
strings to a dynamic buffer. We don't need this snprintf("%s%s%s%s%s")
nonsense.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>

diff --git a/gnutls.c b/gnutls.c
index 6738d04a855a84d7766f6d71603418b3425e5c0e..61e5efcd03e0568f308f35f751d35576fb24fa91 100644 (file)
--- a/gnutls.c
+++ b/gnutls.c
@@ -2099,7 +2099,6 @@ static int verify_peer(gnutls_session_t session)
 
 int openconnect_open_https(struct openconnect_info *vpninfo)
 {
-       const char *default_prio;
        int ssl_sock = -1;
        int err;
 
@@ -2244,9 +2243,10 @@ int openconnect_open_https(struct openconnect_info *vpninfo)
        * 28065ce3896b1b0f87972d0bce9b17641ebb69b9
        */
 
-        if (!strlen(vpninfo->ciphersuite_config)) {
+        if (!vpninfo->ciphersuite_config) {
+               struct oc_text_buf *buf = buf_alloc();
 #ifdef DEFAULT_PRIO
-               default_prio = DEFAULT_PRIO ":%COMPAT";
+               buf_append(buf, "%s", DEFAULT_PRIO ":%COMPAT");
 #else
                /* GnuTLS 3.5.19 and onward remove AES-CBC-HMAC-SHA256 from NORMAL,
                 * but some Cisco servers can't do anything better, so
@@ -2261,13 +2261,30 @@ int openconnect_open_https(struct openconnect_info *vpninfo)
                 * - 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";
+               buf_append(buf, "NORMAL:-VERS-SSL3.0:+SHA256:%%COMPAT");
 #endif
 
-               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" : "");
+               if (vpninfo->pfs)
+                       buf_append(buf, ":-RSA");
+
+               if (vpninfo->no_tls13)
+                       buf_append(buf, ":-VERS-TLS1.3");
+
+               if (vpninfo->allow_insecure_crypto) {
+                       buf_append(buf, ":+3DES-CBC:+ARCFOUR-128:+SHA1");
+                       if (gnutls_check_version_numeric(3,6,0))
+                               buf_append(buf, ":%%VERIFY_ALLOW_SIGN_WITH_SHA1");
+               } else
+                       buf_append(buf, ":-3DES-CBC:-ARCFOUR-128");
+
+               if (buf_error(buf)) {
+                       vpn_progress(vpninfo, PRG_ERR,
+                                    _("Failed to construct GnuTLS priority string\n"));
+                       return buf_free(buf);
+               }
+               vpninfo->ciphersuite_config = buf->data;
+               buf->data = NULL;
+               buf_free(buf);
         }
 
        err = gnutls_priority_set_direct(vpninfo->https_sess,

diff --git a/library.c b/library.c
index 57a8951249fc9e5092f921a983a905c114a75fbc..e6d27184f35a6b02762a8f3faeab02f1d6107e12 100644 (file)
--- a/library.c
+++ b/library.c
@@ -391,6 +391,7 @@ void openconnect_vpninfo_free(struct openconnect_info *vpninfo)
        free(vpninfo->ifname);
        free(vpninfo->dtls_cipher);
        free(vpninfo->peer_cert_hash);
+       free(vpninfo->ciphersuite_config);
 #if defined(OPENCONNECT_OPENSSL)
        free(vpninfo->cstp_cipher);
 #if defined(HAVE_BIO_METH_FREE)

diff --git a/main.c b/main.c
index e520833fb45e17c5b84a0c4eaa1b2155db20fe26..1d398103e73b8bcd1c043c63507fb2550eb10009 100644 (file)
--- a/main.c
+++ b/main.c
@@ -1976,7 +1976,7 @@ int main(int argc, char **argv)
 #endif
                                  );
 
-                       strncpy(vpninfo->ciphersuite_config, config_arg, sizeof(vpninfo->ciphersuite_config) - 1);
+                       vpninfo->ciphersuite_config = keep_config_arg();
                        break;
                default:
                        usage();

diff --git a/openconnect-internal.h b/openconnect-internal.h
index 6fac309f65472bbc82a326194203024dc032871f..50fa043ceb5b7936d7edf6a49ebe9c486f82b8f4 100644 (file)
--- a/openconnect-internal.h
+++ b/openconnect-internal.h
@@ -541,7 +541,7 @@ struct openconnect_info {
        struct oc_tpm2_ctx *tpm2;
 #endif
 #endif /* OPENCONNECT_GNUTLS */
-       char ciphersuite_config[256];
+       char *ciphersuite_config;
        struct oc_text_buf *ttls_pushbuf;
        uint8_t ttls_eap_ident;
        unsigned char *ttls_recvbuf;

diff --git a/openssl.c b/openssl.c
index 8fb03b92a63ceafd1c6b6849ee9be82dd8b19f98..d760172154f681174a2ba8b56f3d9fd14b56bcc9 100644 (file)
--- a/openssl.c
+++ b/openssl.c
@@ -1751,10 +1751,27 @@ int openconnect_open_https(struct openconnect_info *vpninfo)
                if (!vpninfo->no_system_trust)
                        SSL_CTX_set_default_verify_paths(vpninfo->https_ctx);
 
-               if (!strlen(vpninfo->ciphersuite_config)) {
-                       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 (!vpninfo->ciphersuite_config) {
+                       struct oc_text_buf *buf = buf_alloc();
+                       if (vpninfo->pfs)
+                               buf_append(buf, "HIGH:!aNULL:!eNULL:-RSA");
+                       else
+                               buf_append(buf, "DEFAULT");
+
+                       if (vpninfo->allow_insecure_crypto)
+                               buf_append(buf, ":+3DES:+RC4");
+                       else
+                               buf_append(buf, ":-3DES:-RC4");
+
+                       if (buf_error(buf)) {
+                               vpn_progress(vpninfo, PRG_ERR,
+                                            _("Failed to construct OpenSSL cipher list\n"));
+                               return buf_free(buf);
+                       }
+
+                       vpninfo->ciphersuite_config = buf->data;
+                       buf->data = NULL;
+                       buf_free(buf);
                }
 
                if (!SSL_CTX_set_cipher_list(vpninfo->https_ctx, vpninfo->ciphersuite_config)) {