From 44df381ccb90d0c29468c99cae022efa43da4924 Mon Sep 17 00:00:00 2001 From: Filippo Valsorda Date: Mon, 30 Jan 2017 17:54:36 +0000 Subject: [PATCH] crypto/tls: peek at unencrypted alerts to give better errors --- conn.go | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/conn.go b/conn.go index 6cc52a5..d343bd1 100644 --- a/conn.go +++ b/conn.go @@ -703,6 +703,7 @@ func (c *Conn) readRecord(want recordType) error { // Process message. b, c.rawInput = c.in.splitBlock(b, recordHeaderLen+n) + peekedAlert := peekAlert(b) // peek at a possible alert before decryption ok, off, alertValue := c.in.decrypt(b) switch { case !ok && c.phase == discardingEarlyData: @@ -713,8 +714,15 @@ func (c *Conn) readRecord(want recordType) error { case ok && c.phase == discardingEarlyData: c.phase = waitingClientFinished case !ok: + c.in.traceErr, c.out.traceErr = nil, nil // not that interesting c.in.freeBlock(b) - return c.in.setErrorLocked(c.sendAlert(alertValue)) + err := c.sendAlert(alertValue) + // If decryption failed because the message is an unencrypted + // alert, return a more meaningful error message + if alertValue == alertBadRecordMAC && peekedAlert != nil { + err = peekedAlert + } + return c.in.setErrorLocked(err) } b.off = off data := b.data[b.off:] @@ -821,6 +829,19 @@ func (c *Conn) readRecord(want recordType) error { return c.in.err } +// peekAlert looks at a message to spot an unencrypted alert. It must be +// called before decryption to avoid a side channel, and its result must +// only be used if decryption fails, to avoid false positives. +func peekAlert(b *block) error { + if len(b.data) < 7 { + return nil + } + if recordType(b.data[0]) != recordTypeAlert { + return nil + } + return &net.OpError{Op: "remote error", Err: alert(b.data[6])} +} + // sendAlert sends a TLS alert message. // c.out.Mutex <= L. func (c *Conn) sendAlertLocked(err alert) error {