From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id EA3D69BCAF for ; Tue, 21 Nov 2023 13:56:02 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id C58FF994C for ; Tue, 21 Nov 2023 13:55:32 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Tue, 21 Nov 2023 13:55:32 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 1276441572 for ; Tue, 21 Nov 2023 13:55:32 +0100 (CET) Date: Tue, 21 Nov 2023 13:55:31 +0100 From: Christoph Heiss To: Thomas Lamprecht Cc: Proxmox VE development discussion , Markus Frank Message-ID: References: <20231115122334.157407-1-m.frank@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-SPAM-LEVEL: Spam detection results: 0 AWL 0.003 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - Subject: Re: [pve-devel] [PATCH common] fix #5034 ldap attribute regex X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 21 Nov 2023 12:56:03 -0000 On Wed, Nov 15, 2023 at 02:30:06PM +0100, Thomas Lamprecht wrote: > > Am 15/11/2023 um 13:23 schrieb Markus Frank: > > Change regex from "m/^[a-zA-Z0-9]+$/" to "m/^[a-zA-Z0-9\-]+$/" > > to allow hyphen in ldap attribute names for pve & pmg. > > [..] > > --- a/src/PVE/JSONSchema.pm > > +++ b/src/PVE/JSONSchema.pm > > @@ -408,7 +408,7 @@ PVE::JSONSchema::register_format('ldap-simple-attr', \&verify_ldap_simple_attr); > > sub verify_ldap_simple_attr { > > my ($attr, $noerr) = @_; > > > > - if ($attr =~ m/^[a-zA-Z0-9]+$/) { > > + if ($attr =~ m/^[a-zA-Z0-9\-]+$/) { > > Pre-existing, but shouldn't the regex actually be? > > $attr =~ m/^[a-zA-Z][a-zA-Z0-9\-]*$/ > > I.e., start with a letter and then be any of letter, digit or hyphen (minus). > > CCing Christoph, you did a bit more LDAP stuff recently - opinions? Sorry for the late reply, just saw this now. I'd definitely agree with Stefan here, that moving away from regex's for validating LDAP DNs/attributes/etc is the right way, instead of continuously having to fix them up. Even if we try to follow the RFCs as closely as possible, that does unfortunaly still not really guarantee that it is indeed valid and will be _accepted by the server_. Just doing a basic sanity check (e.g. not empty, no spaces) and then querying the actual LDAP server whether that accepts it or not would be IMHO preferable. Plus, it's less work on our side in the long run. PBS does it no differently too, so I'd go the same way here too. Being overly strict does not help anyone, especially with LDAP, which does a lot of weird things.