This service will be undergoing maintenance at 00:00 UTC, 2017-10-23 It is expected to last about 30 minutes
Bug 851148 - libmnl: provide an API to detect Netlink spoofing
libmnl: provide an API to detect Netlink spoofing
Status: CLOSED NOTABUG
Product: Fedora
Classification: Fedora
Component: libmnl (Show other bugs)
19
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: Hushan Jia
Fedora Extras Quality Assurance
: Security
Depends On:
Blocks: 848017 CVE-2012-6689
  Show dependency treegraph
 
Reported: 2012-08-23 07:12 EDT by Florian Weimer
Modified: 2015-02-20 06:49 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: 848017
Environment:
Last Closed: 2015-02-19 09:05:03 EST
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
[PATCH] libmnl: Add filtering support to library as a convienience (6.31 KB, patch)
2013-01-17 15:04 EST, Neil Horman
no flags Details | Diff
libmnl-filter-map.patch (396 bytes, patch)
2013-03-26 05:55 EDT, Florian Weimer
no flags Details | Diff

  None (edit)
Description Florian Weimer 2012-08-23 07:12:13 EDT
+++ This bug was initially created as a clone of Bug #848017 +++

cgdcbxd does not check if the DCB packets have PID/port ID 0 and originated in the kernel.  As a result, unprivileged local users can send messages to cgdcbxd, causing it to alter the cgroup configuration.

In addition, mnl_attr_get_str is used as if the resulting string is NUL-terminated.  This isn't always the case (at least when processing crafted messages).

--- Additional comment from fweimer@redhat.com on 2012-08-14 13:39:03 CEST ---

(In reply to comment #0)
> In addition, mnl_attr_get_str is used as if the resulting string is
> NUL-terminated.  This isn't always the case (at least when processing
> crafted messages).

Based on a discussion with the libmnl folks, mnl_attr_validate(attr, MNL_TYPE_NUL_STRING) should be used instead of mnl_attr_validate(attr, MNL_TYPE_STRING).  This is compatible with what the kernel sends.

--- Additional comment from nhorman@redhat.com on 2012-08-14 17:29:44 CEST ---

I'm not sure thats even necessecary.  Every received message from the kernel is handled in cgdcbxd by mnl_cb_run->__mnl_cb_run.  Internal to that function, we validate the pid and port id in mnl_nlmsg_portid_ok, which validates:

1) that nlmsg_pid is 0
2) That portid is either 0 or matches the expected port id

So it seems, unless I'm missing something that all of your concerns are addressed there

--- Additional comment from fweimer@redhat.com on 2012-08-14 17:39:57 CEST ---

(In reply to comment #2)
> I'm not sure thats even necessecary.  Every received message from the kernel
> is handled in cgdcbxd by mnl_cb_run->__mnl_cb_run.  Internal to that
> function, we validate the pid and port id in mnl_nlmsg_portid_ok, which
> validates:
> 
> 1) that nlmsg_pid is 0
> 2) That portid is either 0 or matches the expected port id
> 
> So it seems, unless I'm missing something that all of your concerns are
> addressed there

Here's an excerpt from a GDB session when processing a packet which was generated by the nobody user:

Breakpoint 2, mnl_nlmsg_portid_ok (nlh=nlh@entry=0x7fffffffcec0, portid=portid@entry=0) at nlmsg.c:238
238		return nlh->nlmsg_pid && portid ? nlh->nlmsg_pid == portid : true;
(gdb) print nlh->nlmsg_pid
$1 = 5971
(gdb) finish
Run till exit from #0  mnl_nlmsg_portid_ok (nlh=nlh@entry=0x7fffffffcec0, portid=portid@entry=0) at nlmsg.c:238
0x00000037f1a01fff in __mnl_cb_run (cb_ctl_array_len=0, cb_ctl_array=0x0, data=0x0, 
    cb_data=0x403182 <cgdcbx_dcb_cb>, portid=0, seq=0, numbytes=<optimized out>, buf=<optimized out>)
    at callback.c:58
