Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Bug 1599545 - Fix assertion and add test for early Key Update message…
… r=mt

Remove an overzealous assertion when a Key Update message is received too early, and add a test for the expected alert condition.

Also adds `TlsEncryptedHandshakeMessageReplacer` for replacing TLS 1.3 encrypted handshake messages. This is a simple implementation where only the first byte of the message is changed to the new type (so as to trigger the desired handler).

Differential Revision: https://phabricator.services.mozilla.com/D54998

--HG--
extra : moz-landing-system : lando
  • Loading branch information
Kevin Jacobs committed Dec 2, 2019
1 parent 6ac251c commit cea71a5
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 1 deletion.
1 change: 1 addition & 0 deletions cpputil/tls_parser.h
Expand Up @@ -31,6 +31,7 @@ const uint8_t kTlsHandshakeCertificateRequest = 13;
const uint8_t kTlsHandshakeCertificateVerify = 15;
const uint8_t kTlsHandshakeClientKeyExchange = 16;
const uint8_t kTlsHandshakeFinished = 20;
const uint8_t kTlsHandshakeKeyUpdate = 24;

const uint8_t kTlsAlertWarning = 1;
const uint8_t kTlsAlertFatal = 2;
Expand Down
31 changes: 31 additions & 0 deletions gtests/ssl_gtest/ssl_keyupdate_unittest.cc
Expand Up @@ -33,6 +33,37 @@ TEST_F(TlsConnectTest, KeyUpdateClient) {
CheckEpochs(4, 3);
}

TEST_F(TlsConnectStreamTls13, KeyUpdateTooEarly_Client) {
StartConnect();
auto filter = MakeTlsFilter<TlsEncryptedHandshakeMessageReplacer>(
server_, kTlsHandshakeFinished, kTlsHandshakeKeyUpdate);
filter->EnableDecryption();

client_->Handshake();
server_->Handshake();
ExpectAlert(client_, kTlsAlertUnexpectedMessage);
client_->Handshake();
client_->CheckErrorCode(SSL_ERROR_RX_UNEXPECTED_KEY_UPDATE);
server_->Handshake();
server_->CheckErrorCode(SSL_ERROR_HANDSHAKE_UNEXPECTED_ALERT);
}

TEST_F(TlsConnectStreamTls13, KeyUpdateTooEarly_Server) {
StartConnect();
auto filter = MakeTlsFilter<TlsEncryptedHandshakeMessageReplacer>(
client_, kTlsHandshakeFinished, kTlsHandshakeKeyUpdate);
filter->EnableDecryption();

client_->Handshake();
server_->Handshake();
client_->Handshake();
ExpectAlert(server_, kTlsAlertUnexpectedMessage);
server_->Handshake();
server_->CheckErrorCode(SSL_ERROR_RX_UNEXPECTED_KEY_UPDATE);
client_->Handshake();
client_->CheckErrorCode(SSL_ERROR_HANDSHAKE_UNEXPECTED_ALERT);
}

TEST_F(TlsConnectTest, KeyUpdateClientRequestUpdate) {
ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3);
Connect();
Expand Down
61 changes: 61 additions & 0 deletions gtests/ssl_gtest/tls_filter.h
Expand Up @@ -454,6 +454,67 @@ class TlsHandshakeDropper : public TlsHandshakeFilter {
}
};

class TlsEncryptedHandshakeMessageReplacer : public TlsRecordFilter {
public:
TlsEncryptedHandshakeMessageReplacer(const std::shared_ptr<TlsAgent>& a,
uint8_t old_ct, uint8_t new_ct)
: TlsRecordFilter(a), old_ct_(old_ct), new_ct_(new_ct) {}

protected:
PacketFilter::Action FilterRecord(const TlsRecordHeader& header,
const DataBuffer& record, size_t* offset,
DataBuffer* output) override {
if (header.content_type() != ssl_ct_application_data) {
return KEEP;
}

uint16_t protection_epoch = 0;
uint8_t inner_content_type;
DataBuffer plaintext;
if (!Unprotect(header, record, &protection_epoch, &inner_content_type,
&plaintext) ||
!plaintext.len()) {
return KEEP;
}

if (inner_content_type != ssl_ct_handshake) {
return KEEP;
}

size_t off = 0;
uint32_t msg_len = 0;
uint32_t msg_type = 255; // Not a real message
do {
if (!plaintext.Read(off, 1, &msg_type) || msg_type == old_ct_) {
break;
}

// Increment and check next messages
if (!plaintext.Read(++off, 3, &msg_len)) {
break;
}
off += 3 + msg_len;
} while (msg_type != old_ct_);

if (msg_type == old_ct_) {
plaintext.Write(off, new_ct_, 1);
}

DataBuffer ciphertext;
bool ok = Protect(spec(protection_epoch), header, inner_content_type,
plaintext, &ciphertext, 0);
if (!ok) {
return KEEP;
}
*offset = header.Write(output, *offset, ciphertext);
return CHANGE;
}

private:
uint8_t old_ct_;
uint8_t new_ct_;
};

class TlsExtensionInjector : public TlsHandshakeFilter {
public:
TlsExtensionInjector(const std::shared_ptr<TlsAgent>& a, uint16_t ext,
Expand Down
1 change: 0 additions & 1 deletion lib/ssl/tls13con.c
Expand Up @@ -798,7 +798,6 @@ tls13_HandleKeyUpdate(sslSocket *ss, PRUint8 *b, unsigned int length)
PORT_Assert(ss->opt.noLocks || ssl_HaveRecvBufLock(ss));
PORT_Assert(ss->opt.noLocks || ssl_HaveSSL3HandshakeLock(ss));

PORT_Assert(ss->firstHsDone);
if (!tls13_IsPostHandshake(ss)) {
FATAL_ERROR(ss, SSL_ERROR_RX_UNEXPECTED_KEY_UPDATE, unexpected_message);
return SECFailure;
Expand Down

0 comments on commit cea71a5

Please sign in to comment.