Fix ICMP handling in probe modules (#470)

* move udp_unreach strings to icmp_unreach_strings since that's what they are. Also move to using PACKET_(IN)VALID instead of random constants for my sanity

* udp module uses abstracted out ICMP validation logic

* udp and tcp use the same style of packet validation

* starting to look like we have reasonable validation for expected protocol as well as ICMP in most places

* more fixups for reasonable packet validation

* tcp synack scan

* always more

* and we're compiling again

* chugging along.

* fix formatting

* compile, get rid of warnings

* standardize UNUSED and PRINT_PACKET_SEP

* successful build

* TCP looks sane

* bacnet looks sane

* fieldsets govern that the right fields are added at the right times

* Trying to start fixing DNS

* tcp fixes now that validation uncovers problems

* dns and udp work

* i

* ntp works

* starting to fix tcp_synack_scan

* synack scan

* moving remote changes here

* some small fixes

* icmp

* no idea on comment

* more sane formatting

* inline correct

* Removign nonsensical command

* char -> uint8_t

* removing cisco to be an example

Co-authored-by: Alex Holland <ajholland77@yahoo.com>

lib/includes.h

@@ -33,6 +33,7 @@
 #include <netinet/ip_icmp.h>
 #include <netinet/udp.h>
 #include <netinet/tcp.h>
+#include <netinet/ip_icmp.h>
 
 #if defined(__NetBSD__)
 #define ICMP_UNREACH_PRECEDENCE_CUTOFF ICMP_UNREACH_PREC_CUTOFF

src/fieldset.c

@@ -31,11 +31,12 @@ void gen_fielddef_set(fielddefset_t *fds, fielddef_t fs[], int len)
 	fds->len += len;
 }
 
-fieldset_t *fs_new_fieldset(void)
+fieldset_t *fs_new_fieldset(fielddefset_t *fds)
 {
 	fieldset_t *f = xcalloc(1, sizeof(fieldset_t));
 	f->len = 0;
 	f->type = FS_FIELDSET;
+	f->fds = fds;
 	return f;
 }
 
@@ -86,6 +87,13 @@ static inline void fs_add_word(fieldset_t *fs, const char *name, int type,
 		    "object added to repeated field does not match type of repeated field.");
 	}
 	field_t *f = &(fs->fields[fs->len]);
+	// if we have a fieldset definition, then we can validate that the name
+	// of the field is as expected
+	if (fs->fds && strcmp(fs->fds->fielddefs[fs->len].name, name)) {
+		log_fatal("fieldset", "added field (%s) is not next expected field (%s).",
+				name, fs->fds->fielddefs[fs->len].name);
+	}
+
 	fs->len++;
 	f->type = type;
 	f->name = name;
@@ -110,7 +118,10 @@ static void fs_modify_word(fieldset_t *fs, const char *name, int type,
 			return;
 		}
 	}
-	fs_add_word(fs, name, type, free_, len, value);
+	// TODO(ZD): We need to test, but this is really unsafe to just add because it 
+	// will all but guarantee that it's in the wront place
+	//fs_add_word(fs, name, type, free_, len, value);
+	log_fatal("fs", "attempting to modify non-existent field");
 }
 
 static char *sanitize_utf8(const char *buf)
@@ -281,6 +292,12 @@ void fs_modify_string(fieldset_t *fs, const char *name, char *value, int free_)
 	fs_modify_word(fs, name, FS_STRING, free_, strlen(value), val);
 }
 
