Merge pull request #1940 from lucas-clemente/unexpected-handshake-messages

fix crash when receiving unexpected handshake messages

internal/handshake/crypto_setup.go

@@ -16,6 +16,9 @@ import (
 	"github.com/marten-seemann/qtls"
 )
 
+// TLS unexpected_message alert
+const alertUnexpectedMessage uint8 = 10
+
 type messageType uint8
 
 // TLS handshake message types.
@@ -333,10 +336,10 @@ func (h *cryptoSetup) checkEncryptionLevel(msgType messageType, encLevel protoco
 	case typeNewSessionTicket:
 		expected = protocol.Encryption1RTT
 	default:
-		return fmt.Errorf("unexpected handshake message: %d", msgType)
+		return qerr.CryptoError(alertUnexpectedMessage, fmt.Sprintf("unexpected handshake message: %d", msgType))
 	}
 	if encLevel != expected {
-		return fmt.Errorf("expected handshake message %s to have encryption level %s, has %s", msgType, expected, encLevel)
+		return qerr.CryptoError(alertUnexpectedMessage, fmt.Sprintf("expected handshake message %s to have encryption level %s, has %s", msgType, expected, encLevel))
 	}
 	return nil
 }
@@ -386,7 +389,8 @@ func (h *cryptoSetup) handleMessageForServer(msgType messageType) bool {
 		}
 		return true
 	default:
-		panic("unexpected handshake message")
+		h.messageErrChan <- qerr.CryptoError(alertUnexpectedMessage, fmt.Sprintf("unexpected handshake message: %d", msgType))
+		return false
 	}
 }
 
@@ -442,7 +446,8 @@ func (h *cryptoSetup) handleMessageForClient(msgType messageType) bool {
 		h.conn.HandlePostHandshakeMessage()
 		return false
 	default:
-		panic("unexpected handshake message: ")
+		h.messageErrChan <- qerr.CryptoError(alertUnexpectedMessage, fmt.Sprintf("unexpected handshake message: %d", msgType))
+		return false
 	}
 }
 

internal/handshake/crypto_setup_test.go

@@ -13,6 +13,7 @@ import (
 	"time"
 
 	"github.com/lucas-clemente/quic-go/internal/protocol"
+	"github.com/lucas-clemente/quic-go/internal/qerr"
 	"github.com/lucas-clemente/quic-go/internal/testdata"
 	"github.com/lucas-clemente/quic-go/internal/utils"
 	"github.com/marten-seemann/qtls"
@@ -141,6 +142,39 @@ var _ = Describe("Crypto Setup TLS", func() {
 		Eventually(done).Should(BeClosed())
 	})
 
+	It("returns Handshake() when a message is received at the wrong encryption level", func() {
+		_, sInitialStream, sHandshakeStream := initStreams()
+		server, err := NewCryptoSetupServer(
+			sInitialStream,
+			sHandshakeStream,
+			ioutil.Discard,
+			protocol.ConnectionID{},
+			nil,
+			&TransportParameters{},
+			func([]byte) {},
+			func(protocol.EncryptionLevel) {},
+			testdata.GetTLSConfig(),
+			utils.DefaultLogger.WithPrefix("server"),
+		)
+		Expect(err).ToNot(HaveOccurred())
+
+		done := make(chan struct{})
+		go func() {
+			defer GinkgoRecover()
+			err := server.RunHandshake()
+			Expect(err).To(BeAssignableToTypeOf(&qerr.QuicError{}))
+			qerr := err.(*qerr.QuicError)
+			Expect(qerr.IsCryptoError()).To(BeTrue())
+			Expect(qerr.ErrorCode).To(BeEquivalentTo(0x100 + int(alertUnexpectedMessage)))
+			Expect(err.Error()).To(ContainSubstring("expected handshake message ClientHello to have encryption level Initial, has Handshake"))
+			close(done)
+		}()
+
+		fakeCH := append([]byte{byte(typeClientHello), 0, 0, 6}, []byte("foobar")...)
+		server.HandleMessage(fakeCH, protocol.EncryptionHandshake) // wrong encryption level
+		Eventually(done).Should(BeClosed())
+	})
+
 	It("returns Handshake() when handling a message fails", func() {
 		_, sInitialStream, sHandshakeStream := initStreams()
 		server, err := NewCryptoSetupServer(
@@ -161,12 +195,16 @@ var _ = Describe("Crypto Setup TLS", func() {
 		go func() {
 			defer GinkgoRecover()
 			err := server.RunHandshake()
-			Expect(err).To(MatchError("expected handshake message ClientHello to have encryption level Initial, has Handshake"))
+			Expect(err).To(BeAssignableToTypeOf(&qerr.QuicError{}))
+			qerr := err.(*qerr.QuicError)
+			Expect(qerr.IsCryptoError()).To(BeTrue())
+			Expect(qerr.ErrorCode).To(BeEquivalentTo(0x100 + int(alertUnexpectedMessage)))
+			Expect(err.Error()).To(ContainSubstring("unexpected handshake message"))
 			close(done)
 		}()
 
-		fakeCH := append([]byte{byte(typeClientHello), 0, 0, 6}, []byte("foobar")...)
-		server.HandleMessage(fakeCH, protocol.EncryptionHandshake) // wrong encryption level
+		fakeCH := append([]byte{byte(typeServerHello), 0, 0, 6}, []byte("foobar")...)
+		server.HandleMessage(fakeCH, protocol.EncryptionInitial) // wrong encryption level
 		Eventually(done).Should(BeClosed())
 	})