Bug 1651160

Summary:	Session resumption and SNI extension
Product:	Red Hat Enterprise Linux 8	Reporter:	Ondrej Moriš <omoris>
Component:	gnutls	Assignee:	Anderson Sasaki <ansasaki>
Status:	CLOSED WONTFIX	QA Contact:	BaseOS QE Security Team <qe-baseos-security>
Severity:	low	Docs Contact:
Priority:	unspecified
Version:	8.0	CC:	hkario, omoris, pvrabec
Target Milestone:	rc
Target Release:	8.0
Hardware:	All
OS:	Linux
Whiteboard:
Fixed In Version:		Doc Type:	If docs needed, set a value
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2018-12-10 15:52:53 UTC	Type:	Bug
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:

Description Ondrej Moriš 2018-11-19 10:35:40 UTC

Description of problem:

This is a follow-up on BZ#1333521, Comment #6, point (4) for RHEL-8. There is a remaining issue with handling SNI extension in GnuTLS during session resumption. Session resume doesn't inspect SNI is client hello before resuming session, that includes situations in which the SNI extension is completely malformed.

Version-Release number of selected component (if applicable):

gnutls-3.6.4-2.el8

How reproducible:

100%

Steps to Reproduce:

0. Prepare keys

   # openssl req -x509 -newkey rsa \
                       -keyout localhost.key \
                       -out localhost.crt \
                       -subj /CN=localhost 
                       -nodes \ 
                       -batch

1. Start GnuTLS server

   # gnutls-serv --http \
                 --port 4433 \
                 --x509keyfile localhost.key \
                 --x509certfile localhost.crt \
                 --disable-client-cert \
                 --priority NORMAL:+VERS-TLS1.2 \
                 --sni-hostname localhost4 \
                 --sni-hostname-fatal

2. Run tlsfuzzer test test-invalid-server-name-extension-resumption.py.

Actual results:

# PYTHONPATH=tlsfuzzer /usr/libexec/platform-python tlsfuzzer/scripts/test-invalid-server-name-extension-resumption.py
sanity ...
OK

Sanity check, SNI ...
OK

Sanity check, bad SNI ...
OK

session resume with malformed SNI ...
Error encountered while processing node <tlsfuzzer.expect.ExpectServerHello object at 0x7fcf948735f8> (child: <tlsfuzzer.expect.ExpectCertificate object at 0x7fcf948736a0>) with last message being: <tlslite.messages.Message object at 0x7fcf94875b70>
Error while processing
Traceback (most recent call last):
  File "tlsfuzzer/scripts/test-invalid-server-name-extension-resumption.py", line 297, in main
    runner.run()
  File "/tmp/tmp.4rmOBy2pLK/tlsfuzzer/tlsfuzzer/runner.py", line 166, in run
    RecordHeader2)))
AssertionError: Unexpected message from peer: Alert(fatal, illegal_parameter)

session resume with different SNI ...
Error encountered while processing node <tlsfuzzer.expect.ExpectServerHello object at 0x7fcf94875048> (child: <tlsfuzzer.expect.ExpectCertificate object at 0x7fcf948750b8>) with last message being: <tlslite.messages.Message object at 0x7fcf94875e80>
Error while processing
Traceback (most recent call last):
  File "tlsfuzzer/scripts/test-invalid-server-name-extension-resumption.py", line 297, in main
    runner.run()
  File "/tmp/tmp.4rmOBy2pLK/tlsfuzzer/tlsfuzzer/runner.py", line 166, in run
    RecordHeader2)))
AssertionError: Unexpected message from peer: Alert(fatal, unrecognized_name)

sanity ...
OK


SNI resumption test version 2
In case the "session resume with malformed SNI" or the
"session resume with different SNI" test case
the server replies with ChangeCipherSpec when we expect
ExpectCertificate, it means it does not follow a "MUST NOT"
clause of RFC 6066 Section 3

Test end
successful: 4
failed: 2
  'session resume with different SNI'
  'session resume with malformed SNI'

Expected results:

All tests passed.

Additional info:

Used tlsfuzzer-0.0.0-alpha6.

Comment 1 Anderson Sasaki 2018-11-27 14:55:39 UTC

It seems the remaining issues were fixed in tlsfuzzer upstream in:

https://github.com/tomato42/tlsfuzzer/commit/412caa96bea8ecea27d15a75a7f4eae18822b06e

Is it possible to update the tlsfuzzer script "test-invalid-server-name-extension-resumption.py" and retry the tests?

Comment 2 Ondrej Moriš 2018-12-07 10:14:38 UTC

I checked the latest version of test script [1]. And yes, it is slightly different from the one used in our beaker test. There were two failures in 

1) session resume with different SNI

When I use the latest version of the test, it is no longer failing. Resolved.

2) session resume with malformed SNI

This test is still failing when using the latest version of the test:

session resume with different SNI ...
Error encountered while processing node <tlsfuzzer.expect.ExpectServerHello object at 0x7fd3f6a8b668> (child: <tlsfuzzer.expect.ExpectCertificate object at 0x7fd3f6a8b780>) with last message being: <tlslite.messages.Message object at 0x7fd3f6a8bf28>
Error while processing
Traceback (most recent call last):
  File "tlsfuzzer/scripts/test-invalid-server-name-extension-resumption.py", line 279, in main
    runner.run()
  File "/tmp/tmp.F2tp0oobTQ/tlsfuzzer/tlsfuzzer/runner.py", line 166, in run
    RecordHeader2)))
AssertionError: Unexpected message from peer: Alert(fatal, unrecognized_name)

Line 238 of [1] causes fail.

[1] https://github.com/tomato42/tlsfuzzer/blob/412caa96bea8ecea27d15a75a7f4eae18822b06e/scripts/test-invalid-server-name-extension-resumption.py

Comment 3 Anderson Sasaki 2018-12-10 14:37:21 UTC

The gnutls upstream do not follow the RFC on SNI resumptions explicitly [1], skipping both tests which were causing the failures.
The PR which enabled the tlsfuzzer tests can be found in [2].

Considering this, I prefer to not change the gnutls behaviour in RHEL 8 and follow the upstream behaviour.
I would close this bug with a WONTFIX and skip these tests as they are in upstream.

Ondrej, do you agree?

[1] https://gitlab.com/gnutls/gnutls/blob/master/tests/suite/tls-fuzzer/gnutls-nocert.json#L63
[2] https://gitlab.com/gnutls/gnutls/merge_requests/442

Comment 4 Ondrej Moriš 2018-12-10 15:45:32 UTC

It sounds reasonable to me. Feel free to close it. I will update the test to skip both tests and keep there a reference to this bugzilla for the future. Thanks for investigation.