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>
Thomas Lamprecht [Mon, 26 Jun 2023 08:43:00 +0000 (10:43 +0200)]
d/control: depend on rsyslog
required for our current tracking center implementation, a central
feature for PMG, which uses rsyslog log files and format.
Note that we evaluated switching to the journal there, but that was
deemed to be too slow (albeit could have only been start-up time
penalty) – anyhow, as of now this is a requirement to get the full
functionality, once the log-tracker can understand other formats in
an efficient way too we can add those as alternatives.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Stoiko Ivanov [Fri, 31 Mar 2023 11:27:47 +0000 (13:27 +0200)]
d/maintscripts: prevent aborting on errors in some commands
in case something goes wrong it is often better to not leave the
packaging state broken.
failure in the commands masked by this patch are either transient
(pmgconfig sync -restart 1 failing when services are masked), or will
be noticed quite instantly (failed database or config initialization
upon first install)
the deb-systemd-invoke change was based on a quick grep in
/var/lib/dpkg/info on my system
I quickly considered masking even more errors (e.g. related to the ucf
handling) - but they don't seem to cause issues (in the past 3 years)
- and if something breaks there it is probably worth to get a report
reported in our community forum:
https://forum.proxmox.com/threads/.125088/
ruledb: match field: validate regular expressions on addition
Do not save rules if they die during an execution test, which is done
by using them once on an empty string.
Since users may have saved already invalid ones, only warn if we
encounter such a regex in 'parse_entity' during execution instead of
dying. Otherwise pmg-smtp-filter will exit and restart, possibly
leading to wrongly denying mails (and possibly sending out NDRs)
before spam checking was done.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> Tested-by: Mira Limbeck <m.limbeck@proxmox.com> Reviewed-by: Mira Limbeck <m.limbeck@proxmox.com>
[ T: touch up commit subject/message ] Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Stoiko Ivanov [Mon, 27 Mar 2023 19:18:13 +0000 (21:18 +0200)]
quarantine: delete Delivered-To and Return-Path when reinjecting
The removal of those 2 headers was dropped in the recent rework for
quarantine delivery.
Leading to mails from quarantine being bounced by postfix 'local'
delivery agent (as the comment in the original code stated)
Reproduced by delivering a mail from quarantine to a postfix instance,
which routes it to a local account
Fixes: e51fe74 ("quarantine: use reinject_local_mail to deliver quarantined mail") Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
Stoiko Ivanov [Fri, 17 Mar 2023 18:44:54 +0000 (19:44 +0100)]
api: quarantine: decode addresses before delivery/userlisting
With the change of using reinject_local_mail for the quarantine
delivery the issue of not properly decoding the entries we get from
the database before delivering became apparent
The database returns utf-8 encoded strings, reinject_local_mail and
add_to_blackwhite expects perl-strings (with wide characters) and
encodes them (a second time) - this patch decodes the database strings
before passing it on.
add_to_black_white is used in a few API calls (via
read_or_modify_user_bw_list), therefore the approach of decode (from
database), and encode (for database) was chosen.
Stoiko Ivanov [Fri, 17 Mar 2023 18:44:53 +0000 (19:44 +0100)]
quarantine: use reinject_local_mail to deliver quarantined mail
the current delivery looks quite similar to reinject_local_mail,
apart from the database handling and sending the mail-contents from a
file instead of a MIME::Entity.
While reparsing the mail might seem expensive, the quarantine code
does so multiple times when users click in the quarantine GUI (see
PMG::HTMLMail, and the attachment quarantine)
The issue of MIME::Parser being lossy [0] (parsing and then printing
the entity, might not return the original mail byte-by-byte), is
already present in our code-base anyways (when the mail gets
quarantined (or sent on) it is from a parsed MIME::Entity).