FIRE-4508 - Enhancements

nser77 commented 1 month ago

Hello, this PR fixes the FIRE-4508 control and adds the following features:

Deep parsing of iptables output with no dependencies.
If the sort binary is present, the output includes only unique records; otherwise, it includes the raw results.
Control if NFQUEUE target is present only in the security and filter tables.

iptables:

-P INPUT ACCEPT
-P FORWARD ACCEPT
-P OUTPUT ACCEPT
-A INPUT -j NFQUEUE --queue-num 0 --queue-bypass
-A INPUT -j NFQUEUE --queue-num 1 --queue-bypass

Results:

[+] Software: firewalls
------------------------------------
  - Checking iptables kernel module                           [ FOUND ]
    - Checking iptables policies of chains                    [ FOUND ]
      - Checking chain INPUT (table: filter, target: ACCEPT)  [ ACCEPT ]
      - Checking chain INPUT (table: filter, target: NFQUEUE)  [ NFQUEUE ]
      - Checking chain INPUT (table: security, target: ACCEPT)  [ ACCEPT ]
    - Checking for empty ruleset                              [ WARNING ]
    - Checking for unused rules                               [ FOUND ]
  - Checking host based firewall                              [ ACTIVE ]

  Suggestions:
  ----------------------------

  * Consider avoid NFQUEUE target if possible (iptables chain INPUT, table: filter) [FIRE-4508]
      https://cisofy.com/lynis/controls/FIRE-4508/

  * Check iptables rules to see which rules are currently not used [FIRE-4513]
      https://cisofy.com/lynis/controls/FIRE-4513/

Please provide feedbacks!

Regards,

teoberi commented 1 month ago

It doesn't work for my firewall!

/lynis/include/tests_firewalls: line 177: shift: 4: shift count out of range

nser77 commented 1 month ago

Thanks!!

Maybe fixed with commit f3ffbb0

teoberi commented 1 month ago

/lynis/include/tests_firewalls: line 178: shift: 4: shift count out of range

Test the changes with a real, production firewall.

wileyhy commented 1 month ago

Something on style:

From CONTRIBUTING.md:

"Variables should be capitalized, with underscore as word separator (e.g. PROCESS_EXISTS=1)."

On Fri, Oct 11, 2024, 06:26 nser77 @.***> wrote:

Thanks!!

Maybe fixed with commit f3ffbb0 https://github.com/CISOfy/lynis/pull/1560/commits/f3ffbb0b486661d4da840d3c791c8bc9e6c073e0

— Reply to this email directly, view it on GitHub https://github.com/CISOfy/lynis/pull/1560#issuecomment-2407412066, or unsubscribe https://github.com/notifications/unsubscribe-auth/AUF2F243Y72UZP3OIRVLXBDZ27GWVAVCNFSM6AAAAABPYREKNKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMBXGQYTEMBWGY . You are receiving this because you are subscribed to this thread.Message ID: @.***>

mboelen commented 1 month ago

Thanks for the great work on this! I got a few suggestions:

I support the remark made by @wileyhy about using the right variable name structure. Also, try to give it a clear name, preferably with a temporary variable name, something like t could become IPTABLES_TABLE. Then at the end of the table, unset the temporary variables. So please rename the variables and then use an 'unset IPTABLES_VAR1 IPTABLES_VAR` at the end of the test.
Use long-format options when possible, as this makes it easier for people to read what a command does. For example --list-rules is better than -S, as it clearly states that it is showing rules.
Did you test with another shell or using shellcheck? We want to avoid 'bashism'.

After those changes, I feel things can be merged and tested by the community.

teoberi commented 1 month ago

ShellCheck finds some problems for FIRE-4508 and offers suggestions for solving them.

nser77 commented 1 month ago

Yees!

I'm working on it.

nser77 commented 1 month ago

Hello, thanks for your feedbacks.

I believe I addressed all suggestions with patch https://github.com/nser77/lynis/tree/nser77-patch-12

Please let me know!

teoberi commented 1 month ago

/lynis/include/tests_firewalls: line 183: shift: 4: shift count out of range

nser77 commented 1 month ago

Hello @teoberi, let's see if commit 6410c5a solves your issue.

Otherwise, I'm not able to reproduce/understand your error: you can try to share here the output of [+] Software: firewalls section or I can share with you my Discord account and we can talk there.

Commit b6bbbf4 introduces the LogText output so you can share that.

Keep me posted plz.

Thanks,

teoberi commented 1 month ago

Now it works but I think you should eliminate duplicates.

[+] Software: firewalls

