lua-lsm: add skb payload accessors and hand out only full socks - #31
Open
chenzongyao200127 wants to merge 6 commits into
Open
lua-lsm: add skb payload accessors and hand out only full socks#31chenzongyao200127 wants to merge 6 commits into
chenzongyao200127 wants to merge 6 commits into
Conversation
A netlink or packet policy has to look at message contents to make a decision, but the skb object only exposed metadata, so such policies could not be written at all. Expose the payload, and the raw protocol number that suites() cannot name, so content-aware policies become possible. Signed-off-by: Zongyao Chen <ZongYao.Chen@linux.alibaba.com>
So policy authors can find the accessors and know a bad read yields nil rather than an error, keeping the decision in the policy's hands. Signed-off-by: Zongyao Chen <ZongYao.Chen@linux.alibaba.com>
On handshake and time-wait paths skb->sk is a request or time-wait sock, which stops short of the fields sock:proto() and sock:suites() read. A policy asking for the protocol number therefore got neighbouring slab bytes, and since those pass for a valid number the rule they feed fails open rather than erroring. Resolve skb->sk through skb_to_full_sk() so a sock handed to a policy is always a full sock, making the guarantee a property of the object rather than of each accessor. skb:full_sk() then duplicates skb:sock() and has no users, so drop it. Signed-off-by: Zongyao Chen <ZongYao.Chen@linux.alibaba.com>
A policy author cannot tell from the method name that a half-open or time-wait connection resolves to the listener or to nil, and would read the nil as "no owning socket" instead of "not a full sock". Signed-off-by: Zongyao Chen <ZongYao.Chen@linux.alibaba.com>
The helper is local to lua_net.c, but skb_ is netcore's prefix and the function sits two lines from skb_copy_bits() and skb_to_full_sk(), so nothing tells a reader it is not a core networking helper. Every other static function in the file carries the module prefix. Signed-off-by: Zongyao Chen <ZongYao.Chen@linux.alibaba.com>
skb:len() was documented as "payload length", but it measures the window starting at skb->data, and each layer advances that pointer past its own header, so at most hooks the window still covers a header. A policy author trusting the word "payload" computes every skb:read() offset from the wrong base, and an out-of-range read yields nil rather than an error, so the mistake never surfaces. Define the window once for both accessors and record where it starts in each hook that passes an skb, since that is the only thing an offset can be computed from. Signed-off-by: Zongyao Chen <ZongYao.Chen@linux.alibaba.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Netlink and packet hooks receive an
sk_buff, but the skb object only exposedmetadata (
sock,protocol,iif,secmark). A policy that has to decide onmessage contents — which netlink command is being issued, for instance — could
not be written at all.
This series adds the payload accessors, fixes a fail-open in
skb:sock(), anddocuments the one thing a content-aware policy cannot guess: where offset 0 is.
skb:len()/skb:read(off, len)—read()returns the bytes as a string,or
nilwhen the range falls outside the window, so the policy keeps theverdict instead of taking a Lua error that would fall back to the hook default
and allow the operation. Reads go through
skb_copy_bits(), so a non-linearskb is stitched together transparently, and one read is bounded by a fixed
256-byte on-stack buffer.
sock:proto()— the rawsk_protocol.suites()renders that number throughthe
IPPROTO_*namespace, which cannot name protocols of other families suchas netlink's
NETLINK_ROUTE.skb:sock()now resolvesskb->skthroughskb_to_full_sk(). On handshakeand time-wait paths
skb->skis a request or time-wait sock, which stops shortof the fields
sock:proto()andsock:suites()read, so a policy asking forthe protocol number got neighbouring slab bytes — and since those pass for a
valid number, the rule they feed failed open. A half-open connection now
resolves to its listener and a time-wait sock yields
nil.skb:full_sk()became a duplicate of
skb:sock()and is dropped.docs/API.mddefines the data window (skb->lenbytes fromskb->data) oncefor both accessors and records where it starts in each hook that passes an skb.
Each layer advances
skb->datapast its own header, so the base is a propertyof the hook, not of the packet — and getting it wrong is quiet: an out-of-range
read yields
nil, an in-range one yields whatever field happens to sit there.Multi-byte fields are decoded in Lua, since the in-kernel Lua has no bit
library; the examples below use
string.byteand modulo arithmetic.Example policies
Deny route deletions over rtnetlink. Under
netlink_sendthe window spanseverything one
sendmsg()wrote, so a batch arrives as consecutivelength-delimited messages and the policy has to walk them.
Log inbound TCP SYNs to a port. Under
socket_sock_rcv_skboffset 0 is the TCPheader on the IPv4/IPv6 TCP path — but the same hook fires for netlink, SCTP,
unix and raw sockets, where the window starts somewhere else, so the family
check is not optional.
skb:sock()can returnnil— no owning socket, or a time-wait sock that isnot a full sock — so a policy that reads through it needs the nil check;
indexing
nilraises, and a Lua error inside a hook falls back to that hook'sdefault, which is "allow" for most hooks. Wrap parsing in
pcall()where thepolicy must fail closed.
Test plan
ip route deldenied,SYNs to port 22 audited).
skb:read()boundaries:off == skb:len()withlen == 0,len == 256,len > 256, negativeoff/len, and a read spanning a non-linear skb.skb:sock()on a SYN (inet_conn_request) returns the listener, and on atime-wait skb returns
nilrather than a mini-sock.docs/API.mdre-checked against the hook call sites for eachrow of the table.