Skip to content

Commit 4c27533

Browse files
sgerbinosgerbino
committed
fix: re-seat the TLS impl's transport pointer on move
Both TLS stream impls referenced their owner's stream_ member, so moving an openssl_stream or wolfssl_stream left the impl reading and writing through the moved-from object's empty any_stream. Store a pointer instead and re-seat it onto the new owner's stream_ in the move constructor and move assignment. Add shared TLS tests: move construction before the handshake and move assignment of a live session followed by data transfer, reads and shutdown after the peer closes the transport without close_notify (stream_truncated), a handshake whose server key is AES-encrypted PEM loaded through the password callback, an unparseable-credentials context failing the handshake cleanly, and a no-SNI client against a server with a servername callback. Encrypted-key decryption is a compile-time wolfSSL feature, so that backend only requires a clean outcome with the callback invoked; a failsafe timer tears down the transport if a feature-limited build stalls the handshake.
1 parent 018e9d7 commit 4c27533

6 files changed

Lines changed: 384 additions & 25 deletions

File tree

‎src/openssl/src/openssl_stream.cpp‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@
3636
3737
Data Flow (using BIO pairs)
3838
---------------------------
39-
App -> SSL_write -> int_bio_ -> BIO_read(ext_bio_) -> out_buf_ -> s_.write_some -> Network
40-
App <- SSL_read <- int_bio_ <- BIO_write(ext_bio_) <- in_buf_ <- s_.read_some <- Network
39+
App -> SSL_write -> int_bio_ -> BIO_read(ext_bio_) -> out_buf_ -> s_->write_some -> Network
40+
App <- SSL_read <- int_bio_ <- BIO_write(ext_bio_) <- in_buf_ <- s_->read_some <- Network
4141
4242
WANT_READ / WANT_WRITE Pattern
4343
------------------------------
@@ -46,8 +46,8 @@
4646
4747
1. Call SSL_read or SSL_write
4848
2. Check for pending output in ext_bio_ via BIO_ctrl_pending
49-
3. If output pending: write to network via s_.write_some
50-
4. If SSL_ERROR_WANT_READ: read from network into ext_bio_ via s_.read_some + BIO_write
49+
3. If output pending: write to network via s_->write_some
50+
4. If SSL_ERROR_WANT_READ: read from network into ext_bio_ via s_->read_some + BIO_write
5151
5. Loop back to step 1
5252
5353
Renegotiation causes cross-direction I/O: SSL_read may need to write
@@ -311,7 +311,9 @@ get_openssl_context(tls_context_data const& cd)
311311

312312
struct openssl_stream::impl
313313
{
314-
capy::any_stream& s_;
314+
// Pointer rather than reference: the owning stream re-seats it
315+
// onto its own stream_ member after a move.
316+
capy::any_stream* s_;
315317
tls_context ctx_;
316318
SSL* ssl_ = nullptr;
317319
BIO* ext_bio_ = nullptr;
@@ -322,7 +324,7 @@ struct openssl_stream::impl
322324

323325
capy::async_mutex io_cm_;
324326

325-
impl(capy::any_stream& s, tls_context ctx) : s_(s), ctx_(std::move(ctx))
327+
impl(capy::any_stream& s, tls_context ctx) : s_(&s), ctx_(std::move(ctx))
326328
{
327329
in_buf_.resize(default_buffer_size);
328330
out_buf_.resize(default_buffer_size);
@@ -379,7 +381,7 @@ struct openssl_stream::impl
379381
co_return lec;
380382
capy::async_mutex::lock_guard io_guard(&io_cm_);
381383
auto [ec, n] = co_await capy::write(
382-
s_, capy::const_buffer(out_buf_.data(), got));
384+
*s_, capy::const_buffer(out_buf_.data(), got));
383385
if (ec)
384386
co_return ec;
385387
}
@@ -393,7 +395,7 @@ struct openssl_stream::impl
393395
if (lec)
394396
co_return lec;
395397
capy::async_mutex::lock_guard io_guard(&io_cm_);
396-
auto [ec, n] = co_await s_.read_some(
398+
auto [ec, n] = co_await s_->read_some(
397399
capy::mutable_buffer(in_buf_.data(), in_buf_.size()));
398400
if (ec)
399401
co_return ec;
@@ -738,6 +740,9 @@ openssl_stream::openssl_stream(openssl_stream&& other) noexcept
738740
, impl_(other.impl_)
739741
{
740742
other.impl_ = nullptr;
743+
// The impl points at its owner's stream_; re-seat it on ours.
744+
if (impl_)
745+
impl_->s_ = &stream_;
741746
}
742747

743748
openssl_stream&
@@ -749,6 +754,9 @@ openssl_stream::operator=(openssl_stream&& other) noexcept
749754
stream_ = std::move(other.stream_);
750755
impl_ = other.impl_;
751756
other.impl_ = nullptr;
757+
// The impl points at its owner's stream_; re-seat it on ours.
758+
if (impl_)
759+
impl_->s_ = &stream_;
752760
}
753761
return *this;
754762
}

