Closed marten-seemann closed 7 months ago
Looks good to me.
My understanding of how these events will be used is something like (for a client):
t=0: Initial EL provisioned; key_updated(client_initial_secret, new=...)
t=0: Initial EL provisioned; key_updated(server_initial_secret, new=...)
...
t=a: Handshake EL provisioned; key_updated(client_handshake_secret, new=...)
t=a; Handshake EL provisioned; key_updated(server_handshake_secret, new=...)
...
t=b; Initial EL dropped; key_discarded(client_initial_secret)
t=b; Initial EL dropped; key_discarded(server_initial_secret)
...
t=c; 1-RTT EL provisioned, key_updated(client_1rtt_secret, generation=0, new=...)
t=c; 1-RTT EL provisioned; key_updated(server_1rtt_secret, generation=0, new=...)
...
t=d; Handshake EL dropped; key_discarded(client_handshake_secret)
t=d; Handshake EL dropped; key_discarded(server_handshake_secret)
...
Key update:
t=m; Key update; key_updated(client_1rtt_secret, generation=1, old=..., new=...)
t=m; Key update; key_updated(server_1rtt_secret, generation=1, old=..., new=...)
(previous keys are kept around for a while to handle in-flight packets)
t=n; Key update completed; key_discarded(client_1rtt_secret, generation=0)
t=n; Key update completed; key_discarded(server_1rtt_secret, generation=0)
This works and gives good understanding to a qlog consumer of when a set of keys is available and handles time periods where multiple keys are available well. LGTM.
The important part here is that the key phase keeps increasing:
Key update:
t=m; Key update; key_updated(client_1rtt_secret, key_phase=1, old=..., new=...)
t=m; Key update; key_updated(server_1rtt_secret, key_phase=1, old=..., new=...)
t=m; Key update; key_updated(client_1rtt_secret, key_phase=2, old=..., new=...)
t=m; Key update; key_updated(server_1rtt_secret, key_phase=2, old=..., new=...)
t=m; Key update; key_updated(client_1rtt_secret, key_phase=3, old=..., new=...)
t=m; Key update; key_updated(server_1rtt_secret, key_phase=3, old=..., new=...)
Fixes #376. Also changing the type to uint64, since I don't see why the number of key updates would be limited to MaxUint32.