Parse SVCB/HTTPS SvcParams list

Add full support for RFC 9460's SvcParams list.

Amend the existing `dns_svcb_rr` record by a vector of new
`dns_svcb_param` records containing aptly typed SvcParamKey and
SvcParamValue pairs.  Example output:

```
@load base/protocols/dns
event dns_HTTPS( c: connection , msg: dns_msg , ans: dns_answer , https: dns_svcb_rr ) {
	for (_, param in https$svc_params)
		print to_json(param);  # filter uninitialised values
}
```

```
$ dig https cloudflare-ech.com +short | tr [:space:] \\n
1
.
alpn="h3,h2"
ipv4hint=104.18.10.118,104.18.11.118
ech=AEX+DQBBHgAgACBGL2e9TiFwjK/w1Zg9AmRm7mgXHz3PjffP0mTFNMxmDQAEAAEAAQASY2xvdWRmbGFyZS1lY2guY29tAAA=
ipv6hint=2606:4700::6812:a76,2606:4700::6812:b76
```

```
{"key":1,"alpn":["h3","h2"]}
{"key":4,"hint":["104.18.10.118","104.18.11.118"]}
{"key":5,"ech":"AEX+DQBBHgAgACBGL2e9TiFwjK/w1Zg9AmRm7mgXHz3PjffP0mTFNMxmDQAEAAEAAQASY2xvdWRmbGFyZS1lY2guY29tAAA="}
{"key":6,"hint":["2606:4700::6812:a76","2606:4700::6812:b76"]}
```

Values with malformed data or belonging to invalid/reserved keys
are passed raw bytes in network order for script-level inspection.

Follow up to "Initial Support to DNS SVCB/HTTPS RR"
https://github.com/zeek/zeek/pull/1808

scripts/base/init-bare.zeek

@@ -3071,12 +3071,30 @@ type dns_loc_rr: record {
 	is_query: count;	##< The RR is a query/Response.
 };
 
-## DNS SVCB and HTTPS RRs
+## A SvcParamKey with an optional SvcParamValue.
+#
+## .. zeek:see:: dns_svcb_rr
+type dns_svcb_param: record {
+	key: count;				##< SvcParamKey
+	mandatory: vector of count &optional;	##< "mandatory" SvcParamKey values
+	alpn: vector of string &optional;	##< "alpn" IDs
+	p: count &optional;			##< "port" number, TCP or UDP
+	hint: vector of addr &optional;		##< "ipv4hint" or "ipv6hint" IP addresses
+	ech: string &optional;			##< "ech" base64 encoded ECHConfigList blob
+	raw: string &optional;			##< reserved key's or malformed value
+};
+
+type dns_svcb_param_vec: vector of dns_svcb_param;
+
+## A SVCB or HTTPS record.
+##
+## See also RFC 9460 - Service Binding and Parameter Specification via the DNS (SVCB and HTTPS Resource Records).
 ##
 ## .. zeek:see:: dns_SVCB dns_HTTPS
 type dns_svcb_rr: record {
-	svc_priority: count;	##< Service priority for the current record, 0 indicates that this record is in AliasMode and cannot carry svc_params; otherwise this is in ServiceMode, and may include svc_params
+	svc_priority: count;				##< Service priority. If zero, the record is in AliasMode and has no SvcParam.
-	target_name: string;	##< Target name, the hostname of the service endpoint.
+	target_name: string;				##< Target name, the hostname of the service endpoint.
+	svc_params: dns_svcb_param_vec &optional;	##< Service parameters, if any.
 };
 
 ## A NAPTR record.

scripts/base/protocols/dns/consts.zeek

@@ -182,8 +182,9 @@ export {
 		[4] = "SHA384",
 	} &default = function(n: count): string { return fmt("digest-%d", n); };
 
