Re: RFC: tls_key_log: report TLS pre-master secrets, other key material

On 30/07/20 6:41 am, Alex Rousskov wrote:
> On 7/15/20 3:14 PM, Alex Rousskov wrote:
> 
>>     I propose to add a new tls_key_log directive to record TLS
>> pre-master secret (and related encryption details) for to- and
>> from-Squid TLS connections. This very useful triage feature is common
>> for browsers and some networking tools. Wireshark supports it[1]. You
>> might know it as SSLKEYLOGFILE. It has been requested by several Squid
>> admins. A draft documentation of the proposed directive is at the end of
>> this email.
>>
>> [1] https://wiki.wireshark.org/TLS#Using_the_.28Pre.29-Master-Secret
>>
>> If you have any feature scope adjustments, implementation wishes, or
>> objections to this feature going in, please let me know!
> 
> 
> FYI: Factory is starting to implement this feature.
> 

Sorry I forgot to reply to this earlier.

Two design points:

1) It seems to me these bits are part of the handshake. So would come in
either as members/args of the %handshake logformat macros (some not yet
implemented) or as secondary %handshake_foo macros in the style %cert_*
macros use.


2) Please do use the logging logic implemented for access_log, just with
the next directive as list of log outputs to write at the appropriate
logging trigger time.

I accept the reasoning for not using access_log directive. This will
need a new log directive with different times when it triggers output
written there. However (most of) the implementation logic of access_log
should be usable for this new output.



>> We propose to structure this new directive so that it is easy to add
>> advanced access_log-like features later if needed (while reusing the
>> corresponding access_log code). For example, if users find that they
>> want to maintain multiple TLS key logs or augment log records with
>> connection details, we can add that support by borrowing access_log
>> options and code without backward compatibility concerns. The new
>> required "if" keyword in front of the ACL list allows for seamless
>> addition of new directive options in the future.
>>


Accepted, provided the directive *does* support access_log feature
addition. The plan below does not meet that criteria. Some changes
inline below to make it do so.


>> ---------- draft squid.conf directive documentation ------------
>>
>> tls_key_log
>>
>> Configures whether and where Squid records pre-master secret and
>> related encryption details for TLS connections accepted or established
>> by Squid. These connections include connections accepted at
>> https_port, TLS connections opened to origin servers/cache_peers/ICAP
>> services, and TLS tunnels bumped by Squid using the SslBump feature.
>> This log (a.k.a. SSLKEYLOGFILE) is meant for triage with traffic
>> inspection tools like Wireshark.
>>
>>     tls_key_log <filename> if <acl>...
>>

Please allow extension points for options and modules:

  tls_key_log stdio:<filename> [options] if <acl>...


The "stdio:" module name is to allow for sharing the access_log config
parser and future expansion to logging modules like daemon: which we
will doubtless be asked for later.



>> WARNING: This log allows anybody to decrypt the corresponding
>> encrypted TLS connections, both in-flight and postmortem.
>>
>> At most one log file is supported at this time. Repeated tls_key_log
>> directives are treated as fatal configuration errors. By default, no
>> log is created or updated.

With ACL support it seems reasonable to support multiple logs. We should
be able to re-use (with minor change to pass the list of log outputs to
the function) the logic access_log has for writing to a list of outputs.


>>
>> If the log file does not exist, Squid creates it. Otherwise, Squid
>> appends an existing log file.
>>
>> The directive is consulted whenever a TLS connection is accepted or
>> established by Squid. TLS connections that fail the handshake may be
>> logged if Squid got enough information to form a log record. A record
>> is logged only if all of the configured ACLs match.
>>
>> Squid does not buffer these log records -- the worker blocks until
>> each record is written. File system buffering may speed things up, but
>> consider placing this triage log in a memory-based partition.
>>
>> This log is rotated based on the logfile_rotate settings.
>>

Please don't use solely that directive. The new directive should have a
rotate=N option of its own. Only using the global directive as a default
if that option is unset.


Amos
_______________________________________________
squid-dev mailing list
[email protected]
http://lists.squid-cache.org/listinfo/squid-dev