- gerrit-log - lists.osmocom.org

[L] Change in osmo-pcap[master]: client: Support generation of pcapng file format
by pespin 10 Jan '25

10 Jan '25

Attention is currently required from: daniel, laforge, pespin. Hello Jenkins Builder, daniel, laforge, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-pcap/+/39265?usp=email to look at the new patch set (#7). The following approvals got outdated and were removed: Verified+1 by Jenkins Builder Change subject: client: Support generation of pcapng file format ...................................................................... client: Support generation of pcapng file format Support transmitting traffic recordings in pcapng towards osmo-pca-server based on VTY config. Old libpcap's file format (.pcap) is still left as default to to keep backward compatibility towards older osmo-pcap-server instances still running. Furthermore, .pcap support is more extended than .pcapng since it's way older. When pcapng file format is selected, osmo-pcap-client will use the same osmo-pcap protocol used in pcap mode, but the payloads encoded in messages will be in pcapng file format instead of pcap. This means, for LINK_HDR message type it will encode a file header with a payload consisting of 1 SHB + N IDB blocks (one for each network interface being monitored). Upon each packet recording received from the monitoring interface, it will encode a LINK_DATA message containing a payload with a encoded pcapng block. This works mostly transparetly on the osmo-pcap-server side, since in general it handles those payloads mostly transparently. Only some sanity checks will need to be updated and improve there to account for the new pcapng blocks. This in turn requires identifying pcap vs pcapng format being recieved from the client, and also placing the proper file suffix when creating the file. This will be done in a follow-up patch modyfing the server side. Related: SYS#5822 Change-Id: I3c80518a1e53a1f77e1aca8dfa83f683f9516ad6 --- M doc/manuals/chapters/client.adoc M include/osmo-pcap/osmo_pcap_client.h M include/osmo-pcap/osmo_pcap_file.h M src/osmo_client_core.c M src/osmo_client_network.c M src/osmo_client_vty.c M src/osmo_pcap_file.c 7 files changed, 534 insertions(+), 10 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-pcap refs/changes/65/39265/7 -- To view, visit https://gerrit.osmocom.org/c/osmo-pcap/+/39265?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: osmo-pcap Gerrit-Branch: master Gerrit-Change-Id: I3c80518a1e53a1f77e1aca8dfa83f683f9516ad6 Gerrit-Change-Number: 39265 Gerrit-PatchSet: 7 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>

1 0

[L] Change in osmo-pcap[master]: client: Introduce osmo_pcap_file to prepare support for pcapng
by pespin 10 Jan '25

10 Jan '25

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-pcap/+/39288?usp=email ) Change subject: client: Introduce osmo_pcap_file to prepare support for pcapng ...................................................................... client: Introduce osmo_pcap_file to prepare support for pcapng This patch is a preparation patch refactoring code to make it easier to add pcapng format in follow-up patch. Different types of encodings are grouped in helper functions. Change-Id: Ifd389a2a32897e101c62fd280ddca984d485d373 --- M include/osmo-pcap/Makefile.am M include/osmo-pcap/osmo_pcap_client.h A include/osmo-pcap/osmo_pcap_file.h M src/Makefile.am M src/osmo_client_core.c M src/osmo_client_network.c A src/osmo_pcap_file.c 7 files changed, 247 insertions(+), 78 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-pcap refs/changes/88/39288/1 diff --git a/include/osmo-pcap/Makefile.am b/include/osmo-pcap/Makefile.am index e377b6a..58c8a59 100644 --- a/include/osmo-pcap/Makefile.am +++ b/include/osmo-pcap/Makefile.am @@ -1,6 +1,7 @@ noinst_HEADERS = \ common.h \ osmo_pcap_client.h \ + osmo_pcap_file.h \ osmo_pcap_server.h \ osmo_tls.h \ wireformat.h \ diff --git a/include/osmo-pcap/osmo_pcap_client.h b/include/osmo-pcap/osmo_pcap_client.h index 442244e..470edb6 100644 --- a/include/osmo-pcap/osmo_pcap_client.h +++ b/include/osmo-pcap/osmo_pcap_client.h @@ -139,7 +139,9 @@ struct osmo_pcap_client_conn *osmo_client_conn_alloc(struct osmo_pcap_client *client, const char *name); void osmo_client_conn_free(struct osmo_pcap_client_conn *conn); void osmo_client_conn_send_data(struct osmo_pcap_client_conn *conn, - struct pcap_pkthdr *hdr, const uint8_t *data); + const struct osmo_pcap_handle *ph, + const struct pcap_pkthdr *pkthdr, + const uint8_t *data); void osmo_client_conn_send_link(struct osmo_pcap_client_conn *conn); void osmo_client_conn_connect(struct osmo_pcap_client_conn *conn); void osmo_client_conn_disconnect(struct osmo_pcap_client_conn *conn); diff --git a/include/osmo-pcap/osmo_pcap_file.h b/include/osmo-pcap/osmo_pcap_file.h new file mode 100644 index 0000000..dfa54d0 --- /dev/null +++ b/include/osmo-pcap/osmo_pcap_file.h @@ -0,0 +1,38 @@ +/* + * Procedures to operate on pcap/pcapng file format + * + * (C) 2025 by sysmocom - s.f.m.c. GmbH <info(a)sysmocom.de> + * All Rights Reserved + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + * + */ + +#pragma once + +#include <unistd.h> +#include <stdint.h> + +/*********************************************************** +* Libpcap File Format (.pcap) +* https://wiki.wireshark.org/Development/LibpcapFileFormat +***********************************************************/ + +#define OSMO_PCAP_FILE_MAGIC 0xa1b2c3d4 + +unsigned int osmo_pcap_file_global_header_size(void); +int osmo_pcap_file_msgb_append_global_header(struct msgb *msg, uint32_t snaplen, uint32_t linktype); + +unsigned int osmo_pcap_file_record_size(const struct pcap_pkthdr *in_hdr); +int osmo_pcap_file_msgb_append_record(struct msgb *msg, const struct pcap_pkthdr *in_hdr, const uint8_t *data); diff --git a/src/Makefile.am b/src/Makefile.am index 00c9ce9..bb70d21 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -20,6 +20,7 @@ osmo_client_network.c \ osmo_client_stats.c \ osmo_client_vty.c \ + osmo_pcap_file.c \ osmo_tls.c \ $(NULL) diff --git a/src/osmo_client_core.c b/src/osmo_client_core.c index a2ab894..538b68a 100644 --- a/src/osmo_client_core.c +++ b/src/osmo_client_core.c @@ -172,7 +172,7 @@ return 0; llist_for_each_entry(conn, &client->conns, entry) - osmo_client_conn_send_data(conn, &hdr, data); + osmo_client_conn_send_data(conn, ph, &hdr, data); return 0; } diff --git a/src/osmo_client_network.c b/src/osmo_client_network.c index 70fe740..12b707b 100644 --- a/src/osmo_client_network.c +++ b/src/osmo_client_network.c @@ -1,6 +1,7 @@ /* * osmo-pcap-client code * + * (C) 2025 by sysmocom - s.f.m.c. GmbH <info(a)sysmocom.de> * (C) 2011-2016 by Holger Hans Peter Freyther <holger(a)moiji-mobile.com> * (C) 2011 by On-Waves * All Rights Reserved @@ -23,7 +24,9 @@ #include <osmo-pcap/osmo_pcap_client.h> #include <osmo-pcap/common.h> #include <osmo-pcap/wireformat.h> +#include <osmo-pcap/osmo_pcap_file.h> +#include <osmocom/core/utils.h> #include <osmocom/core/msgb.h> #include <osmocom/core/rate_ctr.h> #include <osmocom/core/select.h> @@ -160,112 +163,152 @@ } } -void osmo_client_conn_send_data(struct osmo_pcap_client_conn *conn, - struct pcap_pkthdr *in_hdr, const uint8_t *data) +static struct msgb *osmo_client_conn_prepare_msg_data_pcap(struct osmo_pcap_client_conn *conn, + const struct osmo_pcap_handle *ph, + const struct pcap_pkthdr *pkthdr, + const uint8_t *data) { struct osmo_pcap_data *om_hdr; - struct osmo_pcap_pkthdr *hdr; struct msgb *msg; - int offset, ip_len; - struct osmo_pcap_handle *ph; + unsigned int record_size = osmo_pcap_file_record_size(pkthdr); - if (in_hdr->len > in_hdr->caplen) { - LOGCONN(conn, LOGL_ERROR, "Recording truncated packet, len %zu > snaplen %zu\n", - (size_t) in_hdr->len, (size_t) in_hdr->caplen); - rate_ctr_inc2(conn->client->ctrg, CLIENT_CTR_2BIG); - } - - msg = msgb_alloc(in_hdr->caplen + sizeof(*om_hdr) + sizeof(*hdr), "data-data"); + msg = msgb_alloc(sizeof(*om_hdr) + record_size, "pcap-data"); if (!msg) { LOGCONN(conn, LOGL_ERROR, "Failed to allocate\n"); rate_ctr_inc2(conn->client->ctrg, CLIENT_CTR_NOMEM); - return; + return NULL; + } + + om_hdr = (struct osmo_pcap_data *) msgb_put(msg, sizeof(*om_hdr)); + om_hdr->type = PKT_LINK_DATA; + om_hdr->len = htons(record_size); + osmo_pcap_file_msgb_append_record(msg, pkthdr, data); + + return msg; +} + +static struct msgb *osmo_client_conn_prepare_msg_ipip(struct osmo_pcap_client_conn *conn, + const struct osmo_pcap_handle *ph, + const struct pcap_pkthdr *pkthdr, + const uint8_t *data) +{ + struct msgb *msg; + int offset, ip_len; + + offset = get_iphdr_offset(pcap_datalink(ph->handle)); + if (offset < 0) + return NULL; + + ip_len = pkthdr->caplen - offset; + if (ip_len < 0) + return NULL; + + + msg = msgb_alloc(ip_len, "ipip_msg"); + if (!msg) { + LOGP(DCLIENT, LOGL_ERROR, "Failed to allocate data.\n"); + rate_ctr_inc2(conn->client->ctrg, CLIENT_CTR_NOMEM); + return NULL; + } + msg->l2h = msgb_put(msg, ip_len); + memcpy(msg->l2h, data+offset, ip_len); + return msg; +} + +void osmo_client_conn_send_data(struct osmo_pcap_client_conn *conn, + const struct osmo_pcap_handle *ph, + const struct pcap_pkthdr *pkthdr, + const uint8_t *data) +{ + struct osmo_pcap_client *client = conn->client; + struct msgb *msg; + + if (pkthdr->len > pkthdr->caplen) { + LOGCONN(conn, LOGL_ERROR, "Recording truncated packet, len %zu > snaplen %zu\n", + (size_t) pkthdr->len, (size_t) pkthdr->caplen); + rate_ctr_inc2(client->ctrg, CLIENT_CTR_2BIG); } switch (conn->protocol) { case PROTOCOL_OSMOPCAP: - om_hdr = (struct osmo_pcap_data *) msgb_put(msg, sizeof(*om_hdr)); - om_hdr->type = PKT_LINK_DATA; - - msg->l2h = msgb_put(msg, sizeof(*hdr)); - hdr = (struct osmo_pcap_pkthdr *) msg->l2h; - hdr->ts_sec = in_hdr->ts.tv_sec; - hdr->ts_usec = in_hdr->ts.tv_usec; - hdr->caplen = in_hdr->caplen; - hdr->len = in_hdr->len; - - msg->l3h = msgb_put(msg, in_hdr->caplen); - memcpy(msg->l3h, data, in_hdr->caplen); - - om_hdr->len = htons(msgb_l2len(msg)); - rate_ctr_add2(conn->client->ctrg, CLIENT_CTR_BYTES, hdr->caplen); - rate_ctr_inc2(conn->client->ctrg, CLIENT_CTR_PKTS); + msg = osmo_client_conn_prepare_msg_data_pcap(conn, ph, pkthdr, data); break; case PROTOCOL_IPIP: - /* TODO: support capturing from multiple interfaces here: */ - ph = llist_first_entry_or_null(&conn->client->handles, struct osmo_pcap_handle, entry); - if (!ph) { - msgb_free(msg); - return; - } - offset = get_iphdr_offset(pcap_datalink(ph->handle)); - if (offset < 0) { - msgb_free(msg); - return; - } - ip_len = in_hdr->caplen - offset; - if (ip_len < 0) { - msgb_free(msg); - return; - } - msg->l2h = msgb_put(msg, ip_len); - memcpy(msg->l2h, data+offset, ip_len); + msg = osmo_client_conn_prepare_msg_ipip(conn, ph, pkthdr, data); break; default: OSMO_ASSERT(0); } + if (!msg) + return; + + rate_ctr_add2(conn->client->ctrg, CLIENT_CTR_BYTES, pkthdr->caplen); + rate_ctr_inc2(conn->client->ctrg, CLIENT_CTR_PKTS); + write_data(conn, msg); } +static struct msgb *osmo_client_conn_prepare_msg_link_pcap(struct osmo_pcap_client_conn *conn) +{ + struct osmo_pcap_client *client = conn->client; + struct osmo_pcap_data *om_hdr; + struct msgb *msg; + struct osmo_pcap_handle *ph; + int rc; + int linktype; + + ph = llist_first_entry_or_null(&client->handles, struct osmo_pcap_handle, entry); + if (!ph || !ph->handle) { + LOGCONN(conn, LOGL_ERROR, "No pcap_handle not sending link info\n"); + return NULL; + } + linktype = pcap_datalink(ph->handle); + + /* Make sure others have same linktype, .pcap doesn't support different + * linktypes since traffic from all ifaces goes mixed together. */ + llist_for_each_entry(ph, &client->handles, entry) { + if (linktype != pcap_datalink(ph->handle)) { + LOGCONN(conn, LOGL_ERROR, + "File format 'pcap' doesn't support recording from multiple ifaces " + "with different link types!\n"); + return NULL; + } + } + + msg = msgb_alloc(sizeof(*om_hdr) + osmo_pcap_file_global_header_size(), "link-data"); + if (!msg) { + LOGP(DCLIENT, LOGL_ERROR, "Failed to allocate data.\n"); + return NULL; + } + + + om_hdr = (struct osmo_pcap_data *)msgb_put(msg, sizeof(*om_hdr)); + om_hdr->type = PKT_LINK_HDR; + + rc = osmo_pcap_file_msgb_append_global_header(msg, client->snaplen, linktype); + if (rc < 0) { + LOGP(DCLIENT, LOGL_ERROR, "Failed to create pcap file global header.\n"); + msgb_free(msg); + return NULL; + } + + /* Update payload length: */ + om_hdr->len = htons(rc); + return msg; +} + void osmo_client_conn_send_link(struct osmo_pcap_client_conn *conn) { - struct osmo_pcap_handle *ph; - struct pcap_file_header *hdr; - struct osmo_pcap_data *om_hdr; struct msgb *msg; /* IPIP encapsulation has no linktype header */ if (conn->protocol == PROTOCOL_IPIP) return; - /* TODO: support capturing from multiple interfaces here: */ - ph = llist_first_entry_or_null(&conn->client->handles, struct osmo_pcap_handle, entry); - if (!ph || !ph->handle) { - LOGCONN(conn, LOGL_ERROR, "No pcap_handle not sending link info\n"); + msg = osmo_client_conn_prepare_msg_link_pcap(conn); + if (!msg) return; - } - - msg = msgb_alloc(sizeof(*om_hdr) + sizeof(*hdr), "link-data"); - if (!msg) { - LOGP(DCLIENT, LOGL_ERROR, "Failed to allocate data.\n"); - return; - } - - - om_hdr = (struct osmo_pcap_data *) msgb_put(msg, sizeof(*om_hdr)); - om_hdr->type = PKT_LINK_HDR; - om_hdr->len = htons(sizeof(*hdr)); - - hdr = (struct pcap_file_header *) msgb_put(msg, sizeof(*hdr)); - hdr->magic = 0xa1b2c3d4; - hdr->version_major = 2; - hdr->version_minor = 4; - hdr->thiszone = 0; - hdr->sigfigs = 0; - hdr->snaplen = conn->client->snaplen; - hdr->linktype = pcap_datalink(ph->handle); - write_data(conn, msg); } diff --git a/src/osmo_pcap_file.c b/src/osmo_pcap_file.c new file mode 100644 index 0000000..0591682 --- /dev/null +++ b/src/osmo_pcap_file.c @@ -0,0 +1,84 @@ +/* + * osmo-pcap-client code + * + * (C) 2025 by sysmocom - s.f.m.c. GmbH <info(a)sysmocom.de> + * (C) 2011-2016 by Holger Hans Peter Freyther <holger(a)moiji-mobile.com> + * (C) 2011 by On-Waves + * All Rights Reserved + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + * + */ + +#include <pcap/pcap.h> + +#include <osmocom/core/msgb.h> + +#include <osmo-pcap/osmo_pcap_file.h> +#include <osmo-pcap/common.h> +#include <osmo-pcap/wireformat.h> + +/*********************************************************** + * Libpcap File Format (.pcap) + * https://wiki.wireshark.org/Development/LibpcapFileFormat + ***********************************************************/ + +/* Get required length to store a Global Header */ +unsigned int osmo_pcap_file_global_header_size(void) +{ + return sizeof(struct pcap_file_header); +} + +/* Appends a Global Header to msg + * returns number of bytes appended on success, negative on error */ +int osmo_pcap_file_msgb_append_global_header(struct msgb *msg, uint32_t snaplen, uint32_t linktype) +{ + struct pcap_file_header *hdr; + + hdr = (struct pcap_file_header *) msgb_put(msg, sizeof(*hdr)); + hdr->magic = OSMO_PCAP_FILE_MAGIC; + hdr->version_major = 2; + hdr->version_minor = 4; + hdr->thiszone = 0; + hdr->sigfigs = 0; + hdr->snaplen = snaplen; + hdr->linktype = linktype; + + return sizeof(*hdr); +} + +/* Get required length to store a given record (packet) */ +unsigned int osmo_pcap_file_record_size(const struct pcap_pkthdr *in_hdr) +{ + return sizeof(struct osmo_pcap_pkthdr) + in_hdr->caplen; +} + +/* Appends a Record (Packet) Header to msg + * returns number of bytes appended on success, negative on error */ +int osmo_pcap_file_msgb_append_record(struct msgb *msg, const struct pcap_pkthdr *in_hdr, const uint8_t *data) +{ + struct osmo_pcap_pkthdr *hdr; + uint8_t *pkt_payload; + + hdr = (struct osmo_pcap_pkthdr *) msgb_put(msg, sizeof(*hdr)); + hdr->ts_sec = in_hdr->ts.tv_sec; + hdr->ts_usec = in_hdr->ts.tv_usec; + hdr->caplen = in_hdr->caplen; + hdr->len = in_hdr->len; + + pkt_payload = msgb_put(msg, in_hdr->caplen); + memcpy(pkt_payload, data, in_hdr->caplen); + + return osmo_pcap_file_record_size(in_hdr); +} -- To view, visit https://gerrit.osmocom.org/c/osmo-pcap/+/39288?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: osmo-pcap Gerrit-Branch: master Gerrit-Change-Id: Ifd389a2a32897e101c62fd280ddca984d485d373 Gerrit-Change-Number: 39288 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 0

