Stronger guards against session ticket resumption with tls 1.3 and custom parrots?

[max-b]

We seem to be having some issues with some of the parrots and session ticket resumption.

It looks to me like session ticket resumption is not currently supported with tls 1.3 and custom parrots. This snippet of loadSession seems to be re-marshalling the client hello with updateBinders, which assumes that the raw bytes have a extensionPreSharedKey at the end:
https://github.com/getlantern/utls/blob/0c02248f7ce1fa1928b137ff77d6bad4cb0486aa/handshake_client.go#L281-L333

utls/handshake_messages.go

Lines 323 to 351 in 33a2903

	// updateBinders updates the m.pskBinders field, if necessary updating the
	// cached marshaled representation. The supplied binders must have the same
	// length as the current m.pskBinders.
	func (m *clientHelloMsg) updateBinders(pskBinders [][]byte) {
	if len(pskBinders) != len(m.pskBinders) {
	panic("tls: internal error: pskBinders length mismatch")
	}
	for i := range m.pskBinders {
	if len(pskBinders[i]) != len(m.pskBinders[i]) {
	panic("tls: internal error: pskBinders length mismatch")
	}
	}
	m.pskBinders = pskBinders
	if m.raw != nil {
	lenWithoutBinders := len(m.marshalWithoutBinders())
	// TODO(filippo): replace with NewFixedBuilder once CL 148882 is imported.
	b := cryptobyte.NewBuilder(m.raw[:lenWithoutBinders])
	b.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) {
	for _, binder := range m.pskBinders {
	b.AddUint8LengthPrefixed(func(b *cryptobyte.Builder) {
	b.AddBytes(binder)
	})
	}
	})
	if len(b.BytesOrPanic()) != len(m.raw) {
	panic("tls: internal error: failed to update binders")
	}
	}
	}

This has garbled the ends of ClientHellos that we've generated when using session tickets and parrots that support tls1.3.

It seems to me that basically, utls in its present form doesn't support the combination of:

• session ticket resumption
• parrots
• tls 1.3

Am I wrong about that? Are we somehow holding the library incorrectly? I've included some pcaps demonstrating the issue we've been encountering and if it would help, I think I could write up a test which demonstrates the behavior we're encountering. I'd be happy to find out that I'm just missing some key piece somewhere.

If I'm not wrong about this, is there some way to put up stronger guards against doing this?

Lastly, is implementing the pre_shared_key extension on your radar/something you'd be interested in? If it's just a matter of your time/energy/resources, I think our org could at least attempt a PR?

As always, thank you so much for your time, energy and patience on this!!!

chrome-83-clienthello-errs.pcap.gz
firefox-65-clienthello-errs.pcap.gz

[i542873057]

I have meet the same problem like you. Do you have any solution?

Any help to me will be appreciated!

[max-b]

I have meet the same problem like you. Do you have any solution?

Any help to me will be appreciated!

@sergeyfrolov may be able to suggest actual solutions, but we ended up just turning off the session cache functionality when we knew we were using a parrot which supported tls 1.3

[detunized]

I ran into the same issue and it took me a lot of time to diagnose and reproduce this. I would be interested in fixing this but so far lack the needed deep understanding of how the client hello is [un]marshalling works together with the updateBinders stuff.

[gaukas]

I know this issue is kinda old but if anyone is still interested, uTLS now supports real TLS 1.3 (PSK) resumption along with the old TLS 1.2 session-ticket resumption. And thanks to @3andne we revisited and revised a ton of outdated/malfunction code. I believe this issue should be gone at least for the intended use of uTLS's PSK extension(s) but if not, any input is welcome.