New Table: Windows Firewall Rules #7403

aleksmaus · 2021-12-01T15:03:49Z

This change adds the new table windows_firewall_rules as requested in the issue: #7335

There are couple of deviations from the original requirement:

No version column, since it's not available through the API.
Reordered columns slightly.
Using enabled column name instead of active, more consistent with the API naming.
Using INTEGER for enabled column instead of TRUE/FALSE TEXT value.
Renamed rule_action into action field, more consistent with the API naming.
Renamed profile into profiles column that contains decoded comma separated text values like Private,Public. Could create individual rows for each if needed. Let me know.

Screenshot:

I was not sure if we can use ATL CComPtr wrappers for example, it's cleaner this way.
Can rewrite without them if needed. Let me know.

Looking for a feedback if there is anything needs to be changed/added/removed.

Fixes: #7335

linux-foundation-easycla · 2021-12-01T15:03:55Z

The committers are authorized under a signed CLA.

✅ Aleksandr Maus (bb3dee4, e9c3575, 2eadddc, 72a3025, 624e47e)

directionless

Thanks fo much for the implementation! I'm really excited to see this get into osquery

osquery/tables/networking/windows/defender_firewall_rules.cpp

specs/windows/defender_firewall_rules.table

Co-authored-by: seph <seph@kolide.co>

amalone-scwx · 2021-12-01T17:36:27Z

Are these specific to Defender, or does it apply to Windows network firewall rules in-general?

aleksmaus · 2021-12-01T17:50:41Z

Are these specific to Defender, or does it apply to Windows network firewall rules in-general?

This is windows firewall in general, using API defined in:
https://docs.microsoft.com/en-us/windows/win32/api/netfw/

https://en.wikipedia.org/wiki/Windows_Firewall
"Windows Firewall (officially called Windows Defender Firewall in Windows 10), is a firewall component of Microsoft Windows."

amalone-scwx · 2021-12-01T18:39:53Z

Maybe windows_firewall_rules is a better name, so as not to be confused with "Defender ATP"? Also, is there a way to get the ports open in the firewall? Admins will want to know which TCP and UDP ports are open for each rule.

…d addresses and ports columns.

aleksmaus · 2021-12-01T23:00:11Z

Maybe windows_firewall_rules is a better name, so as not to be confused with "Defender ATP"? Also, is there a way to get the ports open in the firewall? Admins will want to know which TCP and UDP ports are open for each rule.

Renamed table defender_firewall_rules to windows_firewall_rules.
Added columns:

local_addresses
remote_addresses
local_ports
remote_ports
icmp_types_codes

amalone-scwx · 2021-12-01T23:40:16Z

LGTM

aleksmaus · 2021-12-08T03:20:24Z

@zwass Thanks! Looks like still blocked on CLA verification. Will check with our team.

zwass · 2021-12-08T04:24:40Z

Thought that might help! I don't seem to have admin access to our EasyCLA account so not sure if there's anything else I can do at the moment.

aleksmaus · 2021-12-08T18:39:37Z

/easycla

alessandrogario

Hello @aleksmaus,

thanks for the PR! I have left some comments

osquery/tables/networking/windows/windows_firewall_rules.cpp

tests/integration/tables/windows_firewall_rules.cpp

aleksmaus · 2021-12-11T14:49:41Z

Hello @aleksmaus,

thanks for the PR! I have left some comments

Thanks for the feedback! Most of the patterns borrowed from the existing code to make it consistent. Will update for suggested changes.

aleksmaus · 2021-12-15T00:41:59Z

@alessandrogario I've made the changes that you requested. Could you take a look if this matches what you meant?
Also let me know if need to get rid of ATL.

directionless · 2021-12-15T02:28:32Z

It looks like the CLA resolved itself. Closing and reopening is the simple way to get it to check.

FWIW a common issue is having commits that aren't tied to your github username in the PR. But for more complex stuff I usually dig up the easyCLA support email address.

osquery/tables/networking/CMakeLists.txt

alessandrogario

This is really good code @aleksmaus! I have left some additional comments, I think we can merge this soon

osquery/tables/networking/windows/windows_firewall_rules.cpp

osquery/tables/networking/windows/windows_firewall_rules.h

osquery/tables/networking/CMakeLists.txt

osquery/tables/networking/windows/windows_firewall_rules.h

tests/integration/tables/windows_firewall_rules.cpp

alessandrogario

We have concluded that ATL should be removed since it does require a valid Visual Studio license, and not everyone contributing to osquery is eligible to use the Community Edition