[S] Change in osmo-mgw[master]: TODO: move mgcp_rtp_end_config
by pespin 10 Jan '25

10 Jan '25

pespin has abandoned this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39218?usp=email ) Change subject: TODO: move mgcp_rtp_end_config ...................................................................... Abandoned -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/39218?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: abandon Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I7f2209f50424903cfefad53b4278b712fda51ce5 Gerrit-Change-Number: 39218 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder

1 0

[XS] Change in osmo-pcap[master]: server: Rename static functions
by pespin 10 Jan '25

10 Jan '25

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-pcap/+/39268?usp=email ) Change subject: server: Rename static functions ...................................................................... server: Rename static functions The current naming of those functions is totally misleadling with regards of the message type it processes. Let's clarify them. Change-Id: I72f21b38b696e80e481043825ad23c3b8d36ab16 --- M src/osmo_server_network.c 1 file changed, 4 insertions(+), 4 deletions(-) Approvals: Jenkins Builder: Verified laforge: Looks good to me, approved diff --git a/src/osmo_server_network.c b/src/osmo_server_network.c index 9750170..a784af2 100644 --- a/src/osmo_server_network.c +++ b/src/osmo_server_network.c @@ -296,7 +296,7 @@ update_last_write(conn, now); } -static int link_data(struct osmo_pcap_conn *conn, struct osmo_pcap_data *data) +static int rx_link_hdr(struct osmo_pcap_conn *conn, struct osmo_pcap_data *data) { struct pcap_file_header *hdr; @@ -460,7 +460,7 @@ /* * Check if we are past the limit or on a day change */ -static int write_data(struct osmo_pcap_conn *conn, struct osmo_pcap_data *data) +static int rx_link_data(struct osmo_pcap_conn *conn, struct osmo_pcap_data *data) { time_t now = time(NULL); int rc; @@ -667,10 +667,10 @@ switch (conn->data->type) { case PKT_LINK_HDR: - return link_data(conn, conn->data); + return rx_link_hdr(conn, conn->data); break; case PKT_LINK_DATA: - return write_data(conn, conn->data); + return rx_link_data(conn, conn->data); break; } } -- To view, visit https://gerrit.osmocom.org/c/osmo-pcap/+/39268?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-pcap Gerrit-Branch: master Gerrit-Change-Id: I72f21b38b696e80e481043825ad23c3b8d36ab16 Gerrit-Change-Number: 39268 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

