Skip to content

Commit

Permalink
bogo fix: user_canceled should be ignored
Browse files Browse the repository at this point in the history
Co-Authored-By: Hannes Rantzsch <hannes.rantzsch@nexenio.com>
  • Loading branch information
reneme and Hannes Rantzsch committed Mar 23, 2022
1 parent 33777bc commit 262bbda
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 6 deletions.
2 changes: 2 additions & 0 deletions src/bogo_shim/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@

"*EarlyData*": "No TLS 1.3 Early Data, yet",

"FailCertCallback-Client-TLS13": "No client auth in TLS 1.3, yet",

"KeyUpdate-RequestACK-UnfinishedWrite": "-read-with-unfinished-write currently not supported in the shim",
"TooManyKeyUpdates": "NYI",

Expand Down
29 changes: 23 additions & 6 deletions src/lib/tls/tls13/tls_channel_impl_13.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,21 @@
#include <botan/tls_messages.h>

namespace {
bool is_closure_alert(const Botan::TLS::Alert& alert)
bool is_user_canceled_alert(const Botan::TLS::Alert& alert)
{
return alert.type() == Botan::TLS::Alert::CLOSE_NOTIFY
|| alert.type() == Botan::TLS::Alert::USER_CANCELED;
return alert.type() == Botan::TLS::Alert::USER_CANCELED;
}

bool is_close_notify_alert(const Botan::TLS::Alert& alert)
{
return alert.type() == Botan::TLS::Alert::CLOSE_NOTIFY;
}

bool is_error_alert(const Botan::TLS::Alert& alert)
{
// In TLS 1.3 all alerts except for closure alerts are considered error alerts.
// (RFC 8446 6.)
return !is_closure_alert(alert);
return !is_close_notify_alert(alert) && !is_user_canceled_alert(alert);
}
}

Expand Down Expand Up @@ -73,6 +77,13 @@ size_t Channel_Impl_13::received_data(const uint8_t input[], size_t input_size)

while(true)
{
// RFC 8446 6.1
// Any data received after a closure alert has been received MUST be ignored.
//
// ... this data might already be in the record layer's read buffer.
if(!m_can_read)
{ return 0; }

auto result = m_record_layer.next_record(m_cipher_state.get());

if(std::holds_alternative<BytesNeeded>(result))
Expand Down Expand Up @@ -251,7 +262,7 @@ void Channel_Impl_13::send_alert(const Alert& alert)
// Each party MUST send a "close_notify" alert before closing its write
// side of the connection, unless it has already sent some error alert.
// This does not have any effect on its read side of the connection.
if(is_closure_alert(alert))
if(is_close_notify_alert(alert))
{
m_can_write = false;
m_cipher_state->clear_write_keys();
Expand Down Expand Up @@ -305,10 +316,16 @@ void Channel_Impl_13::process_alert(const secure_vector<uint8_t>& record)
{
Alert alert(record);

if(is_closure_alert(alert))
if(is_close_notify_alert(alert))
{
m_can_read = false;
m_cipher_state->clear_read_keys();
m_record_layer.clear_read_buffer();
}

if(is_user_canceled_alert(alert))
{
// ignore
}

if(is_error_alert(alert))
Expand Down
6 changes: 6 additions & 0 deletions src/lib/tls/tls13/tls_record_layer_13.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,12 @@ class BOTAN_TEST_API Record_Layer
const std::vector<uint8_t>& data,
Cipher_State* cipher_state=nullptr);

/**
* Clears any data currently stored in the read buffer. This is typically
* used for memory cleanup when the peer sent a CLOSE_NOTIFY alert.
*/
void clear_read_buffer() { zap(m_read_buffer); }

private:
std::vector<uint8_t> m_read_buffer;
Connection_Side m_side;
Expand Down

0 comments on commit 262bbda

Please sign in to comment.