Stoiko Ivanov [Tue, 27 Feb 2024 14:22:03 +0000 (15:22 +0100)]
pmgdb: dump: replace active flag with rules enum
this addresses the odd/unintuitive behavior of `pmgdb dump --active 0`
which currently prints all rules, why printing only the inactive ones
might be expected.
Thomas Lamprecht [Mon, 26 Feb 2024 19:25:34 +0000 (20:25 +0100)]
d/control: bump versioned dependency for log-tracker
while there's actually a cyclic soft-dependency between both of them,
a new log-tracker with an old API just shows on logs from a wrong
time-window, far from ideal, but not hard-breakage either, so keep
this only one way and do not break old pmg-api in new tracking center.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
fix #2971: DKIM: add setting to use From header when signing
Following RFC 5322 [2], we add an option to use the address from the
`From:` header instead of the Envelope From address.
From RFC 7489 [1]:
To illustrate, in relaxed mode, if a validated DKIM signature
successfully verifies with a "d=" domain of "example.com", and the
RFC5322.From address is "alerts@news.example.com", the DKIM "d="
domain and the RFC5322.From domain are considered to be "in
alignment". In strict mode, this test would fail, since the "d="
domain does not exactly match the FQDN of the address.
Mira Limbeck [Tue, 18 Jul 2023 16:01:01 +0000 (18:01 +0200)]
MailTracker: remove timezone offset
The timezone offset is moved to `pmg-log-tracker` in the
compatibility code path. This way the new rfc3339 timestamp, which is
already in UTC, can be used as is.
Signed-off-by: Mira Limbeck <m.limbeck@proxmox.com>
Stoiko Ivanov [Mon, 12 Feb 2024 21:00:00 +0000 (22:00 +0100)]
d/control: bump versioned dependency for postfix
While postfix happily ignores completely unknown settings (e.g. if the
config contains settings not present in an older version of postfix),
the allowed values are handled more strict:
```
fatal: bad boolean configuration: smtpd_forbid_bare_newline = normalize
```
is the result of running this configuration on postfix < 3.7.10 (for
bookworm)
Stoiko Ivanov [Mon, 12 Feb 2024 20:59:59 +0000 (21:59 +0100)]
templates: postfix: adapt to current default setting for smtpsmuggling
postfix recently released new versions (for all supported stable
versions including 3.7.10), which changed the behavior regarding bare
newlines (which originally caused smtp-smuggling [0]).
Instead of directly rejecting smtp sessions when <LF> is used as
command separator, the session continues, however a bare <LF> is not
recognized as end for the DATA command.
The current setting of `smtpd_forbid_bare_newline = yes` (in 3.7.9)
used to behave like the new setting of 'reject'. In 3.7.10 this was
changed and it behaves like `smtpd_forbid_bare_newline = normalize`
(the default for postfix > 3.9)
The current patch simply adapts to the current default naming (yes is
an alias for normalize) - The change in behavior came with the postfix
update shipped in proposed-updates end of January and part of Debian
12.5 (released on 10.02.2024).
As both versions mitigate smtp-smuggling in postfix, and even the more
drastic behavior of rejecting bare <LF> did not cause any problems in
our support-channels and own deployments the patch is not
security-relevant.
Stoiko Ivanov [Thu, 22 Feb 2024 21:06:18 +0000 (22:06 +0100)]
pmgdb: highlight active rules
by printing the ACTIVE in caps. For me this improves quickly skimming
through a ruleset (in a few support cases I wasted quite a bit of time
on the wrong spot - only to see that the rule in question is inactive
anyways)
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com> Tested-by: Friedrich Weber <f.weber@proxmox.com> Tested-by: Alexander Zeidler <a.zeidler@proxmox.com>
Friedrich Weber [Thu, 22 Feb 2024 16:35:35 +0000 (17:35 +0100)]
fix #5189: cluster: avoid sync errors for statistics and quarantine
After restoring a backup from a cluster on a fresh node with
statistics, and then creating a cluster, the following can happen
(node 1 being master and node 2 being a node): `ClusterInfo` on node
1 has no record about the last-synchronized `CStatistic` row id of
node 2. Thus, pmgmirror on node 1 initializes the record with -1 and
tries to synchronize *all* `CStatistic` rows with cid 2 from node 2.
But (some of) these rows may already exist on cid 1, because they
were part of the backup, so pmgmirror on node 1 triggers a Postgres
unique constraint violation, statistics synchronization on node 1
fails, and node 1 remains in the "synchronizing" state.
Fix this as follows: When a new node is added to a cluster, the master
now initializes its `ClusterInfo` record of the last-synchronized
`CStatistic` row id for that node cid with the maximum row id that
exists in the local `CStatistic` for that node cid, or with -1 if the
local `CStatistic` has no row for that node cid. This is valid because
the newly-added node copies the master's `CStatistic` table during
cluster join.
Do the same for the `CMailStore` table, where a similar sync error
could happen e.g. if the table has rows for both node cids, node 2 is
shut down and manually deleted from the cluster.conf, the maxcid is
manually reset to 1, and a fresh node is joined to the cluster and
gets assigned cid 2.
Signed-off-by: Friedrich Weber <f.weber@proxmox.com>
Stoiko Ivanov [Fri, 12 Jan 2024 19:21:51 +0000 (20:21 +0100)]
pmg-smtp-filter: die if processing took longer than the timeout
In case a mail took longer to get processed than the configured
timeout - 1 second - `die` before running any action.
The `die` results in a temporary failure to be reported to the sending
server by PMG::SMTP.pm ("451 4.4.0 detected undelivered mail").
The reason for the 1s extra slack is to have some time to actually
run the action - and also justified that in both cases (postfix
detecting the timeout, and pmg-smtp-filter `die`ing the sender gets
a temporary failure reported back).
Tested with a small filter_timeout setting (30), and a larger sleep in
added in analyze_virus.
Stoiko Ivanov [Fri, 12 Jan 2024 19:21:50 +0000 (20:21 +0100)]
config: postfix: make smtp-filter-timeout configurable
accessing the value in the config-template needs to use the
item virtual method (`pmg.mail.item('filter-timeout')` instead of the
. operator, due to using kebab-case for 'filter-timeout' see [0].
the minimal value is set to 2 as we add 1 second for the rules
processing in the next patch, and postfix has 1 as minimum.
Stoiko Ivanov [Fri, 12 Jan 2024 19:21:48 +0000 (20:21 +0100)]
templates: postfix: set same timeouts for before and after-queue
When a mail takes longer to get processed (observed with some scanned
pages as pdf - where clamav+avast took 2.2 minutes for scanning) the
behavior of the filtering is different between before-queue and
after-queue filtering:
In the before-queue case the timeout is `$smtpd_proxy_timout` (120s)
[0], so the mail does not get processed in time and the listening
smtpd responds with `451 4.3.0 Error: queue file write error`. However
pmg-smtp-filter sends the mail to the smtpd on 10025 once it's done.
the original sender resends the mail due to the 451 error - which
results in the mail getting delivered multiple times (until it gets
removed from the queue on the original sender)
In the after-queue case the timeout is `$lmtp_data_done_timeout`
(pmg-smtp-filter acts as lmtp server) (600s) - so the mail gets
send successfully only once. In case the processing time reaches
600s the behavior is equivalent - but for lmtp postfix logs:
```
...timed out while sending end of data -- message may be sent more
than once
```
The value needs to be set as literal - referring to the builtin
default as `$lmtp_data_done_timeout` does not work.
While the underlying issue of mails getting sent multiple times in
case the timeout is reached is not fixed by this - having the same
timeout in both cases is a good idea and 600s increases the chances
of clamav+avast+custom_check_script+spamassassin to get their job done.
tested by adding a `sleep 200` in PMG::Utils::analyze_virus_clam.
(before the eval block running with the 5 minute timeout)
Stoiko Ivanov [Thu, 22 Feb 2024 12:40:50 +0000 (13:40 +0100)]
backup: reorder tables in order of dependency
Currently we dump the tables manually and print the relevant DDL
statements into the dump-file before copying the data.
The complete backup and dump specific tables part would probably be
best replaced by an appropriate `pg_dump` invocation - but as a
stop-gap measure reorder the tables in order of their dependencies.
Noticed while adding a foreign key constraint on 2 new tables
(objectgroup_attribute and rule_attribute) and running backup+restore.
Dominik Csapak [Thu, 22 Feb 2024 10:04:57 +0000 (11:04 +0100)]
api: reload ruledb on updating objectgroups
previously they only contained metadata (name/comment) so for the
rulesystem that was not relevant. Now with the attributes and/invert
we want to reload the ruledb on update too.
Stoiko Ivanov [Wed, 21 Feb 2024 20:26:51 +0000 (21:26 +0100)]
pmg-smtp-filter: rename proxtest.com to pmg.example
the proxtest.com addresses have been hardcoded for a long while, and
are used for the regression tests.
While the regression tests do not send out mails - certain
configurations might - e.g.:
```
section: mail
ndr_on_block 1
```
noticed while running the regression tests with this setting and
seeing that my queue had messages, which could not be delivered due to
a time-out
replace the addresses with `pmg.example` (longer names make the
test-output wrap to multiple lines causing more churn in the
regressions tests) - pmg.example is safe as .example is reserved (for
such purposes) [0]
Dominik Csapak [Wed, 21 Feb 2024 12:24:35 +0000 (13:24 +0100)]
RuleCache: implement and/invert for what matches
Since what matches are not a simple boolean match, but also can contain
"marks" to mark specific parts of the mail, we must implement some
custom logic for and/invert here.
The goal here is to define that groups are on a per part level,
but the rule operates on the whole mail.
To achieve this we have two different and/invert combine functions, one
for the group level and one for the whole what match.
For per group and/inversion we and 'and-combine' and invert the list of
marks, so if it matches part 1,2 of 1,2,3 the inversion would return 3.
For the rule it only matters if the and/inversion part matches at all,
regardless of the marks. If it matches, the marks will be or'ed.
With this, one can represent many different scenarios that were not
possible before.
Dominik Csapak [Wed, 21 Feb 2024 12:24:27 +0000 (13:24 +0100)]
RuleCache: reorganize how we gather marks and spaminfo
currently, we gather the marks and the spaminfo separately,
with the following properties:
* both are gathered with what matches
* virus and spam matches are global (i.e. not marks but [] to mark the
whole mail)
* only the spam match can differ per user (wl/bl)
* all matches are or'ed (so order and global/per part does not make much
of a difference for matching the rule)
* marks are saved once globally per rule and per target
* spaminfo is saved per target
instead collect spaminfo once and marks per target together. With this,
we can omit the 'global' marks list, since each target has their own
anyway. Also saves the spaminfo only once, since that must be the same
for the mail regardless of target.
Since that shouldn't change the current matching behavior (all marks
are identical for the whole rule), we can simply use the marks from the
first target in `Remove` (the only action where the marks are relevant).
This makes it easier for us when we'll implement and/invert for matches,
as the marks can differ between targets. That is, because the spamlevel
can diverge for them and that can be and-combined with objects that add
marks.
Thomas Lamprecht [Wed, 21 Feb 2024 15:56:35 +0000 (16:56 +0100)]
config: avoid sudden downward glitch in max_filter heurisitic
The recent heuristic change, while having good intentions, added a
glitch in the calculation of max_servers to use between the system
memory boundary of 3840 MiB.
For example, if a VM would e.g., have 3.5 GiB memory it would get 26
max_workers, and if a admin then increased this to 4 GiB, it would get
13 max_workers. This also meant that the VM would use less memory with
4 GiB configured, but when one reduces that, the usage would jump up.
Such effects are rather odd, and thus the heuristic was adapted to be
more linear, with basically no decrease of max_servers due to system
memory increasing.
Make the base_usage a 5/8 fraction of the detected total system
memory, fixate the estimation for per-server-memory usage to 150 MiB
and make the warning differ between violating minimum and recommended
total system memory (with some leeway).
A comparission table of system-memory in the first column, previous
max_servers results in the middle (the caller adds +2 to this, which
is done here too) and the result of the updated calculation in right
most column:
As flooding tests here could not use much more than 4 to 6 processes,
the slightly lower values on very low-memory systems should not matter
– actually they might improve performance even (less memory
contention and lower OOM-kill possibility).
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht [Wed, 21 Feb 2024 14:00:05 +0000 (15:00 +0100)]
filter: skip warning about max_filters on low-memory if manually set
The get_max_filters method gets called on module load to fill the
default value used, at that stage we cannot know if the admin set
max_filters manually or not, so the warning might be bogus and thus
annoying.
Move it over to the smtp-filter executable's startup code, as there we
can check the config if it's max_filters is set or not.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Markus Frank [Thu, 18 Jan 2024 14:55:32 +0000 (15:55 +0100)]
config: adjust max_filters calculation to reflect current memory usage
Change max_filters calculation for systems with recommended memory
setup (>4GB).
The values of 2816 and 150 are based on testing with 4GB, 6GB & 8GB
memory configurations, large and small numbers of added objects and
sending multiple mails simultaneously.
On setups with less memory, it is difficult to completely prevent OOM kills.
So for these setups the calculation remains similar, but a warning is sent.
Related OOM killer problem found in forum:
https://forum.proxmox.com/threads/123531/
Dominik Csapak [Fri, 9 Feb 2024 12:54:26 +0000 (13:54 +0100)]
RuleCache: reorganize to keep group structure
Currently we 'or' combine all objects of a type (from/to/what/when)
regardless of group, so we only keep a single list of all objects.
Since we want to introduce different logic (and/invert) we want to keep
the configured group structure. This patch does this, without changing
the current matching logic (still all 'or'-ed).
Dominik Csapak [Fri, 9 Feb 2024 12:54:25 +0000 (13:54 +0100)]
RuleCache: remove unnecessary copying of marks
two things that are wrong here
* what_match_targets never returns a non empty list
* we copy the list just returned just to append it to itself again
My guess is that we meant to copy the original list, not the just
acquired one, and append it to the one just received. But that never did
make a difference, since we only ever check for defined-ness on that
exact list, and the only Object that this applies to (Spam) always
returns an empty list with the spaminfo (so it's always defined in that
case).
Since this was always the behavior AFAICT, just remove the unnecessary
copy of the list for now. If we encounter any actual bugs with that, we
can still implement it back in the right way (copy the original list).
Dominik Csapak [Thu, 21 Dec 2023 12:05:08 +0000 (13:05 +0100)]
fix #4811: rule db: test regex validity on save
and warn only when it's an invalid regex on execution, because users may
have previously had such rules. Otherwise, pmg-smtp-filter will restart
every time it encounters such a rule.
When testing, 'die' if the regex execution 'warns', so that users cannot
enter a semi-invalid or very wrong regex like '^*foo$'.
do so for every rule type that uses a regex to match
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
[S.I.: add short comment in test_regex sub ] Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
Stoiko Ivanov [Tue, 2 Jan 2024 10:30:39 +0000 (11:30 +0100)]
templates: postfix: forbid_bare_newline on external port
This patch addresses the smtp-smuggling vulnerability [0,1], with the
recommended fix by postfix upstream [2].
Disallowing bare linefeeds instead of crlf should not be a problem
with any standards-compliant MTA.
The internal port allows bare linefeed, since internal clients
(mail-scripts written ages ago, some ancient embedded systems) might
not adhere to the protocol. Additionally the mail-proxy allowlist (the
ip and cidr entries, are the only ones applicable here) is also added
to the global exceptions.
Currently the updated postfix-packages are not published in the
security repositories but only as stable updates [3,4]
However postfix ignores unknown configuration parameters and only
prints a warning to the journal - so the changes to the templates can
already be shipped, for those users who have the stable-updates mirror
enabled.
Tested with the current postfix in bookworm, then updating to the one
in bookworm-updates and running tests with netcat (verified with nc -C
that it still works with the correct line-termination):
```
$ nc -6 pmgtest 25
220 pmgtest.proxmox.com ESMTP Proxmox
EHLO pmgsender.proxmox.com
521 5.5.2 pmgtest.proxmox.com Error: bare <LF> received
```
by disabling pipelining on the external port.
The fix in the postfix config for the smtp-smuggling vulnerability [0]
follows the current recommendation of postfix upstream [1].
by using `smtpd_data_restrictions` instead of the newer
`smtpd_forbid_unauth_pipelining` the fix works for both PMG 7 and 8.
Tested with a handcrafted smtp-smuggling-session and verifying that:
* without the fix I get 2 mails
* with the fix I get 1 mail when sending to the external port, but
still 2 mails when sending to the internal port
Stoiko Ivanov [Fri, 15 Dec 2023 18:08:16 +0000 (19:08 +0100)]
pmg7to8: check for proper grub meta-package for bootmode
This should catch installations from our ISO on non-ZFS in uefi mode,
which won't get the updated grub efi binary installed upon upgrade,
because grub-pc is installed instead of grub-efi-amd64.
Folke Gleumes [Tue, 14 Nov 2023 14:14:07 +0000 (15:14 +0100)]
api: acme: deprecate tos endpoint in favor of new meta endpoint
The ToS endpoint ignored data that is needed to detect if EAB needs to
be used. Instead of adding a new endpoint that does the same request,
the tos endpoint is deprecated and replaced by the meta endpoint,
that returns all information returned by the directory.
user quarantine: use raw pmail for ticket assembly
Currently, the quarantine report does not work if the recipient has
some encodable characters in their local part - e.g.
'some&other@domain.example'
When clicking on the links on the report the user gets still logged
in, the ticket _is_ valid after all, however their quarantine list is
empty, as the API call to `/quarantine/spamusers` returns 403 due to
the (encoded) username from the ticket not matching the (by the API
decoded) one from the request quarantine.
With this patch the username, which is includes in the ticket,
remains 'some&other@domain.example' instead of the encoded
'some&other@domain.example', thus the access check user
comparission work with the correct value again and the listing works
as expected
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
[ TL: commit message additions and rewordings ] Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
cluster: fingerprint parsing: adapt to changed openssl output
currently updating the fingerprints using `pmgcm update-fingerprints`
runs into an error indicating that parsing of the remote node's
fingerprint fails
Note that in that case it would equally work to change the parameter
from `-sha256` to `-SHA256` in the `openssl x509` command above
The change seems small enough to warrant pulling it into stable-7 as
well (although the issue should not occur in systems upgraded
according to our howtos).
system report: skip irrelevant files in /etc/pmg/templates
This patch removes:
* templates which have no changes to the ones in
/var/lib/pmg/templates
* files generated by ucf
from the report. Unmodified files are reported, so that the user can
remove them.
This should make providing support a bit easier - as currenlty I'd
copy each template from the report to `diff` it with the version in
the package, for finding out if there is something relevant.
the new dump_template sub was copied from dir_to_text, in order to
explicitly write which files are skipped.
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
[T: merge in helper method for getting the unmodified templates ] Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
I considered making this a warning, but since unmodified files get
updated to the new versions in /var/lib/pmg/templates by ucf a notice
seems more appropriate.
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
[T: merge in helper method for getting the unmodified templates ] Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
tree-wide: make slurp mode as local as possible for future-proofing
similar to what PMG/TFAConfig.pm already does.
Otherwise, sub-routine calls would still be affected leading to
unexpected results, like the issue fixed by commit "cluster config:
restrict slurp scope to avoid issue parsing network interfaces".
As reported in the community forum [0], there is an edge case, where
querying the network interfaces would not work. In particular, this
could happen if the hostname cannot be resolved to a non-loopback IP
(when installing PMG on Debian and forgetting to adapt /etc/hosts for
example).
The issue manifested as follows:
- When setting up the RESTEnvironemnt, the cluster config is read.
- This reader uses slurp mode by setting the line ending to undef
locally.
- But the subroutine call PVE::Network::get_local_ip() is still part
of that local context.
- When resolving the hostname to a non-loopback IP address failed, the
function would read (via the PVE::INotify module) the network
interfaces file.
- As part of that, /proc/net/dev was read all at once, while the
interface parsing code expects it line-by-line.
- The result for reading network interfaces was cached without having
detected the interfaces in /proc/net/dev.
- When a new request came in, the cached result was used (even
changing the file to invalidate the cache would only work as long
as the cluster config file exists, because otherwise, there would be
an attempt to read the cluster config which would read the updated
version of the interfaces file while slurping again).
fix #4815: pmgsh: fix calling the api paths directly
if we get a command directly, we don't initialize the $rpcenv
variable anymore.
To fix it, make it a local variable of the pmg_command function.
We now make one extra '->get()' call per command (as opposed to
once per program), but that shouldn't cost us anything really.
Reported in the forum: https://forum.proxmox.com/threads/.130008/
projects / pmg-api.git / log