Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Bug 1647752 - Update DTLS 1.3 implementation to draft-38. r=mt
This patch updates DTLS 1.3 to draft-38. Specifically:


  # `ssl_ct_ack` value changes from 25 to 26.
  # AEAD limits in `tls13_UnprotectRecord` enforce a maximum of 2^36-1 (as we only support GCM/ChaCha20 AEADs) decryption failures before the connection is closed.
  # Post-handshake authentication will no longer be negotiated in DTLS 1.3. This allows us to side-step the more convoluted state machine requirements.

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

--HG--
extra : moz-landing-system : lando
  • Loading branch information
Kevin Jacobs committed Jul 8, 2020
1 parent 2998447 commit a412e70
Show file tree
Hide file tree
Showing 13 changed files with 99 additions and 14 deletions.
18 changes: 18 additions & 0 deletions gtests/ssl_gtest/libssl_internals.c
Expand Up @@ -422,6 +422,24 @@ SECStatus SSLInt_AdvanceWriteSeqByAWindow(PRFileDesc *fd, PRInt32 extra) {
return SSLInt_AdvanceWriteSeqNum(fd, to);
}

SECStatus SSLInt_AdvanceDtls13DecryptFailures(PRFileDesc *fd, PRUint64 to) {
sslSocket *ss = ssl_FindSocket(fd);
if (!ss) {
return SECFailure;
}

ssl_GetSpecWriteLock(ss);
ssl3CipherSpec *spec = ss->ssl3.crSpec;
if (spec->cipherDef->type != type_aead) {
ssl_ReleaseSpecWriteLock(ss);
return SECFailure;
}

spec->deprotectionFailures = to;
ssl_ReleaseSpecWriteLock(ss);
return SECSuccess;
}

SSLKEAType SSLInt_GetKEAType(SSLNamedGroup group) {
const sslNamedGroupDef *groupDef = ssl_LookupNamedGroup(group);
if (!groupDef) return ssl_kea_null;
Expand Down
1 change: 1 addition & 0 deletions gtests/ssl_gtest/libssl_internals.h
Expand Up @@ -36,6 +36,7 @@ PRBool SSLInt_DamageEarlyTrafficSecret(PRFileDesc *fd);
SECStatus SSLInt_Set0RttAlpn(PRFileDesc *fd, PRUint8 *data, unsigned int len);
PRBool SSLInt_HasCertWithAuthType(PRFileDesc *fd, SSLAuthType authType);
PRBool SSLInt_SendAlert(PRFileDesc *fd, uint8_t level, uint8_t type);
SECStatus SSLInt_AdvanceDtls13DecryptFailures(PRFileDesc *fd, PRUint64 to);
SECStatus SSLInt_AdvanceWriteSeqNum(PRFileDesc *fd, PRUint64 to);
SECStatus SSLInt_AdvanceReadSeqNum(PRFileDesc *fd, PRUint64 to);
SECStatus SSLInt_AdvanceWriteSeqByAWindow(PRFileDesc *fd, PRInt32 extra);
Expand Down
30 changes: 30 additions & 0 deletions gtests/ssl_gtest/ssl_record_unittest.cc
Expand Up @@ -191,6 +191,36 @@ class ShortHeaderChecker : public PacketFilter {
}
};

TEST_F(TlsConnectDatagram13, AeadLimit) {
Connect();
EXPECT_EQ(SECSuccess, SSLInt_AdvanceDtls13DecryptFailures(server_->ssl_fd(),
(1ULL << 36) - 2));
SendReceive(50);

// Expect this to increment the counter. We should still be able to talk.
client_->SetFilter(std::make_shared<TlsRecordLastByteDamager>(client_));
client_->SendData(10);
server_->ReadBytes(10);
client_->ClearFilter();
client_->ResetSentBytes(50);
SendReceive(60);

// Expect alert when the limit is hit.
client_->SetFilter(std::make_shared<TlsRecordLastByteDamager>(client_));
client_->SendData(10);
ExpectAlert(server_, kTlsAlertBadRecordMac);

// Check the error on both endpoints.
uint8_t buf[10];
PRInt32 rv = PR_Read(server_->ssl_fd(), buf, sizeof(buf));
EXPECT_EQ(-1, rv);
EXPECT_EQ(SSL_ERROR_BAD_MAC_READ, PORT_GetError());

rv = PR_Read(client_->ssl_fd(), buf, sizeof(buf));
EXPECT_EQ(-1, rv);
EXPECT_EQ(SSL_ERROR_BAD_MAC_ALERT, PORT_GetError());
}

