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 2AD3B98902 for ; Wed, 15 Nov 2023 16:12:45 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 0BEE98DE7 for ; Wed, 15 Nov 2023 16:12:45 +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 ; Wed, 15 Nov 2023 16:12:44 +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 0D31D4318B for ; Wed, 15 Nov 2023 16:12:44 +0100 (CET) Message-ID: Date: Wed, 15 Nov 2023 16:12:43 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US To: Thomas Lamprecht , Proxmox VE development discussion , Markus Frank References: <20231115122334.157407-1-m.frank@proxmox.com> <2a01ac45-0918-4973-b20d-8f21cf1dd99c@proxmox.com> <60e53499-e3d2-4f26-8b0c-07b888524c91@proxmox.com> From: Stefan Sterz In-Reply-To: <60e53499-e3d2-4f26-8b0c-07b888524c91@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.086 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 - URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [proxmox.com, jsonschema.pm] 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: Wed, 15 Nov 2023 15:12:45 -0000 On 15.11.23 15:49, Thomas Lamprecht wrote: > Am 15/11/2023 um 14:28 schrieb Stefan Sterz: >> On 15.11.23 13:23, Markus Frank wrote: -- >8 snip 8< -- >> >>> src/PVE/JSONSchema.pm | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm >>> index 49e0d7a..ef58b62 100644 >>> --- 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\-]+$/) { >> >> if i'm not mistaken, this regex should try to filter an `AttributeValue` >> [1]. in case we do stick with this regex approach here, you may want to >> relax this even further, as per the standard: >> >>> If that UTF-8-encoded Unicode string does not have any of the >>> following characters that need escaping, then that string can be used >>> as the string representation >>> of the value. >>> >>> - a space (' ' U+0020) or number sign ('#' U+0023) occurring at >>> the beginning of the string; >>> >>> - a space (' ' U+0020) character occurring at the end of the >>> string; >>> >>> - one of the characters '"', '+', ',', ';', '<', '>', or '\' >>> (U+0022, U+002B, U+002C, U+003B, U+003C, U+003E, or U+005C, >>> respectively); >>> >>> - the null (U+0000) character. >>> > > Ack, so I was wrong, the format might still make sense albeit checking > for above cases would then indeed better, something along the lines of: > > if ($attr !~ /(?:^(?:\s|#))|["+,;<>\0\\]|(?:\s$)/) { > return $attr; > } > > If we leave that regex out completely we should ensure that we don't get > any tainting issues. > > The format could move to PVE::Auth::LDAP too, FWIW, but that's a different > story. just to through this out there, my last attempt at validating this [1] looked something like this: ``` my $escaped = qr!\\(?:[ "#+,;<=>\\]|[0-9a-fA-F]{2})!; my $start = qr!(?:${escaped}|[^"+,;<>\\\0 #])!; my $middle = qr!(?:${escaped}|[^"+,;<>\\\0])!; my $end = qr!(?:${escaped}|[^"+,;<>\\\0 ])!; my $attr_val = qr!("[^"]+"|${start}(?:${middle}*${end})?)!; ``` since things can also be escaped or in quotes, which makes them valid again. could probably be improved here, though. [1]: https://lists.proxmox.com/pipermail/pve-devel/2023-May/056840.html