bpfilter: add in "Quick Start" guide within the "Usage" page #177

ryanbsull · 2024-12-15T23:30:53Z

No description provided.

codecov · 2024-12-15T23:53:22Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 26.93%. Comparing base (dfeea3d) to head (b55f4b3).
Report is 93 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #177      +/-   ##
==========================================
- Coverage   28.54%   26.93%   -1.61%     
==========================================
  Files          54       57       +3     
  Lines        4618     4900     +282     
==========================================
+ Hits         1318     1320       +2     
- Misses       3300     3580     +280

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Ryan B Sullivan <[email protected]>

qdeslandes · 2024-12-17T12:38:03Z

doc/usage/index.rst

+.. code-block:: bash
+
+	> cd path/to/bpfilter/
+	> sudo ./build/output/bpfilter -t -v debug --no-iptables


Use long options here, it's easier to understand for people learning about bpfilter.

qdeslandes · 2024-12-17T12:39:04Z

doc/usage/index.rst

+
+.. code-block:: bash
+
+	> cd path/to/bpfilter/


path/to/bpfilter/ -> path/to/bpfilter/repo

qdeslandes · 2024-12-17T12:39:49Z

doc/usage/index.rst

+.. note::
+	If you run into errors here there may be problems with your system worth diagnosing before continuing
+
+Now let's try changing the filter from ``{192.168.1.1} ACCEPT`` to ``DENY``


DENY -> DROP

qdeslandes · 2024-12-17T12:40:53Z

doc/usage/index.rst

+	--- 192.168.1.1 ping statistics ---
+	4 packets transmitted, 0 packets received, 100.0% packet loss
+
+Congratulations you have now officially used ``bpfilter`` to systematically filter out your own packets. For documentation for more complex filtering options please check under the ``DEVELOPERS`` links and good luck!


"DEVELOPERS link`" -> "DEVELOPERS section"

qdeslandes · 2024-12-17T12:41:15Z

doc/usage/index.rst

+
+	sudo ./build/output/bfcli --str "chain BF_HOOK_NF_LOCAL_IN policy ACCEPT rule ip4.saddr in {192.168.1.1} DROP"
+
+You should now observe a change in the behavior of ``ping``


"behavior of ping" -> "behavior of ping."

doc/usage/index.rst

qdeslandes · 2024-12-17T12:42:51Z

doc/usage/index.rst

+	> cd path/to/bpfilter/
+	> sudo ./build/output/bfcli --str "chain BF_HOOK_NF_LOCAL_IN policy ACCEPT rule ip4.saddr in {192.168.1.1} ACCEPT"
+
+The above program is just to make sure that ``bfcli`` is able to communicate with ``bpfilter``, when you run the commands you should see output from ``bpfilter`` registering that a filter has been loaded.


Detail what the command is doing: "The above command will...", also for a quick example, I would avoid using a set ({...}) and use a single address instead (ip4.saddr eq 192.168.1.1).

Signed-off-by: Ryan Sullivan <[email protected]>

ryanbsull · 2024-12-18T17:39:15Z

@qdeslandes let me know if the additions are acceptable or if more detail is needed

qdeslandes · 2024-12-18T19:55:14Z

doc/usage/index.rst

+
+.. note::
+
+	This is only to be used as an example and a more interactive test to familiarize you with ``bpfilter``, more in-depth information can be found throughout the docs.


"throughout the docs" -> "throughout the documentation"

qdeslandes · 2024-12-18T19:56:33Z

doc/usage/index.rst

+
+.. code-block:: bash
+
+	> cd path/to/bpfilter/repo/


This path should be the same for all the examples. It might be simpler to remove the cd command from the code blocks and mention something along the lines of "we assume the following commands are run from the bpfilter source directory".

qdeslandes · 2024-12-18T20:09:32Z

doc/usage/index.rst

