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>

diff --git a/src/PVE/LDAP.pm b/src/PVE/LDAP.pm
index ff98e367e63265bf76c0f302847c3749eea095a6..342c35213d0f325b3fc0d8a30f7cb626563e2c58 100644 (file)
--- a/src/PVE/LDAP.pm
+++ b/src/PVE/LDAP.pm
@@ -80,6 +80,12 @@ sub get_user_dn {
 
 sub auth_user_dn {
     my ($ldap, $dn, $pw, $noerr) = @_;
+
+    if (!$dn) {
+       return undef if $noerr;
+       die "user dn is empty\n";
+    }
+
     my $res = $ldap->bind($dn, password => $pw);
 
     my $code = $res->code;