netfilter buglog - Mar 2012 - [Bug 780] New: nfnl_handle_packet() doesn't respect NLMSG_DONE

http://bugzilla.netfilter.org/show_bug.cgi?id=780

           Summary: nfnl_handle_packet() doesn't respect NLMSG_DONE
           Product: libnfnetlink
           Version: unspecified
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: major
          Priority: P5
         Component: libnfnetlink
        AssignedTo: netfilter-buglog at lists.netfilter.org
        ReportedBy: dafranke at akamai.com
   Estimated Hours: 0.0


Created attachment 382
  --> http://bugzilla.netfilter.org/attachment.cgi?id=382
Proposed patch

Datagrams sent across a netlink socket from kernel to userspace can contain
trailing garbage of arbitrary length. Userspace is expected to cope with this
by checking for messages of type NLMSG_DONE and doing no further parsing of the
datagram after encountering one. nfnl_handle_packet() doesn't currently do
this
check. As a result, if the length of the trailing garbage is greater than or
equal to NLMSG_SPACE(0) (i.e. NLMSG_ALIGNTO), it tries to parse the garbage as
though it were an additional message. Theoretically, this results in undefined
behavior, since that garbage might be interpretable as a valid message. In
practice, some sanity check fails and nfnl_handle_packet() returns -1.

Attached is a patch which rewrites nfnl_handle_packet() to parse datagrams in
the idiom recommended by the netlink(7) manpage, including correctly handling
NLMSG_DONE.

-- 
Configure bugmail: http://bugzilla.netfilter.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
You are watching all bug changes.

http://bugzilla.netfilter.org/show_bug.cgi?id=780

--- Comment #1 from Daniel Franke <dafranke at akamai.com> 2012-03-28
23:29:18 CEST ---
Created attachment 383
  --> http://bugzilla.netfilter.org/attachment.cgi?id=383
Gentler patch

-- 
Configure bugmail: http://bugzilla.netfilter.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
You are watching all bug changes.

http://bugzilla.netfilter.org/show_bug.cgi?id=780

--- Comment #2 from Daniel Franke <dafranke at akamai.com> 2012-03-28
23:30:45 CEST ---
The first patch I submitted, while "correct", is a bit harsh, since
successful
handling of control messages like NLMSG_DONE requires that the consumer of
libnfnetlink install a subsystem handler for NFNL_SUBSYS_NONE and provide
callbacks for each message type, and AFAIK no downstream code currently does
this. I think it's better to handle control messages right within
nfnl_handle_packet() and never pass them along. I've posted a second patch
which does this.

-- 
Configure bugmail: http://bugzilla.netfilter.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
You are watching all bug changes.

http://bugzilla.netfilter.org/show_bug.cgi?id=780

Daniel Franke <dafranke at akamai.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #382 is|0                           |1
           obsolete|                            |

-- 
Configure bugmail: http://bugzilla.netfilter.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
You are watching all bug changes.

http://bugzilla.netfilter.org/show_bug.cgi?id=780

Jan Engelhardt <jengelh at medozas.de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jengelh at medozas.de
         AssignedTo|netfilter-buglog at lists.netf |pablo at netfilter.org
                   |ilter.org                   |

-- 
Configure bugmail: http://bugzilla.netfilter.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
You are watching all bug changes.
You are watching the assignee of the bug.

http://bugzilla.netfilter.org/show_bug.cgi?id=780

--- Comment #3 from Pablo Neira Ayuso <pablo at netfilter.org> 2012-06-09
02:17:25 CEST ---
Two reasons not to take this patch:

1) nfnl_handle_packet was not designed to handle multi-part netlink message
that usually comes through one dump request.

2) We're moving towards libmnl [1], use that library for your netlink
things.

-- 
Configure bugmail: http://bugzilla.netfilter.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.
You are watching the assignee of the bug.

http://bugzilla.netfilter.org/show_bug.cgi?id=780

Pablo Neira Ayuso <pablo at netfilter.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #4 from Pablo Neira Ayuso <pablo at netfilter.org> 2012-11-17
14:16:12 CET ---
Closing this bug as no incidence was reported since time ago.

-- 
Configure bugmail: http://bugzilla.netfilter.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.
You are watching the assignee of the bug.