‎src/wolfssl/src/wolfssl_stream.cpp‎

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@
3737
3838
Data Flow
3939
---------
40-
App -> wolfSSL_write -> send_callback -> out_buf_ -> s_.write_some -> Network
41-
App <- wolfSSL_read <- recv_callback <- in_buf_ <- s_.read_some <- Network
40+
App -> wolfSSL_write -> send_callback -> out_buf_ -> s_->write_some -> Network
41+
App <- wolfSSL_read <- recv_callback <- in_buf_ <- s_->read_some <- Network
4242
4343
WANT_READ / WANT_WRITE Pattern
4444
------------------------------
@@ -49,7 +49,7 @@
4949
2. If data available: return it immediately
5050
3. If not: return WOLFSSL_CBIO_ERR_WANT_READ or WANT_WRITE
5151
4. wolfSSL_read/write returns WOLFSSL_ERROR_WANT_*
52-
5. Our coroutine does async I/O: co_await s_.read_some() or write_some()
52+
5. Our coroutine does async I/O: co_await s_->read_some() or write_some()
5353
6. Loop back to step 1
5454
5555
Renegotiation causes cross-direction I/O: SSL_read may need to write
@@ -300,7 +300,9 @@ get_wolfssl_native_context(tls_context_data const& cd)
300300

