Skip to content

Commit 2a7d943

Browse files
authored
Merge pull request ClickHouse#79383 from ClickHouse/poco-backport-openssl-buf-fix
Backport Poco fix for concurrent SecureStreamSocket access
2 parents 1a2135c + 284e8c2 commit 2a7d943

File tree

2 files changed

+41
-7
lines changed

2 files changed

+41
-7
lines changed

base/poco/NetSSL_OpenSSL/include/Poco/Net/SecureSocketImpl.h

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,16 +236,21 @@ namespace Net
236236
/// to be able to re-use it again.
237237

238238
private:
239+
using MutexT = Poco::FastMutex;
240+
using LockT = MutexT::ScopedLock;
241+
using UnLockT = Poco::ScopedLockWithUnlock<MutexT>;
242+
239243
SecureSocketImpl(const SecureSocketImpl &);
240244
SecureSocketImpl & operator=(const SecureSocketImpl &);
241245

242246
mutable std::recursive_mutex _mutex;
243-
SSL * _pSSL; // GUARDED_BY _mutex
247+
std::atomic<SSL *> _pSSL;
244248
Poco::AutoPtr<SocketImpl> _pSocket;
245249
Context::Ptr _pContext;
246250
bool _needHandshake;
247251
std::string _peerHostName;
248252
Session::Ptr _pSession;
253+
mutable MutexT _ssl_mutex;
249254

250255
friend class SecureStreamSocketImpl;
251256

base/poco/NetSSL_OpenSSL/src/SecureSocketImpl.cpp

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,8 @@ void SecureSocketImpl::acceptSSL()
103103
std::lock_guard<std::recursive_mutex> lock(_mutex);
104104
poco_assert (!_pSSL);
105105

106+
LockT l(_ssl_mutex);
107+
106108
BIO* pBIO = BIO_new(BIO_s_socket());
107109
if (!pBIO) throw SSLException("Cannot create BIO object");
108110
BIO_set_fd(pBIO, static_cast<int>(_pSocket->sockfd()), BIO_NOCLOSE);
@@ -169,6 +171,8 @@ void SecureSocketImpl::connectSSL(bool performHandshake)
169171
poco_assert (!_pSSL);
170172
poco_assert (_pSocket->initialized());
171173

174+
LockT l(_ssl_mutex);
175+
172176
BIO* pBIO = BIO_new(BIO_s_socket());
173177
if (!pBIO) throw SSLException("Cannot create SSL BIO object");
174178
BIO_set_fd(pBIO, static_cast<int>(_pSocket->sockfd()), BIO_NOCLOSE);
@@ -246,6 +250,8 @@ void SecureSocketImpl::shutdown()
246250
std::lock_guard<std::recursive_mutex> lock(_mutex);
247251
if (_pSSL)
248252
{
253+
UnLockT l(_ssl_mutex);
254+
249255
// Don't shut down the socket more than once.
250256
int shutdownState = SSL_get_shutdown(_pSSL);
251257
bool shutdownSent = (shutdownState & SSL_SENT_SHUTDOWN) == SSL_SENT_SHUTDOWN;
@@ -260,6 +266,7 @@ void SecureSocketImpl::shutdown()
260266
// done with it.
261267
int rc = SSL_shutdown(_pSSL);
262268
if (rc < 0) handleError(rc);
269+
l.unlock();
263270
if (_pSocket->getBlocking())
264271
{
265272
_pSocket->shutdown();
@@ -290,6 +297,9 @@ int SecureSocketImpl::sendBytes(const void* buffer, int length, int flags)
290297
poco_check_ptr (_pSSL);
291298

292299
int rc;
300+
301+
LockT l(_ssl_mutex);
302+
293303
if (_needHandshake)
294304
{
295305
rc = completeHandshake();
@@ -331,6 +341,8 @@ int SecureSocketImpl::receiveBytes(void* buffer, int length, int flags)
331341
poco_assert (_pSocket->initialized());
332342
poco_check_ptr (_pSSL);
333343

344+
LockT l(_ssl_mutex);
345+
334346
/// Special case: just check that we can read from socket
335347
if ((flags & MSG_DONTWAIT) && (flags & MSG_PEEK))
336348
return _pSocket->receiveBytes(buffer, length, flags);
@@ -368,6 +380,8 @@ int SecureSocketImpl::available() const
368380
std::lock_guard<std::recursive_mutex> lock(_mutex);
369381
poco_check_ptr (_pSSL);
370382

383+
LockT l(_ssl_mutex);
384+
371385
return SSL_pending(_pSSL);
372386
}
373387

@@ -464,10 +478,20 @@ bool SecureSocketImpl::isLocalHost(const std::string& hostName)
464478
X509* SecureSocketImpl::peerCertificate() const
465479
{
466480
std::lock_guard<std::recursive_mutex> lock(_mutex);
481+
LockT l(_ssl_mutex);
482+
483+
X509* pCert = nullptr;
484+
467485
if (_pSSL)
468-
return SSL_get1_peer_certificate(_pSSL);
469-
else
470-
return 0;
486+
{
487+
pCert = ::SSL_get_peer_certificate(_pSSL);
488+
489+
if (X509_V_OK != SSL_get_verify_result(_pSSL))
490+
throw CertificateValidationException("SecureSocketImpl::peerCertificate(): "
491+
"Certificate verification error " + Utility::getLastError());
492+
}
493+
494+
return pCert;
471495
}
472496

473497
Poco::Timespan SecureSocketImpl::getMaxTimeoutOrLimit()
@@ -608,6 +632,8 @@ void SecureSocketImpl::reset()
608632
close();
609633
if (_pSSL)
610634
{
635+
LockT l(_ssl_mutex);
636+
611637
SSL_free(_pSSL);
612638
_pSSL = nullptr;
613639
}
@@ -652,9 +678,12 @@ bool SecureSocketImpl::sessionWasReused()
652678
{
653679
std::lock_guard<std::recursive_mutex> lock(_mutex);
654680
if (_pSSL)
655-
return SSL_session_reused(_pSSL) != 0;
656-
else
657-
return false;
681+
{
682+
LockT l(_ssl_mutex);
683+
return ::SSL_session_reused(_pSSL) != 0;
684+
}
685+
686+
return false;
658687
}
659688

660689
void SecureSocketImpl::setBlocking(bool flag)

0 commit comments

Comments
 (0)