58			if (!mnl_nlmsg_portid_ok(nlh, portid)) {
Value returned is $2 = true
(gdb) 

This is actually the documented behavior, "We skip the tracking for netlink message whose portID is zero since it is reserved for event-based kernel notifications.".  I think this function is not suitable for checking that the packet originated in the kernel, and you have to do this check on your own, prior to calling mnl_cb_run.

Perhaps we should ask upstream if this is a bug in libmnl, just to be sure.

--- Additional comment from nhorman@redhat.com on 2012-08-14 18:28:44 CEST ---

agreed, it seems that a non-zero pid and a valid port id should result in a failed check, as that would still allow kernel notifications to go through (as they would have a 0 pid), but catch users.

CCing hushan jia, the libmnl maintainer.  What are your thoughts on the issue?

--- Additional comment from fweimer@redhat.com on 2012-08-15 09:44:31 CEST ---

(In reply to comment #4)
> agreed, it seems that a non-zero pid and a valid port id should result in a
> failed check, as that would still allow kernel notifications to go through
> (as they would have a 0 pid), but catch users.
> 
> CCing hushan jia, the libmnl maintainer.  What are your thoughts on the
> issue?

Please note that this issue is still under embargo.

I've asked Pablo Neira Ayuso (libmnl upstream) for his opinion as well.

--- Additional comment from fweimer@redhat.com on 2012-08-15 14:59:52 CEST ---

(In reply to comment #5)

> I've asked Pablo Neira Ayuso (libmnl upstream) for his opinion as well.

He agrees that this is a bug in libmnl.  We're working on a fix.

--- Additional comment from nhorman@redhat.com on 2012-08-15 15:16:31 CEST ---

copy that, reassigning the component on this bz to libmnl.  Thanks!

--- Additional comment from hushan.jia@gmail.com on 2012-08-16 03:57:06 CEST ---

(In reply to comment #6)
> (In reply to comment #5)
> 
> > I've asked Pablo Neira Ayuso (libmnl upstream) for his opinion as well.
> 
> He agrees that this is a bug in libmnl.  We're working on a fix.
Can you copy me on the email thread? thanks.

--- Additional comment from fweimer@redhat.com on 2012-08-23 11:37:29 CEST ---

(In reply to comment #8)
> Can you copy me on the email thread? thanks.

The discussion resulted in this patch proposal for the kernel:

http://marc.info/?l=linux-netdev&m=134522422125983

This will fix the issue for cgdcbxd only very indirectly, if it is ever accepted (it might break existing userspace software).

I think the current libmnl API does not really provide a way to check message origin.  The source address of a packet is discarded in mnl_socket_recvfrom, and that would be the most natural place to check.  Looking at the port ID in the nlmsghdr header (which is what mnl_nlmsg_portid_ok does) is not actually helpful.

--- Additional comment from fweimer@redhat.com on 2012-08-23 13:10:01 CEST ---

Changing the component back to cgdcbxd because a code change in cgdcbxd will be needed (no transparent fix in libmnl is planned).
Comment 1 Neil Horman 2013-01-17 15:04:18 EST
Created attachment 680486 [details]
[PATCH] libmnl: Add filtering support to library as a convienience


Theres been recent discussion about detecting and discarding unwanted netlink
messages in libmnl, so that we can avoid having applications get spoofed by user
space processes sending messages with malformed netlink headers.  Commonly
applications want to be able to only receive messages from the kernel, but
libmnl currently doesn't offer a mechanism to do that.  This patch adds such a
mechanism.  It creates a function mnl_socket_recvfrom_filter, that adds an
extra function pointer parameter which is used to interrogate recieved frames
and filter them based on a desired criteria.  It also adds a convieninece
function mnl_recvfrom_filter_user which can be passed as the filter agrument in
mnl_socket_recvfrom_filter, so as to prevent individual applications from
re-inventing the wheel over and over again.

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
---
 include/libmnl/libmnl.h |  4 ++
 src/socket.c            | 99 +++++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 92 insertions(+), 11 deletions(-)
Comment 2 Neil Horman 2013-01-17 15:06:41 EST
Note the above is untested, but it builds fine, and looks pretty straightforward.  Since the kernel bits, according to the mailing list appear to have languished, I thought perhaps would be a good alternate solution.  It gives us a common method to do filtering in libmnl without making each application re-invent the same type of filtering). Please let me know what you think.
Comment 3 Florian Weimer 2013-03-26 05:53:46 EDT
I was finally able to build this.  I needed an additional patch which makes the new symbols visible.  We should really try to get this patch upstream, so that we can get an official version for the symbol.

I'm not sure if the retry loop in mnl_socket_recvfrom_filter is actually a good idea.  I think this can cause cgdcbxd to get stuck waiting on one socket if a spoofed message is received which is not quickly followed by a non-spoofed message.  I think mnl_socket_recvfrom_filter should return with an error code if a message is received which is then filtered.  Or we should just expose the sender socket address to the caller and do the filtering there.
Comment 4 Florian Weimer 2013-03-26 05:55:29 EDT
Created attachment 716400 [details]
libmnl-filter-map.patch

Additional patch for exporting the new symbols.
Comment 5 Neil Horman 2013-03-26 07:13:10 EDT
Florian, regarding your suggestion about the loop, My feeling was that the detection could be done in the filter function, since we have the socket address available there.  If thats unclear, we could document it as the point at which to break out of the loop.

If you like I can propose this upstream.  This needs to go to the netfilter-devel list, yes?
Comment 6 Florian Weimer 2013-03-26 07:19:28 EDT
(In reply to comment #5)
> Florian, regarding your suggestion about the loop, My feeling was that the
> detection could be done in the filter function, since we have the socket
> address available there.  If thats unclear, we could document it as the
> point at which to break out of the loop.

I'm worried that the caller performs select()/poll(), calls the receive function to obtain one packet (which should succeed without blocking because select()/poll() indicated that data was available).  But after receiving the that packet, recvmsg() is called again if it was filtered, and that call can now block unexpectedly.

> If you like I can propose this upstream.  This needs to go to the
> netfilter-devel list, yes?

Yes, that would be the right list.
Comment 7 Neil Horman 2013-03-26 09:45:07 EDT
hmm, ok, I see your point.  Ok, given the options, I rather like the error code return a bit better.  Either that, or a second method pointer to a receive function, where the caller can pass some code to do exactly the receive operation that they would like.  I'll recode, and post it shortly.
Comment 8 Jaroslav Reznik 2013-04-05 08:23:39 EDT
This bug appears to have been reported against 'rawhide' during the Fedora 19 development cycle.
Changing version to '19'.

(As we did not run this process for some time, it could affect also pre-Fedora 19 development
cycle bugs. We are very sorry. It will help us with cleanup during Fedora 19 End Of Life. Thank you.)

More information and reason for this action is here:
https://fedoraproject.org/wiki/BugZappers/HouseKeeping/Fedora19
Comment 9 Jaroslav Reznik 2015-02-19 09:05:03 EST
Fedora 19 changed to end-of-life (EOL) status on 2015-01-06. Fedora 19 is
no longer maintained, which means that it will not receive any further
security or bug fix updates. As a result we are closing this bug.

If you can reproduce this bug against a currently maintained version of
Fedora please feel free to reopen this bug against that version. If you
are unable to reopen this bug, please file a new report against the
current release. If you experience problems, please add a comment to this
bug.

Thank you for reporting this bug and we are sorry it could not be fixed.
Comment 10 Florian Weimer 2015-02-20 06:48:15 EST
This was fixed in the kernel.

Note You need to log in before you can comment on or make changes to this bug.