1 0

[S] Change in osmo-pcap[master]: Cleanup and reorder includes in header files
by pespin 10 Jan '25

10 Jan '25

Attention is currently required from: pespin. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-pcap/+/39285?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified-1 by Jenkins Builder Change subject: Cleanup and reorder includes in header files ...................................................................... Cleanup and reorder includes in header files Put all includes in same order of dependencies. Include stdint.h instead of inttypes.h since that's the file actually containing the standard int types. Change-Id: I0467d3c3a2687a7990c0db7371d0c89f97e8400a --- M include/osmo-pcap/osmo_pcap_client.h M include/osmo-pcap/osmo_pcap_server.h M include/osmo-pcap/osmo_tls.h M include/osmo-pcap/wireformat.h M src/osmo_server_network.c 5 files changed, 15 insertions(+), 15 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-pcap refs/changes/85/39285/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-pcap/+/39285?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: osmo-pcap Gerrit-Branch: master Gerrit-Change-Id: I0467d3c3a2687a7990c0db7371d0c89f97e8400a Gerrit-Change-Number: 39285 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Attention: pespin <pespin(a)sysmocom.de>

1 0

[L] Change in osmo-pcap[master]: client: Support generation of pcapng file format
by pespin 10 Jan '25

10 Jan '25

Attention is currently required from: daniel, laforge. pespin has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-pcap/+/39265?usp=email ) Change subject: client: Support generation of pcapng file format ...................................................................... Patch Set 5: This change is ready for review. -- To view, visit https://gerrit.osmocom.org/c/osmo-pcap/+/39265?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-pcap Gerrit-Branch: master Gerrit-Change-Id: I3c80518a1e53a1f77e1aca8dfa83f683f9516ad6 Gerrit-Change-Number: 39265 Gerrit-PatchSet: 5 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Fri, 10 Jan 2025 17:06:52 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No

