From 9af1001682f5ef2579d27b3f27ade19754cca4cd Mon Sep 17 00:00:00 2001
From: Raul Metsma <raul@metsma.ee>
Date: Tue, 18 Aug 2026 15:45:29 +0300
Subject: [PATCH] Validate OCSP response to issuer cert and other cleanups

IB-9057

Signed-off-by: Raul Metsma <raul@metsma.ee>
---
 client/SslCertificate.cpp | 47 ++++++++++++++----------------------
 client/Utils.h            | 51 +++++++++++++++++++++++++++++----------
 2 files changed, 56 insertions(+), 42 deletions(-)

diff --git a/client/SslCertificate.cpp b/client/SslCertificate.cpp
index f43e29df3..ced423af5 100644
--- a/client/SslCertificate.cpp
+++ b/client/SslCertificate.cpp
@@ -20,6 +20,7 @@
 #include "SslCertificate.h"
 
 #include "Common.h"
+#include "Utils.h"
 
 #include <digidocpp/Exception.h>
 #include <digidocpp/crypto/X509Cert.h>
@@ -35,18 +36,6 @@
 #include <openssl/ocsp.h>
 #include <openssl/x509v3.h>
 
-#include <memory>
-
-template<auto D>
-struct free_deleter
-{
-	template<class T>
-	void operator()(T *p) const noexcept
-	{
-		D(p);
-	}
-};
-
 template<typename> struct free_argument;
 template<class T, class R>
 struct free_argument<R (*)(T *)>
@@ -59,13 +48,6 @@ struct free_argument<R (&)(T *)>
 	using type = T;
 };
 
-template<auto F, typename T>
-[[nodiscard]]
-constexpr auto make_unique_ptr(T *t) noexcept
-{
-	return std::unique_ptr<T, free_deleter<F>>(t);
-}
-
 template<class T>
 static auto toQByteArray(T &x)
 {
@@ -356,30 +338,34 @@ SslCertificate::Validity SslCertificate::validateOnline() const
 	if(urls.isEmpty())
 		return Error;
 
-	QEventLoop e;
 	QNetworkAccessManager m;
-	QNetworkAccessManager::connect(&m, &QNetworkAccessManager::finished, &e, &QEventLoop::quit);
-	QNetworkAccessManager::connect(&m, &QNetworkAccessManager::sslErrors, &m,
-		[](QNetworkReply *reply, const QList<QSslError> &errors){
-		reply->ignoreSslErrors(errors);
-	});
 
 	// Get issuer
 	QNetworkRequest r(urls.values(SslCertificate::ad_CAIssuers).first());
 	r.setRawHeader("User-Agent", QStringLiteral("%1/%2 (%3)")
 		.arg(QCoreApplication::applicationName(), QCoreApplication::applicationVersion(), Common::applicationOs()).toUtf8());
+	r.setTransferTimeout(15000);
 	QNetworkReply *repl = m.get(r);
-	e.exec();
+	waitForSignal(&m, &QNetworkAccessManager::finished);
 	QSslCertificate issuer(repl->readAll(), QSsl::Der);
 	repl->deleteLater();
 	if(issuer.isNull())
 		return Error;
 
+	// Verify cert
+	auto *cert = static_cast<X509 *>(handle());
+	auto *issuerCert = static_cast<X509 *>(issuer.handle());
+	if(EVP_PKEY *issuerKey = X509_get0_pubkey(issuerCert);
+		!issuerKey ||
+		X509_check_issued(issuerCert, cert) != X509_V_OK ||
+		X509_verify(cert, issuerKey) != 1)
+		return Invalid;
+
 	// Build request
 	auto ocspReq = make_unique_ptr<OCSP_REQUEST_free>(OCSP_REQUEST_new());
 	if(!ocspReq)
 		return Error;
-	OCSP_CERTID *certId = OCSP_cert_to_id(nullptr, (X509*)handle(), (X509*)issuer.handle());
+	OCSP_CERTID *certId = OCSP_cert_to_id(nullptr, cert, issuerCert);
 	if(!OCSP_request_add0_id(ocspReq.get(), certId))
 		return Error;
 
@@ -387,7 +373,7 @@ SslCertificate::Validity SslCertificate::validateOnline() const
 	r.setUrl(urls.values(SslCertificate::ad_OCSP).first());
 	r.setHeader(QNetworkRequest::ContentTypeHeader, "application/ocsp-request");
 	repl = m.post(r, i2dDer<i2d_OCSP_REQUEST>(ocspReq.get()));
-	e.exec();
+	waitForSignal(&m, &QNetworkAccessManager::finished);
 
 	// Parse response
 	QByteArray respData = repl->readAll();
@@ -401,7 +387,10 @@ SslCertificate::Validity SslCertificate::validateOnline() const
 	auto basic = make_unique_ptr<OCSP_BASICRESP_free>(OCSP_response_get1_basic(resp.get()));
 	if(!basic)
 		return Error;
-	if(OCSP_basic_verify(basic.get(), nullptr, nullptr, OCSP_NOVERIFY) <= 0)
+	auto store = make_unique_ptr<X509_STORE_free>(X509_STORE_new());
+	if(!store || !X509_STORE_add_cert(store.get(), issuerCert))
+		return Error;
+	if(OCSP_basic_verify(basic.get(), nullptr, store.get(), OCSP_PARTIAL_CHAIN) <= 0)
 		return Invalid;
 	int status = -1;
 	if(OCSP_resp_find_status(basic.get(), certId, &status, nullptr, nullptr, nullptr, nullptr) <= 0)
diff --git a/client/Utils.h b/client/Utils.h
index c8c317fe7..1369c9e83 100644
--- a/client/Utils.h
+++ b/client/Utils.h
@@ -24,27 +24,52 @@
 #include <QTimer>
 
 #include <exception>
+#include <future>
 #include <limits>
+#include <memory>
 #include <thread>
 
 namespace {
+	template<auto D>
+	struct free_deleter
+	{
+		template<class T>
+		void operator()(T *p) const noexcept
+		{
+			D(p);
+		}
+	};
+
+	template<auto F, typename T>
+	[[nodiscard]]
+	constexpr auto make_unique_ptr(T *t) noexcept
+	{
+		return std::unique_ptr<T, free_deleter<F>>(t);
+	}
+
 	template <typename F, class... Args>
 	inline auto waitFor(F&& function, Args&& ...args) {
-		std::exception_ptr exception;
-		std::invoke_result_t<F,Args...> result{};
 		QEventLoop l;
-		std::thread([&, function = std::forward<F>(function), ...args = std::forward<Args>(args)]{
-			try {
-				result = std::invoke(function, args...);
-			} catch(...) {
-				exception = std::current_exception();
-			}
-			l.exit();
-		}).detach();
+		using result_t = std::invoke_result_t<F, Args...>;
+		std::packaged_task<result_t()> task(
+			[function = std::forward<F>(function),
+			...args = std::forward<Args>(args)]() mutable -> result_t {
+				return std::invoke(function, args...);
+			});
+		auto future = task.get_future();
+		std::jthread worker([&l, task = std::move(task)]() mutable {
+			task();
+			QMetaObject::invokeMethod(&l, &QEventLoop::quit, Qt::QueuedConnection);
+		});
+		l.exec();
+		return future.get();
+	}
+
+	template <typename Sender, typename Signal>
+	inline void waitForSignal(Sender *sender, Signal signal) {
+		QEventLoop l;
+		QObject::connect(sender, signal, &l, &QEventLoop::quit);
 		l.exec();
-		if(exception)
-			std::rethrow_exception(std::move(exception));
-		return result;
 	}
 
 	template <typename F, class... Args>