osquery/tables/networking/windows/windows_firewall_rules.cpp

alessandrogario

This is excellent work, thank you for this cool table @aleksmaus!

aleksmaus · 2021-12-20T18:01:16Z

This is excellent work, thank you for this cool table @aleksmaus!

Thank you for the code reviews @alessandrogario!

directionless · 2021-12-21T14:31:43Z

specs/windows/windows_firewall_rules.table

+description("Provides the list of Windows firewall rules.")
+schema([
+    Column("name", TEXT, "Friendly name of the rule"),
+    Column("app_name", TEXT, "Friendly name of the application to which the rule applies"),


Is there a non-friendly, but authoritative, way to link this rule to an app? (I haven't looked at the underlying API yet)

Digging through https://docs.microsoft.com/en-us/windows/win32/api/netfw/nn-netfw-inetfwrule

I think get_ServiceName is worth adding, since that's documented as an optional parameter that may take the place of app name.

get_grouping might also be interesting

basically exposed this API: (https://docs.microsoft.com/en-us/windows/win32/api/netfw/nn-netfw-inetfwrule)
and got the description from https://docs.microsoft.com/en-us/windows/win32/api/netfw/nf-netfw-inetfwrule-get_applicationname

aleksmaus · 2021-12-21T16:01:25Z

Added grouping and service_name columns per @directionless request.

Smjert · 2021-12-23T18:10:51Z

Closing and reopening so that it aligns with the latest master.

james-elastic · 2022-01-14T20:56:47Z

Gentle bump, Is there anything else needed to get the pr merged?

…2 to master * commit 'c97a1b416492585e049010deea36d2992b680556': (52 commits) Fix user_time and system_time unit in processes table on M1 (osquery#7473) Add BOOST_USE_ASAN define when enabling Asan (osquery#7469) Removing unnecessary macOS version check (osquery#7451) Add `utc` flag back for compatibility(osquery#7460) Add osquery version to macOS app bundle Info.plist (osquery#7452) Fix submodule cache for macOS CI runner (osquery#7456) New Table: Windows Firewall Rules (osquery#7403) Remove utc flag from example config file (osquery#7437) Update the ATC table `path` column check to be case insensitive (osquery#7442) Fix typos in documentation (osquery#7443) Fix a crash when Yara uses its own strutils functions (osquery#7439) Update `time` table to reflect UTC values (osquery#7276) Update sqlite to version 3.37.0 (osquery#7426) Fix linking of thirdparty_sleuthkit (osquery#7425) Apple Silicon support (osquery#7330) Fix how we disable tables in the fuzzer init method (osquery#7419) Prevent running discovery queries when fuzzing (osquery#7418) Hide the deprecate `antispyware` column in `windows_security_center` (osquery#7411) Fix typo in docs (osquery#7412) CHANGELOG 5.1.0 (osquery#7406) ...

New Table: Windows Defender Firewall Rules

bb3dee4

aleksmaus requested review from a team as code owners December 1, 2021 15:03

directionless reviewed Dec 1, 2021

View reviewed changes

osquery/tables/networking/windows/defender_firewall_rules.cpp Outdated Show resolved Hide resolved

osquery/tables/networking/windows/defender_firewall_rules.cpp Outdated Show resolved Hide resolved

specs/windows/defender_firewall_rules.table Outdated Show resolved Hide resolved

Smjert added virtual tables Windows feature labels Dec 1, 2021

aleksmaus and others added 2 commits December 1, 2021 10:50

Update specs/windows/defender_firewall_rules.table

e9c3575

Co-authored-by: seph <seph@kolide.co>

Split 'profiles' column into three boolean columns

2eadddc

Renamed table defender_firewall_rules to windows_firewall_rules. Adde…

72a3025

…d addresses and ports columns.

aleksmaus changed the title ~~New Table: Windows Defender Firewall Rules~~ New Table: Windows Firewall Rules Dec 2, 2021

Add integration test

624e47e

zwass closed this Dec 8, 2021

zwass reopened this Dec 8, 2021

Cleaner QueryInterface call for the rule

0ab9478

alessandrogario requested changes Dec 11, 2021

View reviewed changes

osquery/tables/networking/windows/windows_firewall_rules.cpp Outdated Show resolved Hide resolved

osquery/tables/networking/windows/windows_firewall_rules.cpp Outdated Show resolved Hide resolved

tests/integration/tables/windows_firewall_rules.cpp Show resolved Hide resolved

aleksmaus requested a review from alessandrogario December 16, 2021 20:09

aleksmaus force-pushed the feature/defender_firewall_rules branch from 17d8d3e to 88f2b1d Compare December 17, 2021 00:53

Address code review feedback

3f8f133

aleksmaus force-pushed the feature/defender_firewall_rules branch from 88f2b1d to 3f8f133 Compare December 17, 2021 00:55

Expose the header for tests

f011781

aleksmaus commented Dec 17, 2021

View reviewed changes

osquery/tables/networking/CMakeLists.txt Show resolved Hide resolved

alessandrogario requested changes Dec 17, 2021

View reviewed changes

Addressed code review feedback

9ae981d

aleksmaus force-pushed the feature/defender_firewall_rules branch from 9a59731 to 9ae981d Compare December 18, 2021 02:41

alessandrogario requested changes Dec 18, 2021

View reviewed changes

Remove ATL

f2476a7

alessandrogario requested changes Dec 19, 2021

View reviewed changes

Address code review feedback

fceaaf9

aleksmaus requested a review from alessandrogario December 19, 2021 16:59

alessandrogario previously approved these changes Dec 20, 2021

View reviewed changes

directionless reviewed Dec 21, 2021

View reviewed changes

Add grouping and service_name columns

ff56e13

aleksmaus dismissed alessandrogario’s stale review via ff56e13 December 21, 2021 16:00

aleksmaus requested review from directionless and alessandrogario December 21, 2021 16:00

Add the new grouping and service_name columns unit test validation

ce417ce

alessandrogario approved these changes Dec 22, 2021

View reviewed changes

Smjert closed this Dec 23, 2021

Smjert reopened this Dec 23, 2021

directionless merged commit 9ecb3f0 into osquery:master Jan 18, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Table: Windows Firewall Rules #7403

New Table: Windows Firewall Rules #7403

aleksmaus commented Dec 1, 2021 •

edited

linux-foundation-easycla bot commented Dec 1, 2021 •

edited

directionless left a comment

amalone-scwx commented Dec 1, 2021

aleksmaus commented Dec 1, 2021

amalone-scwx commented Dec 1, 2021

aleksmaus commented Dec 1, 2021

amalone-scwx commented Dec 1, 2021

aleksmaus commented Dec 8, 2021

zwass commented Dec 8, 2021

aleksmaus commented Dec 8, 2021

alessandrogario left a comment

aleksmaus commented Dec 11, 2021

aleksmaus commented Dec 15, 2021

directionless commented Dec 15, 2021

alessandrogario left a comment

alessandrogario left a comment

alessandrogario left a comment

aleksmaus commented Dec 20, 2021

directionless Dec 21, 2021

directionless Dec 21, 2021

aleksmaus Dec 21, 2021

aleksmaus Dec 21, 2021

aleksmaus commented Dec 21, 2021

Smjert commented Dec 23, 2021

james-elastic commented Jan 14, 2022

New Table: Windows Firewall Rules #7403

New Table: Windows Firewall Rules #7403

Conversation

aleksmaus commented Dec 1, 2021 • edited

linux-foundation-easycla bot commented Dec 1, 2021 • edited

directionless left a comment

Choose a reason for hiding this comment

amalone-scwx commented Dec 1, 2021

aleksmaus commented Dec 1, 2021

amalone-scwx commented Dec 1, 2021

aleksmaus commented Dec 1, 2021

amalone-scwx commented Dec 1, 2021

aleksmaus commented Dec 8, 2021

zwass commented Dec 8, 2021

aleksmaus commented Dec 8, 2021

alessandrogario left a comment

Choose a reason for hiding this comment

aleksmaus commented Dec 11, 2021

aleksmaus commented Dec 15, 2021

directionless commented Dec 15, 2021

alessandrogario left a comment

Choose a reason for hiding this comment

alessandrogario left a comment

Choose a reason for hiding this comment

alessandrogario left a comment

Choose a reason for hiding this comment

aleksmaus commented Dec 20, 2021

directionless Dec 21, 2021

Choose a reason for hiding this comment

directionless Dec 21, 2021

Choose a reason for hiding this comment

aleksmaus Dec 21, 2021

Choose a reason for hiding this comment

aleksmaus Dec 21, 2021

Choose a reason for hiding this comment

aleksmaus commented Dec 21, 2021

Smjert commented Dec 23, 2021

james-elastic commented Jan 14, 2022

aleksmaus commented Dec 1, 2021 •

edited

linux-foundation-easycla bot commented Dec 1, 2021 •

edited