Add systemd support to the config_parser utility #460

Miauwkeru · 2023-11-27T12:28:20Z

Force parsed_data to always be a dictionary
Improve API for config_tree functionality

codecov · 2023-11-27T13:01:25Z

Codecov Report

Attention: 10 lines in your changes are missing coverage. Please review.

Comparison is base (7461516) 73.93% compared to head (3ce0603) 74.02%.

Files	Patch %	Lines
dissect/target/filesystems/config.py	70.96%	9 Missing ⚠️
dissect/target/plugins/os/unix/linux/services.py	95.45%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #460      +/-   ##
==========================================
+ Coverage   73.93%   74.02%   +0.08%     
==========================================
  Files         259      259              
  Lines       20886    20945      +59     
==========================================
+ Hits        15443    15505      +62     
+ Misses       5443     5440       -3

Flag	Coverage Δ
unittests	`74.02% <94.08%> (+0.08%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

tests/plugins/os/unix/linux/test_services.py

Horofic · 2023-12-04T09:15:08Z

dissect/target/plugins/os/unix/linux/services.py

@@ -107,45 +104,19 @@ def should_ignore_file(needle: str, haystack: list) -> bool:
    return False


-def parse_systemd_config(fh: TextIO) -> str:
+def create_systemd_string(parsed_systemd_dict: dict[str, dict]) -> str:


Doesn't this defeat the purpose of the whole config parsing? Ideally you want to make use of the new parsing features, and change the unix services plugin to dynamically include the parsed key / value pairs in the record output.

I just changed it to have the same behavior as before but using the config parser.
It wasn't my intent yet to make a record with dynamic value pairs, tho I can also just do that...

Then this is a heads-up for the @JSCU-CNI , as the output of this plugin will change drastically

Could you provide a changelog or example to showcase the difference in output?

With the current changes, the record would change from (created with rdump -L):

--[ RECORD 1 ]-- hostname = test domain = None ts = 2022-06-03 21:28:07+00:00 name = dbus-fi.w1.wpa_supplicant1.service config = Unit_Description="WPA supplicant" Unit_Before="network.target" Unit_After="dbus.service" Unit_Wants="network.target" Unit_IgnoreOnIsolate="true" Service_Type="dbus" Service_BusName="fi.w1.wpa_supplicant1" Service_ExecStart="/sbin/wpa_supplicant -u -s -O /run/wpa_supplicant" Service_ExecReload="/bin/kill -HUP $MAINPID" Install_WantedBy="multi-user.target" Install_Alias="dbus-fi.w1.wpa_supplicant1.service" source = /etc/systemd/system/dbus-fi.w1.wpa_supplicant1.service _source = /home/user/media/test _classification = None _generated = 2023-12-06 15:38:25.286011+00:00 _version = 1

where all the information is in config, to the following:

--[ RECORD 1 ]-- hostname = test domain = None ts = 2022-06-03 21:28:07+00:00 type = systemd name = dbus-fi.w1.wpa_supplicant1.service source = /etc/systemd/system/dbus-fi.w1.wpa_supplicant1.service Unit_Description = WPA supplicant Unit_Before = network.target Unit_After = dbus.service Unit_Wants = network.target Unit_IgnoreOnIsolate = true Service_Type = dbus Service_BusName = fi.w1.wpa_supplicant1 Service_ExecStart = /sbin/wpa_supplicant -u -s -O /run/wpa_supplicant Service_ExecReload = /bin/kill -HUP $MAINPID Install_WantedBy = multi-user.target Install_Alias = dbus-fi.w1.wpa_supplicant1.service _source = /home/user/media/test _classification = None _generated = 2023-12-06 15:39:41.306051+00:00 _version = 1

Both initd and systemd records would have an additional type field for the type of service.

config would not exists anymore

All previous config values would be inserted into the record itself as key value pairs.

If the the value is empty, it will be set to None

dissect/target/filesystems/config.py

dissect/target/helpers/configutil.py

dissect/target/plugins/general/config.py

tests/plugins/general/test_config.py

dissect/target/filesystems/config.py

Horofic

I'm still torn on whether going the "Filesystem" route is the right course. A lot of functions act like a that of a file system but have some side-effect or caveat you end up needing to explain in your documentation.

This also makes using the API not that intuitive as you would think, because you need to account for these specifics.

Going more into the route what the Registry is currently doing might make the API more clear. Thoughts @Miauwkeru, @Schamper?

dissect/target/filesystems/config.py

Horofic · 2023-12-07T08:51:24Z

dissect/target/filesystems/config.py

@@ -86,15 +95,27 @@ def __init__(
        fs: Filesystem,
        path: str,
        entry: FilesystemEntry,
-        parser_items: Optional[Union[dict, Any]] = None,
+        parser_items: Optional[Union[dict, ConfigurationParser, Any]] = None,


Suggested change

parser_items: Optional[Union[dict, ConfigurationParser, Any]] = None,

parser_items: Optional[Union[dict, ConfigurationParser, str, list]] = None,

dissect/target/helpers/configutil.py

Miauwkeru · 2023-12-08T10:05:33Z

I'm still torn on whether going the "Filesystem" route is the right course. A lot of functions act like a that of a file system but have some side-effect or caveat you end up needing to explain in your documentation.

This also makes using the API not that intuitive as you would think, because you need to account for these specifics.

Going more into the route what the Registry is currently doing might make the API more clear. Thoughts @Miauwkeru, @Schamper?

I can make a pr in the future that would replace the Filesystem route with the Registry one, so we can see whether it would make the API clearer. I don't think it would fit in this PR though.

dissect/target/filesystems/config.py

dissect/target/helpers/configutil.py

- made as_dict behaviour consistent - Able to use Any kind of iterable as ``collapse`` argument - always put everything as dict - add tests for those things

Co-authored-by: Stefan de Reuver <[email protected]>

…lesystem.get

API changes: - made as_dict behaviour consistent - Able to use any kind of Iterable as a `collapse` argument - always put everything as dict - add tests for those for the things above Other additions: - Adds a `ScopeManager` to create a base scoping mechanism - Raise a `FileNotFoundError` when `entry.get()` is None - Improve documentation of everything, using sphinx style. (DIS-2158) --- Co-authored-by: Stefan de Reuver <[email protected]>

Miauwkeru force-pushed the DIS-2158_add-systemd-support-to-config-parser branch 3 times, most recently from 8e335cc to 3ba0dc0 Compare November 27, 2023 12:58

Miauwkeru force-pushed the DIS-2158_add-systemd-support-to-config-parser branch from 1a9bdcd to 67e140e Compare November 29, 2023 15:39

Miauwkeru requested a review from Horofic November 29, 2023 15:39

Miauwkeru marked this pull request as ready for review November 29, 2023 15:47

Horofic requested changes Dec 6, 2023

View reviewed changes

Miauwkeru force-pushed the DIS-2158_add-systemd-support-to-config-parser branch from 7dc2a3e to e53fb6c Compare December 6, 2023 13:43

Miauwkeru requested a review from Horofic December 6, 2023 13:43

Miauwkeru force-pushed the DIS-2158_add-systemd-support-to-config-parser branch 2 times, most recently from 1f50d10 to eb8aca9 Compare December 6, 2023 15:48

Horofic requested changes Dec 8, 2023

View reviewed changes

Miauwkeru force-pushed the DIS-2158_add-systemd-support-to-config-parser branch from 5d6f534 to 4c7a35c Compare December 8, 2023 15:06

Miauwkeru requested a review from Horofic December 8, 2023 15:06

Horofic requested changes Dec 12, 2023

View reviewed changes

Miauwkeru requested a review from Horofic December 12, 2023 12:02

Horofic previously approved these changes Dec 12, 2023

View reviewed changes

Miauwkeru and others added 11 commits December 12, 2023 12:34

Force parsed_data to always be a dictionary

e2c995f

Improve API for config_tree functionality

e6ca3f2

- made as_dict behaviour consistent - Able to use Any kind of iterable as ``collapse`` argument - always put everything as dict - add tests for those things

Add base systemd configuration parser

f7a4959

Add ScopeManager to create a base scoping mechanism

d09923a

Reduce duplication

1f07068

Move weird indentation tests to test_config_util.py

d479120

Update services

ae8d676

Update Match and config mapping in configutil.py

d924f90

Fix tests

4951ed2

Fix odd character in tests

00476d6

Apply suggestions from code review

c5a02af

Co-authored-by: Stefan de Reuver <[email protected]>

Miauwkeru and others added 11 commits December 12, 2023 12:34

Update documentation ConfigurationFilesystem and ConfigurationEntry

f362df3

Update documentation to configutil.py

52e471e

Update documentation general.config.py

661b0fb

Change all instances of seperator to separator

763b3b0

Update typehints for test_services.py

b70975a

Resolve bug, and changed services unix plugin

f01f393

Fix tests

9e19c83

fix linting

a35fed4

Add suggested documentation changes

603d78c

Apply suggestions from code review

54a96e1

Co-authored-by: Stefan de Reuver <[email protected]>

Raise a FileNotFoundError using the previous entry in ConfigurationFi…

7177f42

…lesystem.get

Miauwkeru force-pushed the DIS-2158_add-systemd-support-to-config-parser branch from baedde4 to 7177f42 Compare December 12, 2023 12:34

Fix linting

3ce0603

Miauwkeru dismissed Horofic’s stale review via 3ce0603 December 12, 2023 12:36

Horofic self-requested a review December 12, 2023 12:57

Horofic approved these changes Dec 12, 2023

View reviewed changes

Miauwkeru merged commit 3a298a4 into main Dec 12, 2023

Miauwkeru deleted the DIS-2158_add-systemd-support-to-config-parser branch December 12, 2023 13:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add systemd support to the config_parser utility #460

Add systemd support to the config_parser utility #460

Miauwkeru commented Nov 27, 2023

codecov bot commented Nov 27, 2023 •

edited

Loading

Horofic Dec 4, 2023

Miauwkeru Dec 6, 2023

JSCU-CNI Dec 6, 2023

Miauwkeru Dec 6, 2023

Horofic left a comment

Horofic Dec 7, 2023

Miauwkeru commented Dec 8, 2023

	parser_items: Optional[Union[dict, ConfigurationParser, Any]] = None,
	parser_items: Optional[Union[dict, ConfigurationParser, str, list]] = None,

Add systemd support to the config_parser utility #460

Add systemd support to the config_parser utility #460

Conversation

Miauwkeru commented Nov 27, 2023

codecov bot commented Nov 27, 2023 • edited Loading

Codecov Report

Horofic Dec 4, 2023

Choose a reason for hiding this comment

Miauwkeru Dec 6, 2023

Choose a reason for hiding this comment

JSCU-CNI Dec 6, 2023

Choose a reason for hiding this comment

Miauwkeru Dec 6, 2023

Choose a reason for hiding this comment

Horofic left a comment

Choose a reason for hiding this comment

Horofic Dec 7, 2023

Choose a reason for hiding this comment

Miauwkeru commented Dec 8, 2023

codecov bot commented Nov 27, 2023 •

edited

Loading