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/
users: add endpoint for unlocking the TFA of a user
add /access/users/<userid>/unlock-tfa api call which can be used for
unlocking a user after their TFA got locked due to many failed
consecutive retries.
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Stoiko Ivanov [Tue, 27 Jun 2023 09:00:27 +0000 (11:00 +0200)]
report: adapt to changes in SpamAssassin DNS api
SpamAssassin 4.0 changed the way it does DNS-lookups a bit (switched
to asynchronous lookups) - this broke pmg-system-report, since we use
the SpamAssassin API to check that DNS-resolution works. The reason
for this is that SA used to take only the first entry from
/etc/resolv.conf - and SA being able to do correct resolution is
critical for it to work.
This patch fixes the incompatible use of the DNS-API, but does not
change to the asynchronous model.
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com> Tested-by: Friedrich Weber <f.weber@proxmox.com>
Stoiko Ivanov [Mon, 26 Jun 2023 20:45:10 +0000 (22:45 +0200)]
postgresql compat: cast result from EXTRACT to INTEGER
Postgresql has changed the return type of the EXTRACT function to
numeric from float8 [0] in version 14, and I strongly assume that this
change is the reason why:
`SELECT EXTRACT (EPOCH FROM now());`
now returns a floating point instead of an integer value, which in
turn is not accepted in the prepared statements throughout our
codebase.
Dominik Csapak [Mon, 26 Jun 2023 14:10:26 +0000 (16:10 +0200)]
dbtools: grant permissions public schema for created databases
since postgres 15, the public schema is not world writeable anymore for
security reasons. In our environment, where the db is not externaly
reachable and no database users should exists except the ones we create,
we can safely give the permissions again to be able to use
the root/www-data user without modification of the remaining
code/privileges for postgres.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Dominik Csapak [Mon, 26 Jun 2023 12:30:43 +0000 (14:30 +0200)]
introduce pmg7to8 cli helper
mostly copied from pve7to8 (without the pve specific tests) with some
notable additions to check some basic things for the pmg upgrade:
* check if the cluster is healthy
* check if the services are stopped(pre-upgrade)/started(post-upgrade)
* check if the db was upgraded (post upgrade)
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>