Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(enforcer(bpf)): Block bind(), listen(), recvmsg(), and sendmsg(). #1254

Closed
wants to merge 1 commit into from
Closed

feat(enforcer(bpf)): Block bind(), listen(), recvmsg(), and sendmsg(). #1254

wants to merge 1 commit into from

Conversation

q2ven
Copy link

@q2ven q2ven commented Jun 2, 2023

Currently, the network policy only blocks accept() and connect() syscalls by BPF-LSM. However, it is not sufficient; applications can sneak through the restriction.

For TCP, we can establish a connection without connect() by passing MSG_FASTOPEN to sendto() or sendmsg(). Then, the application can communicate with the server with send() and recv().

Also, we don't block listen(), so the established connection is put into the accept queue even though we cannot pop it out. We should block listen() as well not to consume unneeded memory.

  # python3
  >>> from socket import *
  >>>
  >>> s = socket()
  >>> s.listen()
  >>>
  >>> c = socket()
  >>> MSG_FASTOPEN = 0x20000000
  >>> c.sendto(b'hello', MSG_FASTOPEN, s.getsockname())
  5
  >>> c
  <socket.socket fd=4, family=AddressFamily.AF_INET,
  type=SocketKind.SOCK_STREAM, proto=0,
  laddr=('127.0.0.1', 43396), raddr=('127.0.0.1', 51375)>

For UDP, we can send data by sendmsg() and receive data by bind() and recvmsg().

  >>> s = socket(AF_INET, SOCK_DGRAM, 0)
  >>> s.bind(('0', 0))
  >>>
  >>> c = socket(AF_INET, SOCK_DGRAM, 0)
  >>> c.sendto(b'hello', s.getsockname())
  5
  >>> s.recv(1024)
  b'hello'

Let's block bind(), listen(), recvmsg() and sendmsg().

Purpose of PR?:

Fixes #

I haven't created an issue.

Does this PR introduce a breaking change?

Yes. Users were able to establish a connection even with the network policy blocking TCP, but it should be blocked.

If the changes in this PR are manually verified, list down the scenarios covered::

  • TCP Fast Open without connect()
  • UDP send/recv

Additional information for reviewer? :
Mention if this PR is part of any design or a continuation of previous PRs

Checklist:

  • Bug fix. Fixes #
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • PR Title follows the convention of <type>(<scope>): <subject>
  • Commit has unit tests
  • Commit has integration tests

Currently, the network policy only blocks accept() and connect()
syscalls by BPF-LSM.  However, it is not sufficient; applications
can sneak through the restriction.

For TCP, we can establish a connection without connect() by passing
MSG_FASTOPEN to sendto() or sendmsg().  Then, the application can
communicate with the server with send() and recv().

Also, we don't block listen(), so the established connection is put
into the accept queue even though we cannot pop it out.  We should
block listen() as well not to consume unneeded memory.

  # python3
  >>> from socket import *
  >>>
  >>> s = socket()
  >>> s.listen()
  >>>
  >>> c = socket()
  >>> MSG_FASTOPEN = 0x20000000
  >>> c.sendto(b'hello', MSG_FASTOPEN, s.getsockname())
  5
  >>> c
  <socket.socket fd=4, family=AddressFamily.AF_INET,
  type=SocketKind.SOCK_STREAM, proto=0,
  laddr=('127.0.0.1', 43396), raddr=('127.0.0.1', 51375)>

For UDP, we can send data by sendmsg() and receive data by bind()
and recvmsg().

  >>> s = socket(AF_INET, SOCK_DGRAM, 0)
  >>> s.bind(('0', 0))
  >>>
  >>> c = socket(AF_INET, SOCK_DGRAM, 0)
  >>> c.sendto(b'hello', s.getsockname())
  5
  >>> s.recv(1024)
  b'hello'

Let's block bind(), listen(), recvmsg() and sendmsg().

Signed-off-by: Kuniyuki Iwashima <[email protected]>
@DelusionalOptimist
Copy link
Member

@daemon1024 Please take a look.

@daemon1024
Copy link
Member

daemon1024 commented Jun 5, 2023

Thanks @q2ven for the PR, really appreciate it. I understand there's a problem here and was myself also looking into it. I don't really think we should capture send/recv hooks because these are high frequency hooks and would have major performance impact imo.

What if we block the socket() itself? if the creation of socket is denied then it's not possible to initiate any other request. WDYT?

@q2ven
Copy link
Author

q2ven commented Jun 5, 2023

Thanks for taking a look!
I understand your point and blocking socket() will work too, but we'll lose some flexibility.

I was thinking of implementing address filtering feature so that application can still send/recv data with specific CIDR.
For TCP, we can do so by hooking connect() and inet_csk_clone().
However, we need to hook recv/send for UDP.

But this might be out of KubeArmor's scope ?

@daemon1024
Copy link
Member

Thanks for taking a look!
I understand your point and blocking socket() will work too, but we'll lose some flexibility.

Right. Would you like to update the PR for the same? Appreciate the quick feedback.

I was thinking of implementing address filtering feature so that application can still send/recv data with specific CIDR.
For TCP, we can do so by hooking connect() and inet_csk_clone().
However, we need to hook recv/send for UDP.
But this might be out of KubeArmor's scope ?

I understand this and we have pondered upon it but yes it's not currently on our Roadmap yet. I would appreciate it if you could open an issue and we discuss it there. I would really like to understand how important it is and what all we could do and what are your usecases.

@q2ven
Copy link
Author

q2ven commented Jun 5, 2023

Sure, will update the commit and PR and open an issue later.

@daemon1024
Copy link
Member

@q2ven Can you take a look at #1283, I have added the changes to enforce on socket creation as well and check if it covers the gap you mentioned. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants