Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Bug 1471126 - Provide extra information needed to use record layer se…
…paration, r=ekr

This started as an attempt to remove the cipher spec update callback we use for
testing.  Using the new, public secrets interface should be better for that.

In doing so, it became apparent that we needed more interfaces to NSS to support
the use of these secrets.  In particular:

1. We need to know what the KDF hash function is for a given cipher suite.  This
   allows users of the secret to use the right hash function.

2. We need to know what cipher spec was picked when sending 0-RTT.  NSS
   currently doesn't expose that information.  (When receiving 0-RTT you can
   safely assume that the negotiated cipher suite is good to use.)

3. We need to know what epoch NSS is currently using.  Otherwise, we can't be
   sure which epoch to feed it.  Data from a good epoch is saved, whereas data
   from a bad epoch is lost, so applications need to know.

So this patch adds these functions to the appropriate info functions and uses
that information in tests to remove and re-add protection.

The test changes are considerable.  The main effect of the changes is to rely on
the new functions for managing secrets, rather than the old interface.  But with
the changes in the other CLs for this bug, secrets appear before they are used,
which complicates things considerably.  For that, I've moved more logic into the
TlsCipherSpec class, which now tracks per-epoch state, like sequence numbers and
record drops.

Trial decryption (yep) is used to identify the right cipher spec every time when
decrypting, so tests are no longer tolerant of failures to decrypt.  It's no
longer possible to have a test enable decryption and pass when decryption fails;
this is particularly true for some parameterized tests that assumed it was OK to
enable decryption even for TLS 1.2 and earlier.

--HG--
extra : rebase_source : 4d5a752d0b9837db2ddee9cef481ed7fb588b62d
extra : amend_source : 2559f37290e31c70a4591b11f30b84f5640c86e7
extra : source : 6d5ddd89089058ed7be42a17d92e195a31aec46e
extra : histedit_source : aa847484ab6b1826d1494052b20f29a2136a3644
  • Loading branch information
martinthomson committed Oct 23, 2018
1 parent 7b3eabb commit c88db17
Show file tree
Hide file tree
Showing 21 changed files with 621 additions and 423 deletions.
20 changes: 20 additions & 0 deletions automation/abi-check/expected-report-libssl3.so.txt
@@ -0,0 +1,20 @@

2 functions with some indirect sub-type change:

[C]'function SECStatus SSL_GetCipherSuiteInfo(PRUint16, SSLCipherSuiteInfo*, PRUintn)' at sslinfo.c:326:1 has some indirect sub-type changes:
parameter 2 of type 'SSLCipherSuiteInfo*' has sub-type changes:
in pointed to type 'typedef SSLCipherSuiteInfo' at sslt.h:433:1:
underlying type 'struct SSLCipherSuiteInfoStr' at sslt.h:366:1 changed:
type size changed from 768 to 832 (in bits)
1 data member insertion:
'SSLHashType SSLCipherSuiteInfoStr::kdfHash', at offset 768 (in bits) at sslt.h:429:1

[C]'function SECStatus SSL_GetPreliminaryChannelInfo(PRFileDesc*, SSLPreliminaryChannelInfo*, PRUintn)' at sslinfo.c:111:1 has some indirect sub-type changes:
parameter 2 of type 'SSLPreliminaryChannelInfo*' has sub-type changes:
in pointed to type 'typedef SSLPreliminaryChannelInfo' at sslt.h:379:1:
underlying type 'struct SSLPreliminaryChannelInfoStr' at sslt.h:333:1 changed:
type size changed from 160 to 192 (in bits)
1 data member insertion:
'PRUint16 SSLPreliminaryChannelInfoStr::zeroRttCipherSuite', at offset 160 (in bits) at sslt.h:375:1


46 changes: 0 additions & 46 deletions gtests/ssl_gtest/libssl_internals.c
Expand Up @@ -297,38 +297,6 @@ SSLKEAType SSLInt_GetKEAType(SSLNamedGroup group) {
return groupDef->keaType;
}

SECStatus SSLInt_SetCipherSpecChangeFunc(PRFileDesc *fd,
sslCipherSpecChangedFunc func,
void *arg) {
sslSocket *ss;

ss = ssl_FindSocket(fd);
if (!ss) {
return SECFailure;
}

ss->ssl3.changedCipherSpecFunc = func;
ss->ssl3.changedCipherSpecArg = arg;

return SECSuccess;
}

PK11SymKey *SSLInt_CipherSpecToKey(const ssl3CipherSpec *spec) {
return spec->keyMaterial.key;
}

SSLCipherAlgorithm SSLInt_CipherSpecToAlgorithm(const ssl3CipherSpec *spec) {
return spec->cipherDef->calg;
}

const PRUint8 *SSLInt_CipherSpecToIv(const ssl3CipherSpec *spec) {
return spec->keyMaterial.iv;
}

PRUint16 SSLInt_CipherSpecToEpoch(const ssl3CipherSpec *spec) {
return spec->epoch;
}

void SSLInt_SetTicketLifetime(uint32_t lifetime) {
ssl_ticket_lifetime = lifetime;
}
Expand Down Expand Up @@ -360,20 +328,6 @@ void SSLInt_RolloverAntiReplay(void) {
tls13_AntiReplayRollover(ssl_TimeUsec());
}