TEST_F(TlsConnectDatagram13, ShortHeadersClient) {
Connect();
client_->SetOption(SSL_ENABLE_DTLS_SHORT_HEADER, PR_TRUE);
Expand Down
2 changes: 1 addition & 1 deletion gtests/ssl_gtest/tls_agent.cc
Expand Up @@ -1154,7 +1154,7 @@ void TlsAgent::ReadBytes(size_t amount) {
}
}

void TlsAgent::ResetSentBytes() { send_ctr_ = 0; }
void TlsAgent::ResetSentBytes(size_t bytes) { send_ctr_ = bytes; }

void TlsAgent::SetOption(int32_t option, int value) {
ASSERT_TRUE(EnsureTlsSetup());
Expand Down
2 changes: 1 addition & 1 deletion gtests/ssl_gtest/tls_agent.h
Expand Up @@ -182,7 +182,7 @@ class TlsAgent : public PollTarget {
uint16_t zeroRttSuite = TLS_NULL_WITH_NULL_NULL);
void RemovePsk(std::string label);
void ReadBytes(size_t max = 16384U);
void ResetSentBytes(); // Hack to test drops.
void ResetSentBytes(size_t bytes = 0); // Hack to test drops.
void EnableExtendedMasterSecret();
void CheckExtendedMasterSecret(bool expected);
void CheckEarlyDataAccepted(bool expected);
Expand Down
21 changes: 21 additions & 0 deletions lib/ssl/dtls13con.c
Expand Up @@ -185,6 +185,27 @@ dtls13_SendAckCb(sslSocket *ss)
(void)dtls13_SendAck(ss);
}

/* Limits from draft-ietf-tls-dtls13-38; section 4.5.3. */
PRBool
dtls13_AeadLimitReached(ssl3CipherSpec *spec)
{
if (spec->version >= SSL_LIBRARY_VERSION_TLS_1_3) {
switch (spec->cipherDef->calg) {
case ssl_calg_chacha20:
case ssl_calg_aes_gcm:
return spec->deprotectionFailures >= (1ULL << 36);
#ifdef UNSAFE_FUZZER_MODE
case ssl_calg_null:
return PR_FALSE;
#endif
default:
PORT_Assert(0);
break;
}
}
return PR_FALSE;
}

/* Zero length messages are very simple to check. */
static PRBool
dtls_IsEmptyMessageAcknowledged(sslSocket *ss, PRUint16 msgSeq, PRUint32 offset)
Expand Down
1 change: 1 addition & 0 deletions lib/ssl/dtls13con.h
Expand Up @@ -31,6 +31,7 @@ void dtls13_HolddownTimerCb(sslSocket *ss);
void dtls_ReceivedFirstMessageInFlight(sslSocket *ss);
SECStatus dtls13_MaskSequenceNumber(sslSocket *ss, ssl3CipherSpec *spec,
PRUint8 *hdr, PRUint8 *cipherText, PRUint32 cipherTextLen);
PRBool dtls13_AeadLimitReached(ssl3CipherSpec *spec);

CK_MECHANISM_TYPE tls13_SequenceNumberEncryptionMechanism(SSLCipherAlgorithm bulkAlgorithm);

Expand Down
6 changes: 3 additions & 3 deletions lib/ssl/ssl3con.c
Expand Up @@ -519,7 +519,7 @@ ssl3_DecodeContentType(int msgType)
rv = "application_data (23)";
break;
case ssl_ct_ack:
rv = "ack (25)";
rv = "ack (26)";
break;
default:
sprintf(line, "*UNKNOWN* record type! (%d)", msgType);
Expand Down Expand Up @@ -13024,8 +13024,8 @@ ssl3_HandleRecord(sslSocket *ss, SSL3Ciphertext *cText)
return SECSuccess;
}

