1601402 - Post handshake auth doesn't interact well with KeyUpdate

Status: NEW

(NSS :: Libraries, defect, P3)

Description

[Alicja Kario]

When the client replies to server's CertificateRequest with a KeyUpdate and Certificate messages, the connection is aborted by server (usually while client is writing the CertificateVerify message ).

No error messages are printed by selfserv.

tlsfuzzer test-tls13-post-handshake-auth.py errors out with:

post-handshake authentication with KeyUpdate ...
Error encountered while processing node <tlsfuzzer.messages.CertificateVerifyGenerator object at 0x7fdba7708110> (child: <tlsfuzzer.messages.FinishedGenerator object at 0x7fdba7708190>) with last message being: <tlslite.messages.CertificateVerify object at 0x7fdba7721810>
Error while processing
Traceback (most recent call last):
  File "scripts/test-tls13-post-handshake-auth.py", line 458, in main
    runner.run()
  File "/home/hkario/dev/tlsfuzzer/tlsfuzzer/runner.py", line 262, in run
    raise AssertionError("Unexpected closure from peer")
AssertionError: Unexpected closure from peer

If I read the code correctly, it looks to me like SSL_ForceHandshake() can't handle a KeyUpdate message before Certificate (despite the RFC allowing it there)

Testing with a9ba652046e6 (current default branch) and tlsfuzzer from https://github.com/tomato42/tlsfuzzer/pull/551

Comment 1

[Alicja Kario]

the script needs key and certificate trusted by server (provided as PEM files using -k and -c options) and to work with selfserv also requires passing --pha-as-reply --pha-in-sanity options

Comment 2

[J.C. Jones [:jcj] (he/they)]

Can you take a look at this, please?

Comment 3

[Kevin Jacobs [:kjacobs]]

It appears that we have a gtest for this case [1], and I've been unable to induce any failures with variations of that test. I'm attaching a trace log in which the server doesn't receive the Certificate message after entering KeyUpdate.

I'm not very familiar with the tlsfuzzer code, but is it possible this is a test bug?

[1] https://searchfox.org/mozilla-central/source/security/nss/gtests/ssl_gtest/ssl_auth_unittest.cc#342

Comment 4

[Kevin Jacobs [:kjacobs]]

Attachment: log.txt

Comment 5

[Alicja Kario]

Attachment: PHA_with_key_update.zip

I'm not very familiar with the tlsfuzzer code, but is it possible this is a test bug?

it's always a possibility, but doesn't look to be in this case – the connection was closed by server when we were sending a reply to Certificate Request message, while writing Certificate Verify message to the socket

actually I'm afraid that it's worse than what I said in the opening comment: after Certificate Request, the client sends Key Update, as a reply to that, server sends Application Data with the HTTP reply and immediately after, Alert close_notify – i.e. it doesn't wait for Certificate message from client at all, and also mishandles KeyUpdate request (as it does send app data but no KeyUpdate of its own)

pcap capture and key log file attached

More complete reproducer:
(generate /tmp/client/key.pem and /tmp/client/cert.pem with certificates trusted by sql:/tmp/nssdb with "server" certificate)

pushd /tmp &&
wget https://raw.githubusercontent.com/redhat-qe-security/certgen/master/certgen/lib.sh &&
source lib.sh &&
x509KeyGen ca &&
x509KeyGen server &&
x509KeyGen client &&
x509SelfSign ca &&
x509CertSign --CA ca server &&
x509CertSign --CA ca -t webclient client &&
x509Key --pkcs12 --with-cert server &&
mkdir /tmp/nssdb &&
certutil -N -d sql:/tmp/nssdb --empty-password &&
certutil -A -d sql:/tmp/nssdb -n ca -i /tmp/ca/cert.pem -t 'cTC,cTC,' -a &&
pk12util -i /tmp/server/bundle.p12 -d sql:/tmp/nssdb -W '' &&
popd

Start server:

LD_LIBRARY_PATH="$NSS_DIR/lib" SSLKEYLOGFILE=/tmp/keys.txt "$NSS_DIR/bin/selfserv" -d sql:/tmp/nssdb -p 4433 -V tls1.0: -H 1 -n server -u -rrr -E

Execute client:

PYTHONPATH=. python scripts/test-tls13-post-handshake-auth.py -k /tmp/client/key.pem -c /tmp/client/cert.pem --pha-as-reply --pha-in-sanity 'post-handshake authentication with KeyUpdate'

Client output:

post-handshake authentication with KeyUpdate ...
Error encountered while processing node <tlsfuzzer.messages.CertificateVerifyGenerator object at 0x7f1ffebcf110> (child: <tlsfuzzer.messages.FinishedGenerator object at 0x7f1ffebcf190>) with last message being: <tlslite.messages.CertificateVerify object at 0x7f1ffebe8810>
Error while processing
Traceback (most recent call last):
  File "scripts/test-tls13-post-handshake-auth.py", line 458, in main
    runner.run()
  File "/home/hkario/dev/tlsfuzzer/tlsfuzzer/runner.py", line 262, in run
    raise AssertionError("Unexpected closure from peer")
AssertionError: Unexpected closure from peer

Basic post-handshake authentication test case
Check if server will accept PHA, check if server rejects invalid
signatures on PHA CertificateVerify, etc.
version: 1

Test end
successful: 0
failed: 1
  'post-handshake authentication with KeyUpdate'

Comment 6

[Alicja Kario]

(the above was with the NSS as of fc636973ad06392 and tlsfuzzer from https://github.com/tomato42/tlsfuzzer/pull/551, d132f915e0cf0)

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:jcj, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 8

[Kevin Jacobs [:kjacobs]]

Thanks for the additional info. After sending the CR, the handshake is driven. Libssl first handles the KeyUpdate, then enters idle_handshake and returns. I.e., it handles the KU instead of the intended PHA response. This leaves nss in a keyUpdateDeferred state. The server then sends its data and closes the connection normally. We entirely miss the PHA response as there is no subsequent read or SSL_ForceHandshake.

For PHA, the RFC allows an arbitrary number of messages between sending the CR and receiving the C/CV/Fin, meaning we could be in this "deferred" state indefinitely. As noted in coment 5, selfserv.c proceeds with sending Application Data while in this state, which goes against 4.6.3.

@Daiki, as this code is fairly recent, do you have an opinion and/or recall the rationale for deferring key update response?

Comment 9

[Daiki Ueno [:ueno]]

I (only vaguely) remember this was added in the context of: https://phabricator.services.mozilla.com/D21935#757338