Build up TLS 1.3 record-layer tests.
This also adds a missing check to the C half to ensure fake record types are
always correct, to keep implementations honest.
Change-Id: I1d65272e647ffa67018c721d52c639f8ba47d647
Reviewed-on: https://boringssl-review.googlesource.com/8510
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
diff --git a/crypto/err/ssl.errordata b/crypto/err/ssl.errordata
index dfe4ac8..64249ad 100644
--- a/crypto/err/ssl.errordata
+++ b/crypto/err/ssl.errordata
@@ -58,6 +58,7 @@
SSL,157,INAPPROPRIATE_FALLBACK
SSL,158,INVALID_COMMAND
SSL,159,INVALID_MESSAGE
+SSL,251,INVALID_OUTER_RECORD_TYPE
SSL,160,INVALID_SSL_SESSION
SSL,161,INVALID_TICKET_KEYS_LENGTH
SSL,162,LENGTH_MISMATCH
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 2b42093..d9cae0c 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -4611,6 +4611,7 @@
#define SSL_R_X509_LIB 248
#define SSL_R_X509_VERIFICATION_SETUP_PROBLEMS 249
#define SSL_R_SHUTDOWN_WHILE_IN_INIT 250
+#define SSL_R_INVALID_OUTER_RECORD_TYPE 251
#define SSL_R_SSLV3_ALERT_CLOSE_NOTIFY 1000
#define SSL_R_SSLV3_ALERT_UNEXPECTED_MESSAGE 1010
#define SSL_R_SSLV3_ALERT_BAD_RECORD_MAC 1020
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index 2a5565b..c9593ff 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -853,6 +853,19 @@
// CECPQ1BadNewhopePart corrupts the Newhope part of a CECPQ1 key exchange,
// as a trivial proof that it is actually used.
CECPQ1BadNewhopePart bool
+
+ // RecordPadding is the number of bytes of padding to add to each
+ // encrypted record in TLS 1.3.
+ RecordPadding int
+
+ // OmitRecordContents, if true, causes encrypted records in TLS 1.3 to
+ // be missing their body and content type. Padding, if configured, is
+ // still added.
+ OmitRecordContents bool
+
+ // OuterRecordType, if non-zero, is the outer record type to use instead
+ // of application data.
+ OuterRecordType recordType
}
func (c *Config) serverInit() {
diff --git a/ssl/test/runner/conn.go b/ssl/test/runner/conn.go
index 3b4d0b7..e8f6857 100644
--- a/ssl/test/runner/conn.go
+++ b/ssl/test/runner/conn.go
@@ -523,17 +523,24 @@
case cipher.Stream:
c.XORKeyStream(payload, payload)
case *tlsAead:
- contentTypeLen := 0
- if hc.version >= VersionTLS13 {
- contentTypeLen = 1
- }
payloadLen := len(b.data) - recordHeaderLen - explicitIVLen
- b.resize(len(b.data) + contentTypeLen + c.Overhead())
+ paddingLen := 0
if hc.version >= VersionTLS13 {
- b.data[payloadLen+recordHeaderLen] = byte(typ)
- payloadLen += 1
- // TODO(nharper): Add ProtocolBugs to add
- // padding.
+ payloadLen++
+ paddingLen = hc.config.Bugs.RecordPadding
+ }
+ if hc.config.Bugs.OmitRecordContents {
+ payloadLen = 0
+ }
+ b.resize(recordHeaderLen + explicitIVLen + payloadLen + paddingLen + c.Overhead())
+ if hc.version >= VersionTLS13 {
+ if !hc.config.Bugs.OmitRecordContents {
+ b.data[payloadLen+recordHeaderLen-1] = byte(typ)
+ }
+ for i := 0; i < hc.config.Bugs.RecordPadding; i++ {
+ b.data[payloadLen+recordHeaderLen+i] = 0
+ }
+ payloadLen += paddingLen
}
nonce := hc.outSeq[:]
if c.explicitNonce {
@@ -762,8 +769,9 @@
b, c.rawInput = c.in.splitBlock(b, recordHeaderLen+n)
ok, off, encTyp, err := c.in.decrypt(b)
if c.vers >= VersionTLS13 && c.in.cipher != nil {
- // TODO(nharper): Check that outer type (typ) is
- // application data.
+ if typ != recordTypeApplicationData {
+ return 0, nil, c.in.setErrorLocked(fmt.Errorf("tls: outer record type is not application data"))
+ }
typ = encTyp
}
if !ok {
@@ -971,8 +979,10 @@
b.resize(recordHeaderLen + explicitIVLen + m)
b.data[0] = byte(typ)
if c.vers >= VersionTLS13 && c.out.cipher != nil {
- // TODO(nharper): Add a ProtocolBugs to skip this.
b.data[0] = byte(recordTypeApplicationData)
+ if outerType := c.config.Bugs.OuterRecordType; outerType != 0 {
+ b.data[0] = byte(outerType)
+ }
}
vers := c.vers
if vers == 0 || vers >= VersionTLS13 {
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 709cd8d..3b95b2c 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -5298,6 +5298,59 @@
})
}
+func addTLS13RecordTests() {
+ testCases = append(testCases, testCase{
+ name: "TLS13-RecordPadding",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ MinVersion: VersionTLS13,
+ Bugs: ProtocolBugs{
+ RecordPadding: 10,
+ },
+ },
+ })
+
+ testCases = append(testCases, testCase{
+ name: "TLS13-EmptyRecords",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ MinVersion: VersionTLS13,
+ Bugs: ProtocolBugs{
+ OmitRecordContents: true,
+ },
+ },
+ shouldFail: true,
+ expectedError: ":DECRYPTION_FAILED_OR_BAD_RECORD_MAC:",
+ })
+
+ testCases = append(testCases, testCase{
+ name: "TLS13-OnlyPadding",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ MinVersion: VersionTLS13,
+ Bugs: ProtocolBugs{
+ OmitRecordContents: true,
+ RecordPadding: 10,
+ },
+ },
+ shouldFail: true,
+ expectedError: ":DECRYPTION_FAILED_OR_BAD_RECORD_MAC:",
+ })
+
+ testCases = append(testCases, testCase{
+ name: "TLS13-WrongOuterRecord",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ MinVersion: VersionTLS13,
+ Bugs: ProtocolBugs{
+ OuterRecordType: recordTypeHandshake,
+ },
+ },
+ shouldFail: true,
+ expectedError: ":INVALID_OUTER_RECORD_TYPE:",
+ })
+}
+
func worker(statusChan chan statusMsg, c chan *testCase, shimPath string, wg *sync.WaitGroup) {
defer wg.Done()
@@ -5398,6 +5451,7 @@
addCurveTests()
addCECPQ1Tests()
addKeyExchangeInfoTests()
+ addTLS13RecordTests()
for _, async := range []bool{false, true} {
for _, splitHandshake := range []bool{false, true} {
for _, protocol := range []protocol{tls, dtls} {
diff --git a/ssl/tls_record.c b/ssl/tls_record.c
index f1e866f..e357ed98 100644
--- a/ssl/tls_record.c
+++ b/ssl/tls_record.c
@@ -255,6 +255,13 @@
if (ssl->s3->have_version &&
ssl3_protocol_version(ssl) >= TLS1_3_VERSION &&
ssl->s3->aead_read_ctx != NULL) {
+ /* The outer record type is always application_data. */
+ if (type != SSL3_RT_APPLICATION_DATA) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_OUTER_RECORD_TYPE);
+ *out_alert = SSL_AD_DECODE_ERROR;
+ return ssl_open_record_error;
+ }
+
do {
if (!CBS_get_last_u8(out, &type)) {
OPENSSL_PUT_ERROR(SSL, SSL_R_DECRYPTION_FAILED_OR_BAD_RECORD_MAC);
