Allow BuildHandshakeState to inspect ClientHello before setting SessionTicket/PSK

[adotkhan]

The TLS-PSK changes introduced in 8094658 sets the SessionTicket or PSK (either from cache or explicitly set) on the first call to (*UConn)BuildHandshakeState and locks the sessionController, preventing library user from inspecting the ClientHello to then craft an appropriate SessionTicket / PSK.

Our use case involves building a UConn with 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 calling BuildHandshakeState first.

Specifically, with regards to PSK / SessionTicket, the following doc comment on BuildHandshakeState does not apply to SetSessionTicketExtension/SetPskExtension, since the sessionController is locked after the first BuildHandshakeState call and further change to these extensions is disallowed:

// BuildHandshakeState is automatically called before uTLS performs handshake,
// amd should only be called explicitly to inspect/change fields of
// default/mimicked ClientHello.

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.

[gaukas]

Looks interesting, I am gonna give a pass on this PR later this week.

Thanks for contributing to uTLS, @adotkhan!

[gaukas]

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)

[adotkhan]

Thank you for the review @gaukas, much appreciated.

I have applied your suggestion regarding removing ExtMasterSecret bool field from MakeClientSessionState to avoid breaking the API.

cbf02ab reverts BuildHandshakeState behavior to what it was before, and adds a new BuildHandshakeStateWithoutSession which I believe would resolve the concerns you raised.

Since we only needed to inspect a partial/incomplete Client Hello, as more manual use case, I believe it makes sense to not change BuildHandshakeState.
This also avoids potential issues with having to remember to call lockSessionState.

[adotkhan]

I missed what you mentioned regarding marshalling the Client Hello after the uLoadSession call.
I believe f061fb7 should resolve the issues raised better.

[gaukas]

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]

Looks good to me, thanks for patching BuildHandshakeState and adding Extended Master Secret-related methods to *ClientSessionState!

[gaukas]

Released in v1.6.7!