Limit the number of warning alerts silently consumed.
Per review comments on
https://boringssl-review.googlesource.com/#/c/4112/.
Change-Id: I82cacf67c6882e64f6637015ac41945522699797
Reviewed-on: https://boringssl-review.googlesource.com/5041
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index edebba1..d1b7db7 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -705,10 +705,6 @@
// preferences to be ignored.
IgnorePeerCurvePreferences bool
- // SendWarningAlerts, if non-zero, causes every record to be prefaced by
- // a warning alert.
- SendWarningAlerts alert
-
// BadFinished, if true, causes the Finished hash to be broken.
BadFinished bool
diff --git a/ssl/test/runner/conn.go b/ssl/test/runner/conn.go
index 84741fb..ea9f3bb 100644
--- a/ssl/test/runner/conn.go
+++ b/ssl/test/runner/conn.go
@@ -799,13 +799,8 @@
// sendAlert sends a TLS alert message.
// c.out.Mutex <= L.
-func (c *Conn) sendAlertLocked(err alert) error {
- switch err {
- case alertNoRenegotiation, alertCloseNotify:
- c.tmp[0] = alertLevelWarning
- default:
- c.tmp[0] = alertLevelError
- }
+func (c *Conn) sendAlertLocked(level byte, err alert) error {
+ c.tmp[0] = level
c.tmp[1] = byte(err)
if c.config.Bugs.FragmentAlert {
c.writeRecord(recordTypeAlert, c.tmp[0:1])
@@ -813,8 +808,8 @@
} else {
c.writeRecord(recordTypeAlert, c.tmp[0:2])
}
- // closeNotify is a special case in that it isn't an error:
- if err != alertCloseNotify {
+ // Error alerts are fatal to the connection.
+ if level == alertLevelError {
return c.out.setErrorLocked(&net.OpError{Op: "local error", Err: err})
}
return nil
@@ -823,9 +818,17 @@
// sendAlert sends a TLS alert message.
// L < c.out.Mutex.
func (c *Conn) sendAlert(err alert) error {
+ level := byte(alertLevelError)
+ if err == alertNoRenegotiation || err == alertCloseNotify {
+ level = alertLevelWarning
+ }
+ return c.SendAlert(level, err)
+}
+
+func (c *Conn) SendAlert(level byte, err alert) error {
c.out.Lock()
defer c.out.Unlock()
- return c.sendAlertLocked(err)
+ return c.sendAlertLocked(level, err)
}
// writeV2Record writes a record for a V2ClientHello.
@@ -841,13 +844,6 @@
// to the connection and updates the record layer state.
// c.out.Mutex <= L.
func (c *Conn) writeRecord(typ recordType, data []byte) (n int, err error) {
- if typ != recordTypeAlert && c.config.Bugs.SendWarningAlerts != 0 {
- alert := make([]byte, 2)
- alert[0] = alertLevelWarning
- alert[1] = byte(c.config.Bugs.SendWarningAlerts)
- c.writeRecord(recordTypeAlert, alert)
- }
-
if c.isDTLS {
return c.dtlsWriteRecord(typ, data)
}
@@ -1113,7 +1109,7 @@
}
if c.config.Bugs.SendSpuriousAlert != 0 {
- c.sendAlertLocked(c.config.Bugs.SendSpuriousAlert)
+ c.sendAlertLocked(alertLevelError, c.config.Bugs.SendSpuriousAlert)
}
// SSL 3.0 and TLS 1.0 are susceptible to a chosen-plaintext
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index d7ae850..d17b048 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -207,6 +207,9 @@
// sendEmptyRecords is the number of consecutive empty records to send
// before and after the test message.
sendEmptyRecords int
+ // sendWarningAlerts is the number of consecutive warning alerts to send
+ // before and after the test message.
+ sendWarningAlerts int
}
var testCases = []testCase{
@@ -955,23 +958,6 @@
expectedError: ":WRONG_CURVE:",
},
{
- name: "SendWarningAlerts",
- config: Config{
- Bugs: ProtocolBugs{
- SendWarningAlerts: alertAccessDenied,
- },
- },
- },
- {
- protocol: dtls,
- name: "SendWarningAlerts-DTLS",
- config: Config{
- Bugs: ProtocolBugs{
- SendWarningAlerts: alertAccessDenied,
- },
- },
- },
- {
name: "BadFinished",
config: Config{
Bugs: ProtocolBugs{
@@ -1156,6 +1142,28 @@
shouldFail: true,
expectedError: ":TOO_MANY_EMPTY_FRAGMENTS:",
},
+ {
+ name: "SendWarningAlerts-Pass",
+ sendWarningAlerts: 4,
+ },
+ {
+ protocol: dtls,
+ name: "SendWarningAlerts-DTLS-Pass",
+ sendWarningAlerts: 4,
+ },
+ {
+ name: "SendWarningAlerts",
+ sendWarningAlerts: 5,
+ shouldFail: true,
+ expectedError: ":TOO_MANY_WARNING_ALERTS:",
+ },
+ {
+ name: "SendWarningAlerts-Async",
+ sendWarningAlerts: 5,
+ flags: []string{"-async"},
+ shouldFail: true,
+ expectedError: ":TOO_MANY_WARNING_ALERTS:",
+ },
}
func doExchange(test *testCase, config *Config, conn net.Conn, messageLen int, isResume bool) error {
@@ -1295,6 +1303,10 @@
tlsConn.Write(nil)
}
+ for i := 0; i < test.sendWarningAlerts; i++ {
+ tlsConn.SendAlert(alertLevelWarning, alertUnexpectedMessage)
+ }
+
if test.renegotiate {
if test.renegotiateCiphers != nil {
config.CipherSuites = test.renegotiateCiphers
@@ -1334,6 +1346,10 @@
tlsConn.Write(nil)
}
+ for i := 0; i < test.sendWarningAlerts; i++ {
+ tlsConn.SendAlert(alertLevelWarning, alertUnexpectedMessage)
+ }
+
buf := make([]byte, len(testMessage))
if test.protocol == dtls {
bufTmp := make([]byte, len(buf)+1)