Skip to content

Commit 4784eb0

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 44a1785 commit 4784eb0

6 files changed

Lines changed: 376 additions & 25 deletions

File tree

‎src/openssl/src/openssl_stream.cpp‎

Lines changed: 12 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,7 @@ get_openssl_context(tls_context_data const& cd)
311311

312312
struct openssl_stream::impl
313313
{
314-
capy::any_stream& s_;
314+
capy::any_stream* s_;
315315
tls_context ctx_;
316316
SSL* ssl_ = nullptr;
317317
BIO* ext_bio_ = nullptr;
@@ -322,7 +322,7 @@ struct openssl_stream::impl
322322

323323
capy::async_mutex io_cm_;
324324

325-
impl(capy::any_stream& s, tls_context ctx) : s_(s), ctx_(std::move(ctx))
325+
impl(capy::any_stream& s, tls_context ctx) : s_(&s), ctx_(std::move(ctx))
326326
{
327327
in_buf_.resize(default_buffer_size);
328328
out_buf_.resize(default_buffer_size);
@@ -379,7 +379,7 @@ struct openssl_stream::impl
379379
co_return lec;
380380
capy::async_mutex::lock_guard io_guard(&io_cm_);
381381
auto [ec, n] = co_await capy::write(
382-
s_, capy::const_buffer(out_buf_.data(), got));
382+
*s_, capy::const_buffer(out_buf_.data(), got));
383383
if (ec)
384384
co_return ec;
385385
}
@@ -393,7 +393,7 @@ struct openssl_stream::impl
393393
if (lec)
394394
co_return lec;
395395
capy::async_mutex::lock_guard io_guard(&io_cm_);
396-
auto [ec, n] = co_await s_.read_some(
396+
auto [ec, n] = co_await s_->read_some(
397397
capy::mutable_buffer(in_buf_.data(), in_buf_.size()));
398398
if (ec)
399399
co_return ec;
@@ -738,6 +738,8 @@ openssl_stream::openssl_stream(openssl_stream&& other) noexcept
738738
, impl_(other.impl_)
739739
{
740740
other.impl_ = nullptr;
741+
if (impl_)
742+
impl_->s_ = &stream_;
741743
}
742744

743745
openssl_stream&
@@ -749,6 +751,8 @@ openssl_stream::operator=(openssl_stream&& other) noexcept
749751
stream_ = std::move(other.stream_);
750752
impl_ = other.impl_;
751753
other.impl_ = nullptr;
754+
if (impl_)
755+
impl_->s_ = &stream_;
752756
}
753757
return *this;
754758
}

‎src/wolfssl/src/wolfssl_stream.cpp‎

Lines changed: 21 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,7 @@ get_wolfssl_native_context(tls_context_data const& cd)
300300