Checking iptables kernel module [ FOUND ]
- Checking iptables policies of chains [ FOUND ]
  - Checking chain INPUT (table: filter, target: ACCEPT) [ ACCEPT ]
  - Checking chain INPUT (table: filter, target: DROP) [ DROP ]
  - Checking chain INPUT (table: filter, target: DROP) [ DROP ]
  - Checking chain INPUT (table: filter, target: ACCEPT) [ ACCEPT ]
  - Checking chain INPUT (table: filter, target: ACCEPT) [ ACCEPT ]
  - Checking chain INPUT (table: filter, target: DROP) [ DROP ]
  - Checking chain INPUT (table: filter, target: DROP) [ DROP ]
  - Checking chain INPUT (table: filter, target: DROP) [ DROP ] ...

mboelen commented 1 month ago

Thanks for all your work. To avoid getting an increasingly big merge, I have merged it now. This way we can move on and add new changes in smaller pull requests. I will comb through the code to make a bit more in line with the rest of the code and submit it to the main tree.

mboelen commented 1 month ago

Additional thought: I noticed you introduced a suggestion (to the screen), while the comment on top of the test says they are currently disabled. My suggestion for this suggestion: make it as actionable as possible.

nser77 commented 1 month ago

Ok thank you for your patience!

nser77 commented 1 month ago

Now it works but I think you should eliminate duplicates.

[+] Software: firewalls

Checking iptables kernel module [ FOUND ]

Checking iptables policies of chains [ FOUND ]

Checking chain INPUT (table: filter, target: ACCEPT) [ ACCEPT ]

Checking chain INPUT (table: filter, target: DROP) [ DROP ]

Checking chain INPUT (table: filter, target: DROP) [ DROP ]

Checking chain INPUT (table: filter, target: ACCEPT) [ ACCEPT ]

Checking chain INPUT (table: filter, target: ACCEPT) [ ACCEPT ]

Checking chain INPUT (table: filter, target: DROP) [ DROP ]

Checking chain INPUT (table: filter, target: DROP) [ DROP ]

Checking chain INPUT (table: filter, target: DROP) [ DROP ] ...

Thanks @teoberi !

In resume:

We want to sort and return unique output values and we need to use \n (new line) as separator for our queue.
We need to parse our output queue with \n as separator.
Each line must have 4 values.

While testing the changes that have been merged, I noticed that FIRE-4508 doesn't work with some distributions due to some comptability errors.

After some researches, the problem seems connected to the shell environment where lynis runs.

In my OpenSuse and RockyLinux test environements:

NAME="openSUSE Leap"
VERSION="15.6"
ID="opensuse-leap"
ID_LIKE="suse opensuse"
VERSION_ID="15.6"
PRETTY_NAME="openSUSE Leap 15.6"
ANSI_COLOR="0;32"
CPE_NAME="cpe:/o:opensuse:leap:15.6"
BUG_REPORT_URL="https://bugs.opensuse.org"
HOME_URL="https://www.opensuse.org/"
DOCUMENTATION_URL="https://en.opensuse.org/Portal:Leap"
LOGO="distributor-logo-Leap"

NAME="Rocky Linux"
VERSION="9.4 (Blue Onyx)"
ID="rocky"
ID_LIKE="rhel centos fedora"
VERSION_ID="9.4"
PLATFORM_ID="platform:el9"
PRETTY_NAME="Rocky Linux 9.4 (Blue Onyx)"
ANSI_COLOR="0;32"
LOGO="fedora-logo-icon"
CPE_NAME="cpe:/o:rocky:rocky:9::baseos"
HOME_URL="https://rockylinux.org/"
BUG_REPORT_URL="https://bugs.rockylinux.org/"
SUPPORT_END="2032-05-31"
ROCKY_SUPPORT_PRODUCT="Rocky-Linux-9"
ROCKY_SUPPORT_PRODUCT_VERSION="9.4"
REDHAT_SUPPORT_PRODUCT="Rocky Linux"
REDHAT_SUPPORT_PRODUCT_VERSION="9.4"

shell is a link to bash:

sw1:~ # ls -la $(which sh)
lrwxrwxrwx 1 root root 4 Aug 22 08:05 /usr/bin/sh -> bash
sw1:~/lynis # sh -c "echo 'hello\nhello'"
hello\nhello

but in Debian/Ubuntu it points to dash:

root@rr01:/opt/lynis# ls -la $(which sh)
lrwxrwxrwx 1 root root 4 Mar 31  2024 /usr/bin/sh -> dash
root@router01:/opt/lynis# sh -c "echo 'hello\nhello'"
hello
hello

I'm working on it and trying to figure out what's the best solution.

Thanks again!

nser77 commented 1 month ago

Hello @teoberi , patch https://github.com/nser77/lynis/tree/nser77-patch-1 should solve those portability issues.

Kindly requesting your review while I'm trying to address the other community suggestions.

https://pubs.opengroup.org/onlinepubs/9699919799/utilities/echo.html

Thanks!

teoberi commented 1 month ago

Now output it looks OK!

