Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions dissect/target/plugins/os/unix/_os.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def __init__(self, target: Target):
super().__init__(target)
self._add_mounts()
self._add_devices()
self._hostname_dict = self._parse_hostname_string()
self._hostname, self._domain = self._parse_hostname_string()
self._hosts_dict = self._parse_hosts_string()
self._os_release = self._parse_os_release()

Expand Down Expand Up @@ -149,14 +149,13 @@ def architecture(self) -> str | None:

@export(property=True)
def hostname(self) -> str | None:
hosts_string = self._hosts_dict.get("hostname", "localhost")
return self._hostname_dict.get("hostname", hosts_string)
return self._hostname or self._hosts_dict.get("hostname", "localhost")

@export(property=True)
def domain(self) -> str | None:
domain = self._hostname_dict.get("domain", "localhost")
domain = self._domain or "localhost"
if domain == "localhost":
domain = self._hosts_dict["hostname", "localhost"]
domain = self._hosts_dict.get("hostname", "localhost")
Copy link
Copy Markdown
Member

@Schamper Schamper Apr 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hah, that looked like a fun bug.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just because we're here anyway, could you add a unit test that would cover this scenario?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you ask the author of this piece of code instead (iirc @Horofic)? Not sure what the original purpose is of setting domain to "localhost".

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oof.

I've committed some small changes in 43505c6 and added a simple test. The second parameterised test case was my original intent. Which I believe, mimicked Debian, but not 100% clear on that anymore. This case intentionally fails now, feel free to implement it or take it in the direction you believe is more correct!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the content of /etc/hosts contains 127.0.0.1 mydomain (and no /etc/hostname is found) in my opinion the parsed hostname should be mydomain. In this limited context we have no way to safely determine the difference between a hostname and domain.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very fair. I adjusts the test case in 9394c1c and added some additional test cases just to be sure. Please see if you agree with them. I also adjusted some other tests to take into account some of the localhost behaviour.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I think leaving the domain to None makes more sense instead of assuming localhost.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ill change the behaviour back later today.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted in 3f6f6bb and adjusted the behaviour in 343c12a. This should do the trick now.

if domain == self.hostname:
return domain # domain likely not defined, so localhost is the domain.
return domain
Expand All @@ -167,14 +166,14 @@ def os(self) -> str:

def _parse_hostname_string(
self, paths: list[tuple[str, Callable[[Path], str] | None]] | None = None
) -> dict[str, str] | None:
"""Returns a dict containing the hostname and domain name portion of the path(s) specified.
) -> tuple[str | None, str | None]:
"""Returns a tuple containing respectively the hostname and domain name portion of the path(s) specified.

Args:
paths (list): list of tuples with paths and callables to parse the path or None

Returns:
Dictionary with ``hostname`` and ``domain`` keys.
Tuple with ``hostname`` and ``domain`` strings.
"""
hostname = None
domain = None
Expand All @@ -201,7 +200,8 @@ def _parse_hostname_string(

break # break whenever a valid hostname is found

return {"hostname": hostname if hostname else None, "domain": domain if domain else None}
# can be an empty string due to splitting of hostname and domain
return hostname if hostname else None, domain if domain else None

def _parse_rh_legacy(self, path: Path) -> str | None:
hostname = None
Expand Down
8 changes: 6 additions & 2 deletions dissect/target/plugins/os/unix/bsd/openbsd/_os.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
class OpenBsdPlugin(BsdPlugin):
def __init__(self, target: Target):
super().__init__(target)
self._hostname_dict = self._parse_hostname_string([("/etc/myname", None)])
self._hostname, self._domain = self._parse_hostname_string([("/etc/myname", None)])

@classmethod
def detect(cls, target: Target) -> Filesystem | None:
Expand All @@ -30,4 +30,8 @@ def version(self) -> str | None:

@export(property=True)
def hostname(self) -> str | None:
return self._hostname_dict.get("hostname", None)
return self._hostname

@export(property=True)
def domain(self) -> str | None:
return self._domain
6 changes: 3 additions & 3 deletions tests/plugins/os/unix/test__os.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,10 @@ def test_parse_hostname_string(
) -> None:
fs_unix.map_file_fh(path, BytesIO(file_content))

hostname_dict = target_unix._os._parse_hostname_string()
hostname, domain = target_unix._os._parse_hostname_string()

assert hostname_dict["hostname"] == expected_hostname
assert hostname_dict["domain"] == expected_domain
assert hostname == expected_hostname
assert domain == expected_domain


def test_users(target_unix_users: Target) -> None:
Expand Down