-
Notifications
You must be signed in to change notification settings - Fork 82
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP]: Add key logging for TLS 1.3 and TLS 1.2 (#523) #542
base: master
Are you sure you want to change the base?
Conversation
(as the CI verifies that tests can be executed after every commit, you will have to squash/fixup the commits together) |
a964b2b
to
67bd0c8
Compare
ssl_key_log_file.writelines( | ||
"{0} {1} {2}\n".format( | ||
label, | ||
binascii.hexlify(client_random).decode().upper(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using b2a_hex()
from compat
would be better:
tlslite-ng/tlslite/utils/compat.py
Line 68 in c93df82
def b2a_hex(b): |
@@ -2250,6 +2275,12 @@ def handshakeServerAsync(self, verifierDB=None, | |||
for result in self._handshakeWrapperAsync(handshaker, checker): | |||
yield result | |||
|
|||
# Log client random and master secret for version < TLS1.3 | |||
if self.sslkeylogfile and self.version < (3, 4): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but why log it only when it's not TLS 1.3?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The client random label + master secret isn't needed for TLS 1.3. These are the labels for 1.3
$ openssl s_client -connect www.example.com:443 -keylogfile /dev/stdout -quiet 2>/dev/null
# SSL/TLS secrets log file, generated by OpenSSL
SERVER_HANDSHAKE_TRAFFIC_SECRET 073529f81ea749dbf6ab08114ea89d2690b36c5958e3785d4597514b8d5264c3 83bd3e0b65e29d749d130baa7dc9e5a52d748280948415b526b9ce6d62501e462e89b64f56b1ec77708166c359c4f9b7
EXPORTER_SECRET 073529f81ea749dbf6ab08114ea89d2690b36c5958e3785d4597514b8d5264c3 bdd7bda6b8ed3b85171db62902e0da071600e15669421a09ea73ff1813c905165f0af65990cc81f3f4cb0e74bd854cbd
SERVER_TRAFFIC_SECRET_0 073529f81ea749dbf6ab08114ea89d2690b36c5958e3785d4597514b8d5264c3 1dc871b90ce46bb3e3f8844da41ce6e64a280c80eb807383ce73d09fd1ec0c78f7c68e993dbdcbf8d9ad5d637ca439d1
CLIENT_HANDSHAKE_TRAFFIC_SECRET 073529f81ea749dbf6ab08114ea89d2690b36c5958e3785d4597514b8d5264c3 eb4ff0bdac69c3727b6f6a2d9a7bc944f56355e6f14c8dc566cc4279650175c414879b7345027226e780b313a99fa6c6
CLIENT_TRAFFIC_SECRET_0 073529f81ea749dbf6ab08114ea89d2690b36c5958e3785d4597514b8d5264c3 113f23d155205273758f8758ec66057a925eb156b633ee6480d13b201470ecff2b7edb92e2f51a4a71af4fecfad1c3a5
@@ -1323,6 +1327,15 @@ def _clientTLS13Handshake(self, settings, session, clientHello, | |||
self._handshake_hash, | |||
prfName) | |||
|
|||
# TLS1.3 log Client and Server traffic secrets for SSLKEYLOGFILE | |||
if self.sslkeylogfile: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be cleaner to have the check if we should log stuff inside the method to log it...
|
||
def create_connection(hostname='www.example.com', port=443, settings=HandshakeSettings()): | ||
raw_socket = socket(AF_INET, SOCK_STREAM) | ||
raw_socket.connect((hostname, port)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we definitely shouldn't be making real connections in the unit tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(as in to external servers)
@tomato42 considering how people may use this library, this may not be process or thread safe. I'm thinking about changing this to use logger class that can be passed to the TLSConnection's init which can handle IO in a safer way. Also taking into consideration your comments |
yes, we can have two |
work in progress for SSLKEYLOGFILE support
fixes #523
This change is