mirror of
https://github.com/XTLS/REALITY.git
synced 2025-08-24 15:38:36 +00:00
crypto/tls: use decode alert for handshake msg unmarshal err
Previously if instances of the handshakeMessage interface returned false from unmarshal(), indicating an umarshalling error, the crypto/tls package would emit an unexpected_message alert. This commit changes to use a decode_error alert for this condition instead. The usage-pattern of the handshakeMessage interface is that we switch on the message type, invoke a specific concrete handshakeMessage type's unmarshal function, and then return it to the caller on success. At this point the caller looks at the message type and can determine if the message was unexpected or not. If it was unexpected, the call-sites emit the correct error for that case. Only the caller knows the current protocol state and allowed message types, not the generic handshake decoding logic. With the above in mind, if we find that within the unmarshal logic for a specific message type that the data we have in hand doesn't match the protocol syntax we should emit a decode_error. An unexpected_message error isn't appropriate because we don't yet know if the message is unexpected or not, only that the message can't be decoded based on the spec's syntax for the type the message claimed to be. Notably one unit test, TestQUICPostHandshakeKeyUpdate, had to have its test data adjusted because it was previously not testing the right thing: it was double-encoding the type & length prefix data for a key update message and expecting the QUIC logic to reject it as an inappropriate post-handshake message. In reality it was being rejected sooner as an invalid key update message from the double-encoding and this was masked by the previous alert for this condition matching the expected alert. Finally, changing our alert allows enabling a handful of BoGo tests related to duplicate extensions of the form "DuplicateExtension[Server|Client]-TLS-[TLS1|TLS11|TLS12|TLS13]". One test remains skipped (DuplicateExtensionClient-TLS-TLS13), as it requires additional follow-up. Updates #72006 Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5d1 Reviewed-on: https://go-review.googlesource.com/c/go/+/673738 Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: David Chase <drchase@google.com> Auto-Submit: Daniel McCarney <daniel@binaryparadox.net> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Filippo Valsorda <filippo@golang.org>
This commit is contained in:
parent
5a0e0628ae
commit
3c80a18847
2
conn.go
2
conn.go
@ -1242,7 +1242,7 @@ func (c *Conn) unmarshalHandshakeMessage(data []byte, transcript transcriptHash)
|
||||
data = append([]byte(nil), data...)
|
||||
|
||||
if !m.unmarshal(data) {
|
||||
return nil, c.in.setErrorLocked(c.sendAlert(alertUnexpectedMessage))
|
||||
return nil, c.in.setErrorLocked(c.sendAlert(alertDecodeError))
|
||||
}
|
||||
|
||||
if transcript != nil {
|
||||
|
Loading…
Reference in New Issue
Block a user