[+] Software: firewalls
------------------------------------
  - Checking iptables kernel module                           [ FOUND ]
    - Checking iptables policies of chains                    [ FOUND ]
      - Chain INPUT (table: filter, target: ACCEPT)           [ ACCEPT ]
      - Chain INPUT (table: filter, target: DROP)             [ DROP ]
      - Chain INPUT (table: security, target: ACCEPT)         [ ACCEPT ]
    - Checking for empty ruleset                              [ OK ]
    - Checking for unused rules                               [ FOUND ]
  - Checking host based firewall                              [ ACTIVE ]

In lynis.log I don't know, you have to check if that's what you want.

024-10-16 10:03:11 ==== 2024-10-16 10:03:11 Performing test ID FIRE-4508 (Check used policies of iptables chains) 2024-10-16 10:03:11 Hardening: assigned partial number of hardening points (1 of 3). Currently having 121 points (out of 177) 2024-10-16 10:03:11 Hardening: assigned maximum number of hardening points for this item (3). Currently having 124 points (out of 180) 2024-10-16 10:03:11 Hardening: assigned maximum number of hardening points for this item (3). Currently having 127 points (out of 183) .... 2024-10-16 10:03:11 Hardening: assigned maximum number of hardening points for this item (3). Currently having 363 points (out of 423) 2024-10-16 10:03:11 Hardening: assigned maximum number of hardening points for this item (3). Currently having 366 points (out of 426) 2024-10-16 10:03:11 Hardening: assigned maximum number of hardening points for this item (3). Currently having 369 points (out of 429) 2024-10-16 10:03:11 Hardening: assigned partial number of hardening points (1 of 3). Currently having 370 points (out of 432) 2024-10-16 10:03:11 Hardening: assigned partial number of hardening points (1 of 3). Currently having 371 points (out of 435) 2024-10-16 10:03:11 Hardening: assigned partial number of hardening points (1 of 3). Currently having 372 points (out of 438) 2024-10-16 10:03:11 Info: sorting output 2024-10-16 10:03:11 Result: Found target 'ACCEPT' for chain 'INPUT' (table: filter) 2024-10-16 10:03:11 Result: Found target 'DROP' for chain 'INPUT' (table: filter) 2024-10-16 10:03:11 Info: sorting output 2024-10-16 10:03:12 Info: sorting output 2024-10-16 10:03:12 Info: sorting output 2024-10-16 10:03:12 Hardening: assigned partial number of hardening points (1 of 3). Currently having 121 points (out of 177) 2024-10-16 10:03:12 Info: sorting output 2024-10-16 10:03:12 Result: Found target 'ACCEPT' for chain 'INPUT' (table: security) 2024-10-16 10:03:12 ====

nser77 commented 1 month ago

Hi, I opened this PR: #1561 wher I solved the community issues; kindly requesting your inputs.

On the other hand, I believe there is still much work to be done here:

I will address the NFQUEUE and ReportSuggestion community suggestions in a separated PR.
We should take care about the iptables-legacy, which is not NFT based.
We might have other stacks to analyze, like ip6tables, arptables and ebtables.
We may want to take care about the br_netfilter.ko module and it's usage with iptables.

What do you think?

Thanks!

teoberi commented 1 month ago

We can continue the discussion in the new PR.

nser77 commented 1 month ago

Additional thought: I noticed you introduced a suggestion (to the screen), while the comment on top of the test says they are currently disabled. My suggestion for this suggestion: make it as actionable as possible.

Hello, I don't know how to make it actionable but now I understand what the code's comment means; https://cisofy.com/lynis/controls/ has not a FIRE-4508 page; if you can guide me on how-to I can do that.

Otherwise, I can make the following changes and use Report function instead of ReportSuggestion:

                                if [ "${3}" = "NFQUEUE" ]
                                then
                                    # NFQUEUE is an ip(6)tables target that passes the packet to userspace 
                                    #  using the nfnetlink_queue handler.
                                    #  A userspace application can inspect and modify the packet if desired
                                    #  then must drop or reinject it into the kernel; more informations
                                    #  can be found here: man 8 iptables-extensions
                                    #
                                    # libnetfilter_queue is a userspace library providing an API to packets 
                                    #  that have been queued by the kernel packet filter; more informations
                                    #  can be found here: https://netfilter.org/projects/libnetfilter_queue/doxygen/html/
                                    #
                                    # In other words, NFQUEUE target can be used to perform MiTM attacks from the ip(6)tables stack;
                                    #  a real example can be found in the Scapy community:
                                    #  https://github.com/secdev/scapy/blob/master/doc/notebooks/Scapy%20in%2015%20minutes.ipynb
                                    #ReportSuggestion "${TEST_NO}" "Consider avoid ${3} target if possible (iptables chain ${2}, table: ${1})"
                                    Report "${TEST_NO}" "Consider avoid ${3} target if possible (iptables chain ${2}, table: ${1})"
                                fi

CISOfy / lynis

FIRE-4508 - Enhancements #1560

[+] Software: firewalls

[+] Software: firewalls