crypto/tls: limit number of consecutive warning alerts

In the current implementation, it is possible for a client to
continuously send warning alerts, which are just dropped on the floor
inside readRecord.

This can enable scenarios in where someone can try to continuously
send warning alerts to the server just to keep it busy.

This CL implements a simple counter that triggers an error if
we hit the warning alert limit.

Fixes #22543

Change-Id: Ief0ca10308cf5a4dea21a5a67d3e8f6501912da6
Reviewed-on: https://go-review.googlesource.com/75750
Reviewed-by: Adam Langley <agl@golang.org>
Reviewed-by: Filippo Valsorda <hi@filippo.io>
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This commit is contained in:
filewalkwithme 2017-11-03 03:45:04 +01:00 committed by Adam Langley
parent 64bffb76ca
commit ff1bc5469f
3 changed files with 73 additions and 4 deletions

View File

@ -33,6 +33,7 @@ const (
maxCiphertext = 16384 + 2048 // maximum ciphertext payload length maxCiphertext = 16384 + 2048 // maximum ciphertext payload length
recordHeaderLen = 5 // record header length recordHeaderLen = 5 // record header length
maxHandshake = 65536 // maximum handshake we support (protocol max is 16 MB) maxHandshake = 65536 // maximum handshake we support (protocol max is 16 MB)
maxWarnAlertCount = 5 // maximum number of consecutive warning alerts
minVersion = VersionTLS10 minVersion = VersionTLS10
maxVersion = VersionTLS12 maxVersion = VersionTLS12

16
conn.go
View File

@ -94,6 +94,10 @@ type Conn struct {
bytesSent int64 bytesSent int64
packetsSent int64 packetsSent int64
// warnCount counts the number of consecutive warning alerts received
// by Conn.readRecord. Protected by in.Mutex.
warnCount int
// activeCall is an atomic int32; the low bit is whether Close has // activeCall is an atomic int32; the low bit is whether Close has
// been called. the rest of the bits are the number of goroutines // been called. the rest of the bits are the number of goroutines
// in Conn.Write. // in Conn.Write.
@ -658,6 +662,11 @@ Again:
return c.in.setErrorLocked(err) return c.in.setErrorLocked(err)
} }
if typ != recordTypeAlert && len(data) > 0 {
// this is a valid non-alert message: reset the count of alerts
c.warnCount = 0
}
switch typ { switch typ {
default: default:
c.in.setErrorLocked(c.sendAlert(alertUnexpectedMessage)) c.in.setErrorLocked(c.sendAlert(alertUnexpectedMessage))
@ -675,6 +684,13 @@ Again:
case alertLevelWarning: case alertLevelWarning:
// drop on the floor // drop on the floor
c.in.freeBlock(b) c.in.freeBlock(b)
c.warnCount++
if c.warnCount > maxWarnAlertCount {
c.sendAlert(alertUnexpectedMessage)
return c.in.setErrorLocked(errors.New("tls: too many warn alerts"))
}
goto Again goto Again
case alertLevelError: case alertLevelError:
c.in.setErrorLocked(&net.OpError{Op: "remote error", Err: alert(data[1])}) c.in.setErrorLocked(&net.OpError{Op: "remote error", Err: alert(data[1])})

View File

@ -566,6 +566,58 @@ func TestConnCloseWrite(t *testing.T) {
} }
} }
func TestWarningAlertFlood(t *testing.T) {
ln := newLocalListener(t)
defer ln.Close()
server := func() error {
sconn, err := ln.Accept()
if err != nil {
return fmt.Errorf("accept: %v", err)
}
defer sconn.Close()
serverConfig := testConfig.Clone()
srv := Server(sconn, serverConfig)
if err := srv.Handshake(); err != nil {
return fmt.Errorf("handshake: %v", err)
}
defer srv.Close()
_, err = ioutil.ReadAll(srv)
if err == nil {
return errors.New("unexpected lack of error from server")
}
const expected = "too many warn"
if str := err.Error(); !strings.Contains(str, expected) {
return fmt.Errorf("expected error containing %q, but saw: %s", expected, str)
}
return nil
}
errChan := make(chan error, 1)
go func() { errChan <- server() }()
clientConfig := testConfig.Clone()
conn, err := Dial("tcp", ln.Addr().String(), clientConfig)
if err != nil {
t.Fatal(err)
}
defer conn.Close()
if err := conn.Handshake(); err != nil {
t.Fatal(err)
}
for i := 0; i < maxWarnAlertCount+1; i++ {
conn.sendAlert(alertNoRenegotiation)
}
if err := <-errChan; err != nil {
t.Fatal(err)
}
}
func TestCloneFuncFields(t *testing.T) { func TestCloneFuncFields(t *testing.T) {
const expectedCount = 5 const expectedCount = 5
called := 0 called := 0