utls: [BUG] (Fake|Utls)PreSharedKeyExtension

There appears to be a bug/typo, which I’ll clarify. In short, the bug is in newSessionController, where we should set these to nil: https://github.com/refraction-networking/utls/blob/df6e4c827aa5895583d53e9ccc5139077af27786/u_session_controller.go#L57-L58

---

The SessionController was designed to manage all session-related operations. Initially, it had ownership over extensions.

But this posed a challenge. The ClientHelloSpec might have a different set of PSK/Session extensions. For clarity, the original design had the SessionController take charge of the extensions, overwriting those from the spec. This leads to the design that newSessionController produces a non-nil session and a psk extension. A downside of this approach was that it altered the signature of a public method, and we didn’t have a clear vision for v2.0 yet.

To circumvent this, I pivoted to a strategy where the Spec owns the extensions, ensuring the method signature remains unchanged. However, users should be able to override psk/session extensions before invoking ApplyPreset. These overriding extensions should exclusively reside in the SessionController. In such cases, the SessionController have ownership over the extensions.

This necessitated the creation of the syncSessionExts method to discern which extension takes precedence. The guiding principle is straightforward:

1. If both the user and the spec offer an extension, prioritize the user’s.
2. If the spec lacks an extension and the user attempts to provide one, it’s deemed an error.
3. In the absence of a user-provided extension, revert to the spec’s version, if available.

Yet, during this overhaul, I overlooked the newSessionController method. As illustrated in the comments, deep in my mind I believe that the extension would default to nil unless overridden by the user: https://github.com/refraction-networking/utls/blob/df6e4c827aa5895583d53e9ccc5139077af27786/u_session_controller.go#L271-L273

Sorry about the delay in getting a PR sorted… life and all that 😄

@VeNoMouS No problem! Thanks for your efforts!

Ok, i’ll dive deeper into FakePreSharedKeyExtension and see if i can fix what ever is broken with it (if a PSK is present) … 🤞

To clarify on the intended behaviors:

• If RealPSKResumption is set to true: parsing from raw bytes will discard the ticket and binders from the raw bytes, but use the available ones from ClientSessionCache instead.
• If RealPSKResumption is false: parsing from raw bytes with legitimate PSK tickets and binders IS GUARANTEED TO FAIL, given that:
  • the server will check the ticket and find it is legitimate, and proceed with verifying the binder. If we are sending binders from the original raw bytes (that’s the intended behavior), the binder will not match the expected value which is calculated from all previous bytes in ClientHello.
  • If we were to re-calculated binders, it will be a different challenge for us to find the shared secret for that specific ticket.

Interesting observation. I don’t believe we are ready to parse raw byte ClientHello into PSK yet. On the other hand it is theoretically not possible to resume the connection with just the PSK ticket and binder from raw bytes.

That said, it might worth debugging and fixing to make at least FakePreSharedKeyExtension work with raw bytes. But note that in the end, unexpected things would happen, such as server rejecting you for submitting a valid ticket with an incorrect binder, etc.

Tagging @3andne for visibility.