Skip to content
Snippets Groups Projects
Commit ef39a26f authored by Sébastien Blin's avatar Sébastien Blin Committed by Adrien Béraud
Browse files

tls_session: fix verifyOcsp potential crashes

verifyOcsp takes a callback with arguments passed by reference.
However, if the method finishes (verifyCertificateWrapper) cert
will be destroyed and references potentially invalidated.
So, instead of waiting on a condition variable use a future to
be sure that the cert is used before destruction.
The callback is always called by sendOcspRequest with a similar
timeout so also remove the useless timeout.

Change-Id: If1a8eba70e13d2d613b758ad24629efa4fe57bb3
GitLab: #577
parent 9727bd57
No related branches found
No related tags found
No related merge requests found
...@@ -107,8 +107,6 @@ static constexpr auto RX_OOO_TIMEOUT = std::chrono::milliseconds(1500); ...@@ -107,8 +107,6 @@ static constexpr auto RX_OOO_TIMEOUT = std::chrono::milliseconds(1500);
static constexpr int ASYMETRIC_TRANSPORT_MTU_OFFSET static constexpr int ASYMETRIC_TRANSPORT_MTU_OFFSET
= 20; // when client, if your local IP is IPV4 and server is IPV6; you must reduce your MTU to = 20; // when client, if your local IP is IPV4 and server is IPV6; you must reduce your MTU to
// avoid packet too big error on server side. the offset is the difference in size of IP headers // avoid packet too big error on server side. the offset is the difference in size of IP headers
static constexpr auto OCSP_VERIFICATION_TIMEOUT = std::chrono::seconds(
2); // Time to wait for an OCSP verification
static constexpr auto OCSP_REQUEST_TIMEOUT = std::chrono::seconds( static constexpr auto OCSP_REQUEST_TIMEOUT = std::chrono::seconds(
2); // Time to wait for an ocsp-request 2); // Time to wait for an ocsp-request
...@@ -685,16 +683,15 @@ TlsSession::TlsSessionImpl::verifyCertificateWrapper(gnutls_session_t session) ...@@ -685,16 +683,15 @@ TlsSession::TlsSessionImpl::verifyCertificateWrapper(gnutls_session_t session)
} }
// OCSP (Online Certificate Service Protocol) { // OCSP (Online Certificate Service Protocol) {
bool ocsp_done = false; std::promise<int> v;
std::mutex cv_m; std::future<int> f = v.get_future();
std::condition_variable cv;
std::unique_lock<std::mutex> cv_lk(cv_m);
gnutls_x509_crt_t issuer_crt = cert.issuer ? cert.issuer->cert : nullptr; gnutls_x509_crt_t issuer_crt = cert.issuer ? cert.issuer->cert : nullptr;
verifyOcsp(ocspUrl, cert, issuer_crt, [&](const int status) { verifyOcsp(ocspUrl, cert, issuer_crt, [&](const int status) {
if (status == GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE) { if (status == GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE) {
// OCSP URI is absent, don't fail the verification by overwritting the user-set one. // OCSP URI is absent, don't fail the verification by overwritting the user-set one.
JAMI_WARN("Skipping OCSP verification %s: request failed", cert.getUID().c_str()); JAMI_WARN("Skipping OCSP verification %s: request failed", cert.getUID().c_str());
v.set_value(verified);
} else { } else {
if (status != GNUTLS_E_SUCCESS) { if (status != GNUTLS_E_SUCCESS) {
JAMI_ERR("OCSP verification failed for %s: %s (%i)", JAMI_ERR("OCSP verification failed for %s: %s (%i)",
...@@ -702,18 +699,12 @@ TlsSession::TlsSessionImpl::verifyCertificateWrapper(gnutls_session_t session) ...@@ -702,18 +699,12 @@ TlsSession::TlsSessionImpl::verifyCertificateWrapper(gnutls_session_t session)
gnutls_strerror(status), gnutls_strerror(status),
status); status);
} }
verified = status; v.set_value(status);
} }
std::lock_guard<std::mutex> cv_lk(cv_m);
ocsp_done = true;
cv.notify_all();
}); });
cv.wait_for(cv_lk, std::chrono::seconds(OCSP_VERIFICATION_TIMEOUT), [&ocsp_done] { f.wait();
return ocsp_done;
});
// } OCSP
return verified; return f.get();
} }
void void
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment