]> www.infradead.org Git - users/dwmw2/openconnect.git/commitdiff
Remove static ui_vpninfo hack for ENGINE callbacks
authorDavid Woodhouse <dwmw2@infradead.org>
Fri, 12 Oct 2018 18:49:45 +0000 (11:49 -0700)
committerDavid Woodhouse <dwmw2@infradead.org>
Fri, 12 Oct 2018 18:49:45 +0000 (11:49 -0700)
This doesn't seem to be needed; all the TPM engines (even v1) handle
the callback properly now.

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

index eb64ed5732856b36523b68481d5b05463a0ebc48..e4c840d2082e19c6ea3bf34f2429c71fc9e51b62 100644 (file)
--- a/openssl.c
+++ b/openssl.c
@@ -335,12 +335,9 @@ struct ui_form_opt {
 };
 
 #ifdef HAVE_ENGINE
- /* Ick. But there is no way to pass this sanely through OpenSSL */
-static struct openconnect_info *ui_vpninfo;
-
 static int ui_open(UI *ui)
 {
-       struct openconnect_info *vpninfo = ui_vpninfo; /* Ick */
+       struct openconnect_info *vpninfo = UI_get0_user_data(ui);
        struct ui_data *ui_data;
 
        if (!vpninfo || !vpninfo->process_auth_form)
@@ -354,6 +351,7 @@ static int ui_open(UI *ui)
        ui_data->last_opt = &ui_data->form.opts;
        ui_data->vpninfo = vpninfo;
        ui_data->form.auth_id = (char *)"openssl_ui";
+
        UI_add_user_data(ui, ui_data);
 
        return 1;
@@ -433,42 +431,10 @@ static int ui_close(UI *ui)
        return 1;
 }
 
-static UI_METHOD *create_openssl_ui(struct openconnect_info *vpninfo)
+static UI_METHOD *create_openssl_ui(void)
 {
        UI_METHOD *ui_method = UI_create_method((char *)"AnyConnect VPN UI");
 
-       /* There is a race condition here because of the use of the
-          static ui_vpninfo pointer. This sucks, but it's OpenSSL's
-          fault and in practice it's *never* going to hurt us.
-
-          This UI is only used for loading certificates from a TPM; for
-          PKCS#12 and PEM files we hook the passphrase request differently.
-          The ui_vpninfo variable is set here, and is used from ui_open()
-          when the TPM ENGINE decides it needs to ask the user for a PIN.
-
-          The race condition exists because theoretically, there
-          could be more than one thread using libopenconnect and
-          trying to authenticate to a VPN server, within the *same*
-          process. And if *both* are using certificates from the TPM,
-          and *both* manage to be within that short window of time
-          between setting ui_vpninfo and invoking ui_open() to fetch
-          the PIN, then one connection's ->process_auth_form() could
-          get a PIN request for the *other* connection.
-
-          However, the only thing that ever does run libopenconnect more
-          than once from the same process is KDE's NetworkManager support,
-          and NetworkManager doesn't *support* having more than one VPN
-          connected anyway, so first that would have to be fixed and then
-          you'd have to connect to two VPNs simultaneously by clicking
-          'connect' on both at *exactly* the same time and then getting
-          *really* unlucky.
-
-          Oh, and the KDE support won't be using OpenSSL anyway because of
-          licensing conflicts... so although this sucks, I'm not going to
-          lose sleep over it.
-       */
-       ui_vpninfo = vpninfo;
-
        /* Set up a UI method of our own for password/passphrase requests */
        UI_method_set_opener(ui_method, ui_open);
        UI_method_set_writer(ui_method, ui_write);
@@ -648,11 +614,12 @@ static int load_tpm_certificate(struct openconnect_info *vpninfo,
                }
                vpninfo->cert_password = NULL;
                free(vpninfo->cert_password);
-       } else {
-               /* Provide our own UI method to handle the PIN callback. */
-               meth = create_openssl_ui(vpninfo);
        }
-       key = ENGINE_load_private_key(e, vpninfo->sslkey, meth, NULL);
+
+       /* Provide our own UI method to handle the PIN callback. */
+       meth = create_openssl_ui();
+
+       key = ENGINE_load_private_key(e, vpninfo->sslkey, meth, vpninfo);
        if (meth)
                UI_destroy_method(meth);
        if (!key) {