Change in osmo-pcap[master]: server: Improve verification of messages from client

[Pau Espin Pedrol]

Pau Espin Pedrol has uploaded this change for review. ( https://gerrit.osmocom.org/11280


Change subject: server: Improve verification of messages from client
......................................................................

server: Improve verification of messages from client

Take the chance to define SERVER_MAX_DATA_SIZE as pcap payload, which we
can later match to configurable snaplen parameter.

Change-Id: I45d4c59026faf1108c0976eb6ad8c270e3577dbf
---
M include/osmo-pcap/osmo_pcap_server.h
M src/osmo_server_network.c
2 files changed, 38 insertions(+), 8 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-pcap refs/changes/80/11280/1

diff --git a/include/osmo-pcap/osmo_pcap_server.h b/include/osmo-pcap/osmo_pcap_server.h
index c1d318e..cdcdb70 100644
--- a/include/osmo-pcap/osmo_pcap_server.h
+++ b/include/osmo-pcap/osmo_pcap_server.h
@@ -91,7 +91,7 @@
 	int state;
 	int pend;
 	int reopen;
-	char buf[SERVER_MAX_DATA_SIZE + sizeof(struct osmo_pcap_data)];
+	char buf[sizeof(struct osmo_pcap_data) + sizeof(struct osmo_pcap_pkthdr) + SERVER_MAX_DATA_SIZE];
 	struct osmo_pcap_data *data;
 
 	/* statistics */
diff --git a/src/osmo_server_network.c b/src/osmo_server_network.c
index 695090d..8eb7567 100644
--- a/src/osmo_server_network.c
+++ b/src/osmo_server_network.c
@@ -192,12 +192,15 @@
 {
 	struct pcap_file_header *hdr;
 
-	if (data->len != sizeof(*hdr)) {
-		LOGP(DSERVER, LOGL_ERROR, "The pcap_file_header does not fit.\n");
+	hdr = (struct pcap_file_header *) &data->data[0];
+
+	if (hdr->snaplen > SERVER_MAX_DATA_SIZE) {
+		LOGP(DSERVER, LOGL_ERROR,
+		     "The recvd pcap_file_header contains too big snaplen %zu > %zu\n",
+		     (size_t) hdr->snaplen, (size_t) SERVER_MAX_DATA_SIZE);
 		return -1;
 	}
 
-	hdr = (struct pcap_file_header *) &data->data[0];
 	if (!conn->no_store && conn->local_fd < 0) {
 		conn->file_hdr = *hdr;
 		restart_pcap(conn);
@@ -335,6 +338,36 @@
 	return do_read_tls(conn, buf, size);
 }
 
+static bool pcap_data_valid(struct osmo_pcap_conn *conn)
+{
+	unsigned int min_len, max_len;
+	switch ((enum OsmoPcapDataType) conn->data->type) {
+	case PKT_LINK_HDR:
+		if (conn->data->len != sizeof(struct pcap_file_header)) {
+			LOGP(DSERVER, LOGL_ERROR,
+			     "Implausible llink_hdr length: %u != %zu\n",
+			     conn->data->len, sizeof(struct osmo_pcap_pkthdr));
+			return false;
+		}
+		break;
+	case PKT_LINK_DATA:
+		min_len = sizeof(struct osmo_pcap_pkthdr);
+		max_len = SERVER_MAX_DATA_SIZE + sizeof(struct osmo_pcap_pkthdr);
+		if (conn->data->len < min_len || conn->data->len > max_len) {
+			LOGP(DSERVER, LOGL_ERROR,
+			     "Implausible data length: %u < %u <= %u\n",
+			     min_len, conn->data->len, max_len);
+			return false;
+		}
+		break;
+	default:
+		LOGP(DSERVER, LOGL_ERROR, "Unknown data type %" PRIx8 "\n",
+		     conn->data->type);
+		return false;
+	}
+	return true;
+}
+
 static int read_cb_initial(struct osmo_pcap_conn *conn)
 {
 	int rc;
@@ -354,11 +387,8 @@
 	} else if (conn->pend == 0) {
 		conn->data->len = ntohs(conn->data->len);
 
-		if (conn->data->len > SERVER_MAX_DATA_SIZE) {
-			LOGP(DSERVER, LOGL_ERROR,
-			     "Implausible data length: %u\n", conn->data->len);
+		if (!pcap_data_valid(conn))
 			return -1;
-		}
 
 		conn->state = STATE_DATA;
 		conn->pend = conn->data->len;

-- 
To view, visit https://gerrit.osmocom.org/11280
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-pcap
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I45d4c59026faf1108c0976eb6ad8c270e3577dbf
Gerrit-Change-Number: 11280
Gerrit-PatchSet: 1
Gerrit-Owner: Pau Espin Pedrol <pespin at sysmocom.de>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20181008/9d2b1413/attachment.htm>