Skip to content

security/maltrail: don't override the Check Hostheaders setting - #5679

Open
Lux-WorldPC wants to merge 1 commit into
opnsense:masterfrom
Lux-WorldPC:maltrail-check-host-domains
Open

Lux-WorldPC wants to merge 1 commit into
opnsense:masterfrom
Lux-WorldPC:maltrail-check-host-domains

Conversation

@Lux-WorldPC

Copy link
Copy Markdown
Contributor

Important notices

Before you submit a pull request, we ask you kindly to acknowledge the following:

If AI was used, please disclose:

  • Model used: Claude Opus 5 (via Claude Code)
  • Extent of AI involvement: used to locate the offending line, to check maltrail upstream
    (core/settings.py) for the duplicate-key behaviour, and to trace the template history.
    The fix is the one-line deletion proposed by the reporter in os-maltrail: CHECK_HOST_DOMAINS appears twice #5489. I reviewed and verified
    each of those points before opening this PR.

Describe the problem

CHECK_HOST_DOMAINS is emitted twice in the generated maltrail.conf: once from the
checkhostheader conditional, and again a few lines below as a hardcoded false.

maltrail's read_config() walks the file top to bottom and ends each iteration with a plain
config[name] = value, with no guard, so the last occurrence wins. The trailing hardcoded line
therefore always overrides the GUI setting and pins CHECK_HOST_DOMAINS to false.

The hardcoded line dates back to the initial template. The conditional block was later inserted
directly above it in cdf3620 ("security/maltrail: add hostheader checking", #3144) without removing
it, so Services -> Maltrail -> General -> Check Hostheaders has had no effect since the option was
introduced.


Describe the proposed solution

Drop the leftover hardcoded CHECK_HOST_DOMAINS false so the emitted value is the one selected in
the GUI. This is the fix suggested by @thwien in the issue.

Users who had the toggle off are unaffected (the conditional still emits false); users who had it
on now get the true they asked for.

I left PLUGIN_REVISION alone, assuming the bump is done on your side as usual - happy to add it if
you prefer.


Related issue

Closes #5489

The template emitted CHECK_HOST_DOMAINS twice: once from the checkhostheader
conditional and again below as a hardcoded false.  maltrail's read_config()
assigns config[name] = value unguarded while walking the file top to bottom,
so the last occurrence won and the GUI toggle never took effect.

The hardcoded line predates the option; cdf3620 added the conditional right
above it without removing it.

Closes opnsense#5489

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@Lux-WorldPC

Copy link
Copy Markdown
Contributor Author

Adding the verification I ran, in case it is useful for review.

Tested on OPNsense 26.1.11_10 with os-maltrail 1.10_3, through the GUI: Services > Maltrail >
General
for the toggle, then the Sensor page to apply, since that is what triggers the
template reload. The template file on the box was byte-identical to master plus this diff
(sha256 a28de8ac...728f).

Before, with "Check Hostheaders" enabled:

# grep -n CHECK_HOST_DOMAINS /usr/local/share/maltrail/maltrail.conf
15:CHECK_HOST_DOMAINS true
17:CHECK_HOST_DOMAINS false

maltrail's own parser resolves that to the second occurrence:

>>> read_config('/usr/local/share/maltrail/maltrail.conf')
>>> config.CHECK_HOST_DOMAINS
False

After this change, same setting: a single CHECK_HOST_DOMAINS true, and read_config()
returns True. With the setting disabled: a single CHECK_HOST_DOMAINS false, and False.

AI disclosure, as in the PR description: Claude Opus 5 via Claude Code ran this test and
drafted this comment. I reviewed each command and its output.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

os-maltrail: CHECK_HOST_DOMAINS appears twice

1 participant