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>
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 6bb5d640e3)
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
(cherry picked from commit a9fa415783)
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
According to rfc2986 the only valid version is 0. No newer rfc changed
that.
See section 4.1:
https://www.rfc-editor.org/rfc/rfc2986#section-4.1
Manually verifying the CSR with openssl results in the following error:
```
$ openssl req -in bad.csr -text -noout
Certificate Request:
Data:
Version: Unknown (2)
```
Signed-off-by: Mira Limbeck <m.limbeck@proxmox.com>
with a rough check and a slight amount of hope that nothing else in
the module dependent on turning that off for the whole module
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
to please perlcritic's highest severity level and as it makes just
sense to contain such things to the smallest possible scope.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
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>
This is a pattern that can be found often in Proxmox VE's API stack,
so implement it centrally here for re-use.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
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.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
If one sets a param hard-coded to the same vallue in all but one case
it often is a nicer to negate it and but it at the end.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Otherwise the created vlan bridge has the default MTU, which is
unexpected when the original bridge has some other MTU configured.
We already do this for the firewall bridges, so we should do so too for
the vlan bridges.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
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>
It's possible to have a
/proc/sys/net/ipv6/ directory
but no
/proc/sys/net/ipv6/conf/$iface/disable_ipv6
Signed-off-by: Alexandre Derumier <aderumier@odiso.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>
