Peter M. Groen / FlashMQ

Commit cc5f85e1660bd3d7deacebe81fbc2fd27560d388

Authored by Wiebe Cazemier
1 parent 8609c607

Fix SSL crash on SSL_write retry 

When SSL wants the app to retry, it stands to reason that the local
write buffer needs to be grown (read: reallocated) to hold the
application bytes. At that point, the buffer is no longer where it was
originally.

This fixes segfaults.
Inline Side-by-side
Showing 3 changed files with 15 additions and 11 deletions
iowrapper.cpp
  View file @cc5f85e
... ... @@ -22,8 +22,8 @@ License along with FlashMQ. If not, see <https://www.gnu.org/licenses/>.
22 22 #include "logger.h"
23 23 #include "client.h"
24 24  
25   -IncompleteSslWrite::IncompleteSslWrite(const void *buf, size_t nbytes) :
26   - buf(buf),
  25 +IncompleteSslWrite::IncompleteSslWrite(size_t nbytes) :
  26 + valid(true),
27 27 nbytes(nbytes)
28 28 {
29 29  
... ... @@ -31,13 +31,13 @@ IncompleteSslWrite::IncompleteSslWrite(const void *buf, size_t nbytes) :
31 31  
32 32 void IncompleteSslWrite::reset()
33 33 {
34   - buf = nullptr;
  34 + valid = false;
35 35 nbytes = 0;
36 36 }
37 37  
38 38 bool IncompleteSslWrite::hasPendingWrite() const
39 39 {
40   - return buf != nullptr;
  40 + return valid;
41 41 }
42 42  
43 43 void IncompleteWebsocketRead::reset()
... ... @@ -264,7 +264,6 @@ ssize_t IoWrapper::writeOrSslWrite(int fd, const void *buf, size_t nbytes, IoWra
264 264 }
265 265 else
266 266 {
267   - const void *buf_ = buf;
268 267 size_t nbytes_ = nbytes;
269 268  
270 269 /*
... ... @@ -273,7 +272,6 @@ ssize_t IoWrapper::writeOrSslWrite(int fd, const void *buf, size_t nbytes, IoWra
273 272 */
274 273 if (this->incompleteSslWrite.hasPendingWrite())
275 274 {
276   - buf_ = this->incompleteSslWrite.buf;
277 275 nbytes_ = this->incompleteSslWrite.nbytes;
278 276 }
279 277  
... ... @@ -285,7 +283,7 @@ ssize_t IoWrapper::writeOrSslWrite(int fd, const void *buf, size_t nbytes, IoWra
285 283  
286 284 ERR_clear_error();
287 285 char sslErrorBuf[OPENSSL_ERROR_STRING_SIZE];
288   - n = SSL_write(ssl, buf_, nbytes_);
  286 + n = SSL_write(ssl, buf, nbytes_);
289 287  
290 288 if (n <= 0)
291 289 {
... ... @@ -295,7 +293,7 @@ ssize_t IoWrapper::writeOrSslWrite(int fd, const void *buf, size_t nbytes, IoWra
295 293 {
296 294 logger->logf(LOG_DEBUG, "SSL Write is incomplete: %d. Will be retried later.", err);
297 295 *error = IoWrapResult::Wouldblock;
298   - IncompleteSslWrite sslAction(buf_, nbytes_);
  296 + IncompleteSslWrite sslAction(nbytes_);
299 297 this->incompleteSslWrite = sslAction;
300 298 if (err == SSL_ERROR_WANT_READ)
301 299 this->sslWriteWantsRead = true;
... ...
iowrapper.h
  View file @cc5f85e
... ... @@ -56,17 +56,21 @@ enum class WebsocketOpcode
56 56 Unknown = 0xF
57 57 };
58 58  
59   -/*
  59 +/**
  60 + * @brief The IncompleteSslWrite struct facilities the SSL retry
  61 + *
60 62 * OpenSSL doc: "When a write function call has to be repeated because SSL_get_error(3) returned
61 63 * SSL_ERROR_WANT_READ or SSL_ERROR_WANT_WRITE, it must be repeated with the same arguments"
  64 + *
  65 + * Note that we use SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER.
62 66 */
63 67 struct IncompleteSslWrite
64 68 {
65   - const void *buf = nullptr;
  69 + bool valid = false;
66 70 size_t nbytes = 0;
67 71  
68 72 IncompleteSslWrite() = default;
69   - IncompleteSslWrite(const void *buf, size_t nbytes);
  73 + IncompleteSslWrite(size_t nbytes);
70 74 bool hasPendingWrite() const;
71 75  
72 76 void reset();
... ...
listener.cpp
  View file @cc5f85e
... ... @@ -86,6 +86,8 @@ void Listener::loadCertAndKeyFromConfig()
86 86 sslctx = std::make_unique<SslCtxManager>();
87 87 SSL_CTX_set_options(sslctx->get(), SSL_OP_NO_SSLv3); // TODO: config option
88 88 SSL_CTX_set_options(sslctx->get(), SSL_OP_NO_TLSv1); // TODO: config option
  89 +
  90 + SSL_CTX_set_mode(sslctx->get(), SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER);
89 91 }
90 92  
91 93 if (SSL_CTX_use_certificate_file(sslctx->get(), sslFullchain.c_str(), SSL_FILETYPE_PEM) != 1)
... ...