301301
struct wolfssl_stream::impl
302302
{
303-
capy::any_stream& s_;
303+
// Pointer rather than reference: the owning stream re-seats it
304+
// onto its own stream_ member after a move.
305+
capy::any_stream* s_;
304306
tls_context ctx_;
305307
WOLFSSL* ssl_ = nullptr;
306308
bool used_ = false;
@@ -336,7 +338,7 @@ struct wolfssl_stream::impl
336338
// Renegotiation can cause both TLS read/write to access the socket
337339
capy::async_mutex io_cm_;
338340

339-
impl(capy::any_stream& s, tls_context ctx) : s_(s), ctx_(std::move(ctx))
341+
impl(capy::any_stream& s, tls_context ctx) : s_(&s), ctx_(std::move(ctx))
340342
{
341343
read_in_buf_.resize(default_buffer_size);
342344
read_out_buf_.resize(default_buffer_size);
@@ -501,7 +503,7 @@ struct wolfssl_stream::impl
501503
co_return {lec, total_read};
502504
}
503505
capy::async_mutex::lock_guard io_guard(&io_cm_);
504-
auto [rec, rn] = co_await s_.read_some(rbuf);
506+
auto [rec, rn] = co_await s_->read_some(rbuf);
505507
if (rec)
506508
{
507509
if (rec == make_error_code(capy::error::eof))
@@ -535,7 +537,7 @@ struct wolfssl_stream::impl
535537
}
536538
capy::async_mutex::lock_guard io_guard(&io_cm_);
537539
auto [wec, wn] = co_await capy::write(
538-
s_,
540+
*s_,
539541
capy::const_buffer(
540542
read_out_buf_.data(), read_out_len_));
541543
read_out_len_ = 0;
@@ -614,7 +616,7 @@ struct wolfssl_stream::impl
614616
}
615617
capy::async_mutex::lock_guard io_guard(&io_cm_);
616618
auto [wec, wn] = co_await capy::write(
617-
s_,
619+
*s_,
618620
capy::const_buffer(
619621
write_out_buf_.data(), write_out_len_));
620622
write_out_len_ = 0;
@@ -644,7 +646,7 @@ struct wolfssl_stream::impl
644646
}
645647
capy::async_mutex::lock_guard io_guard(&io_cm_);
646648
auto [wec, wn] = co_await capy::write(
647-
s_,
649+
*s_,
648650
capy::const_buffer(
649651
write_out_buf_.data(), write_out_len_));
650652
write_out_len_ = 0;
@@ -673,7 +675,7 @@ struct wolfssl_stream::impl
673675
co_return {lec, total_written};
674676
}
675677
capy::async_mutex::lock_guard io_guard(&io_cm_);
676-
auto [rec, rn] = co_await s_.read_some(rbuf);
678+
auto [rec, rn] = co_await s_->read_some(rbuf);
677679
if (rec)
678680
{
679681
current_op_ = nullptr;
@@ -742,7 +744,7 @@ struct wolfssl_stream::impl
742744
}
743745
capy::async_mutex::lock_guard io_guard(&io_cm_);
744746
auto [wec, wn] = co_await capy::write(
745-
s_,
747+
*s_,
746748
capy::const_buffer(
747749
read_out_buf_.data(), read_out_len_));
748750
read_out_len_ = 0;
@@ -768,7 +770,7 @@ struct wolfssl_stream::impl
768770
}
769771
capy::async_mutex::lock_guard io_guard(&io_cm_);
770772
auto [wec, wn] = co_await capy::write(
771-
s_,
773+
*s_,
772774
capy::const_buffer(
773775
read_out_buf_.data(), read_out_len_));
774776
read_out_len_ = 0;
@@ -794,7 +796,7 @@ struct wolfssl_stream::impl
794796
break;
795797
}
796798
capy::async_mutex::lock_guard io_guard(&io_cm_);
797-
auto [rec, rn] = co_await s_.read_some(rbuf);
799+
auto [rec, rn] = co_await s_->read_some(rbuf);
798800
if (rec)
799801
{
800802
ec = rec;
@@ -814,7 +816,7 @@ struct wolfssl_stream::impl
814816
}
815817
capy::async_mutex::lock_guard io_guard(&io_cm_);
816818
auto [wec, wn] = co_await capy::write(
817-
s_,
819+
*s_,
818820
capy::const_buffer(
819821
read_out_buf_.data(), read_out_len_));
820822
read_out_len_ = 0;
@@ -870,7 +872,7 @@ struct wolfssl_stream::impl
870872
}
871873
capy::async_mutex::lock_guard io_guard(&io_cm_);
872874
auto [wec, wn] = co_await capy::write(
873-
s_,
875+
*s_,
874876
capy::const_buffer(
875877
read_out_buf_.data(), read_out_len_));
876878
read_out_len_ = 0;
@@ -892,7 +894,7 @@ struct wolfssl_stream::impl
892894
}
893895
capy::async_mutex::lock_guard io_guard(&io_cm_);
894896
auto [wec, wn] = co_await capy::write(
895-
s_,
897+
*s_,
896898
capy::const_buffer(
897899
read_out_buf_.data(), read_out_len_));
898900
read_out_len_ = 0;
@@ -918,7 +920,7 @@ struct wolfssl_stream::impl
918920
break;
919921
}
920922
capy::async_mutex::lock_guard io_guard(&io_cm_);
921-
auto [rec, rn] = co_await s_.read_some(rbuf);
923+
auto [rec, rn] = co_await s_->read_some(rbuf);
922924
if (rec)
923925
break; // EOF or socket error during shutdown read - acceptable
924926
read_in_len_ += rn;
@@ -1029,6 +1031,9 @@ wolfssl_stream::wolfssl_stream(wolfssl_stream&& other) noexcept
10291031
, impl_(other.impl_)
10301032
{
10311033
other.impl_ = nullptr;
1034+
// The impl points at its owner's stream_; re-seat it on ours.
1035+
if (impl_)
1036+
impl_->s_ = &stream_;
10321037
}
10331038

