1580286 - NSS rejects TLS 1.2 records with large padding with SHA384 HMAC

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Alicja Kario]

When NSS receives a record with maximum amount of application data (2**14 bytes) and maximum amount of padding (256 bytes) and the connection is using SHA384 HMAC, the record is rejected with record_overflow alert.

This is RFC 5246 non compliance (as the record is valid).

Old versions of GnuTLS are known to generate this kind of records: https://gitlab.com/gnutls/gnutls/issues/811

Reproducer is available in tlsfuzzer:
https://github.com/tomato42/tlsfuzzer/pull/585
test-atypical-padding.py

result:

...
2^14 bytes of AppData with 256 bytes of padding (SHA384) ...
Error encountered while processing node <tlsfuzzer.expect.ExpectApplicationData object at 0x7f812d3d9310> (child: <tlsfuzzer.messages.AlertGenerator object at 0x7f812d3d9350>) with last message being: <tlslite.messages.Message object at 0x7f812d3f9890>
Error while processing
Traceback (most recent call last):
  File "scripts/test-atypical-padding.py", line 532, in main
    runner.run()
  File "/home/hkario/dev/tlsfuzzer/tlsfuzzer/runner.py", line 225, in run
    RecordHeader2)))
AssertionError: Unexpected message from peer: Alert(fatal, record_overflow)
...
version: 2

Test end
successful: 9
failed: 1
  '2^14 bytes of AppData with 256 bytes of padding (SHA384)'

server output:

selfserv: HDX PR_Read returned error -12263:
SSL received a record that exceeded the maximum permissible length.

this is with d2485b1c997e (3.47 in-development)

Comment 1

[Daiki Ueno [:ueno]]

This is fairly easy to fix: account for IV size in MAX_EXPANSION:
https://searchfox.org/mozilla-central/source/security/nss/lib/ssl/ssl3con.c#12660

The problem is that it is hard to test. While we could use the tlsfuzzer test, it would require different server setup, which is not currently supported in the infrastructure. The other option would be to add a means to specify random padding for TLS 1.2 GenericBlockCipher and all the ciphers in TLS 1.3. I'm going to attach a patch in the latter option.

Comment 2

[Daiki Ueno [:ueno]]

Attachment: Bug 1580286, add function to set callback for setting record padding

This adds SSLExp_SetRecordPaddingCallback that allows callers to control record padding behavior based on the content type and the size of plaintext.

This is particularly useful for testing record_size_limit extension, but also would be useful for hiding the plaintext length in some applications.

Comment 3

[Daiki Ueno [:ueno]]

Comment on attachment 9093947 [details]
Bug 1580286, add function to set callback for setting record padding

I realized that this can be actually tested by tlsfuzzer, without adding a new API to libssl. I'll submit a new patch soon, but in a separate patch as I wonder if the API might be perhaps useful.

Comment 4

[Daiki Ueno [:ueno]]

Attachment: Bug 1580286, account for IV size when checking TLS 1.2 records

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).

Comment 5

[Daiki Ueno [:ueno]]

Thanks for the review; pushed as:
https://hg.mozilla.org/projects/nss/rev/03039d4fad572d1213357caae32bcd81ed07e10a