From: Friedrich Weber <f.weber@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
Christoph Heiss <c.heiss@proxmox.com>
Subject: Re: [pve-devel] [PATCH common/access-control 0/5] improve LDAP DN and bind creds checking on creation/change
Date: Thu, 20 Jul 2023 14:42:10 +0200 [thread overview]
Message-ID: <7e04b1df-0c19-f44c-bfaa-66d35890602d@proxmox.com> (raw)
In-Reply-To: <20230719155157.696828-1-c.heiss@proxmox.com>
Tested against slapd 2.4.47+dfsg-3+deb10u6. I quite like the connection
check when creating/updating the realm, and also, it seems sensible to
delegate DN validation to Net::LDAP.
I noticed one bug: Weirdly, updating the realm via CLI or manually via
API now errors out for me (the connection details are correct):
$ cat /etc/pve/domains.cfg
pam: pam
comment Linux PAM standard authentication
pve: pve
comment Proxmox VE authentication server
default 0
ldap: ldap
comment foo
base_dn dc=example,dc=com
server1 [...]
user_attr uid
bind_dn cn=admin,dc=example,dc=com
default 0
secure 0
$ pveum realm modify ldap -comment foo
update auth server failed: Expected 'PeerHost' at
/usr/share/perl5/Net/LDAP.pm line 173.
$ http --verify no PUT
'https://[...]:8006/api2/json/access/domains/ldap' comment=foo [...]
HTTP/1.1 500 update auth server failed: Expected 'PeerHost' at
/usr/share/perl5/Net/LDAP.pm line 173.
On 19/07/2023 17:51, Christoph Heiss wrote:
> tl;dr implements the result of the discussion in [0].
>
> First, this removes the dreaded LDAP DN regex, replacing it instead with
> a proper schema format, which does validation using
> Net::LDAP::Util::canonical_dn().
>
> Further, upon saving a LDAP realm in the UI, it tries to connect & bind
> using the provided credentials, providing the user with immediate
> feedback whether they are valid or not.
>
> The same approach is already implemented in PBS [1].
>
> Changes was done against slapd 2.5.13+dfsg-5.
>
> [0] https://lists.proxmox.com/pipermail/pve-devel/2023-May/056839.html
> [1] https://git.proxmox.com/?p=proxmox-backup.git;a=commitdiff;h=5210f3b5
> [1] https://lists.proxmox.com/pipermail/pbs-devel/2023-June/006237.html
>
> pve-common:
>
> Christoph Heiss (3):
> schema: add `ldap-dn` format for validating LDAP distinguished names
> test: add test cases for new 'ldap-dn' schema format
> ldap: handle errors explicitly everywhere instead of simply `die`ing
>
> debian/control | 2 ++
> src/PVE/JSONSchema.pm | 12 +++++++++
> src/PVE/LDAP.pm | 11 ++++----
> test/Makefile | 9 +++++++
> test/ldap_dn_format_test.pl | 54 +++++++++++++++++++++++++++++++++++++
> 5 files changed, 83 insertions(+), 5 deletions(-)
>
> pve-access-control:
>
> Christoph Heiss (2):
> ldap: validate LDAP DNs using the `ldap-dn` schema format
> ldap: check bind credentials with LDAP directory directly on change
>
> src/PVE/Auth/LDAP.pm | 28 ++++++++++++++++------------
> 1 file changed, 16 insertions(+), 12 deletions(-)
>
> --
> 2.41.0
>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
next prev parent reply other threads:[~2023-07-20 12:42 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-19 15:51 Christoph Heiss
2023-07-19 15:51 ` [pve-devel] [PATCH common 1/5] schema: add `ldap-dn` format for validating LDAP distinguished names Christoph Heiss
2023-07-19 15:51 ` [pve-devel] [PATCH common 2/5] test: add test cases for new 'ldap-dn' schema format Christoph Heiss
2023-07-19 15:51 ` [pve-devel] [PATCH common 3/5] ldap: handle errors explicitly everywhere instead of simply `die`ing Christoph Heiss
2023-07-19 15:51 ` [pve-devel] [PATCH access-control 4/5] ldap: validate LDAP DNs using the `ldap-dn` schema format Christoph Heiss
2023-07-19 15:51 ` [pve-devel] [PATCH access-control 5/5] ldap: check bind credentials with LDAP directory directly on change Christoph Heiss
2023-07-20 12:42 ` Friedrich Weber [this message]
2023-07-20 13:30 ` [pve-devel] [PATCH common/access-control 0/5] improve LDAP DN and bind creds checking on creation/change Christoph Heiss
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7e04b1df-0c19-f44c-bfaa-66d35890602d@proxmox.com \
--to=f.weber@proxmox.com \
--cc=c.heiss@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.