if (IS_DTLS(ss) ||
(ss->sec.isServer &&
if ((IS_DTLS(ss) && !dtls13_AeadLimitReached(spec)) ||
(!IS_DTLS(ss) && ss->sec.isServer &&
ss->ssl3.hs.zeroRttIgnore == ssl_0rtt_ignore_trial)) {
/* Silently drop the packet unless we sent a fatal alert. */
if (ss->ssl3.fatalAlertSent) {
Expand Down
2 changes: 1 addition & 1 deletion lib/ssl/ssl3prot.h
Expand Up @@ -14,7 +14,7 @@ typedef PRUint16 SSL3ProtocolVersion;
/* version numbers are defined in sslproto.h */

/* DTLS 1.3 is still a draft. */
#define DTLS_1_3_DRAFT_VERSION 34
#define DTLS_1_3_DRAFT_VERSION 38

typedef PRUint16 ssl3CipherSuite;
/* The cipher suites are defined in sslproto.h */
Expand Down
5 changes: 4 additions & 1 deletion lib/ssl/sslspec.h
Expand Up @@ -169,8 +169,11 @@ struct ssl3CipherSpecStr {
* content type octet. */
PRUint16 recordSizeLimit;

/* Masking context used for DTLS 1.3 */
/* DTLS 1.3: Sequence number masking context. */
SSLMaskingContext *maskContext;

/* DTLS 1.3: Count of decryption failures for the given key. */
PRUint64 deprotectionFailures;
};

typedef void (*sslCipherSpecChangedFunc)(void *arg,
Expand Down
2 changes: 1 addition & 1 deletion lib/ssl/sslt.h
Expand Up @@ -41,7 +41,7 @@ typedef enum {
ssl_ct_alert = 21,
ssl_ct_handshake = 22,
ssl_ct_application_data = 23,
ssl_ct_ack = 25
ssl_ct_ack = 26
} SSLContentType;

typedef enum {
Expand Down
4 changes: 4 additions & 0 deletions lib/ssl/tls13con.c
Expand Up @@ -5704,6 +5704,10 @@ tls13_UnprotectRecord(sslSocket *ss,
cText->buf->buf, /* in */
cText->buf->len); /* inlen */
if (rv != SECSuccess) {
if (IS_DTLS(ss)) {
spec->deprotectionFailures++;
}

SSL_TRC(3,
("%d: TLS13[%d]: record has bogus MAC",
SSL_GETPID(), ss->fd));
Expand Down
19 changes: 13 additions & 6 deletions lib/ssl/tls13exthandle.c
Expand Up @@ -997,10 +997,13 @@ tls13_ClientSendPostHandshakeAuthXtn(const sslSocket *ss,
TLSExtensionData *xtnData,
sslBuffer *buf, PRBool *added)
{
SSL_TRC(3, ("%d: TLS13[%d]: send post_handshake_auth extension",
SSL_GETPID(), ss->fd));

*added = ss->opt.enablePostHandshakeAuth;
/* Only one post-handshake message is supported: a single
* NST immediately following the client Finished. */
if (!IS_DTLS(ss)) {
SSL_TRC(3, ("%d: TLS13[%d]: send post_handshake_auth extension",
SSL_GETPID(), ss->fd));
*added = ss->opt.enablePostHandshakeAuth;
}
return SECSuccess;
}

Expand All @@ -1017,8 +1020,12 @@ tls13_ServerHandlePostHandshakeAuthXtn(const sslSocket *ss,
return SECFailure;
}

/* Keep track of negotiated extensions. */
xtnData->negotiated[xtnData->numNegotiated++] = ssl_tls13_post_handshake_auth_xtn;
/* Only one post-handshake message is supported: a single
* NST immediately following the client Finished. */
if (!IS_DTLS(ss)) {
/* Keep track of negotiated extensions. */
xtnData->negotiated[xtnData->numNegotiated++] = ssl_tls13_post_handshake_auth_xtn;
}

return SECSuccess;
}
Expand Down

0 comments on commit a412e70

Please sign in to comment.