Merge pull request #334 from tomato42/resumption-cipher-mismatch

Resumption cipher mismatch
tlsfuzzer · Dec 20, 2018 · 3696909 · 3696909
2 parents 7c1b350 + c2d29ff
commit 3696909
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 46 deletions.
diff --git a/tests/tlstest.py b/tests/tlstest.py
@@ -822,7 +822,7 @@ def connect():
         connection.handshakeClientCert(serverName=address[0], session=session,
                                        settings=settings)
     except TLSRemoteAlert as e:
-        assert(str(e) == "handshake_failure")
+        assert(str(e) == "illegal_parameter")
     else:
         raise AssertionError("No exception raised")
     connection.close()
@@ -1649,7 +1649,7 @@ def server_bind(self):
         connection.handshakeServer(certChain=x509Chain, privateKey=x509Key,
                                    sessionCache=sessionCache)
     except TLSLocalAlert as e:
-        assert(str(e) == "handshake_failure")
+        assert(str(e) == "illegal_parameter")
     else:
         raise AssertionError("no exception raised")
     connection.close()

diff --git a/tlslite/tlsconnection.py b/tlslite/tlsconnection.py
@@ -2837,54 +2837,60 @@ def _serverGetClientHello(self, settings, cert_chain, verifierDB,
         if clientHello.session_id and sessionCache:
             session = None
 
-            #Check in the session cache
-            if sessionCache and not session:
-                try:
-                    session = sessionCache[clientHello.session_id]
-                    if not session.resumable:
-                        raise AssertionError()
-                    #Check for consistency with ClientHello
-                    if session.cipherSuite not in cipherSuites:
-                        for result in self._sendError(\
-                                AlertDescription.handshake_failure):
-                            yield result
-                    if session.cipherSuite not in clientHello.cipher_suites:
-                        for result in self._sendError(\
-                                AlertDescription.handshake_failure):
-                            yield result
-                    if clientHello.srp_username:
-                        if not session.srpUsername or \
-                            clientHello.srp_username != bytearray(session.srpUsername, "utf-8"):
-                            for result in self._sendError(\
-                                    AlertDescription.handshake_failure):
-                                yield result
-                    if clientHello.server_name:
-                        if not session.serverName or \
-                            clientHello.server_name != bytearray(session.serverName, "utf-8"):
-                            for result in self._sendError(\
-                                    AlertDescription.handshake_failure):
-                                yield result                    
-                    if session.encryptThenMAC and \
-                            not clientHello.getExtension(
-                                    ExtensionType.encrypt_then_mac):
-                        for result in self._sendError(\
+            # Check if the session there is good enough and consistent with
+            # new Client Hello
+            try:
+                session = sessionCache[clientHello.session_id]
+                if not session.resumable:
+                    raise AssertionError()
+                # Check if we are willing to use that old cipher still
+                if session.cipherSuite not in cipherSuites:
+                    session = None
+                    raise KeyError()
+                # Check for consistency with ClientHello
+                # see RFC 5246 section 7.4.1.2, description of
+                # cipher_suites
+                if session.cipherSuite not in clientHello.cipher_suites:
+                    for result in self._sendError(
+                            AlertDescription.illegal_parameter):
+                        yield result
+                if clientHello.srp_username:
+                    if not session.srpUsername or \
+                            clientHello.srp_username != \
+                            bytearray(session.srpUsername, "utf-8"):
+                        for result in self._sendError(
                                 AlertDescription.handshake_failure):
                             yield result
-                    # if old session used EMS, new connection MUST use EMS
-                    if session.extendedMasterSecret and \
-                            not clientHello.getExtension(
-                                    ExtensionType.extended_master_secret):
-                        for result in self._sendError(\
+                if clientHello.server_name:
+                    if not session.serverName or \
+                            clientHello.server_name != \
+                            bytearray(session.serverName, "utf-8"):
+                        for result in self._sendError(
                                 AlertDescription.handshake_failure):
                             yield result
-                    # if old session didn't use EMS but new connection
-                    # advertises EMS, create a new session
-                    elif not session.extendedMasterSecret and \
-                            clientHello.getExtension(
-                                    ExtensionType.extended_master_secret):
-                        session = None
-                except KeyError:
-                    pass
+                if session.encryptThenMAC and \
+                        not clientHello.getExtension(
+                                ExtensionType.encrypt_then_mac):
+                    for result in self._sendError(
+                            AlertDescription.illegal_parameter):
+                        yield result
+                # if old session used EMS, new connection MUST use EMS
+                if session.extendedMasterSecret and \
+                        not clientHello.getExtension(
+                                ExtensionType.extended_master_secret):
+                    # RFC 7627, section 5.2 explicitly requires
+                    # handshake_failure
+                    for result in self._sendError(
+                            AlertDescription.handshake_failure):
+                        yield result
+                # if old session didn't use EMS but new connection
+                # advertises EMS, create a new session
+                elif not session.extendedMasterSecret and \
+                        clientHello.getExtension(
+                                ExtensionType.extended_master_secret):
+                    session = None
+            except KeyError:
+                pass
 
             #If a session is found..
             if session: