Performance improvement proposal
Paolo De Santis <[email protected]> Mon, 30 Sep 2024 23:46:55 +0200
| Newsgroups | gmane.comp.web.wget.general |
|---|---|
| Message-ID | <CAHDkLY0NFhsm8FGRDcNau_=NQgQZ-H44Mqb_soWg+WM_QO4vyg@mail.gmail.com> |
Hello, After upgrading from openssl 1.x to 3.x, I've seen significant performance degradation in a particular scenario where wget was used to perform multiple HEAD requests, targeting a server with a self-signed certificate, thus using the option "--check-certificate=quiet". That led me to dig a bit deeper and find an open issue <https://github.com/openssl/openssl/issues/18814>, regarding the speed of "SSL_CTX_load_verify_locations" in openssl 3.x. As a workaround, when the user requests that the certificate is not checked at all, I'd propose to skip CA certificates loading, thus avoiding unnecessary function calls (proposed implementation in "skip-ca-loading.patch"). In addition, when the user sets the quiet flag "-q" and, at the same time, uses the option "--no-check-certificate", I believe the program should behave as if the user had set "--check-certificate=quiet", because the warning would not be printed anyways (proposed implementation in "quiet-cert-check.patch"). Feel free to contact me if you want to discuss further about these possible changes. Thanks for your work! Have a great day! -- Paolo De Santis
skip-ca-loading.patch
(application/octet-stream, 5 KB)
diff --git a/src/gnutls.c b/src/gnutls.c
index cfcdf671..d7791794 100644
--- a/src/gnutls.c
+++ b/src/gnutls.c
@@ -114,6 +114,11 @@ ssl_init (void)
gnutls_certificate_set_verify_flags (credentials,
GNUTLS_VERIFY_ALLOW_X509_V1_CA_CRT);
+ /* If opt.check_cert equals CHECK_CERT_QUIET, we can return early and
+ avoid loading CA certificates. */
+ if (opt.check_cert == CHECK_CERT_QUIET)
+ return true;
+
#if GNUTLS_VERSION_MAJOR >= 3
if (!opt.ca_directory)
ncerts = gnutls_certificate_set_x509_system_trust (credentials);
@@ -1041,13 +1046,15 @@ ssl_check_certificate (int fd, const char *host)
goto out;
}
- _CHECK_CERT (GNUTLS_CERT_INVALID, _("%s: The certificate of %s is not trusted.\n"));
- _CHECK_CERT (GNUTLS_CERT_SIGNER_NOT_FOUND, _("%s: The certificate of %s doesn't have a known issuer.\n"));
- _CHECK_CERT (GNUTLS_CERT_REVOKED, _("%s: The certificate of %s has been revoked.\n"));
- _CHECK_CERT (GNUTLS_CERT_SIGNER_NOT_CA, _("%s: The certificate signer of %s was not a CA.\n"));
- _CHECK_CERT (GNUTLS_CERT_INSECURE_ALGORITHM, _("%s: The certificate of %s was signed using an insecure algorithm.\n"));
- _CHECK_CERT (GNUTLS_CERT_NOT_ACTIVATED, _("%s: The certificate of %s is not yet activated.\n"));
- _CHECK_CERT (GNUTLS_CERT_EXPIRED, _("%s: The certificate of %s has expired.\n"));
+ if (opt.check_cert != CHECK_CERT_QUIET) {
+ _CHECK_CERT (GNUTLS_CERT_INVALID, _("%s: The certificate of %s is not trusted.\n"));
+ _CHECK_CERT (GNUTLS_CERT_SIGNER_NOT_FOUND, _("%s: The certificate of %s doesn't have a known issuer.\n"));
+ _CHECK_CERT (GNUTLS_CERT_REVOKED, _("%s: The certificate of %s has been revoked.\n"));
+ _CHECK_CERT (GNUTLS_CERT_SIGNER_NOT_CA, _("%s: The certificate signer of %s was not a CA.\n"));
+ _CHECK_CERT (GNUTLS_CERT_INSECURE_ALGORITHM, _("%s: The certificate of %s was signed using an insecure algorithm.\n"));
+ _CHECK_CERT (GNUTLS_CERT_NOT_ACTIVATED, _("%s: The certificate of %s is not yet activated.\n"));
+ _CHECK_CERT (GNUTLS_CERT_EXPIRED, _("%s: The certificate of %s has expired.\n"));
+ }
if (gnutls_certificate_type_get (ctx->session) == GNUTLS_CRT_X509)
{
@@ -1080,6 +1087,10 @@ ssl_check_certificate (int fd, const char *host)
success = false;
goto crt_deinit;
}
+
+ if (opt.check_cert == CHECK_CERT_QUIET)
+ goto pkp_verify;
+
if (now < gnutls_x509_crt_get_activation_time (cert))
{
logprintf (LOG_NOTQUIET, _("The certificate has not yet been activated\n"));
@@ -1100,6 +1111,7 @@ ssl_check_certificate (int fd, const char *host)
}
xfree(sni_hostname);
+ pkp_verify:
pinsuccess = pkp_pin_peer_pubkey (cert, opt.pinnedpubkey);
if (!pinsuccess)
{
diff --git a/src/openssl.c b/src/openssl.c
index a9708753..ba8d2718 100644
--- a/src/openssl.c
+++ b/src/openssl.c
@@ -342,6 +342,17 @@ ssl_init (void)
goto error;
}
+ /* SSL_VERIFY_NONE instructs OpenSSL not to abort SSL_connect if the
+ certificate is invalid. We verify the certificate separately in
+ ssl_check_certificate, which provides much better diagnostics
+ than examining the error stack after a failed SSL_connect. */
+ SSL_CTX_set_verify (ssl_ctx, SSL_VERIFY_NONE, NULL);
+
+ /* If opt.check_cert equals CHECK_CERT_QUIET, we can return early and
+ avoid loading CA certificates. */
+ if (opt.check_cert == CHECK_CERT_QUIET)
+ return true;
+
SSL_CTX_set_default_verify_paths (ssl_ctx);
SSL_CTX_load_verify_locations (ssl_ctx, opt.ca_cert, opt.ca_directory);
@@ -388,12 +399,6 @@ ssl_init (void)
X509_STORE_set_flags (store, X509_V_FLAG_CRL_CHECK | X509_V_FLAG_CRL_CHECK_ALL);
}
- /* SSL_VERIFY_NONE instructs OpenSSL not to abort SSL_connect if the
- certificate is invalid. We verify the certificate separately in
- ssl_check_certificate, which provides much better diagnostics
- than examining the error stack after a failed SSL_connect. */
- SSL_CTX_set_verify (ssl_ctx, SSL_VERIFY_NONE, NULL);
-
/* Use the private key from the cert file unless otherwise specified. */
if (opt.cert_file && !opt.private_key)
{
@@ -1064,6 +1069,9 @@ ssl_check_certificate (int fd, const char *host)
xfree (issuer);
}
+ if (opt.check_cert == CHECK_CERT_QUIET)
+ goto pkp_verify;
+
vresult = SSL_get_verify_result (conn);
if (vresult != X509_V_OK)
{
@@ -1240,13 +1248,13 @@ ssl_check_certificate (int fd, const char *host)
}
}
- pinsuccess = pkp_pin_peer_pubkey (cert, opt.pinnedpubkey);
- if (!pinsuccess)
- {
- logprintf (LOG_ALWAYS, _("The public key does not match pinned public key!\n"));
- success = false;
- }
-
+ pkp_verify:
+ pinsuccess = pkp_pin_peer_pubkey (cert, opt.pinnedpubkey);
+ if (!pinsuccess)
+ {
+ logprintf (LOG_ALWAYS, _("The public key does not match pinned public key!\n"));
+ success = false;
+ }
if (success)
DEBUGP (("X509 certificate successfully verified and matches host %s\n",
quiet-cert-check.patch
(application/octet-stream, 394 B)
diff --git a/src/main.c b/src/main.c
index 77b1a0b6..9ce4856a 100644
--- a/src/main.c
+++ b/src/main.c
@@ -1636,6 +1636,9 @@ main (int argc, char **argv)
if (opt.quiet && opt.show_progress == -1)
opt.show_progress = false;
+ if (opt.quiet && opt.check_cert == CHECK_CERT_OFF)
+ opt.check_cert = CHECK_CERT_QUIET;
+
/* Sanity checks. */
if (opt.verbose && opt.quiet)
{