From 117f033c413820739e6679c926a39a5b3f45ff79 Mon Sep 17 00:00:00 2001 From: Pablo Neira Ayuso Date: Sun, 4 Apr 2010 02:32:35 +0200 Subject: check source of the netlink message and fix sequence tracking logic This patch changes the callback handlers to include netlink portID checking. Thus, we avoid that any malicious process can spoof messages. If portid, sequence number of the message is != 0, we check if the message is what we expect. This allows to use the same netlink channel for dumps (portid, seq != 0) and event-based notifications (portid, seq == 0). Signed-off-by: Pablo Neira Ayuso --- examples/genl-family-get.c | 5 +++-- examples/rtnl-link-dump.c | 5 +++-- examples/rtnl-link-dump2.c | 5 +++-- examples/rtnl-link-dump3.c | 5 +++-- examples/rtnl-link-event.c | 2 +- examples/rtnl-link-set.c | 5 +++-- examples/rtnl-route-dump.c | 5 +++-- include/libmnl/libmnl.h | 9 ++++++--- src/callback.c | 17 ++++++++++++----- src/msg.c | 22 ++++++++++++++++++---- 10 files changed, 55 insertions(+), 25 deletions(-) diff --git a/examples/genl-family-get.c b/examples/genl-family-get.c index 00f601c..fbe1bf1 100644 --- a/examples/genl-family-get.c +++ b/examples/genl-family-get.c @@ -187,7 +187,7 @@ int main(int argc, char *argv[]) struct nlmsghdr *nlh; struct genlmsghdr *genl; int ret; - unsigned int seq; + unsigned int seq, portid; if (argc != 2) { printf("%s [family name]\n", argv[0]); @@ -216,6 +216,7 @@ int main(int argc, char *argv[]) perror("mnl_socket_bind"); exit(EXIT_FAILURE); } + portid = mnl_socket_get_portid(nl); if (mnl_socket_sendto(nl, nlh, mnl_nlmsg_get_len(nlh)) < 0) { perror("mnl_socket_send"); @@ -224,7 +225,7 @@ int main(int argc, char *argv[]) ret = mnl_socket_recvfrom(nl, buf, sizeof(buf)); while (ret > 0) { - ret = mnl_cb_run(buf, ret, seq, data_cb, NULL); + ret = mnl_cb_run(buf, ret, seq, portid, data_cb, NULL); if (ret <= 0) break; ret = mnl_socket_recvfrom(nl, buf, sizeof(buf)); diff --git a/examples/rtnl-link-dump.c b/examples/rtnl-link-dump.c index 9e3f114..42843aa 100644 --- a/examples/rtnl-link-dump.c +++ b/examples/rtnl-link-dump.c @@ -69,7 +69,7 @@ int main() struct nlmsghdr *nlh; struct rtgenmsg *rt; int ret; - unsigned int seq; + unsigned int seq, portid; nlh = mnl_nlmsg_put_header(buf); nlh->nlmsg_type = RTM_GETLINK; @@ -88,6 +88,7 @@ int main() perror("mnl_socket_bind"); exit(EXIT_FAILURE); } + portid = mnl_socket_get_portid(nl); if (mnl_socket_sendto(nl, nlh, mnl_nlmsg_get_len(nlh)) < 0) { perror("mnl_socket_send"); @@ -96,7 +97,7 @@ int main() ret = mnl_socket_recvfrom(nl, buf, sizeof(buf)); while (ret > 0) { - ret = mnl_cb_run(buf, ret, seq, data_cb, NULL); + ret = mnl_cb_run(buf, ret, seq, portid, data_cb, NULL); if (ret <= MNL_CB_STOP) break; ret = mnl_socket_recvfrom(nl, buf, sizeof(buf)); diff --git a/examples/rtnl-link-dump2.c b/examples/rtnl-link-dump2.c index dc44c54..3c62006 100644 --- a/examples/rtnl-link-dump2.c +++ b/examples/rtnl-link-dump2.c @@ -60,7 +60,7 @@ int main() struct nlmsghdr *nlh; struct rtgenmsg *rt; int ret; - unsigned int seq; + unsigned int seq, portid; nlh = mnl_nlmsg_put_header(buf); nlh->nlmsg_type = RTM_GETLINK; @@ -79,6 +79,7 @@ int main() perror("mnl_socket_bind"); exit(EXIT_FAILURE); } + portid = mnl_socket_get_portid(nl); if (mnl_socket_sendto(nl, nlh, mnl_nlmsg_get_len(nlh)) < 0) { perror("mnl_socket_send"); @@ -87,7 +88,7 @@ int main() ret = mnl_socket_recvfrom(nl, buf, sizeof(buf)); while (ret > 0) { - ret = mnl_cb_run(buf, ret, seq, data_cb, NULL); + ret = mnl_cb_run(buf, ret, seq, portid, data_cb, NULL); if (ret <= MNL_CB_STOP) break; ret = mnl_socket_recvfrom(nl, buf, sizeof(buf)); diff --git a/examples/rtnl-link-dump3.c b/examples/rtnl-link-dump3.c index d5e4458..ce59f9f 100644 --- a/examples/rtnl-link-dump3.c +++ b/examples/rtnl-link-dump3.c @@ -58,7 +58,7 @@ int main() struct nlmsghdr *nlh; struct rtgenmsg *rt; int ret; - unsigned int seq; + unsigned int seq, portid; nlh = mnl_nlmsg_put_header(buf); nlh->nlmsg_type = RTM_GETLINK; @@ -77,6 +77,7 @@ int main() perror("mnl_socket_bind"); exit(EXIT_FAILURE); } + portid = mnl_socket_get_portid(nl); if (mnl_socket_sendto(nl, nlh, mnl_nlmsg_get_len(nlh)) < 0) { perror("mnl_socket_send"); @@ -85,7 +86,7 @@ int main() ret = mnl_socket_recvfrom(nl, buf, sizeof(buf)); while (ret > 0) { - ret = mnl_cb_run(buf, ret, seq, data_cb, NULL); + ret = mnl_cb_run(buf, ret, seq, portid, data_cb, NULL); if (ret <= MNL_CB_STOP) break; ret = mnl_socket_recvfrom(nl, buf, sizeof(buf)); diff --git a/examples/rtnl-link-event.c b/examples/rtnl-link-event.c index 3e25b6f..84daf01 100644 --- a/examples/rtnl-link-event.c +++ b/examples/rtnl-link-event.c @@ -82,7 +82,7 @@ int main() ret = mnl_socket_recvfrom(nl, buf, sizeof(buf)); while (ret > 0) { - ret = mnl_cb_run(buf, ret, 0, data_cb, NULL); + ret = mnl_cb_run(buf, ret, 0, 0, data_cb, NULL); if (ret <= 0) break; ret = mnl_socket_recvfrom(nl, buf, sizeof(buf)); diff --git a/examples/rtnl-link-set.c b/examples/rtnl-link-set.c index 36bf355..d7327dd 100644 --- a/examples/rtnl-link-set.c +++ b/examples/rtnl-link-set.c @@ -14,7 +14,7 @@ int main(int argc, char *argv[]) struct nlmsghdr *nlh; struct ifinfomsg *ifm; int ret; - unsigned int seq, oper; + unsigned int seq, portid, oper; if (argc != 3) { printf("Usage: %s [ifname] [up|down]\n", argv[0]); @@ -50,6 +50,7 @@ int main(int argc, char *argv[]) perror("mnl_socket_bind"); exit(EXIT_FAILURE); } + portid = mnl_socket_get_portid(nl); mnl_nlmsg_print(nlh); @@ -64,7 +65,7 @@ int main(int argc, char *argv[]) exit(EXIT_FAILURE); } - ret = mnl_cb_run(buf, ret, seq, NULL, NULL); + ret = mnl_cb_run(buf, ret, seq, portid, NULL, NULL); if (ret == -1){ perror("callback"); exit(EXIT_FAILURE); diff --git a/examples/rtnl-route-dump.c b/examples/rtnl-route-dump.c index eb36bbc..c3fd577 100644 --- a/examples/rtnl-route-dump.c +++ b/examples/rtnl-route-dump.c @@ -198,7 +198,7 @@ int main() struct nlmsghdr *nlh; struct rtmsg *rtm; int ret; - unsigned int seq; + unsigned int seq, portid; nlh = mnl_nlmsg_put_header(buf); nlh->nlmsg_type = RTM_GETROUTE; @@ -217,6 +217,7 @@ int main() perror("mnl_socket_bind"); exit(EXIT_FAILURE); } + portid = mnl_socket_get_portid(nl); if (mnl_socket_sendto(nl, nlh, mnl_nlmsg_get_len(nlh)) < 0) { perror("mnl_socket_send"); @@ -225,7 +226,7 @@ int main() ret = mnl_socket_recvfrom(nl, buf, sizeof(buf)); while (ret > 0) { - ret = mnl_cb_run(buf, ret, 0, data_cb, NULL); + ret = mnl_cb_run(buf, ret, seq, portid, data_cb, NULL); if (ret <= MNL_CB_STOP) break; ret = mnl_socket_recvfrom(nl, buf, sizeof(buf)); diff --git a/include/libmnl/libmnl.h b/include/libmnl/libmnl.h index 6a2b8a6..8502eeb 100644 --- a/include/libmnl/libmnl.h +++ b/include/libmnl/libmnl.h @@ -49,6 +49,9 @@ extern struct nlmsghdr *mnl_nlmsg_next(const struct nlmsghdr *nlh, int *len); /* Netlink sequence tracking */ extern int mnl_nlmsg_seq_ok(const struct nlmsghdr *nlh, unsigned int seq); +/* Netlink portID checking */ +int mnl_nlmsg_portid_ok(const struct nlmsghdr *nlh, unsigned int portid); + /* Netlink header getters */ extern uint16_t mnl_nlmsg_get_len(const struct nlmsghdr *nlh); extern void *mnl_nlmsg_get_data(const struct nlmsghdr *nlh); @@ -135,10 +138,10 @@ extern int mnl_attr_parse_nested(const struct nlattr *attr, mnl_attr_cb_t cb, vo typedef int (*mnl_cb_t)(const struct nlmsghdr *nlh, void *data); extern int mnl_cb_run(const char *buf, int numbytes, unsigned int seq, - mnl_cb_t cb_data, void *data); + unsigned int portid, mnl_cb_t cb_data, void *data); -extern int mnl_cb_run2(const char *buf, int numbytes, - unsigned int seq, mnl_cb_t cb_data, void *data, +extern int mnl_cb_run2(const char *buf, int numbytes, unsigned int seq, + unsigned int portid, mnl_cb_t cb_data, void *data, mnl_cb_t *cb_ctl_array, unsigned int cb_ctl_array_len); /* diff --git a/src/callback.c b/src/callback.c index 1b18b2a..f6637b3 100644 --- a/src/callback.c +++ b/src/callback.c @@ -48,7 +48,8 @@ static mnl_cb_t default_cb_array[NLMSG_MIN_TYPE] = { * mnl_cb_run2 - callback runqueue for netlink messages * @buf: buffer that contains the netlink messages * @numbytes: number of bytes stored in the buffer - * @seq: sequence number that we expect to receive (use zero to skip) + * @seq: sequence number that we expect to receive + * @portid: Netlink PortID that we expect to receive * @cb_data: callback handler for data messages * @data: pointer to data that will be passed to the data callback handler * @cb_ctl_array: array of custom callback handlers from control messages @@ -66,13 +67,18 @@ static mnl_cb_t default_cb_array[NLMSG_MIN_TYPE] = { * This function propagates the callback return value. */ int mnl_cb_run2(const char *buf, int numbytes, unsigned int seq, - mnl_cb_t cb_data, void *data, + unsigned int portid, mnl_cb_t cb_data, void *data, mnl_cb_t *cb_ctl_array, unsigned int cb_ctl_array_len) { int ret = MNL_CB_OK; struct nlmsghdr *nlh = (struct nlmsghdr *)buf; while (mnl_nlmsg_ok(nlh, numbytes)) { + /* check message source */ + if (!mnl_nlmsg_portid_ok(nlh, portid)) { + errno = EINVAL; + return -1; + } /* perform sequence tracking */ if (!mnl_nlmsg_seq_ok(nlh, seq)) { errno = EILSEQ; @@ -107,7 +113,8 @@ out: * mnl_cb_run - callback runqueue for netlink messages (simplified version) * @buf: buffer that contains the netlink messages * @numbytes: number of bytes stored in the buffer - * @seq: sequence number that we expect to receive (use zero to skip) + * @seq: sequence number that we expect to receive + * @portid: Netlink PortID that we expect to receive * @cb_data: callback handler for data messages * @data: pointer to data that will be passed to the data callback handler * @@ -122,7 +129,7 @@ out: * This function propagates the callback return value. */ int mnl_cb_run(const char *buf, int numbytes, unsigned int seq, - mnl_cb_t cb_data, void *data) + unsigned int portid, mnl_cb_t cb_data, void *data) { - return mnl_cb_run2(buf, numbytes, seq, cb_data, data, NULL, 0); + return mnl_cb_run2(buf, numbytes, seq, portid, cb_data, data, NULL, 0); } diff --git a/src/msg.c b/src/msg.c index 6b603b2..fc6a7c8 100644 --- a/src/msg.c +++ b/src/msg.c @@ -184,13 +184,27 @@ void *mnl_nlmsg_get_tail(const struct nlmsghdr *nlh) * @seq: last sequence number used to send a message * * This functions returns 1 if the sequence tracking is fulfilled, otherwise - * 0 is returned. If seq is 0, then the sequence tracking is skipped. This - * value is generally used by the kernel for asynchronous notifications, - * for that reason, this library consider that it is reserved. + * 0 is returned. We skip the tracking for netlink messages whose sequence + * number is zero since it is usually reserved for event-based kernel + * notifications. */ int mnl_nlmsg_seq_ok(const struct nlmsghdr *nlh, unsigned int seq) { - return seq ? nlh->nlmsg_seq == seq : 1; + return nlh->nlmsg_seq ? nlh->nlmsg_seq == seq : 1; +} + +/** + * mnl_nlmsg_portid_ok - perform portID origin check + * @nlh: current netlink message that we are handling + * @seq: netlink portid that we want to check + * + * This functions return 1 if the origin is fulfilled, otherwise + * 0 is returned. We skip the tracking for netlink message whose portID + * is zero since it is reserved for event-based kernel notifications. + */ +int mnl_nlmsg_portid_ok(const struct nlmsghdr *nlh, unsigned int portid) +{ + return nlh->nlmsg_pid ? nlh->nlmsg_pid == portid : 1; } /* XXX: rework this, please */ -- cgit v1.2.3