1 0

[S] Change in osmo-pcap[master]: Makefile.am: proper format and reorder source file lists
by pespin 10 Jan '25

10 Jan '25

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-pcap/+/39286?usp=email ) Change subject: Makefile.am: proper format and reorder source file lists ...................................................................... Makefile.am: proper format and reorder source file lists Change-Id: Ie39bd0348f4f329539c1d9dabc8fa87f3e4bbefe --- M include/osmo-pcap/Makefile.am M src/Makefile.am 2 files changed, 8 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-pcap refs/changes/86/39286/1 diff --git a/include/osmo-pcap/Makefile.am b/include/osmo-pcap/Makefile.am index b71e70c..e377b6a 100644 --- a/include/osmo-pcap/Makefile.am +++ b/include/osmo-pcap/Makefile.am @@ -1 +1,7 @@ -noinst_HEADERS = common.h osmo_pcap_client.h osmo_pcap_server.h wireformat.h osmo_tls.h +noinst_HEADERS = \ + common.h \ + osmo_pcap_client.h \ + osmo_pcap_server.h \ + osmo_tls.h \ + wireformat.h \ + $(NULL) diff --git a/src/Makefile.am b/src/Makefile.am index b35227c..00c9ce9 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -17,9 +17,9 @@ osmo_client_main.c \ osmo_common.c \ osmo_client_core.c \ - osmo_client_vty.c \ osmo_client_network.c \ osmo_client_stats.c \ + osmo_client_vty.c \ osmo_tls.c \ $(NULL) -- To view, visit https://gerrit.osmocom.org/c/osmo-pcap/+/39286?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: osmo-pcap Gerrit-Branch: master Gerrit-Change-Id: Ie39bd0348f4f329539c1d9dabc8fa87f3e4bbefe Gerrit-Change-Number: 39286 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 0

[S] Change in osmo-pcap[master]: Cleanup and reorder includes in header files
by pespin 10 Jan '25