+	> cd path/to/bpfilter/
+	> sudo ./build/output/bfcli --str "chain BF_HOOK_NF_LOCAL_IN policy ACCEPT rule ip4.saddr eq 192.168.1.1 ACCEPT"
+
+The above command is just to make sure that ``bfcli`` is able to communicate with ``bpfilter``, but it's still worth working through it. This command is telling ``bpfilter`` to check incoming traffic from the ``BF_HOOK_NF_LOCAL_IN`` hook location and accept those connections. Then the command asks ``bpfilter`` to check to see if the incoming IP address is equal to ``192.168.1.1`` (yourself) and to accept the connection if that is true. When you run the commands you should see output from ``bpfilter`` registering that a filter has been loaded.


This part is confusing and inaccurate:

"bpfilter to check incoming traffic": it sends a request to bpfilter to create a new filter. bpfilter itself doesn't process the traffic

"accept those connections": "connections" when it comes to packet filtering have a very specific meaning (e.g. TCP connection), bpfilter's programs works at the packet level

"asks bpfilter to check": bpfilter won't check anything but generate a program that does

"192.168.1.1 (yourself)": that is very specific to one's network.

I would prefer something simpler and more straightforward such as:

The command above will send a request to the bpfilter daemon to create a chain and attach it to the BF_HOOK_NF_LOCAL_IN hook, located after the packet has been routed in the kernel network stack. A single rule is defined, which will accept the packet if its source IPv4 address is 192.168.1.1. If the rule doesn't match the packet, the chain's policy is applied and the packet is accepted. In its current shape, the chain will always accept incoming packets.

qdeslandes · 2024-12-18T20:14:10Z

doc/usage/index.rst

+	> ping 192.168.1.1
+	... [attempting to ping] ...
+	--- 192.168.1.1 ping statistics ---
+	4 packets transmitted, 0 packets received, 100.0% packet loss


We define a rule to block a specific IP and the ping fails, but does this do what you expect it to do? The command above creates a filter for BF_HOOK_LOCAL_IN, i.e. an input hook, meaning packets coming to the local host with the source IP 192.168.1.1 are accepted. It won't block packets going out of the local host, but the answer coming back from 192.168.1.1.

qdeslandes · 2024-12-18T20:14:44Z

doc/usage/index.rst

+	--- 192.168.1.1 ping statistics ---
+	4 packets transmitted, 0 packets received, 100.0% packet loss
+
+Congratulations you have now officially used ``bpfilter`` to systematically filter out your own packets. For documentation for more complex filtering options please check under the ``DEVELOPERS`` section and good luck!


systematically filter out your own packets

We only filter packets coming from 192.168.1.1.

Signed-off-by: Ryan B Sullivan <[email protected]>

qdeslandes

Almost there, one last nit. Also, please fix the command regarding the usage of the long options in the examples.

qdeslandes · 2024-12-19T13:07:36Z

doc/usage/index.rst

@@ -56,7 +59,7 @@ While the ``bpfilter`` daemon runs, now we will open up a separate window to use
 	> cd path/to/bpfilter/
 	> sudo ./build/output/bfcli --str "chain BF_HOOK_NF_LOCAL_IN policy ACCEPT rule ip4.saddr eq 192.168.1.1 ACCEPT"

-The above command is just to make sure that ``bfcli`` is able to communicate with ``bpfilter``, but it's still worth working through it. This command is telling ``bpfilter`` to check incoming traffic from the ``BF_HOOK_NF_LOCAL_IN`` hook location and accept those connections. Then the command asks ``bpfilter`` to check to see if the incoming IP address is equal to ``192.168.1.1`` (yourself) and to accept the connection if that is true. When you run the commands you should see output from ``bpfilter`` registering that a filter has been loaded.
+The command above will send a request to the bpfilter daemon to create a chain and attach it to the BF_HOOK_NF_LOCAL_IN hook, located after the packet has been routed in the kernel network stack. A single rule is defined, which will accept the packet if its source IPv4 address is 192.168.1.1. If the rule doesn't match the packet, the chain's policy is applied and the packet is accepted. In its current shape, the chain will always accept incoming packets.


Use double backticks around BF_HOOK_NF_LOCAL_IN and the IP address.

…in explanatory section Signed-off-by: Ryan B Sullivan <[email protected]>

