Merge pull request #2690 in SNORT/snort3 from ~KATHARVE/snort3:h2_in_hi to master

Squashed commit of the following:

commit 955281029abbb6d30732b10660a5edde2594f78a
Author: Katura Harvey <katharve@cisco.com>
Date:   Tue Jan 5 14:59:13 2021 -0500

    http_inspect: abort on HTTP/2 connection preface

diff --git a/src/service_inspectors/http_inspect/http_cutter.cc b/src/service_inspectors/http_inspect/http_cutter.cc
index c8fa2c49b10e46b9424bfd6abf6768998a349fe3..a071347698b4cc2495f77d0e4575935739f8606d 100644 (file)
--- a/src/service_inspectors/http_inspect/http_cutter.cc
+++ b/src/service_inspectors/http_inspect/http_cutter.cc
@@ -114,18 +114,42 @@ ScanResult HttpStartCutter::cut(const uint8_t* buffer, uint32_t length,
     return SCAN_NOT_FOUND;
 }
 
-HttpStartCutter::ValidationResult HttpRequestCutter::validate(uint8_t octet, HttpInfractions*,
-    HttpEventGen*)
+HttpStartCutter::ValidationResult HttpRequestCutter::validate(uint8_t octet,
+    HttpInfractions* infractions, HttpEventGen*)
 {
     // Request line must begin with a method. There is no list of all possible methods because
     // extension is allowed, so there is no absolute way to tell whether something is a method.
     // Instead we verify that all its characters are drawn from the RFC list of valid token
     // characters, that it is followed by a whitespace character, and that it is at most 80
     // characters long. There is nothing special or specified about 80. It is just more than any
-    // reasonable method name would be.
+    // reasonable method name would be. Additionally we check for the first 16 bytes of the HTTP/2
+    // connection preface, which would otherwise pass the aforementioned check.
 
     static const int max_method_length = 80;
+    static const int preface_len = 16;
+    static const int h1_test_len_in_preface = 4;
+    static const uint8_t h2_connection_preface[] = { 'P', 'R', 'I', ' ', '*', ' ', 'H', 'T', 'T',
+        'P', '/', '2', '.', '0', '\r', '\n' };
 
+    if (check_h2)
+    {
+        if (octet == h2_connection_preface[octets_checked])
+        {
+            octets_checked++;
+            if (octets_checked >= preface_len)
+            {
+                *infractions += INF_HTTP2_IN_HI;
+                return V_BAD;
+            }
+            return V_TBD;
+        }
+        else
+        {
+            if (octets_checked >= h1_test_len_in_preface)
+                return V_GOOD;
+            check_h2 = false;
+        }
+    }
     if ((octet == ' ') || (octet == '\t'))
         return V_GOOD;
     if (!token_char[octet] || ++octets_checked > max_method_length)

diff --git a/src/service_inspectors/http_inspect/http_cutter.h b/src/service_inspectors/http_inspect/http_cutter.h
index 093259137ab8086e775eff53ac7ffcd881b91689..f49e14d8aeca67c52aa8377297d6c51a28ab9823 100644 (file)
--- a/src/service_inspectors/http_inspect/http_cutter.h
+++ b/src/service_inspectors/http_inspect/http_cutter.h
@@ -74,6 +74,7 @@ class HttpRequestCutter : public HttpStartCutter
 {
 private:
     uint32_t octets_checked = 0;
+    bool check_h2 = true;
     ValidationResult validate(uint8_t octet, HttpInfractions*, HttpEventGen*) override;
 };
 

diff --git a/src/service_inspectors/http_inspect/http_enum.h b/src/service_inspectors/http_inspect/http_enum.h
index 8dbe56f7ccd67a083b303adcaaf64dca03ca81ae..7be0f0f7a49f701466d9b6e2b334416629a4aa7e 100755 (executable)
--- a/src/service_inspectors/http_inspect/http_enum.h
+++ b/src/service_inspectors/http_inspect/http_enum.h
@@ -243,6 +243,7 @@ enum Infraction
     INF_100_CONNECT_RESP,
     INF_EARLY_CONNECT_RESPONSE,
     INF_MALFORMED_CD_FILENAME,
+    INF_HTTP2_IN_HI,
     INF__MAX_VALUE
 };