10 Jan '25

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-pcap/+/39285?usp=email ) Change subject: Cleanup and reorder includes in header files ...................................................................... Cleanup and reorder includes in header files Put all includes in same order of dependencies. Include stdint.h instead of inttypes.h since that's the file actually containing the standard int types. Change-Id: I0467d3c3a2687a7990c0db7371d0c89f97e8400a --- M include/osmo-pcap/osmo_pcap_client.h M include/osmo-pcap/osmo_pcap_server.h M include/osmo-pcap/osmo_tls.h M include/osmo-pcap/wireformat.h 4 files changed, 14 insertions(+), 15 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-pcap refs/changes/85/39285/1 diff --git a/include/osmo-pcap/osmo_pcap_client.h b/include/osmo-pcap/osmo_pcap_client.h index 09a4674..442244e 100644 --- a/include/osmo-pcap/osmo_pcap_client.h +++ b/include/osmo-pcap/osmo_pcap_client.h @@ -21,8 +21,6 @@ */ #pragma once -#include "osmo_tls.h" - #include <inttypes.h> #include <pcap.h> @@ -32,6 +30,8 @@ #include <osmocom/core/rate_ctr.h> #include <osmocom/core/write_queue.h> +#include <osmo-pcap/osmo_tls.h> + #define WQUEUE_MAXLEN_DEFAULT 1000 enum pcap_client_ctr { diff --git a/include/osmo-pcap/osmo_pcap_server.h b/include/osmo-pcap/osmo_pcap_server.h index 988ac96..da39d99 100644 --- a/include/osmo-pcap/osmo_pcap_server.h +++ b/include/osmo-pcap/osmo_pcap_server.h @@ -21,21 +21,20 @@ */ #pragma once -#include "wireformat.h" -#include "osmo_tls.h" - -#include <osmocom/core/select.h> -#include <osmocom/core/linuxlist.h> -#include <osmocom/core/write_queue.h> - +#include <stdbool.h> +#include <time.h> #include <sys/socket.h> #include <netinet/in.h> #include <arpa/inet.h> #include <pcap.h> -#include <stdbool.h> -#include <time.h> +#include <osmocom/core/select.h> +#include <osmocom/core/linuxlist.h> +#include <osmocom/core/write_queue.h> + +#include <osmo-pcap/wireformat.h> +#include <osmo-pcap/osmo_tls.h> struct rate_ctr_group; struct rate_ctr_group_desc; diff --git a/include/osmo-pcap/osmo_tls.h b/include/osmo-pcap/osmo_tls.h index 1b15168..716604e 100644 --- a/include/osmo-pcap/osmo_tls.h +++ b/include/osmo-pcap/osmo_tls.h @@ -20,12 +20,12 @@ */ #pragma once -#include <gnutls/gnutls.h> -#include <gnutls/abstract.h> - #include <stdbool.h> #include <stdint.h> +#include <gnutls/gnutls.h> +#include <gnutls/abstract.h> + struct osmo_fd; struct osmo_wqueue; struct osmo_pcap_client_conn; diff --git a/include/osmo-pcap/wireformat.h b/include/osmo-pcap/wireformat.h index 3838830..40b5b0e 100644 --- a/include/osmo-pcap/wireformat.h +++ b/include/osmo-pcap/wireformat.h @@ -21,7 +21,7 @@ */ #pragma once -#include <inttypes.h> +#include <stdint.h> #include <pcap.h> enum OsmoPcapDataType { -- To view, visit https://gerrit.osmocom.org/c/osmo-pcap/+/39285?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: osmo-pcap Gerrit-Branch: master Gerrit-Change-Id: I0467d3c3a2687a7990c0db7371d0c89f97e8400a Gerrit-Change-Number: 39285 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 0

[L] Change in osmo-pcap[master]: server: Support storing of pcapng format
by pespin 10 Jan '25