ryanbsull · 2024-12-30T15:39:46Z

Hey, @qdeslandes just wanted to ping regarding this

doc/usage/index.rst

Signed-off-by: Ryan B Sullivan <[email protected]>

qdeslandes · 2025-01-01T13:37:29Z

Hi @ryanbsull ,

Non-code changes are usually not the most straightforward for some reason, thanks for pushing through, I appreciate your contribution. This change is now merged!

ryanbsull · 2025-01-01T16:53:51Z

Thanks!

ryanbsull requested a review from qdeslandes as a code owner December 15, 2024 23:30

facebook-github-bot added the cla signed label Dec 15, 2024

ryanbsull force-pushed the quick_start_docs branch 2 times, most recently from 6de6520 to b55f4b3 Compare December 15, 2024 23:43

bpfilter: add in "Quick Start" guide within the "Usage" page

5dc54fb

Signed-off-by: Ryan B Sullivan <[email protected]>

ryanbsull force-pushed the quick_start_docs branch from b55f4b3 to 5dc54fb Compare December 15, 2024 23:56

qdeslandes requested changes Dec 17, 2024

View reviewed changes

bpfilter: added improvements to "Quick Start" guide under "Usage"

f6617a2

Signed-off-by: Ryan Sullivan <[email protected]>

qdeslandes requested changes Dec 18, 2024

View reviewed changes

bpfilter: further improvements to documentation for clarity

12819a6

Signed-off-by: Ryan B Sullivan <[email protected]>

qdeslandes reviewed Dec 19, 2024

View reviewed changes

qdeslandes self-requested a review December 19, 2024 13:09

bpfilter: used more verbose options in examples and fixed formatting …

659a3b7

…in explanatory section Signed-off-by: Ryan B Sullivan <[email protected]>

qdeslandes requested changes Dec 31, 2024

View reviewed changes

doc/usage/index.rst Show resolved Hide resolved

doc/usage/index.rst Outdated Show resolved Hide resolved

bpfilter: remove extra newline and redundant code in example

ed32e4e

Signed-off-by: Ryan B Sullivan <[email protected]>

qdeslandes approved these changes Jan 1, 2025

View reviewed changes

qdeslandes merged commit c8f2d66 into facebook:main Jan 1, 2025
11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bpfilter: add in "Quick Start" guide within the "Usage" page #177

bpfilter: add in "Quick Start" guide within the "Usage" page #177

ryanbsull commented Dec 15, 2024

codecov bot commented Dec 15, 2024 •

edited

Loading

qdeslandes Dec 17, 2024

qdeslandes Dec 17, 2024

qdeslandes Dec 17, 2024

qdeslandes Dec 17, 2024

qdeslandes Dec 17, 2024

qdeslandes Dec 17, 2024

ryanbsull commented Dec 18, 2024

qdeslandes Dec 18, 2024

qdeslandes Dec 18, 2024

qdeslandes Dec 18, 2024

qdeslandes Dec 18, 2024

qdeslandes Dec 18, 2024

qdeslandes left a comment

qdeslandes Dec 19, 2024

ryanbsull commented Dec 30, 2024

qdeslandes commented Jan 1, 2025

ryanbsull commented Jan 1, 2025


		sudo ./build/output/bfcli --str "chain BF_HOOK_NF_LOCAL_IN policy ACCEPT rule ip4.saddr in {192.168.1.1} DROP"

		You should now observe a change in the behavior of ``ping``


		.. note::

		This is only to be used as an example and a more interactive test to familiarize you with ``bpfilter``, more in-depth information can be found throughout the docs.

bpfilter: add in "Quick Start" guide within the "Usage" page #177

bpfilter: add in "Quick Start" guide within the "Usage" page #177

Conversation

ryanbsull commented Dec 15, 2024

codecov bot commented Dec 15, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryanbsull commented Dec 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qdeslandes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryanbsull commented Dec 30, 2024

qdeslandes commented Jan 1, 2025

ryanbsull commented Jan 1, 2025

codecov bot commented Dec 15, 2024 •

edited

Loading