Friedrich Weber [Tue, 13 Jun 2023 13:04:25 +0000 (15:04 +0200)]
ldap: fail authentication if dn is empty
This fixes an issue with LDAP servers that accept anonymous binds with
a non-empty password: If a user exists in the PVE LDAP realm, but PVE
cannot find the corresponding LDAP entry during login, they could log
in with any non-empty password.
This issue affects only LDAP realms. AD realms are not affected
because they perform no username->dn mapping.
At least the following LDAP server configurations seem to accept a
bind with empty DN and non-empty password and are affected:
* OpenLDAP with anonymous binds and the non-default setting
`olcAllows: bind_anon_cred` enabled.
* AD (when used in an LDAP realm instead of an AD realm). However, for
the issue to trigger, the LDAP search for the username->dn mapping
has to succeed but return zero results. This can happen, for
example, if the LDAP realm has (1) a bind DN set or (2) no bind DN
set and AD was manually configured to allow anonymous LDAP searches
for user entries.
The situation that a user exists in the PVE realm but is missing in
the LDAP directory can occur, for example, (1) if the user was created
manually or (2) if the LDAP entry is deleted or the base DN is
changed, but the LDAP realm has not been re-synced with
remove-vanished.
The username->dn mapping is performed by `get_user_dn`, which performs
an LDAP search. If the LDAP search for the user entry succeeds but
returns zero results (e.g. if the entry does not exist), `get_user_dn`
returns undef. Then, `auth_user_dn` is called with $dn being undef and
the user-provided $pw and performs an LDAP simple bind with these
credentials. If $pw is empty, Net::LDAP throws an error, but if it is
non-empty, it performs an LDAP bind with an empty DN and the password
provided by the user. If the LDAP server accepts this bind, the user
is logged in.
To fix this, `auth_user_dn` now dies/returns (depending on the $noerr
parameter) if the dn is falsy, which is the case for undef and the
empty string.
The issue was originally reported by forum user ITKR [0].
[0] https://forum.proxmox.com/threads/128788/
Suggested-by: Dominik Csapak <d.csapak@proxmox.com> Suggested-by: Thomas Lamprecht <t.lamprecht@proxmox.com> Signed-off-by: Friedrich Weber <f.weber@proxmox.com>
Fiona Ebner [Fri, 19 May 2023 09:18:16 +0000 (11:18 +0200)]
d/control: record dependency on libanyevent-perl
It's not just a build-dependency. Noticed during an sbuild of
qemu-server that would fail with, because it couldn't locate the
AnyEvent module used in RESTEnvironment.pm.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
(cherry picked from commit 6bb5d640e3eb31e2395993d3880fc179d0ab3df8) Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht [Wed, 17 May 2023 06:50:37 +0000 (08:50 +0200)]
makefile: convert to use simple parenthesis
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
(cherry picked from commit a9fa4157831f5d5742a53926e15d15e34b0e5850) Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Dominik Csapak [Mon, 27 Mar 2023 08:26:32 +0000 (10:26 +0200)]
fix #4615: REST environment: improve AnyEvent detectíon in child cleanup
I assumed that the 'priv' and 'pub' RESTEnvironment types always
contained an AnyEvent eventloop, but this is actually not the case in
pvestatd and pvescheduler.
But it depended on the used model that AnyEvent used (and auto
detected) if this wrong assumption worked or not. With the
AnyEvent::Impl::Perl there weren't any problems and it seemingly
worked by accident, but when using AnyEvent::Impl::EV (which is
autodetected and used when libev-perl is installed) it interfered
with our SIG_CHLD handlers and only ever called them once. (Not clear
why this happens, maybe because AnyEvent is not setup correctly).
This patch uses $AnyEvent::MODEL as a detection instead since this is
`undef` until the first AnyEvent watcher is created, which should be
only the case where we really use AnyEvent, such as pveproxy and
pvedaemon.
Fixes: 6870afa ("RESTEnvironment: better SIGCHLD handling in AnyEvent event loop") Link: https://lists.proxmox.com/pipermail/pve-devel/2023-March/056057.html Suggested-by: Thomas Lamprecht <t.lamprecht@proxmox.com> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
print_errs (which is also called internally by die_now) will only
'warn' the collected error stack if the log level is set to tracing.
otherwise, it will just return the error message(s) corresponding to
the error stack as string.
while they are not always the most user-friendly ones, they do
provide additional context that might help to find out what is
actually causing a particular failure. both helpers here actually
provide a meaninful user friendly context (via $msg) as final line.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
[ T: resolve merge conflict due to dropped warn helper ] Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Dominik Csapak [Mon, 20 Feb 2023 10:08:28 +0000 (11:08 +0100)]
RESTEnvironment: better SIGCHLD handling in AnyEvent event loop
when we're in an API server that uses AnyEvent, we must postpone
the worker_reaper, since it calls 'active_workers' which might already
be called and then we're inside the lock twice (flocks are per process
for us, see PVE::Tools::lock_file)
This resulted in an error like this:
close (rename) atomic file '/var/log/pve/tasks/active' failed: No such file or directory
We use the fact that only 'pub' and 'priv' RESTEnvironment types are an
api server with anyevent. For other types we call it like before.
Christian Ebner [Wed, 11 Jan 2023 13:32:20 +0000 (14:32 +0100)]
tools: Add callback based filtering for logfile dump
This patch introduces callback based filtering functionality for logfile dumps.
Further, the `dump_logfile` function is split into a reusable part for dumps
generated based on a filehandle. The state parameter can be used to keep the
state for multiple consecutive function invocations.
Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
The dump_logfile now returns the whole log file if the limit
parameter is set to 0. This must be done explicitly though, as in the
case of 'limit' being undefined, the default as before, 50 will be
used.
Signed-off-by: Daniel Tschlatscher <d.tschlatscher@proxmox.com>
Dominik Csapak [Thu, 10 Nov 2022 10:36:30 +0000 (11:36 +0100)]
PBSClient: file_restore_list: add extraParams and use timeout
under some conditions, like when calling it in the api where we have
a 30s pveproxy limit, we want to make use of the '--timeout' parameter
of the file-restore binary, but we may want to call it in the future
where we don't want add timeout.
To achieve that, add an extendable 'extra_params' hash parameter to
'file_restore_list' and use the timeout from there.
Thomas Lamprecht [Sun, 13 Nov 2022 14:48:22 +0000 (15:48 +0100)]
d/control: record breaks for older qemu-server/pve-container
as we now auto-detect if the 'bridge-disable-mac-learning' is set in
the Network::tap_plug method and disable learning if so, we need to
ensure that the qemu-server and pve-container can cope with that by
manually registering the guests MAC into the FDB.
So this certainly isn't a hard break, but as network is dead for the
guest on update, if that option is set and the new qemu-server and/or
pve-container packages ain't yet updated, it seems still worthy of a
break.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht [Sun, 13 Nov 2022 12:54:57 +0000 (13:54 +0100)]
procfs tools: modernize write_proc_entry
that unless stuff is just hard to read and against our code style.
note that there's also basically the same helper in SysFSTools, and
neither is really dependent on sysfs or procfs semantics, so both
probably should go away..
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht [Sun, 13 Nov 2022 10:54:32 +0000 (11:54 +0100)]
tests: section config: comment need for warn on debugging
as often only warn really makes it out of perl/our pit of std out/err
handling (e.g., I had a case where neither print STDERR nor syslog
worked, but warn did)
also, the tests are rather brittle w.r.t their expect_fail variant,
as the actual expected error should be enforced.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht [Sun, 13 Nov 2022 10:50:40 +0000 (11:50 +0100)]
job registry: avoid injecting the section id unconditionally in configs
this can result in a broken config due to it getting written out on
write_config serialization, and if a plugin did not declare `id` as
an option it understood (none do currently), it would then fail the
next parse, far from ideal...
As the section ID is available already anyway we should probably just
drop this, but for now avoid rushed changes and just make it
conforming to section config semantics and check if the option is
actually understood by the respective section type we're working on.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
pbs client: backup fs tree: drop namespace parameter
Instead, use the one from the initial configuration. The only current
caller is in PMG and the namespace parameter set there agrees with
the one from the initial configuration, so this is not actually a
breaking change.
All existing callers for functions with namespaced parameters just
re-use the one that's passed in via the initial configuration already,
so there is no need for namespaced parameters currently.
If the need for one PBS client to handle multiple namespaces arises, a
set_namespace() function could be added, or the relevant functions
could take an additional parameter, either for just the namespace or
like $cmd_opts in restore_pxar().
pbs client: default to configured namespace for non-namespaced parameters
For get_snapshots(), also set the default when no namespaced parameter
is present at all.
This would break any callers that have a namespace in the initial
config and explicitly don't set it for a later call, but the only
such caller is restore_pxar() in PMG, which /should/ be using the
namespace!
In other words, this implicitly fixes the restore_pxar() call in PMG
and avoids the need to extract the namespace from the configuration
(which already is present in the client) on the call site for all
functions that currently take a namespaced parameter.
Otherwise, there will be
Result: {
"data": null
}
when calling via a CLI tool for example. This also makes it consistent
with PVE in preparation to switch to using PBSClient there.
Thomas Lamprecht [Wed, 19 Oct 2022 10:29:35 +0000 (12:29 +0200)]
cgroup: move get_cpuunits helper from qemu-server as clamp_cpu_shares
Based on a patch from Fiona[0] that proposed to move it to
guest-common, rather go for common where the CGroup module resides to
avoid having to touch multiple sites if this changes another time
(hopefully not)
cgroup: get mode by checking /sys/fs/cgroup mount point
Since even in pure unified layouts there may be a `name=systemd` v1
cgroup mounted additionally (manually or potentially via
systemd-nspawn apparently), we should check what's actually mounted
at `/sys/fs/cgroup` rather than whether v1 cgroups exist.
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
- ovsint port mtu need to be set with ""ovs-vsctl set mtu-request"
- update mtu on already existing interfaces (fwbr,fwln,tap,veth)
if existing tap|veth interface is replugged on a different mtu bridge
Dominik Csapak [Fri, 12 Aug 2022 09:29:48 +0000 (11:29 +0200)]
SysFSTools: get name from mediated device types
Some vendors also provide a 'name' file here for the type, which, in case of
NVIDIA, is the official name for the vGPU type in their documentation,
so extract and return it too (if it exists).
proc fs tools: handle proc/stat without guest values
PMG is often run as a container, and in certain environments (like
Virtuozzo 7), the last two values (guest and guest_nice) might not be
present. This led to a division by zero, because the total value was
never updated.
Reported in the community forum:
https://forum.proxmox.com/threads/106896/
Acked-by: Wolfgang Bumiller <w.bumiller@proxmox.com> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
tools: use int() on all integer syscall parameters
this should fix an issue where users with custom id mappings
get bad ownership on intermediate directories caused by the
rootuid/gid being the string "100000" in perl instead of the
number 100000...
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>