Allow BuildHandshakeState to inspect ClientHello before setting SessionTicket/PSK#301
Conversation
|
Looks interesting, I am gonna give a pass on this PR later this week. Thanks for contributing to uTLS, @adotkhan! |
gaukas
left a comment
gaukas
left a comment
There was a problem hiding this comment.
Thanks for the pull request. Overall it looks good to me, but it would be much better if we can follow a less breaking manner (see comments for MakeClientSessionState in u_public.go).
I also shared my two cents on separating (*UConn).lockSessionState out of the last few lines in (*UConn).BuildHandshakeState. I agree we probably should do something like that, but I would kindly ask you to at least expore if there are any further change we can add to make it more natural (comments in u_conn.go)
This partially reverts ebe5d66 and introduces BuildHandshakeStateWithoutSession.
|
Thank you for the review @gaukas, much appreciated. I have applied your suggestion regarding removing cbf02ab reverts BuildHandshakeState behavior to what it was before, and adds a new Since we only needed to inspect a partial/incomplete Client Hello, as more manual use case, I believe it makes sense to not change |
|
I missed what you mentioned regarding marshalling the Client Hello after the |
|
Much obliged! Feel free to mark this pull request ready-for-review and re-request a review from me if you are done with it. |
gaukas
left a comment
gaukas
left a comment
There was a problem hiding this comment.
Looks good to me, thanks for patching BuildHandshakeState and adding Extended Master Secret-related methods to *ClientSessionState!
…onTicket/PSK (#301) * Lock sessionController only on last call to BuildHandshakeState * Add public getter/setter for SessionState.extMasterSecret * Fix breaking exported MakeClientSessionState * Revert `(*UConn).BuildHandshakeState` to lock session controller This partially reverts ebe5d66 and introduces BuildHandshakeStateWithoutSession. * fix: Marshal the Client Hello after loading session --------- Signed-off-by: Gaukas Wang <[email protected]>
|
Released in v1.6.7! |
…onTicket/PSK (refraction-networking#301) * Lock sessionController only on last call to BuildHandshakeState * Add public getter/setter for SessionState.extMasterSecret * Fix breaking exported MakeClientSessionState * Revert `(*UConn).BuildHandshakeState` to lock session controller This partially reverts ebe5d66 and introduces BuildHandshakeStateWithoutSession. * fix: Marshal the Client Hello after loading session --------- Signed-off-by: Gaukas Wang <[email protected]>
…onTicket/PSK (refraction-networking#301) * Lock sessionController only on last call to BuildHandshakeState * Add public getter/setter for SessionState.extMasterSecret * Fix breaking exported MakeClientSessionState * Revert `(*UConn).BuildHandshakeState` to lock session controller This partially reverts ebe5d66 and introduces BuildHandshakeStateWithoutSession. * fix: Marshal the Client Hello after loading session --------- Signed-off-by: Gaukas Wang <[email protected]>
…onTicket/PSK (refraction-networking#301) * Lock sessionController only on last call to BuildHandshakeState * Add public getter/setter for SessionState.extMasterSecret * Fix breaking exported MakeClientSessionState * Revert `(*UConn).BuildHandshakeState` to lock session controller This partially reverts ebe5d66 and introduces BuildHandshakeStateWithoutSession. * fix: Marshal the Client Hello after loading session --------- Signed-off-by: Gaukas Wang <[email protected]>
…onTicket/PSK (refraction-networking#301) * Lock sessionController only on last call to BuildHandshakeState * Add public getter/setter for SessionState.extMasterSecret * Fix breaking exported MakeClientSessionState * Revert `(*UConn).BuildHandshakeState` to lock session controller This partially reverts ebe5d66 and introduces BuildHandshakeStateWithoutSession. * fix: Marshal the Client Hello after loading session --------- Signed-off-by: Gaukas Wang <[email protected]>
2 participants
The TLS-PSK changes introduced in 8094658 sets the SessionTicket or PSK (either from cache or explicitly set) on the first call to
(*UConn)BuildHandshakeStateand locks thesessionController, preventing library user from inspecting the ClientHello to then craft an appropriate SessionTicket / PSK.Our use case involves building a
UConnwith a potentially randomzied ClientHello and adding a SessionTicket to it encrypted with an out-of-band shared secret. To craft the appropriate SessionTicket (since the ClientHello parameters need to agree with it), the ClientHello is inspected for example to check whether EMS is present by callingBuildHandshakeStatefirst.Specifically, with regards to PSK / SessionTicket, the following doc comment on
BuildHandshakeStatedoes not apply toSetSessionTicketExtension/SetPskExtension, since the sessionController is locked after the firstBuildHandshakeStatecall and further change to these extensions is disallowed:This draft PR suggests one workaround, it's admittedly a fragile solution, however a review by uTLS maintainers is much appreicated towards finding an appropriate solution.