Mirror/zeek
1
0
Fork 0
mirror of https://github.com/zeek/zeek.git synced 2025-10-02 14:48:21 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

ssl: Cap number of alerts parsed from SSL record

Browse source 
Limit the number of events raised from an SSL record with content_type
alert (21) to a configurable maximum number (default 10). For TLS 1.3,
the limit is set to 1 as specified in the RFC. Add a new weird cases
where the limit is exceeded.

OSS-Fuzz managed to generate a reproducer that raised ~660k ssl_plaintext
and ssl_alert events given ~810kb of input data. This change prevents this
with hopefully no negative side-effect in the real-world.
This commit is contained in:
Arne Welzel 2023-10-18 18:43:33 +02:00
parent a5b94f04fd
commit c960d279a2
4 changed files with 46 additions and 1 deletions
Download patch file Download diff file Expand all files Collapse all files

5
NEWS
View file

@ -187,6 +187,11 @@ Changed Functionality
- The input framework now provides better information in error messages when
encountering missing non-optional field while loading data.
- The SSL analyzer will now parse a configurable maximum of 10 SSL Alerts per
SSL message. For TLS 1.3, the maximum is implicitly 1 as defined by RFC 8446.
If there are more alerts, a new weird "SSL_excessive_alerts_in_record" is raised.
For non-TLS 1.3, the maximum can be redefined via ``SSL::max_alerts_per_record``.
Deprecated Functionality
------------------------

5
scripts/base/init-bare.zeek
View file

@ -4490,6 +4490,11 @@ const SSL::dtls_max_version_errors = 10 &redef;
## Maximum number of invalid version errors to report in one DTLS connection.
const SSL::dtls_max_reported_version_errors = 1 &redef;
## Maximum number of Alert messages parsed from an SSL record with
## content_type alert (21). The remaining alerts are discarded. For
## TLS 1.3 connections, this is implicitly 1 as defined by RFC 8446.
const SSL::max_alerts_per_record = 10 &redef;
}
module GLOBAL;

1
src/analyzer/protocol/ssl/consts.bif
View file

@ -1,2 +1,3 @@
const SSL::dtls_max_version_errors: count;
const SSL::dtls_max_reported_version_errors: count;
const SSL::max_alerts_per_record: count;

36
src/analyzer/protocol/ssl/ssl-dtls-protocol.pac
View file

@ -5,7 +5,7 @@
type PlaintextRecord(rec: SSLRecord) = case rec.content_type of {
CHANGE_CIPHER_SPEC -> ch_cipher : ChangeCipherSpec(rec);
ALERT -> alert : Alert(rec);
ALERT -> alerts : Alerts(rec);
HEARTBEAT -> heartbeat: Heartbeat(rec);
APPLICATION_DATA -> app_data : ApplicationData(rec);
default -> unknown_record : UnknownRecord(rec);
@ -44,6 +44,11 @@ type ChangeCipherSpec(rec: SSLRecord) = record {
# Alert Protocol (7.2.)
######################################################################
type Alerts(rec: SSLRecord) = record {
alerts: Alert(rec)[] &length=$context.connection.cap_alert_messages_length(rec.length);
rest: bytestring &restofdata &transient;
} &length=rec.length;
type Alert(rec: SSLRecord) = record {
level : uint8;
description: uint8;
@ -87,6 +92,10 @@ type CiphertextRecord(rec: SSLRecord) = record {
# binpac analyzer for SSL including
######################################################################
%extern{
#include "zeek/analyzer/protocol/ssl/consts.bif.h"
%}
refine connection SSL_Conn += {
%member{
@ -131,4 +140,29 @@ refine connection SSL_Conn += {
return 0;
%}
function cap_alert_messages_length(record_length: int) : int
%{
int alert_length = record_length;
int max_length = zeek::BifConst::SSL::max_alerts_per_record * 2;
// With TLS 1.3, enforce a single alert.
//
// From https://datatracker.ietf.org/doc/html/rfc8446//section-5.1
//
// Alert messages (Section 6) MUST NOT be fragmented across records, and
// multiple alert messages MUST NOT be coalesced into a single
// record. In other words, a record with an Alert type MUST contain
// exactly one message.
if ( determine_tls13() )
max_length = 2;
if ( alert_length > max_length )
{
zeek_analyzer()->Weird("SSL_excessive_alerts_in_record", zeek::util::fmt("%d", alert_length / 2));
alert_length = max_length;
}
return alert_length;
%}
};