10 Jan '25

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-pcap/+/39287?usp=email ) Change subject: server: Support storing of pcapng format ...................................................................... server: Support storing of pcapng format Improve existing logic doing validation on payload data and generalize the file_hdr field to account for the possibily of receiving recordings in pcapng format. This requires, during LINK_HDR when file header is received, discering whether the recording is being done in pcap vs pcapng, (and in pcapng figuring out the endianness of the file) and then applying validation based on that knowledge. With that knowledge, osmo-pcap-server can now store the pcap(ng) file using the proper suffix for the file. Related: SYS#5822 Change-Id: I275d28ab418a1514fa9c5c7c20f3d831cc6af8bb --- M include/osmo-pcap/osmo_pcap_file.h M include/osmo-pcap/osmo_pcap_server.h M src/Makefile.am M src/osmo_pcap_file.c M src/osmo_server_network.c M tests/rotate_localtime/Makefile.am 6 files changed, 335 insertions(+), 48 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-pcap refs/changes/87/39287/1 diff --git a/include/osmo-pcap/osmo_pcap_file.h b/include/osmo-pcap/osmo_pcap_file.h index a12df89..3daad99 100644 --- a/include/osmo-pcap/osmo_pcap_file.h +++ b/include/osmo-pcap/osmo_pcap_file.h @@ -29,6 +29,8 @@ OSMO_PCAP_FMT_PCAPNG, }; +int osmo_pcap_file_discover_fmt(const uint8_t *data, size_t data_len, enum osmo_pcap_fmt *result_fmt); + /*********************************************************** * Libpcap File Format (.pcap) * https://wiki.wireshark.org/Development/LibpcapFileFormat @@ -123,6 +125,11 @@ /* ... Options ... */ } __attribute__((packed)); +int osmo_pcapng_file_is_swapped(const uint8_t *data, size_t data_len); +uint16_t osmo_pcapng_file_read_uint16(const uint8_t *data, bool endian_swapped); +uint32_t osmo_pcapng_file_read_uint32(const uint8_t *data, bool endian_swapped); +uint64_t osmo_pcapng_file_read_uint64(const uint8_t *data, bool endian_swapped); + /* Helper APIs to encode blocks: */ struct osmo_pcapng_file_shb_pars { diff --git a/include/osmo-pcap/osmo_pcap_server.h b/include/osmo-pcap/osmo_pcap_server.h index da39d99..f44781a 100644 --- a/include/osmo-pcap/osmo_pcap_server.h +++ b/include/osmo-pcap/osmo_pcap_server.h @@ -35,6 +35,7 @@ #include <osmo-pcap/wireformat.h> #include <osmo-pcap/osmo_tls.h> +#include <osmo-pcap/osmo_pcap_file.h> struct rate_ctr_group; struct rate_ctr_group_desc; @@ -88,7 +89,10 @@ char *curr_filename; /* pcap stuff */ - struct pcap_file_header file_hdr; + enum osmo_pcap_fmt file_fmt; + bool pcapng_endian_swapped; + uint8_t *file_hdr; + uint32_t file_hdr_len; /* last time */ struct tm last_write; diff --git a/src/Makefile.am b/src/Makefile.am index bb70d21..0fb16a2 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -35,6 +35,7 @@ osmo_pcap_server_SOURCES = \ osmo_server_main.c \ osmo_common.c \ + osmo_pcap_file.c \ osmo_server_vty.c \ osmo_server_network.c \ osmo_server_stats.c \ diff --git a/src/osmo_pcap_file.c b/src/osmo_pcap_file.c index ddd32ed..8e59a37 100644 --- a/src/osmo_pcap_file.c +++ b/src/osmo_pcap_file.c @@ -29,6 +29,40 @@ #include <osmo-pcap/common.h> #include <osmo-pcap/wireformat.h> +/* Find out whether data contains a start of a .pcap or .pcapng file. + * Returns 0 on success, negative on error. */ +int osmo_pcap_file_discover_fmt(const uint8_t *data, size_t data_len, enum osmo_pcap_fmt *result_fmt) +{ + const struct pcapng_block_header *bh; + const struct pcapng_section_header_block *shb; + + /* Check for .pcap: */ + if (data_len >= sizeof(struct pcap_file_header) && + ((struct pcap_file_header *)data)->magic == OSMO_PCAP_FILE_MAGIC) { + *result_fmt = OSMO_PCAP_FMT_PCAP; + return 0; + } + + /* Check for .pcapng: */ + if (data_len < sizeof(struct pcapng_block_header) + + sizeof(struct pcapng_section_header_block) + + sizeof(uint32_t)) + return -1; + + bh = (const struct pcapng_block_header *)data; + /* BLOCK_TYPE_SHB has the same value regardless of byte order */ + if (bh->block_type != BLOCK_TYPE_SHB) + return -1; + + shb = (const struct pcapng_section_header_block *)&bh->block_body[0]; + if (shb->magic != OSMO_PCAPNG_FILE_MAGIC && + shb->magic != OSMO_PCAPNG_FILE_MAGIC_SWAPPED) + return -2; + + *result_fmt = OSMO_PCAP_FMT_PCAPNG; + return 0; +} + /*********************************************************** * Libpcap File Format (.pcap) * https://wiki.wireshark.org/Development/LibpcapFileFormat @@ -179,6 +213,64 @@ return sizeof(*opth); } +static int osmo_pcapng_file_shb_is_swapped(const struct pcapng_section_header_block *shb) +{ + if (shb->magic == OSMO_PCAPNG_FILE_MAGIC_SWAPPED) + return 1; + if (shb->magic == OSMO_PCAPNG_FILE_MAGIC) + return 0; + return -1; +} + +/* 1: true, 0: false, negative: error */ +int osmo_pcapng_file_is_swapped(const uint8_t *data, size_t data_len) +{ + const struct pcapng_block_header *bh = (const struct pcapng_block_header *)data; + const struct pcapng_section_header_block *shb; + + if (data_len < sizeof(struct pcapng_block_header) + + sizeof(struct pcapng_section_header_block) + + sizeof(uint32_t)) + return -1; + + /* BLOCK_TYPE_SHB has the same value regardless of byte order */ + if (bh->block_type != BLOCK_TYPE_SHB) + return -1; + + shb = (const struct pcapng_section_header_block *)&bh->block_body[0]; + return osmo_pcapng_file_shb_is_swapped(shb); +} + +uint16_t osmo_pcapng_file_read_uint16(const uint8_t *data, bool endian_swapped) +{ + uint16_t val; + + memcpy(&val, data, sizeof(val)); + if (!endian_swapped) + return val; + return (val >> 8) | (val << 8); +} + +uint32_t osmo_pcapng_file_read_uint32(const uint8_t *data, bool endian_swapped) +{ + uint32_t val; + + memcpy(&val, data, sizeof(val)); + if (!endian_swapped) + return val; + return __builtin_bswap32(val); +} + +uint64_t osmo_pcapng_file_read_uint64(const uint8_t *data, bool endian_swapped) +{ + uint64_t val; + + memcpy(&val, data, sizeof(val)); + if (!endian_swapped) + return val; + return __builtin_bswap64(val); +} + /* Get required length to store a given record (packet) */ unsigned int osmo_pcapng_file_shb_size(const struct osmo_pcapng_file_shb_pars *pars) { diff --git a/src/osmo_server_network.c b/src/osmo_server_network.c index 64d772a..0a0763b 100644 --- a/src/osmo_server_network.c +++ b/src/osmo_server_network.c @@ -108,7 +108,7 @@ talloc_free(event_name); pcap_zmq_send(conn->server->zmq_publ, - &conn->file_hdr, sizeof(conn->file_hdr), + conn->file_hdr, conn->file_hdr_len, ZMQ_SNDMORE); pcap_zmq_send(conn->server->zmq_publ, &data->data[0], data->len, @@ -268,10 +268,11 @@ conn->server->base_path, strerror(errno)); return; } - conn->curr_filename = talloc_asprintf(conn, "%s/trace-%s-%d%.2d%.2d_%.2d%.2d%.2d.pcap", + conn->curr_filename = talloc_asprintf(conn, "%s/trace-%s-%d%.2d%.2d_%.2d%.2d%.2d.%s", real_base_path, conn->name, tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday, - tm.tm_hour, tm.tm_min, tm.tm_sec); + tm.tm_hour, tm.tm_min, tm.tm_sec, + conn->file_fmt == OSMO_PCAP_FMT_PCAP ? "pcap" : "pcapng"); free(real_base_path); if (!conn->curr_filename) { LOGP(DSERVER, LOGL_ERROR, "Failed to assemble filename for %s.\n", conn->name); @@ -285,8 +286,8 @@ return; } - rc = write(conn->local_fd, &conn->file_hdr, sizeof(conn->file_hdr)); - if (rc != sizeof(conn->file_hdr)) { + rc = write(conn->local_fd, conn->file_hdr, conn->file_hdr_len); + if (rc != conn->file_hdr_len) { LOGP(DSERVER, LOGL_ERROR, "Failed to write the header: %d\n", errno); close(conn->local_fd); conn->local_fd = -1; @@ -296,27 +297,116 @@ update_last_write(conn, now); } -/* returns >0 on success, <= 0 on failure (closes conn) */ -static int rx_link_hdr(struct osmo_pcap_conn *conn, struct osmo_pcap_data *data) +static int validate_link_hdr_pcap(const struct osmo_pcap_conn *conn, const struct osmo_pcap_data *data) { struct pcap_file_header *hdr; hdr = (struct pcap_file_header *) &data->data[0]; - + if (data->len < sizeof(struct pcap_file_header)) { + LOGP(DSERVER, LOGL_ERROR, + "Implausible llink_hdr length: %u < %zu\n", + data->len, sizeof(struct pcap_file_header)); + return -1; + } if (hdr->snaplen > conn->server->max_snaplen) { LOGP(DSERVER, LOGL_ERROR, - "The recvd pcap_file_header contains too big snaplen %zu > %zu\n", - (size_t) hdr->snaplen, (size_t) conn->server->max_snaplen); + "The recvd pcap_file_header contains too big snaplen %zu > %zu\n", + (size_t) hdr->snaplen, (size_t) conn->server->max_snaplen); + return -1; + } + return 0; +} + +static int validate_link_hdr_pcapng(const struct osmo_pcap_conn *conn, const struct osmo_pcap_data *data) +{ + + const struct pcapng_block_header *bh = (const struct pcapng_block_header *)&data->data[0]; + const size_t pcapnb_bh_min_len = sizeof(struct pcapng_block_header) + sizeof(uint32_t); + uint32_t block_total_length, block_type; + + if (data->len < pcapnb_bh_min_len) { + LOGP(DSERVER, LOGL_ERROR, "Implausible data length: %u < %zu\n", + data->len, pcapnb_bh_min_len); return -1; } + block_total_length = osmo_pcapng_file_read_uint32((uint8_t *)&bh->block_total_length, conn->pcapng_endian_swapped); + if (block_total_length & 0x00000003) { + LOGP(DSERVER, LOGL_ERROR, "Implausible pcapng block total length not multiple of 4: %u\n", + block_total_length); + return -1; + } + + /* We have a SHB block + N IDB blocks here, so it most probably won't be equal + * since we are only checking the 1st SHB block here: */ + if (block_total_length > data->len) { + LOGP(DSERVER, LOGL_ERROR, "Implausible pcapng block total length: %u > %u\n", + block_total_length, data->len); + return -1; + } + + block_type = osmo_pcapng_file_read_uint32((uint8_t *)&bh->block_type, conn->pcapng_endian_swapped); + if (block_type != BLOCK_TYPE_SHB) { + LOGP(DSERVER, LOGL_DEBUG, "Implausible pcapng Block Header type %u vs exp %u\n", + block_type, BLOCK_TYPE_SHB); + return -1; + } + + /* TODO: validate each idb->snaplen in data->data to be <= conn->server->max_snaplen */ + + return 0; +} + +static int validate_link_hdr(const struct osmo_pcap_conn *conn, const struct osmo_pcap_data *data) +{ + + /* Validation checks: */ + switch (conn->file_fmt) { + case OSMO_PCAP_FMT_PCAP: + return validate_link_hdr_pcap(conn, data); + case OSMO_PCAP_FMT_PCAPNG: + return validate_link_hdr_pcapng(conn, data); + default: + OSMO_ASSERT(0); + } +} + +/* returns >0 on success, <= 0 on failure (closes conn) */ +static int rx_link_hdr(struct osmo_pcap_conn *conn, struct osmo_pcap_data *data) +{ + int rc; + + rc = osmo_pcap_file_discover_fmt(data->data, data->len, &conn->file_fmt); + if (rc < 0) + return rc; + + if (conn->file_fmt == OSMO_PCAP_FMT_PCAPNG) { + rc = osmo_pcapng_file_is_swapped(data->data, data->len); + if (rc < 0) { + LOGP(DSERVER, LOGL_ERROR, "Unable to figure out pcapng file endianness\n"); + return rc; + } + conn->pcapng_endian_swapped = !!rc; + } + + /* Validation checks: */ + if ((rc = validate_link_hdr(conn, data)) < 0) + return rc; + if (conn->store && conn->local_fd < 0) { /* First received link hdr in conn */ - conn->file_hdr = *hdr; + talloc_free(conn->file_hdr); + conn->file_hdr = talloc_size(conn, data->len); + memcpy(conn->file_hdr, data->data, data->len); + conn->file_hdr_len = data->len; restart_pcap(conn); - } else if (memcmp(&conn->file_hdr, hdr, sizeof(*hdr)) != 0) { + } else if (conn->file_hdr_len != data->len || + memcmp(&conn->file_hdr, data->data, data->len) != 0) { /* Client changed the link hdr in conn */ - conn->file_hdr = *hdr; + talloc_free(conn->file_hdr); + conn->file_hdr = talloc_size(conn, data->len); + memcpy(conn->file_hdr, data->data, data->len); + conn->file_hdr_len = data->len; restart_pcap(conn); } @@ -460,12 +550,95 @@ return true; } +static int validate_link_data_pcap(const struct osmo_pcap_conn *conn, const struct osmo_pcap_data *data) +{ + unsigned int min_len, max_len; + + min_len = sizeof(struct osmo_pcap_pkthdr); + max_len = conn->server->max_snaplen + sizeof(struct osmo_pcap_pkthdr); + if (data->len < min_len || data->len > max_len) { + LOGP(DSERVER, LOGL_ERROR, "Implausible data length: %u < %u <= %u\n", + min_len, data->len, max_len); + return -1; + } + return 0; +} + +/* pcapng: validate size of payload (pkt) in EPB doesn't go through snaplen. */ +static int validate_link_data_pcapng(const struct osmo_pcap_conn *conn, const struct osmo_pcap_data *data) +{ + + const struct pcapng_block_header *bh = (const struct pcapng_block_header *)&data->data[0]; + const struct pcapng_enhanced_packet_block *epb; + const size_t pcapnb_bh_min_len = sizeof(struct pcapng_block_header) + sizeof(uint32_t); + uint32_t block_total_length, block_type, captured_len; + + if (data->len < pcapnb_bh_min_len) { + LOGP(DSERVER, LOGL_ERROR, "Implausible data length: %u < %zu\n", + data->len, pcapnb_bh_min_len); + return -1; + } + + block_total_length = osmo_pcapng_file_read_uint32((uint8_t *)&bh->block_total_length, conn->pcapng_endian_swapped); + if (block_total_length & 0x00000003) { + LOGP(DSERVER, LOGL_ERROR, "Implausible pcapng block total length not multiple of 4: %u\n", + block_total_length); + return -1; + } + + if (block_total_length != data->len) { + LOGP(DSERVER, LOGL_ERROR, "Implausible pcapng block total length: %u != %u\n", + block_total_length, data->len); + return -1; + } + + block_type = osmo_pcapng_file_read_uint32((uint8_t *)&bh->block_type, conn->pcapng_endian_swapped); + switch (block_type) { + case BLOCK_TYPE_EPB: + if (data->len < pcapnb_bh_min_len + sizeof(struct pcapng_enhanced_packet_block)) { + LOGP(DSERVER, LOGL_ERROR, "Implausible data length: %u < %zu\n", + data->len, pcapnb_bh_min_len + sizeof(struct pcapng_enhanced_packet_block)); + return -1; + } + epb = (struct pcapng_enhanced_packet_block *)&bh->block_body[0]; + captured_len = osmo_pcapng_file_read_uint32((uint8_t *)&epb->captured_len, conn->pcapng_endian_swapped); + if (captured_len > conn->server->max_snaplen) { + LOGP(DSERVER, LOGL_ERROR, "Implausible pcapng EPB captured length: %u > %u\n", + captured_len, conn->server->max_snaplen); + return -1; + } + break; + default: + LOGP(DSERVER, LOGL_DEBUG, "Unexpected pcapng Block Header type %u\n", block_type); + /* Other types exist which we don't specifically support right now, but which may + * be sent by newer osmo-pcap-client. Allow storing it without further checks. */ + break; + } + return 0; +} + +static int validate_link_data(const struct osmo_pcap_conn *conn, const struct osmo_pcap_data *data) +{ + /* Validation checks: */ + switch (conn->file_fmt) { + case OSMO_PCAP_FMT_PCAP: + return validate_link_data_pcap(conn, data); + case OSMO_PCAP_FMT_PCAPNG: + return validate_link_data_pcapng(conn, data); + default: + OSMO_ASSERT(0); + } +} + /* returns >0 on success, <= 0 on failure (closes conn) */ static int rx_link_data(struct osmo_pcap_conn *conn, struct osmo_pcap_data *data) { time_t now = time(NULL); int rc; + if ((rc = validate_link_data(conn, data)) < 0) + return rc; + zmq_send_client_data(conn, data); if (!conn->store) { @@ -499,6 +672,42 @@ talloc_free(conn); } +static inline size_t calc_data_max_len(const struct osmo_pcap_server *server) +{ + size_t data_max_len; + + /* Some safe value regarding variable size options in a given pcapng block... */ + const size_t pcapng_max_len_opt = 4096; + const size_t pcapng_max_iface_len = 256; + + /* Maximum of the 2 types of .pcap blocks: */ + data_max_len = OSMO_MAX(sizeof(struct pcap_file_header), + sizeof(struct osmo_pcap_pkthdr) + server->max_snaplen); + + /* pcapng SHB: */ + const size_t pcapng_shb_max_len = sizeof(struct pcapng_block_header) + + sizeof(struct pcapng_section_header_block) + + pcapng_max_len_opt + + sizeof(uint32_t); + /* pcapng IDB: */ + const size_t pcapng_idb_max_len = sizeof(struct pcapng_block_header) + + sizeof(struct pcapng_iface_descr_block) + + pcapng_max_len_opt + + sizeof(uint32_t); + /* hdr_link for pcapng (SHB + N*IDB) */ + const size_t pcapg_hdr_link_max_len = pcapng_shb_max_len + // SHB + (pcapng_max_iface_len * pcapng_idb_max_len); // N * IDB + data_max_len = OSMO_MAX(data_max_len, pcapg_hdr_link_max_len); + + /* pcapng EPB: */ + const size_t pcapng_epb_max_len = sizeof(struct pcapng_block_header) + + sizeof(struct pcapng_enhanced_packet_block) + + pcapng_max_len_opt + + sizeof(uint32_t); + data_max_len = OSMO_MAX(data_max_len, pcapng_epb_max_len + server->max_snaplen); + return data_max_len; +} + static struct osmo_pcap_conn *osmo_pcap_conn_alloc(struct osmo_pcap_server *server, const char *name) { @@ -512,8 +721,7 @@ return NULL; } - conn->data_max_len = OSMO_MAX(sizeof(struct pcap_file_header), - sizeof(struct osmo_pcap_pkthdr) + server->max_snaplen); + conn->data_max_len = calc_data_max_len(server); conn->data = talloc_zero_size(conn, sizeof(struct osmo_pcap_data) + conn->data_max_len); /* a bit nasty. we do not work with ids but names */ desc = talloc_zero(conn, struct rate_ctr_group_desc); @@ -586,36 +794,6 @@ 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 = conn->server->max_snaplen + 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; -} - /* Read segment header, struct osmo_pcap_data (without payload) * returns >0 on success, <= 0 on failure (closes conn) */ static int read_cb_initial(struct osmo_pcap_conn *conn) @@ -640,8 +818,11 @@ conn->data->len = ntohs(conn->data->len); - if (!pcap_data_valid(conn)) + if (conn->data->len > conn->data_max_len) { + LOGP(DSERVER, LOGL_ERROR, "Implausible data length: %u > %zu (snaplen %u)\n", + conn->data->len, conn->data_max_len, conn->server->max_snaplen); return -1; + } conn->state = STATE_DATA; conn->pend = conn->data->len; @@ -766,7 +947,8 @@ { close_connection(client); - memset(&client->file_hdr, 0, sizeof(client->file_hdr)); + TALLOC_FREE(client->file_hdr); + client->file_hdr_len = 0; client->rem_wq.bfd.fd = new_fd; if (osmo_fd_register(&client->rem_wq.bfd) != 0) { LOGP(DSERVER, LOGL_ERROR, "Failed to register fd.\n"); diff --git a/tests/rotate_localtime/Makefile.am b/tests/rotate_localtime/Makefile.am index fca9ff8..265e4fe 100644 --- a/tests/rotate_localtime/Makefile.am +++ b/tests/rotate_localtime/Makefile.am @@ -28,6 +28,7 @@ $(NULL) rotate_localtime_test_LDADD = \ + $(top_builddir)/src/osmo_pcap_file.o \ $(top_builddir)/src/osmo_server_network.o \ $(top_builddir)/src/osmo_server_stats.o \ $(top_builddir)/src/osmo_tls.o \ -- To view, visit https://gerrit.osmocom.org/c/osmo-pcap/+/39287?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: osmo-pcap Gerrit-Branch: master Gerrit-Change-Id: I275d28ab418a1514fa9c5c7c20f3d831cc6af8bb Gerrit-Change-Number: 39287 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 0

[S] Change in osmo-bsc[master]: nokia_site: change RSL bootstrap logic
by pespin 10 Jan '25

10 Jan '25

Attention is currently required from: csaba.sipos. pespin has posted comments on this change by csaba.sipos. ( https://gerrit.osmocom.org/c/osmo-bsc/+/39277?usp=email ) Change subject: nokia_site: change RSL bootstrap logic ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/39277?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I7319a64f069d239a582547bc422058ff5848d2e2 Gerrit-Change-Number: 39277 Gerrit-PatchSet: 2 Gerrit-Owner: csaba.sipos <metro4(a)freemail.hu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: csaba.sipos <metro4(a)freemail.hu> Gerrit-Comment-Date: Fri, 10 Jan 2025 14:40:27 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

2026

2025

2024

2023

2022

gerrit-log