+void fs_modify_constchar(fieldset_t *fs, const char *name, const char *value)
+{
+	field_val_t val = {.ptr = (char*) value};
+	fs_modify_word(fs, name, FS_STRING, 0, strlen(value), val);
+}
+
 void fs_modify_uint64(fieldset_t *fs, const char *name, uint64_t value)
 {
 	field_val_t val = {.num = value};
@@ -378,7 +395,7 @@ void fs_generate_full_fieldset_translation(translation_t *t,
 
 fieldset_t *translate_fieldset(fieldset_t *fs, translation_t *t)
 {
-	fieldset_t *retv = fs_new_fieldset();
+	fieldset_t *retv = fs_new_fieldset(NULL);
 	if (!retv) {
 		log_fatal("fieldset",
 			  "unable to allocate space for translated field set");

src/fieldset.h

@@ -62,6 +62,7 @@ typedef struct field {
 typedef struct fieldset {
 	int len;
 	field_t fields[MAX_FIELDS];
+	fielddefset_t *fds;
 	// only used for repeated.
 	int inner_type; // type of repeated element. e.g., FS_STRING
 	int type;       // REPEATED or FIELDSET
@@ -80,7 +81,7 @@ typedef struct translation {
 	int translation[MAX_FIELDS];
 } translation_t;
 
-fieldset_t *fs_new_fieldset(void);
+fieldset_t *fs_new_fieldset(fielddefset_t*);
 
 fieldset_t *fs_new_repeated_field(int type, int free_);
 fieldset_t *fs_new_repeated_uint64(void);

src/output_modules/module_json.c

@@ -104,7 +104,9 @@ json_object *fs_to_jsonobj(fieldset_t *fs)
 	json_object *obj = json_object_new_object();
 	for (int i = 0; i < fs->len; i++) {
 		field_t *f = &(fs->fields[i]);
-		json_object_object_add(obj, f->name, field_to_jsonobj(f));
+		if (f->type != FS_NULL) {
+			json_object_object_add(obj, f->name, field_to_jsonobj(f));
+		}
 	}
 	return obj;
 }
@@ -115,7 +117,7 @@ int json_output_to_file(fieldset_t *fs)
 		return EXIT_SUCCESS;
 	}
 	json_object *record = fs_to_jsonobj(fs);
-	fprintf(file, "%s\n", json_object_to_json_string(record));
+	fprintf(file, "%s\n", json_object_to_json_string_ext(record, JSON_C_TO_STRING_PLAIN));
 	fflush(file);
 	check_and_log_file_error(file, "json");
 	json_object_put(record);

src/probe_modules/module_bacnet.c

@@ -36,7 +36,7 @@ static inline uint8_t get_invoke_id(uint32_t *validation)
 }
 
 int bacnet_init_perthread(void *buf, macaddr_t *src, macaddr_t *gw,
-			  __attribute__((unused)) port_h_t dst_port, void **arg)
+			  UNUSED port_h_t dst_port, void **arg)
 {
 	memset(buf, 0, MAX_PACKET_SIZE);
 	struct ether_header *eth_header = (struct ether_header *)buf;
@@ -102,87 +102,58 @@ int bacnet_make_packet(void *buf, size_t *buf_len,
 int bacnet_validate_packet(const struct ip *ip_hdr, uint32_t len,
 			   uint32_t *src_ip, uint32_t *validation)
 {
-	// this will reject packets that aren't UDP or ICMP
-	if (!udp_do_validate_packet(ip_hdr, len, src_ip, validation,
-				    num_ports)) {
-		return 0;
+	// this will reject packets that aren't UDP or ICMP and fully process ICMP
+	// packets
+	if (udp_do_validate_packet(ip_hdr, len, src_ip, validation, num_ports,
+				   zconf.target_port) == PACKET_INVALID) {
+		return PACKET_INVALID;
 	}
 	if (ip_hdr->ip_p == IPPROTO_UDP) {
-		struct udphdr *udp =
-		    (struct udphdr *)((char *)ip_hdr + ip_hdr->ip_hl * 4);
-		uint16_t sport = ntohs(udp->uh_sport);
-		if (sport != zconf.target_port) {
-			return 0;
+		struct udphdr *udp = get_udp_header(ip_hdr, len);
+		if (!udp) {
+			return PACKET_INVALID;
 		}
-		if (udp->uh_ulen < sizeof(struct udphdr)) {
-			return 0;
+		const size_t min_len =
+		    sizeof(struct udphdr) + sizeof(struct bacnet_vlc);
+		if (udp->uh_ulen < min_len) {
+			return PACKET_INVALID;
 		}
-		if (udp->uh_ulen <
-		    sizeof(struct udphdr) + sizeof(struct bacnet_vlc)) {
-			return 0;
-		}
-		struct bacnet_vlc *vlc = (struct bacnet_vlc *)&udp[1];
+		struct bacnet_vlc *vlc =
+		    (struct bacnet_vlc *)get_udp_payload(udp, len);
 		if (vlc->type != ZMAP_BACNET_TYPE_IP) {
-			return 0;
+			return PACKET_INVALID;
 		}
 	}
-	return 1;
+	return PACKET_VALID;
 }
 
 void bacnet_process_packet(const u_char *packet, uint32_t len, fieldset_t *fs,
-			   __attribute__((unused)) uint32_t *validation,
-			   __attribute__((unused)) struct timespec ts)
+			   UNUSED uint32_t *validation, UNUSED struct timespec ts)
 {
-	uint32_t ip_offset = sizeof(struct ether_header);
-	struct ip *ip_hdr = (struct ip *)&packet[ip_offset];
-
+	struct ip *ip_hdr = get_ip_header(packet, len);
+	assert(ip_hdr);
 	if (ip_hdr->ip_p == IPPROTO_UDP) {
-		uint32_t udp_offset = ip_offset + ip_hdr->ip_hl * 4;
-		assert(udp_offset + sizeof(struct udphdr) < len);
-		struct udphdr *udp = (struct udphdr *)&packet[udp_offset];
-		fs_add_string(fs, "classification", (char *)"ntp", 0);
-		fs_add_bool(fs, "success", 1);
+		struct udphdr *udp = get_udp_header(ip_hdr, len);
+		assert(udp);
 		fs_add_uint64(fs, "sport", ntohs(udp->uh_sport));
 		fs_add_uint64(fs, "dport", ntohs(udp->uh_dport));
-		fs_add_null(fs, "icmp_responder");
-		fs_add_null(fs, "icmp_type");
-		fs_add_null(fs, "icmp_code");
-		fs_add_null(fs, "icmp_unreach_str");
-
+		fs_add_constchar(fs, "classification", "bacnet");
+		fs_add_bool(fs, "success", 1);
+		fs_add_null_icmp(fs);
+		uint32_t udp_offset = sizeof(struct ether_header) + ip_hdr->ip_hl * 4;
 		uint32_t payload_offset = udp_offset + sizeof(struct udphdr);
 		assert(payload_offset < len);
-		const uint8_t *payload = &packet[payload_offset];
+		uint8_t *payload = get_udp_payload(udp, len);
 		uint32_t payload_len = len - payload_offset;
-		fs_add_binary(fs, "udp_payload", payload_len, (void *)payload,
-			      0);
+		fs_add_binary(fs, "udp_payload", payload_len, (void *)payload, 0);
+		fs_add_null_icmp(fs);
 	} else if (ip_hdr->ip_p == IPPROTO_ICMP) {
-		struct icmp *icmp =
-		    (struct icmp *)((char *)ip_hdr + ip_hdr->ip_hl * 4);
-		struct ip *ip_inner =
-		    (struct ip *)((char *)icmp + ICMP_UNREACH_HEADER_SIZE);
-
-		fs_modify_string(fs, "saddr",
-				 make_ip_str(ip_inner->ip_dst.s_addr), 1);
-		fs_add_string(fs, "classification", (char *)"icmp-unreach", 0);
-		fs_add_bool(fs, "success", 0);
 		fs_add_null(fs, "sport");
 		fs_add_null(fs, "dport");
-		fs_add_string(fs, "icmp_responder",
-			      make_ip_str(ip_hdr->ip_src.s_addr), 1);
-		fs_add_uint64(fs, "icmp_type", icmp->icmp_type);
-		fs_add_uint64(fs, "icmp_code", icmp->icmp_code);
-		fs_add_null(fs, "icmp_unreach_str");
-		fs_add_null(fs, "udp_payload");
-	} else {
-		fs_add_string(fs, "classification", (char *)"other", 0);
+		fs_add_constchar(fs, "classification", "icmp");
 		fs_add_bool(fs, "success", 0);
-		fs_add_null(fs, "sport");
-		fs_add_null(fs, "dport");
-		fs_add_null(fs, "icmp_responder");
-		fs_add_null(fs, "icmp_type");
-		fs_add_null(fs, "icmp_code");
-		fs_add_null(fs, "icmp_unreach_str");
 		fs_add_null(fs, "udp_payload");
+		fs_populate_icmp_from_iphdr(ip_hdr, len, fs);
 	}
 }
 
@@ -193,23 +164,11 @@ int bacnet_global_initialize(struct state_conf *conf)
 }
 
 static fielddef_t fields[] = {
-    {.name = "classification",
-     .type = "string",
-     .desc = "packet classification"},
-    {.name = "success",
-     .type = "bool",
-     .desc = "is  response considered success"},
     {.name = "sport", .type = "int", .desc = "UDP source port"},
     {.name = "dport", .type = "int", .desc = "UDP destination port"},
-    {.name = "icmp_responder",
-     .type = "string",
-     .desc = "Source IP of ICMP_UNREACH messages"},
-    {.name = "icmp_type", .type = "int", .desc = "icmp message type"},
-    {.name = "icmp_code", .type = "int", .desc = "icmp message sub type code"},
-    {.name = "icmp_unreach_str",
-     .type = "string",
-     .desc = "for icmp_unreach responses, the string version of icmp_code "},
+    CLASSIFICATION_SUCCESS_FIELDSET_FIELDS,
     {.name = "udp_payload", .type = "binary", .desc = "UDP payload"},
+    ICMP_FIELDSET_FIELDS,
 };
 
 probe_module_t module_bacnet = {