Skip to content

Commit

Permalink
Bug 1580286, account for IV size when checking TLS 1.2 records, r=mt
Browse files Browse the repository at this point in the history
Summary:
This increases the limit of record expansion by 16 so that it doesn't
reject maximum block padding when HMAC-SHA384 is used.

To test this, tlsfuzzer is updated to the latest version
(commit 80d7932ead1d8dae6e555cfd2b1c4c5beb2847df).

Reviewers: mt

Reviewed By: mt

Bug #: 1580286

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

--HG--
extra : amend_source : bab488421c7625fc6c27248afc68270bc0f9ee6b
  • Loading branch information
ueno committed Sep 23, 2019
1 parent c7bac67 commit f847f14
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 7 deletions.
5 changes: 3 additions & 2 deletions gtests/ssl_gtest/ssl_recordsize_unittest.cc
Expand Up @@ -230,14 +230,15 @@ TEST_P(TlsConnectTls13, RecordSizePlaintextExceed) {

// Tweak the ciphertext of server records so that they greatly exceed the limit.
// This requires a much larger expansion than for plaintext to trigger the
// guard, which runs before decryption (current allowance is 304 octets).
// guard, which runs before decryption (current allowance is 320 octets,
// see MAX_EXPANSION in ssl3con.c).
TEST_P(TlsConnectTls13, RecordSizeCiphertextExceed) {
EnsureTlsSetup();

client_->SetOption(SSL_RECORD_SIZE_LIMIT, 64);
Connect();

auto server_expand = MakeTlsFilter<TlsRecordExpander>(server_, 320);
auto server_expand = MakeTlsFilter<TlsRecordExpander>(server_, 336);
server_->SendData(100);

client_->ExpectReadWriteError();
Expand Down
5 changes: 3 additions & 2 deletions lib/ssl/ssl3con.c
Expand Up @@ -12658,8 +12658,9 @@ ssl3_GetCipherSpec(sslSocket *ss, SSL3Ciphertext *cText)

/* MAX_EXPANSION is the amount by which a record might plausibly be expanded
* when protected. It's the worst case estimate, so the sum of block cipher
* padding (up to 256 octets) and HMAC (48 octets for SHA-384). */
#define MAX_EXPANSION (256 + 48)
* padding (up to 256 octets), HMAC (48 octets for SHA-384), and IV (16
* octets for AES). */
#define MAX_EXPANSION (256 + 48 + 16)

/* if cText is non-null, then decipher and check the MAC of the
* SSL record from cText->buf (typically gs->inbuf)
Expand Down
20 changes: 20 additions & 0 deletions tests/tlsfuzzer/config.json.in
Expand Up @@ -160,5 +160,25 @@
]
}
]
},
{
"server_command": [
"@SELFSERV@", "-w", "nss", "-d", "@SERVERDIR@",
"-V", "tls1.0:", "-H", "1",
"-n", "rsa",
"-c", ":C028",
"-p", "@PORT@"
],
"server_hostname": "@HOSTADDR@",
"server_port": @PORT@,
"tests" : [
{
"name" : "test-atypical-padding.py",
"arguments": [
"-p", "@PORT@",
"2^14 bytes of AppData with 256 bytes of padding (SHA384)"
]
}
]
}
]
6 changes: 3 additions & 3 deletions tests/tlsfuzzer/tlsfuzzer.sh
Expand Up @@ -44,11 +44,11 @@ tlsfuzzer_init()
if [ ! -d "$TLSFUZZER" ]; then
# Can't use git-copy.sh here, as tlsfuzzer doesn't have any tags
git clone -q https://github.com/tomato42/tlsfuzzer/ "$TLSFUZZER"
git -C "$TLSFUZZER" checkout a40ce4085052a4da9a05f9149b835a76c194a0c6
git -C "$TLSFUZZER" checkout 80d7932ead1d8dae6e555cfd2b1c4c5beb2847df

# We could use tlslite-ng from pip, but the pip command installed
# on TC is too old to support --pre
${QADIR}/../fuzz/config/git-copy.sh https://github.com/tomato42/tlslite-ng/ v0.8.0-alpha18 tlslite-ng
${QADIR}/../fuzz/config/git-copy.sh https://github.com/tomato42/tlslite-ng/ v0.8.0-alpha27 tlslite-ng

pushd "$TLSFUZZER"
ln -s ../tlslite-ng/tlslite tlslite
Expand Down Expand Up @@ -99,7 +99,7 @@ tlsfuzzer_cleanup()
tlsfuzzer_run_tests()
{
pushd "${HOSTDIR}/tlsfuzzer/${TLSFUZZER}"
PYTHONPATH=. python tests/scripts_retention.py config.json "${BINDIR}/selfserv"
PYTHONPATH=. python tests/scripts_retention.py config.json "${BINDIR}/selfserv" 512
html_msg $? 0 "tlsfuzzer" "Run successfully"
popd
}
Expand Down

0 comments on commit f847f14

Please sign in to comment.