Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Bug 1252745 - Fix Signed Certificate Timestamps for TLS 1.3, r=ttaubert
--HG--
extra : rebase_source : 7573e1e230a665764a8c54ff931abf2a50c024af
extra : amend_source : e92fd60d246e77b33928f18bebce45a347c93a70
  • Loading branch information
martinthomson committed Nov 10, 2016
1 parent 0cfb4ed commit 74a5edd
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 39 deletions.
24 changes: 12 additions & 12 deletions gtests/ssl_gtest/ssl_cert_ext_unittest.cc
Expand Up @@ -23,8 +23,8 @@ namespace nss_test {
// by the relevant callbacks on the client.
class SignedCertificateTimestampsExtractor {
public:
SignedCertificateTimestampsExtractor(TlsAgent* client) {
client->SetAuthCertificateCallback(
SignedCertificateTimestampsExtractor(TlsAgent* client) : client_(client) {
client_->SetAuthCertificateCallback(
[&](TlsAgent* agent, bool checksig, bool isServer) -> SECStatus {
const SECItem* scts = SSL_PeerSignedCertTimestamps(agent->ssl_fd());
EXPECT_TRUE(scts);
Expand All @@ -34,7 +34,7 @@ class SignedCertificateTimestampsExtractor {
auth_timestamps_.reset(new DataBuffer(scts->data, scts->len));
return SECSuccess;
});
client->SetHandshakeCallback([&](TlsAgent* agent) {
client_->SetHandshakeCallback([&](TlsAgent* agent) {
const SECItem* scts = SSL_PeerSignedCertTimestamps(agent->ssl_fd());
ASSERT_TRUE(scts);
handshake_timestamps_.reset(new DataBuffer(scts->data, scts->len));
Expand All @@ -47,9 +47,13 @@ class SignedCertificateTimestampsExtractor {

EXPECT_TRUE(handshake_timestamps_);
EXPECT_EQ(timestamps, *handshake_timestamps_);

const SECItem* current = SSL_PeerSignedCertTimestamps(client_->ssl_fd());
EXPECT_EQ(timestamps, DataBuffer(current->data, current->len));
}

private:
TlsAgent* client_;
std::unique_ptr<DataBuffer> auth_timestamps_;
std::unique_ptr<DataBuffer> handshake_timestamps_;
};
Expand All @@ -60,7 +64,7 @@ static const SECItem kSctItem = {siBuffer, const_cast<uint8_t*>(kSctValue),
static const DataBuffer kSctBuffer(kSctValue, sizeof(kSctValue));

// Test timestamps extraction during a successful handshake.
TEST_P(TlsConnectGenericPre13, SignedCertificateTimestampsHandshake) {
TEST_P(TlsConnectGeneric, SignedCertificateTimestampsHandshake) {
EnsureTlsSetup();
EXPECT_EQ(SECSuccess, SSL_SetSignedCertTimestamps(server_->ssl_fd(),
&kSctItem, ssl_kea_rsa));
Expand All @@ -72,11 +76,9 @@ TEST_P(TlsConnectGenericPre13, SignedCertificateTimestampsHandshake) {
Connect();

timestamps_extractor.assertTimestamps(kSctBuffer);
const SECItem* c_timestamps = SSL_PeerSignedCertTimestamps(client_->ssl_fd());
EXPECT_EQ(SECEqual, SECITEM_CompareItem(&kSctItem, c_timestamps));
}

TEST_P(TlsConnectGenericPre13, SignedCertificateTimestampsConfig) {
TEST_P(TlsConnectGeneric, SignedCertificateTimestampsConfig) {
static const SSLExtraServerCertData kExtraData = {ssl_auth_rsa_sign, nullptr,
nullptr, &kSctItem};

Expand All @@ -91,13 +93,11 @@ TEST_P(TlsConnectGenericPre13, SignedCertificateTimestampsConfig) {
Connect();

timestamps_extractor.assertTimestamps(kSctBuffer);
const SECItem* c_timestamps = SSL_PeerSignedCertTimestamps(client_->ssl_fd());
EXPECT_EQ(SECEqual, SECITEM_CompareItem(&kSctItem, c_timestamps));
}

// Test SSL_PeerSignedCertTimestamps returning zero-length SECItem
// when the client / the server / both have not enabled the feature.
TEST_P(TlsConnectGenericPre13, SignedCertificateTimestampsInactiveClient) {
TEST_P(TlsConnectGeneric, SignedCertificateTimestampsInactiveClient) {
EnsureTlsSetup();
EXPECT_EQ(SECSuccess, SSL_SetSignedCertTimestamps(server_->ssl_fd(),
&kSctItem, ssl_kea_rsa));
Expand All @@ -107,7 +107,7 @@ TEST_P(TlsConnectGenericPre13, SignedCertificateTimestampsInactiveClient) {
timestamps_extractor.assertTimestamps(DataBuffer());
}

TEST_P(TlsConnectGenericPre13, SignedCertificateTimestampsInactiveServer) {
TEST_P(TlsConnectGeneric, SignedCertificateTimestampsInactiveServer) {
EnsureTlsSetup();
EXPECT_EQ(SECSuccess,
SSL_OptionSet(client_->ssl_fd(), SSL_ENABLE_SIGNED_CERT_TIMESTAMPS,
Expand All @@ -118,7 +118,7 @@ TEST_P(TlsConnectGenericPre13, SignedCertificateTimestampsInactiveServer) {
timestamps_extractor.assertTimestamps(DataBuffer());
}

TEST_P(TlsConnectGenericPre13, SignedCertificateTimestampsInactiveBoth) {
TEST_P(TlsConnectGeneric, SignedCertificateTimestampsInactiveBoth) {
EnsureTlsSetup();
SignedCertificateTimestampsExtractor timestamps_extractor(client_);

Expand Down
7 changes: 2 additions & 5 deletions lib/ssl/ssl3con.c
Expand Up @@ -6802,7 +6802,6 @@ ssl3_HandleServerHello(sslSocket *ss, SSL3Opaque *b, PRUint32 length)

loser:
/* Clean up the temporary pointer to the handshake buffer. */
ss->xtnData.signedCertTimestamps.data = NULL;
ss->xtnData.signedCertTimestamps.len = 0;
ssl_MapLowLevelError(errCode);
return SECFailure;
Expand Down Expand Up @@ -7001,14 +7000,12 @@ ssl3_HandleServerHelloPart2(sslSocket *ss, const SECItem *sidBytes,
ssl3_ExtensionNegotiated(ss, ssl_extended_master_secret_xtn);

/* Copy Signed Certificate Timestamps, if any. */
if (ss->xtnData.signedCertTimestamps.data) {
if (ss->xtnData.signedCertTimestamps.len) {
rv = SECITEM_CopyItem(NULL, &sid->u.ssl3.signedCertTimestamps,
&ss->xtnData.signedCertTimestamps);
ss->xtnData.signedCertTimestamps.len = 0;
if (rv != SECSuccess)
goto loser;
/* Clean up the temporary pointer to the handshake buffer. */
ss->xtnData.signedCertTimestamps.data = NULL;
ss->xtnData.signedCertTimestamps.len = 0;
}

ss->ssl3.hs.isResuming = PR_FALSE;
Expand Down
13 changes: 6 additions & 7 deletions lib/ssl/ssl3exthandle.c
Expand Up @@ -2426,18 +2426,17 @@ ssl3_ServerSendSignedCertTimestampXtn(const sslSocket *ss, TLSExtensionData *xtn
rv = ssl3_ExtAppendHandshakeNumber(ss,
ssl_signed_cert_timestamp_xtn,
2);
if (rv != SECSuccess)
goto loser;
if (rv != SECSuccess) {
return -1;
}
/* extension_data */
rv = ssl3_ExtAppendHandshakeVariable(ss, scts->data, scts->len, 2);
if (rv != SECSuccess)
goto loser;
if (rv != SECSuccess) {
return -1;
}
}

return extension_length;

loser:
return -1;
}

SECStatus
Expand Down
2 changes: 1 addition & 1 deletion lib/ssl/sslcert.c
Expand Up @@ -274,7 +274,7 @@ ssl_PopulateSignedCertTimestamps(sslServerCert *sc,
if (sc->signedCertTimestamps.len) {
SECITEM_FreeItem(&sc->signedCertTimestamps, PR_FALSE);
}
if (signedCertTimestamps) {
if (signedCertTimestamps && signedCertTimestamps->len) {
return SECITEM_CopyItem(NULL, &sc->signedCertTimestamps,
signedCertTimestamps);
}
Expand Down
28 changes: 14 additions & 14 deletions lib/ssl/tls13con.c
Expand Up @@ -2009,18 +2009,6 @@ tls13_HandleServerHelloPart2(sslSocket *ss)
if (ssl3_ClientExtensionAdvertised(ss, ssl_tls13_pre_shared_key_xtn)) {
SSL_AtomicIncrementLong(&ssl3stats->hsh_sid_cache_misses);
}
/* Copy Signed Certificate Timestamps, if any. */
if (ss->xtnData.signedCertTimestamps.data) {
rv = SECITEM_CopyItem(NULL, &sid->u.ssl3.signedCertTimestamps,
&ss->xtnData.signedCertTimestamps);
if (rv != SECSuccess) {
FATAL_ERROR(ss, SEC_ERROR_NO_MEMORY, internal_error);
return SECFailure;
}
/* Clean up the temporary pointer to the handshake buffer. */
ss->xtnData.signedCertTimestamps.data = NULL;
ss->xtnData.signedCertTimestamps.len = 0;
}
if (sid->cached == in_client_cache) {
/* If we tried to resume and failed, let's not try again. */
ss->sec.uncache(sid);
Expand Down Expand Up @@ -2395,11 +2383,23 @@ tls13_HandleCertificate(sslSocket *ss, SSL3Opaque *b, PRUint32 length)
rv = tls13_HandleCertificateEntry(ss, &certList, first,
&cert);
if (rv != SECSuccess) {
ss->xtnData.signedCertTimestamps.len = 0;
return SECFailure;
}

if (first) {
ss->sec.peerCert = cert;

if (ss->xtnData.signedCertTimestamps.len) {
sslSessionID *sid = ss->sec.ci.sid;
rv = SECITEM_CopyItem(NULL, &sid->u.ssl3.signedCertTimestamps,
&ss->xtnData.signedCertTimestamps);
ss->xtnData.signedCertTimestamps.len = 0;
if (rv != SECSuccess) {
FATAL_ERROR(ss, SEC_ERROR_NO_MEMORY, internal_error);
return SECFailure;
}
}
} else {
ssl3CertNode *c = PORT_ArenaNew(ss->ssl3.peerCertArena,
ssl3CertNode);
Expand Down Expand Up @@ -3975,8 +3975,8 @@ static const struct {
{ ssl_tls13_early_data_xtn, ExtensionSendEncrypted },
{ ssl_next_proto_nego_xtn, ExtensionNotUsed },
{ ssl_renegotiation_info_xtn, ExtensionNotUsed },
{ ssl_signed_cert_timestamp_xtn, ExtensionSendEncrypted },
{ ssl_cert_status_xtn, ExtensionSendEncrypted },
{ ssl_signed_cert_timestamp_xtn, ExtensionSendCertificate },
{ ssl_cert_status_xtn, ExtensionSendCertificate },
{ ssl_tls13_ticket_early_data_info_xtn, ExtensionNewSessionTicket },
{ ssl_tls13_cookie_xtn, ExtensionSendHrr }
};
Expand Down

0 comments on commit 74a5edd

Please sign in to comment.