-	## SVCB/HTTPS SvcParam keys, as defined in
+	## SVCB/HTTPS SvcParam keys as defined in
-	## https://www.ietf.org/archive/id/draft-ietf-dnsop-svcb-https-07.txt, sec 14.3.2
+	## https://datatracker.ietf.org/doc/html/rfc9460#name-initial-contents
+	## Keep in sync with src/analyzer/protocol/dns/DNS.h SVCPARAM_Key.
 	const svcparam_keys = {
 		[0] = "mandatory",
 		[1] = "alpn",

src/analyzer/protocol/dns/DNS.cc

@@ -8,7 +8,9 @@
 #include <sys/types.h>
 #include <cctype>
 
+#include "zeek/Base64.h"
 #include "zeek/Event.h"
+#include "zeek/IPAddr.h"
 #include "zeek/NetVar.h"
 #include "zeek/RunState.h"
 #include "zeek/Val.h"
@@ -1587,17 +1589,161 @@ bool DNS_Interpreter::ParseRR_CAA(detail::DNS_MsgInfo* msg, const u_char*& data,
     return rdlength == 0;
 }
 
+VectorValPtr DNS_Interpreter::Parse_SvcParams(const u_char*& data, int& len, int svc_params_len) {
+    static auto dns_svcb_param_vec = id::find_type<VectorType>("dns_svcb_param_vec");
+    auto svc_params = make_intrusive<VectorVal>(dns_svcb_param_vec);
+
+    // Each service parameter is at least four bytes, two for key and value length each.
+    while ( svc_params_len >= 4 ) {
+        static auto dns_svcb_param = id::find_type<RecordType>("dns_svcb_param");
+        auto svc_param = make_intrusive<RecordVal>(dns_svcb_param);
+
+        const uint16_t key = ExtractShort(data, len);
+        uint16_t value_len = ExtractShort(data, len);
+        int item_len_parsed = 0;
+        svc_params_len -= 4;
+
+        if ( value_len > svc_params_len ) {
+            analyzer->Weird("DNS_SVCB_param_value_toobig", util::fmt("%d capped to %d", value_len, svc_params_len));
+            value_len = svc_params_len;
+            goto malformed;
+        }
+
+        svc_param->Assign(0, zeek::val_mgr->Count(key));
+
+        switch ( key ) {
+            case detail::mandatory: // list of keys
+            {
+                if ( value_len == 0 || value_len % 2 != 0 ) {
+                    analyzer->Weird("DNS_SVCB_mandatory_length_invalid");
+                    goto malformed;
+                }
+
+                auto mandatory = make_intrusive<VectorVal>(id::index_vec);
+
+                while ( item_len_parsed + 2 <= value_len ) {
+                    mandatory->Append(zeek::val_mgr->Count(ExtractShort(data, len)));
+                    item_len_parsed += 2;
+                }
+
+                svc_param->Assign(1, std::move(mandatory));
+                break;
+            }
+
+            case detail::alpn: // list of length-prefixed (1 octet) ALPN IDs
+            {
+                auto alpn = make_intrusive<VectorVal>(id::string_vec);
+
+                while ( item_len_parsed + 2 < value_len ) {
+                    const uint8_t alpn_len = ExtractByte(data, len);
+                    item_len_parsed += 1;
+
+                    if ( alpn_len == 0 || alpn_len > 255 || alpn_len + item_len_parsed > value_len ) {
+                        // Account for already consumed data first.
+                        value_len -= item_len_parsed;
+                        analyzer->Weird("DNS_SVCB_alpn_length_invalid");
+                        goto malformed;
+                    }
+
+                    alpn->Append(zeek::make_intrusive<zeek::StringVal>(alpn_len, reinterpret_cast<const char*>(data)));
+                    data += alpn_len;
+                    len -= alpn_len;
+                    item_len_parsed += alpn_len;
+                }
+
+                if ( alpn->Size() > 0 )
+                    svc_param->Assign(2, std::move(alpn));
+                break;
+            }
+
+            case detail::no_default_alpn:
+                if ( value_len > 0 ) {
+                    analyzer->Weird("DNS_SVCB_nodefaultalpn_value");
+                    goto malformed;
+                }
+                break;
+
+            case detail::port: // port
+                if ( value_len != 2 ) {
+                    analyzer->Weird("DNS_SVCB_port_length_invalid");
+                    break;
+                }
+
+                svc_param->Assign(3, zeek::val_mgr->Count(ExtractShort(data, len)));
+                item_len_parsed += 2;
+                break;
+
+            case detail::ipv4hint: // list of IPs
+            case detail::ipv6hint: // list of IPs
+            {
+                const bool is_ipv4 = key == detail::ipv4hint;
+                const int addr_len = is_ipv4 ? 4 : 16;
+
+                if ( value_len % addr_len != 0 ) {
+                    analyzer->Weird("DNS_SVCB_hint_length_invalid");
+                    goto malformed;
+                }
+
+                static auto addr_vec = id::find_type<VectorType>("addr_vec");
+                auto hint = make_intrusive<VectorVal>(addr_vec);
+
+                while ( item_len_parsed + addr_len <= value_len ) {
+                    const auto addr = zeek::IPAddr(is_ipv4 ? IPv4 : IPv6, reinterpret_cast<const uint32_t*>(data),
+                                                   zeek::IPAddr::Network);
+                    hint->Append(zeek::make_intrusive<zeek::AddrVal>(addr));
+
+                    data += addr_len;
+                    len -= addr_len;
+                    item_len_parsed += addr_len;
+                }
+
+                if ( hint->Size() > 0 )
+                    svc_param->Assign(4, std::move(hint));
+                break;
+            }
+
+            case detail::ech: // ECHConfigList
+            {
+                const String* ech = ExtractStream(data, len, value_len);
+                item_len_parsed += value_len;
+
+                // Convert binary blob to presentation format.
+                String* b64 = zeek::detail::encode_base64(ech, nullptr, analyzer->Conn());
+                delete ech;
+
+                svc_param->Assign(5, zeek::make_intrusive<zeek::StringVal>(b64));
+                break;
+            }
+
+            default:
+                analyzer->Weird("DNS_SVCB_key_reserved_or_invalid");
+            malformed:
+                svc_param->Assign(6, zeek::make_intrusive<StringVal>(ExtractStream(data, len, value_len)));
+                item_len_parsed += value_len;
+                break;
+        }
+
+        svc_params->Append(std::move(svc_param));
+        svc_params_len -= value_len;
+    }
+
+    return svc_params;
+}
+
+/**
+ * https://datatracker.ietf.org/doc/html/rfc9460#name-rdata-wire-format
+ */
 bool DNS_Interpreter::ParseRR_SVCB(detail::DNS_MsgInfo* msg, const u_char*& data, int& len, int rdlength,
                                    const u_char* msg_start, const RR_Type& svcb_type) {
     const u_char* data_start = data;
     // the smallest SVCB/HTTPS rr is 3 bytes:
     // the first 2 bytes are for the svc priority, and the third byte is root (0x0)
     if ( len < 3 ) {
-        analyzer->Weird("DNS_SVBC_wrong_length");
+        analyzer->Weird("DNS_SVCB_wrong_length");
         return false;
     }
 
-    uint16_t svc_priority = ExtractShort(data, len);
+    const uint16_t svc_priority = ExtractShort(data, len);
 
     u_char target_name[513];
     int name_len = sizeof(target_name) - 1;
@@ -1613,29 +1759,23 @@ bool DNS_Interpreter::ParseRR_SVCB(detail::DNS_MsgInfo* msg, const u_char*& data
         name_end = target_name + 1;
     }
 
-    SVCB_DATA svcb_data = {svc_priority,
-                           make_intrusive<StringVal>(new String(target_name, name_end - target_name, true))};
-
-    // TODO: parse svcparams
-    // we consume all the remaining raw data (svc params) but do nothing.
-    // this should be removed if the svc param parser is ready
     std::ptrdiff_t parsed_bytes = data - data_start;
-    if ( parsed_bytes < rdlength ) {
+    int svc_params_len = rdlength - parsed_bytes;
-        len -= (rdlength - parsed_bytes);
+    VectorValPtr svc_params = nullptr;
-        data += (rdlength - parsed_bytes);
+
+    if ( svc_params_len > 0 ) {
+        if ( svc_priority == 0 )
+            analyzer->Weird("DNS_SVCB_aliasmode_with_params");
+
+        svc_params = Parse_SvcParams(data, len, svc_params_len);
     }
 
-    switch ( svcb_type ) {
+    SVCB_DATA svcb_data = {svc_priority,
-        case detail::TYPE_SVCB:
+                           make_intrusive<StringVal>(new String(target_name, name_end - target_name, true)),
-            analyzer->EnqueueConnEvent(dns_SVCB, analyzer->ConnVal(), msg->BuildHdrVal(), msg->BuildAnswerVal(),
+                           std::move(svc_params)};
-                                       msg->BuildSVCB_Val(svcb_data));
+
-            break;
+    analyzer->EnqueueConnEvent(svcb_type == detail::TYPE_SVCB ? dns_SVCB : dns_HTTPS, analyzer->ConnVal(),
-        case detail::TYPE_HTTPS:
+                               msg->BuildHdrVal(), msg->BuildAnswerVal(), msg->BuildSVCB_Val(svcb_data));
-            analyzer->EnqueueConnEvent(dns_HTTPS, analyzer->ConnVal(), msg->BuildHdrVal(), msg->BuildAnswerVal(),
-                                       msg->BuildSVCB_Val(svcb_data));
-            break;
-        default: break; // unreachable. for suppressing compiler warnings.
-    }
     return true;
 }
 
@@ -1947,8 +2087,9 @@ RecordValPtr DNS_MsgInfo::BuildSVCB_Val(const SVCB_DATA& svcb) {
 
     r->Assign(0, svcb.svc_priority);
     r->Assign(1, svcb.target_name);
+    if ( svcb.svc_params )
+        r->Assign(2, std::move(svcb.svc_params));
 
-    // TODO: assign values to svcparams
     return r;
 }
 

src/analyzer/protocol/dns/DNS.h

@@ -146,8 +146,9 @@ enum DNSSEC_Digest : uint8_t {
     SHA384 = 4,
 };
 
-///< all keys are defined in RFC draft
+// SVCB/HTTPS SvcParam keys as defined in
-///< https://datatracker.ietf.org/doc/html/draft-ietf-dnsop-svcb-https-07#section-14.3.2
+// https://datatracker.ietf.org/doc/html/rfc9460#name-initial-contents
+// Keep in sync with scripts/base/protocols/dns/consts.zeek svcparam_keys.
 enum SVCPARAM_Key : uint8_t {
     mandatory = 0,
     alpn = 1,
@@ -278,6 +279,7 @@ struct LOC_DATA {
 struct SVCB_DATA {
     uint16_t svc_priority; // 2
     StringValPtr target_name;
+    VectorValPtr svc_params;
 };
 
 class DNS_MsgInfo {
@@ -360,6 +362,8 @@ protected:
 
     String* ExtractStream(const u_char*& data, int& len, int sig_len);
 
+    VectorValPtr Parse_SvcParams(const u_char*& data, int& len, int svc_params_len);
+
     bool ParseRR_Name(detail::DNS_MsgInfo* msg, const u_char*& data, int& len, int rdlength, const u_char* msg_start);
     bool ParseRR_SOA(detail::DNS_MsgInfo* msg, const u_char*& data, int& len, int rdlength, const u_char* msg_start);
     bool ParseRR_MX(detail::DNS_MsgInfo* msg, const u_char*& data, int& len, int rdlength, const u_char* msg_start);

testing/btest/Baseline/scripts.base.protocols.dns.ech/output

@@ -0,0 +1,8 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+[key=4, mandatory=<uninitialized>, alpn=<uninitialized>, p=<uninitialized>, hint=[213.108.108.101], ech=<uninitialized>, raw=<uninitialized>]
+[key=5, mandatory=<uninitialized>, alpn=<uninitialized>, p=<uninitialized>, hint=<uninitialized>, ech=AMD+DQA8agAgACCuY19tSB4Tb5cnVHw9eCEj629o/whTJgMysNszoM7KSgAEAAEAAQANY292ZXIuZGVmby5pZQAA/g0APEcAIAAg8InmybO7/fiQqDA30bs6zU4TEkcHY3ExOgVOmpIPcWoABAABAAEADWNvdmVyLmRlZm8uaWUAAP4NADwiACAAIO4CLZ79TKIxJXvbhF13BQo7n8/umXWXCI4dydnNfjoFAAQAAQABAA1jb3Zlci5kZWZvLmllAAA=, raw=<uninitialized>]
+[key=6, mandatory=<uninitialized>, alpn=<uninitialized>, p=<uninitialized>, hint=[2a00:c6c0:0:116:5::10], ech=<uninitialized>, raw=<uninitialized>]
+[key=1, mandatory=<uninitialized>, alpn=[h3, h2], p=<uninitialized>, hint=<uninitialized>, ech=<uninitialized>, raw=<uninitialized>]
+[key=4, mandatory=<uninitialized>, alpn=<uninitialized>, p=<uninitialized>, hint=[104.18.10.118, 104.18.11.118], ech=<uninitialized>, raw=<uninitialized>]
+[key=5, mandatory=<uninitialized>, alpn=<uninitialized>, p=<uninitialized>, hint=<uninitialized>, ech=AEX+DQBBdAAgACDB6UNjy9kyv48V6cEOb99HnrfJuiTGKjW9A05sDxhcKQAEAAEAAQASY2xvdWRmbGFyZS1lY2guY29tAAA=, raw=<uninitialized>]
+[key=6, mandatory=<uninitialized>, alpn=<uninitialized>, p=<uninitialized>, hint=[2606:4700::6812:a76, 2606:4700::6812:b76], ech=<uninitialized>, raw=<uninitialized>]

testing/btest/Baseline/scripts.base.protocols.dns.https/output

@@ -1,2 +1,2 @@
 ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
-[svc_priority=1, target_name=.]
+[svc_priority=1, target_name=., svc_params=[[key=1, mandatory=<uninitialized>, alpn=[h3, h3-29, h3-28, h3-27, h2], p=<uninitialized>, hint=<uninitialized>, ech=<uninitialized>, raw=<uninitialized>], [key=4, mandatory=<uninitialized>, alpn=<uninitialized>, p=<uninitialized>, hint=[104.16.132.229, 104.16.133.229], ech=<uninitialized>, raw=<uninitialized>], [key=6, mandatory=<uninitialized>, alpn=<uninitialized>, p=<uninitialized>, hint=[2606:4700::6810:84e5, 2606:4700::6810:85e5], ech=<uninitialized>, raw=<uninitialized>]]]

testing/btest/Baseline/scripts.base.protocols.dns.svcb/output

@@ -1,2 +1,2 @@
 ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
-[svc_priority=0, target_name=foo.example.com]
+[svc_priority=0, target_name=foo.example.com, svc_params=<uninitialized>]

testing/btest/scripts/base/protocols/dns/ech.zeek

@@ -0,0 +1,10 @@
+# @TEST-EXEC: zeek -r $TRACES/dns/ech.pcap %INPUT > output
+# @TEST-EXEC: btest-diff output
+
+@load policy/protocols/dns/auth-addl
+
+event dns_HTTPS(c: connection, msg: dns_msg, ans: dns_answer, https: dns_svcb_rr)
+    {
+    for (_, param in https$svc_params)
+        print param;
+    }