10341039
wolfssl_stream&
@@ -1040,6 +1045,9 @@ wolfssl_stream::operator=(wolfssl_stream&& other) noexcept
10401045
stream_ = std::move(other.stream_);
10411046
impl_ = other.impl_;
10421047
other.impl_ = nullptr;
1048+
// The impl points at its owner's stream_; re-seat it on ours.
1049+
if (impl_)
1050+
impl_->s_ = &stream_;
10431051
}
10441052
return *this;
10451053
}

‎test/unit/openssl_stream.cpp‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,10 @@ struct openssl_stream_test
118118
test::testSni(make_stream);
119119
test::testSniCallback(make_stream);
120120
test::testMtls(make_stream);
121+
test::testMoveSemantics(make_stream);
122+
test::testAbruptClose(make_stream);
123+
test::testEncryptedKey(make_stream);
124+
test::testInvalidContextHandshake(make_stream);
121125

122126
test::testReset(make_stream, cert_modes);
123127
test::testResetViaHandshake(make_stream, cert_modes);

‎test/unit/test_utils.hpp‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -624,6 +624,67 @@ make_wrong_ca_context()
624624
return ctx;
625625
}
626626

627+
/** server_key_pem encrypted with AES-128-CBC (traditional PEM).
628+
Passphrase: "test-password"
629+
Command:
630+
openssl rsa -traditional -in server_key.pem -aes128 -passout pass:test-password
631+
*/
632+
inline constexpr char const* encrypted_server_key_pem =
633+
"-----BEGIN RSA PRIVATE KEY-----\n"
634+
"Proc-Type: 4,ENCRYPTED\n"
635+
"DEK-Info: AES-128-CBC,8042604E303B1469DCCA597EA6555AA9\n"
636+
"\n"
637+
"c6KSpVasopI6/gsa+NOVnOsMGJG8Y1emu3J4aLsCoYkHWQT7axL/ZLTH7AjRn4Sj\n"
638+
"HD+gP/4QA6UMdVqShesc4zd1bEhq2RV3yir6ast6IfXB3zpq4ggK2cLMYYOo4VwF\n"
639+
"jKP4XTN8KL3xLQssWdahPsPnkFBR0M/VCo6YCpuZi8zc5iAOHzkGVXf3rCkRORtO\n"
640+
"5qssbqV33FzwHPeH0PuKWZxcApYfLSRA7vt/bqIxg49WhXGJMhkv4qFpSoaiZ5Tc\n"
641+
"HlNLfrTbG+A0zUh6f3Oeqm6F95LP7yi2TGxM7oGiJtkwksWoEvS4d9p+NmRNKr0i\n"
642+
"gZJjOUKq9oxPzFdcRS1H7AkDjraw3q1NTxrcFlXbjtxDm9t6GkLuwXfUz0n31IL9\n"
643+
"/jQX3/N3eVKp8mHMRWmdtllmK0/XeYBH5A4MKpBz4/smuDoStMsmezP4+0qYyE2H\n"
644+
"YUHaTlTrJQBG/BpIJmJk7BfzqGoTg3dgSNxMc1gYpCwcK0X/w+6HzEOtMjGWGhv1\n"
645+
"Cbq03wMH8BLdIFMNRpnN0cLUBJItnn58AxDg0JsIii7jxEpAVVpl3tTC5Lk3q97k\n"
646+
"15fpGhMKya8iZ0EFs6jdjuJoGNl7+KnMoet0cQ1CyV5uSzus6qBdz6ATHsF8KTRj\n"
647+
"xNCXc1cbEt8A2soazFBUVeN1MQFt5yTKLCEz1Sb4M9Wn3IT07JUpWjGZLIrz9KWe\n"
648+
"JU2oKjSxQmKbAPmpRRe84MR5O8qCytCAstR/GG+qU1HZsW/bgB/2RTY7kcTeOpke\n"
649+
"/jw2iZUygNkXgCiik+LqwAbchkVw6ImRYCTdaXfq+bFwV+2JBUMfotCgxawFVaDc\n"
650+
"EdEF18g01II/xxr9HGA6hCJioP4curRiJNeqqs0S+4nJAxv8IiesdbmzbxBzAPql\n"
651+
"bzcfmrMH2qrDEJg1NiPaMODwfcdpgfjE5yMewI+nCRuOWSVPzk092zmPI9qv1xFK\n"
652+
"B+76CzH88yGESip3x2Zsce1FB/HV2CBcvzjIkkMtHdFzpXidg0bS7vDG4WTyGdti\n"
653+
"9z3K31rD0qi6IRtQlLiHZIaKhSMNqPnfePRCru0S4rHWGcVtdrsKaHL201meLSPi\n"
654+
"Iw82DRbNVs/lLPGtMJ0DzKbJvTMWFoC0zZ/nIaRa2hi7nd/Ig0oD19cVIkB4IWPp\n"
655+
"tuR53NZs9EsEqjNnNmn5ftdTAO8P50EulaaPjJggIF59C9u74hhgI+UvMrnOhq0C\n"
656+
"nlyfUgl6nS0WgBc+rtCdB59xppPobefyZ06Rud4QFJmz/wdazXzuimPCkpNDv/8t\n"
657+
"lbS952xleT2dYawtho4K+ZMV6q2SFfjAXZjhy/fbVP1vqqXOZPeEC4n070ITifHO\n"
658+
"GN/5evJpcYxohXB2paKkwHWeN9oaolBXBpW5zZKNacEcu/uFWjN8UYuxO/9/Wls7\n"
659+
"yimyYsmoYtbxVTH5H+UE4TlaIhGXy46VzYRkEYx43m7laIPdyyctQV4cb7Xec9ZS\n"
660+
"o/UWTdrl/e0k0UoftWRH4a5RCubDQ21khoyTabVSfiJY48GEAUExGItiG6C7KZ/S\n"
661+
"xXW6nBG4TmkXJYEwVNJfVygvVDjFPrHtKhFLHs7nsigDK0jNvTbAXNhhCrSMkLC8\n"
662+
"-----END RSA PRIVATE KEY-----\n";
663+
664+
/// Passphrase matching `encrypted_server_key_pem`.
665+
inline constexpr char const* encrypted_key_password = "test-password";
666+
667+
/** Create a server context whose key needs the password callback. */
668+
inline tls_context
669+
make_encrypted_key_server_context(bool& callback_invoked)
670+
{
671+
tls_context ctx;
672+
ctx.use_certificate(
673+
server_cert_pem,
674+
tls_file_format::pem); // NOLINT(bugprone-unused-return-value)
675+
ctx.set_password_callback(
676+
[&callback_invoked](std::size_t, tls_password_purpose) {
677+
callback_invoked = true;
678+
return std::string(encrypted_key_password);
679+
});
680+
ctx.use_private_key(
681+
encrypted_server_key_pem,
682+
tls_file_format::pem); // NOLINT(bugprone-unused-return-value)
683+
ctx.set_verify_mode(
684+
tls_verify_mode::none); // NOLINT(bugprone-unused-return-value)
685+
return ctx;
686+
}
687+
627688
/** Create a context that requires peer verification but has no cert. */
628689
inline tls_context
629690
make_verify_no_cert_context()

0 commit comments

Comments
 (0)