SECStatus SSLInt_GetEpochs(PRFileDesc *fd, PRUint16 *readEpoch,
PRUint16 *writeEpoch) {
sslSocket *ss = ssl_FindSocket(fd);
if (!ss || !readEpoch || !writeEpoch) {
return SECFailure;
}

ssl_GetSpecReadLock(ss);
*readEpoch = ss->ssl3.crSpec->epoch;
*writeEpoch = ss->ssl3.cwSpec->epoch;
ssl_ReleaseSpecReadLock(ss);
return SECSuccess;
}

SECStatus SSLInt_HasPendingHandshakeData(PRFileDesc *fd, PRBool *pending) {
sslSocket *ss = ssl_FindSocket(fd);
if (!ss) {
Expand Down
10 changes: 0 additions & 10 deletions gtests/ssl_gtest/libssl_internals.h
Expand Up @@ -39,17 +39,7 @@ SECStatus SSLInt_AdvanceWriteSeqNum(PRFileDesc *fd, PRUint64 to);
SECStatus SSLInt_AdvanceReadSeqNum(PRFileDesc *fd, PRUint64 to);
SECStatus SSLInt_AdvanceWriteSeqByAWindow(PRFileDesc *fd, PRInt32 extra);
SSLKEAType SSLInt_GetKEAType(SSLNamedGroup group);
SECStatus SSLInt_GetEpochs(PRFileDesc *fd, PRUint16 *readEpoch,
PRUint16 *writeEpoch);
SECStatus SSLInt_HasPendingHandshakeData(PRFileDesc *fd, PRBool *pending);

SECStatus SSLInt_SetCipherSpecChangeFunc(PRFileDesc *fd,
sslCipherSpecChangedFunc func,
void *arg);
PRUint16 SSLInt_CipherSpecToEpoch(const ssl3CipherSpec *spec);
PK11SymKey *SSLInt_CipherSpecToKey(const ssl3CipherSpec *spec);
SSLCipherAlgorithm SSLInt_CipherSpecToAlgorithm(const ssl3CipherSpec *spec);
const PRUint8 *SSLInt_CipherSpecToIv(const ssl3CipherSpec *spec);
void SSLInt_SetTicketLifetime(uint32_t lifetime);
SECStatus SSLInt_SetSocketMaxEarlyDataSize(PRFileDesc *fd, uint32_t size);
void SSLInt_RolloverAntiReplay(void);
Expand Down
11 changes: 6 additions & 5 deletions gtests/ssl_gtest/ssl_auth_unittest.cc
Expand Up @@ -273,9 +273,7 @@ class TlsReplaceSignatureSchemeFilter : public TlsHandshakeFilter {
TlsReplaceSignatureSchemeFilter(const std::shared_ptr<TlsAgent>& a,
SSLSignatureScheme scheme)
: TlsHandshakeFilter(a, {kTlsHandshakeCertificateVerify}),
scheme_(scheme) {
EnableDecryption();
}
scheme_(scheme) {}

protected:
virtual PacketFilter::Action FilterHandshake(const HandshakeHeader& header,
Expand Down Expand Up @@ -552,7 +550,9 @@ TEST_P(TlsConnectTls12, SignatureAlgorithmDrop) {

TEST_P(TlsConnectTls13, UnsupportedSignatureSchemeAlert) {
EnsureTlsSetup();
MakeTlsFilter<TlsReplaceSignatureSchemeFilter>(server_, ssl_sig_none);
auto filter =
MakeTlsFilter<TlsReplaceSignatureSchemeFilter>(server_, ssl_sig_none);
filter->EnableDecryption();

ConnectExpectAlert(client_, kTlsAlertIllegalParameter);
server_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT);
Expand All @@ -563,8 +563,9 @@ TEST_P(TlsConnectTls13, InconsistentSignatureSchemeAlert) {
EnsureTlsSetup();

// This won't work because we use an RSA cert by default.
MakeTlsFilter<TlsReplaceSignatureSchemeFilter>(
auto filter = MakeTlsFilter<TlsReplaceSignatureSchemeFilter>(
server_, ssl_sig_ecdsa_secp256r1_sha256);
filter->EnableDecryption();

ConnectExpectAlert(client_, kTlsAlertIllegalParameter);
server_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT);
Expand Down
5 changes: 3 additions & 2 deletions gtests/ssl_gtest/ssl_damage_unittest.cc
Expand Up @@ -62,7 +62,6 @@ TEST_P(TlsConnectGenericPre13, DamageServerSignature) {
EnsureTlsSetup();
auto filter = MakeTlsFilter<TlsLastByteDamager>(
server_, kTlsHandshakeServerKeyExchange);
filter->EnableDecryption();
ExpectAlert(client_, kTlsAlertDecryptError);
ConnectExpectFail();
client_->CheckErrorCode(SEC_ERROR_BAD_SIGNATURE);
Expand All @@ -84,7 +83,9 @@ TEST_P(TlsConnectGeneric, DamageClientSignature) {
server_->RequestClientAuth(true);
auto filter = MakeTlsFilter<TlsLastByteDamager>(
client_, kTlsHandshakeCertificateVerify);
filter->EnableDecryption();
if (version_ >= SSL_LIBRARY_VERSION_TLS_1_3) {
filter->EnableDecryption();
}
server_->ExpectSendAlert(kTlsAlertDecryptError);
// Do these handshakes by hand to avoid race condition on
// the client processing the server's alert.
Expand Down

0 comments on commit c88db17

Please sign in to comment.