301301
struct wolfssl_stream::impl
302302
{
303-
capy::any_stream& s_;
303+
capy::any_stream* s_;
304304
tls_context ctx_;
305305
WOLFSSL* ssl_ = nullptr;
306306
bool used_ = false;
@@ -336,7 +336,7 @@ struct wolfssl_stream::impl
336336
// Renegotiation can cause both TLS read/write to access the socket
337337
capy::async_mutex io_cm_;
338338

339-
impl(capy::any_stream& s, tls_context ctx) : s_(s), ctx_(std::move(ctx))
339+
impl(capy::any_stream& s, tls_context ctx) : s_(&s), ctx_(std::move(ctx))
340340
{
341341
read_in_buf_.resize(default_buffer_size);
342342
read_out_buf_.resize(default_buffer_size);
@@ -501,7 +501,7 @@ struct wolfssl_stream::impl
501501
co_return {lec, total_read};
502502
}
503503
capy::async_mutex::lock_guard io_guard(&io_cm_);
504-
auto [rec, rn] = co_await s_.read_some(rbuf);
504+
auto [rec, rn] = co_await s_->read_some(rbuf);
505505
if (rec)
506506
{
507507
if (rec == make_error_code(capy::error::eof))
@@ -535,7 +535,7 @@ struct wolfssl_stream::impl
535535
}
536536
capy::async_mutex::lock_guard io_guard(&io_cm_);
537537
auto [wec, wn] = co_await capy::write(
538-
s_,
538+
*s_,
539539
capy::const_buffer(
540540
read_out_buf_.data(), read_out_len_));
541541
read_out_len_ = 0;
@@ -614,7 +614,7 @@ struct wolfssl_stream::impl
614614
}
615615
capy::async_mutex::lock_guard io_guard(&io_cm_);
616616
auto [wec, wn] = co_await capy::write(
617-
s_,
617+
*s_,
618618
capy::const_buffer(
619619
write_out_buf_.data(), write_out_len_));
620620
write_out_len_ = 0;
@@ -644,7 +644,7 @@ struct wolfssl_stream::impl
644644
}
645645
capy::async_mutex::lock_guard io_guard(&io_cm_);
646646
auto [wec, wn] = co_await capy::write(
647-
s_,
647+
*s_,
648648
capy::const_buffer(
649649
write_out_buf_.data(), write_out_len_));
650650
write_out_len_ = 0;
@@ -673,7 +673,7 @@ struct wolfssl_stream::impl
673673
co_return {lec, total_written};
674674
}
675675
capy::async_mutex::lock_guard io_guard(&io_cm_);
676-
auto [rec, rn] = co_await s_.read_some(rbuf);
676+
auto [rec, rn] = co_await s_->read_some(rbuf);
677677
if (rec)
678678
{
679679
current_op_ = nullptr;
@@ -742,7 +742,7 @@ struct wolfssl_stream::impl
742742
}
743743
capy::async_mutex::lock_guard io_guard(&io_cm_);
744744
auto [wec, wn] = co_await capy::write(
745-
s_,
745+
*s_,
746746
capy::const_buffer(
747747
read_out_buf_.data(), read_out_len_));
748748
read_out_len_ = 0;
@@ -768,7 +768,7 @@ struct wolfssl_stream::impl
768768
}
769769
capy::async_mutex::lock_guard io_guard(&io_cm_);
770770
auto [wec, wn] = co_await capy::write(
771-
s_,
771+
*s_,
772772
capy::const_buffer(
773773
read_out_buf_.data(), read_out_len_));
774774
read_out_len_ = 0;
@@ -794,7 +794,7 @@ struct wolfssl_stream::impl
794794
break;
795795
}
796796
capy::async_mutex::lock_guard io_guard(&io_cm_);
797-
auto [rec, rn] = co_await s_.read_some(rbuf);
797+
auto [rec, rn] = co_await s_->read_some(rbuf);
798798
if (rec)
799799
{
800800
ec = rec;
@@ -814,7 +814,7 @@ struct wolfssl_stream::impl
814814
}
815815
capy::async_mutex::lock_guard io_guard(&io_cm_);
816816
auto [wec, wn] = co_await capy::write(
817-
s_,
817+
*s_,
818818
capy::const_buffer(
819819
read_out_buf_.data(), read_out_len_));
820820
read_out_len_ = 0;
@@ -870,7 +870,7 @@ struct wolfssl_stream::impl
870870
}
871871
capy::async_mutex::lock_guard io_guard(&io_cm_);
872872
auto [wec, wn] = co_await capy::write(
873-
s_,
873+
*s_,
874874
capy::const_buffer(
875875
read_out_buf_.data(), read_out_len_));
876876
read_out_len_ = 0;
@@ -892,7 +892,7 @@ struct wolfssl_stream::impl
892892
}
893893
capy::async_mutex::lock_guard io_guard(&io_cm_);
894894
auto [wec, wn] = co_await capy::write(
895-
s_,
895+
*s_,
896896
capy::const_buffer(
897897
read_out_buf_.data(), read_out_len_));
898898
read_out_len_ = 0;
@@ -918,7 +918,7 @@ struct wolfssl_stream::impl
918918
break;
919919
}
920920
capy::async_mutex::lock_guard io_guard(&io_cm_);
921-
auto [rec, rn] = co_await s_.read_some(rbuf);
921+
auto [rec, rn] = co_await s_->read_some(rbuf);
922922
if (rec)
923923
break; // EOF or socket error during shutdown read - acceptable
924924
read_in_len_ += rn;
@@ -1029,6 +1029,8 @@ wolfssl_stream::wolfssl_stream(wolfssl_stream&& other) noexcept
10291029
, impl_(other.impl_)
10301030
{
10311031
other.impl_ = nullptr;
1032+
if (impl_)
1033+
impl_->s_ = &stream_;
10321034
}
10331035

10341036
wolfssl_stream&
@@ -1040,6 +1042,8 @@ wolfssl_stream::operator=(wolfssl_stream&& other) noexcept
10401042
stream_ = std::move(other.stream_);
10411043
impl_ = other.impl_;
10421044
other.impl_ = nullptr;
1045+
if (impl_)
1046+
impl_->s_ = &stream_;
10431047
}
10441048
return *this;
10451049
}

‎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)