Fold ssl_open_record_fatal_alert into ssl_open_record_error.
The only difference is whether there's an alert to send back, but we'll
need to allow an "error without alert" in several cases anyway:
1. If the server sees an HTTP request or garbage instead of a
ClientHello, it shouldn't send an alert.
2. Resurfaced errors.
Just make zero signal no alert for now. Later on, I'm thinking we might
just want to put the alert into the outgoing buffer and make it further
uniform.
This also gives us only one error state to keep track of rather than
two.
Bug: 206
Change-Id: Ia821d9f89abd2ca6010e8851220d4e070bc42fa1
Reviewed-on: https://boringssl-review.googlesource.com/21286
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 6b4bf81..032ec7e 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -4651,7 +4651,6 @@
kDiscard,
kIncompleteRecord,
kAlertCloseNotify,
- kAlertFatal,
kError,
};
@@ -4665,9 +4664,8 @@
// - kIncompleteRecord if |in| did not contain a complete record.
// - kAlertCloseNotify if a record was successfully processed but is a
// close_notify alert.
-// - kAlertFatal if a record was successfully processed but is a fatal alert.
// - kError if an error occurred or the record is invalid. |*out_alert| will be
-// set to an alert to emit.
+// set to an alert to emit, or zero if no alert should be emitted.
OPENSSL_EXPORT OpenRecordResult OpenRecord(SSL *ssl, Span<uint8_t> *out,
size_t *out_record_len,
uint8_t *out_alert,
diff --git a/ssl/d1_pkt.cc b/ssl/d1_pkt.cc
index a3095e1..91fc647 100644
--- a/ssl/d1_pkt.cc
+++ b/ssl/d1_pkt.cc
@@ -192,11 +192,10 @@
case ssl_open_record_close_notify:
return 0;
- case ssl_open_record_fatal_alert:
- return -1;
-
case ssl_open_record_error:
- ssl3_send_alert(ssl, SSL3_AL_FATAL, alert);
+ if (alert != 0) {
+ ssl3_send_alert(ssl, SSL3_AL_FATAL, alert);
+ }
return -1;
}
diff --git a/ssl/internal.h b/ssl/internal.h
index 32199aa..b9a597c 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -764,7 +764,6 @@
ssl_open_record_discard,
ssl_open_record_partial,
ssl_open_record_close_notify,
- ssl_open_record_fatal_alert,
ssl_open_record_error,
};
@@ -786,11 +785,11 @@
// If a record was successfully processed but should be discarded, it returns
// |ssl_open_record_discard|.
//
-// If a record was successfully processed but is a close_notify or fatal alert,
-// it returns |ssl_open_record_close_notify| or |ssl_open_record_fatal_alert|.
+// If a record was successfully processed but is a close_notify, it returns
+// |ssl_open_record_close_notify|.
//
-// On failure, it returns |ssl_open_record_error| and sets |*out_alert| to an
-// alert to emit.
+// On failure or fatal alert, it returns |ssl_open_record_error| and sets
+// |*out_alert| to an alert to emit, or zero if no alert should be emitted.
enum ssl_open_record_t tls_open_record(SSL *ssl, uint8_t *out_type, CBS *out,
size_t *out_consumed, uint8_t *out_alert,
uint8_t *in, size_t in_len);
diff --git a/ssl/s3_pkt.cc b/ssl/s3_pkt.cc
index f7470ae..5334145 100644
--- a/ssl/s3_pkt.cc
+++ b/ssl/s3_pkt.cc
@@ -178,11 +178,10 @@
case ssl_open_record_close_notify:
return 0;
- case ssl_open_record_fatal_alert:
- return -1;
-
case ssl_open_record_error:
- ssl3_send_alert(ssl, SSL3_AL_FATAL, alert);
+ if (alert != 0) {
+ ssl3_send_alert(ssl, SSL3_AL_FATAL, alert);
+ }
return -1;
}
@@ -547,6 +546,7 @@
ssl->s3->send_shutdown = ssl_shutdown_close_notify;
} else {
assert(level == SSL3_AL_FATAL);
+ assert(desc != SSL_AD_CLOSE_NOTIFY);
ssl->s3->send_shutdown = ssl_shutdown_fatal_alert;
}
diff --git a/ssl/tls_record.cc b/ssl/tls_record.cc
index 5eeff3c..f9033e6 100644
--- a/ssl/tls_record.cc
+++ b/ssl/tls_record.cc
@@ -562,7 +562,8 @@
OPENSSL_PUT_ERROR(SSL, SSL_AD_REASON_OFFSET + alert_descr);
BIO_snprintf(tmp, sizeof(tmp), "%d", alert_descr);
ERR_add_error_data(2, "SSL alert number ", tmp);
- return ssl_open_record_fatal_alert;
+ *out_alert = 0; // No alert to send back to the peer.
+ return ssl_open_record_error;
}
*out_alert = SSL_AD_ILLEGAL_PARAMETER;
@@ -603,8 +604,6 @@
return OpenRecordResult::kIncompleteRecord;
case ssl_open_record_close_notify:
return OpenRecordResult::kAlertCloseNotify;
- case ssl_open_record_fatal_alert:
- return OpenRecordResult::kAlertFatal;
case ssl_open_record_error:
return OpenRecordResult::kError;
}