From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 4AA3A1FF184 for ; Thu, 20 Nov 2025 13:33:14 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id EF5096799; Thu, 20 Nov 2025 13:33:18 +0100 (CET) Message-ID: <2b34b2be-63a8-43b4-aae3-70532dd4b1d8@proxmox.com> Date: Thu, 20 Nov 2025 13:33:16 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta To: Proxmox VE development discussion , Robert Obkircher References: <20251120121039.100300-1-r.obkircher@proxmox.com> Content-Language: en-US From: Thomas Lamprecht In-Reply-To: <20251120121039.100300-1-r.obkircher@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1763641964875 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.072 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 PROLO_LEO1 0.1 Meta Catches all Leo drug variations so far RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH v3 pve-storage] fix #6900: correctly detect PBS API tokens in storage plugin 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: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" Am 20.11.25 um 13:10 schrieb Robert Obkircher: > The PBS storage plugin used PVE code to detect if an API token was > entered in the username field. This lead to bad requests for some > valid PBS tokens which are not valid PVE tokens. Examples are > "root@pam!1234" and "root@pam!_-". > > Relax the token pattern to allow token names and realms that start > with numbers or underscores. Also allow single character token names, > which are allowed on the backend even though they can't be created > through the PBS Web UI. > > Signed-off-by: Robert Obkircher > --- Please add a short changelog for new patch revisions in this area. Placing it here ensures that it will be visible for review, but doesn't gets picked up by git am, as that info should not contain anything relevant for the persistent git history, rather just meta info for helping review. FWICT it would spell out something like: changelog v2 -> v3: * drop importing unused PVE::Auth::Plugin module in this case. > src/PVE/Storage/PBSPlugin.pm | 19 +++++++++++++++++-- > 1 file changed, 17 insertions(+), 2 deletions(-) > > diff --git a/src/PVE/Storage/PBSPlugin.pm b/src/PVE/Storage/PBSPlugin.pm > index 5842004..17b2e2d 100644 > --- a/src/PVE/Storage/PBSPlugin.pm > +++ b/src/PVE/Storage/PBSPlugin.pm > @@ -701,6 +701,21 @@ my sub snapshot_files_encrypted { > return $any && $all; > } > > +# SAFE_ID_REGEX_STR: &str = r"(?:[A-Za-z0-9_][A-Za-z0-9._\-]*)"; IMO there not much of a point in having the rust code source as literal comment here, that might change and then one might not be able to draw that assumption anymore. Rather, just explicitly state where they come from and why we do not reuse the PVE specific regex here, something like: # We cannot use the PVE API token regexes as we're stricter in PVE, so some tokens that would # be valid for PBS would get rejected. Adapt over the PBS ones from > +my $safe_id_regex = qr/(?:[A-Za-z0-9_][A-Za-z0-9\._\-]*)/; > + > +# TOKEN_NAME_REGEX_STR: = SAFE_ID_REGEX_STR > +my $token_name_regex = $safe_id_regex; > + > +# USER_NAME_REGEX_STR: &str = r"(?:[^\s:/[:cntrl:]]+)"; > +my $user_name_regex = qr/(?:[^\s:\/\p{PosixCntrl}]+)/; > + > +# USER_ID_REGEX_STR: &str = concatcp!(USER_NAME_REGEX_STR, r"@", SAFE_ID_REGEX_STR); > +my $user_id_regex = qr/${user_name_regex}\@${safe_id_regex}/; > + > +# APITOKEN_ID_REGEX_STR: &str = concatcp!(USER_ID_REGEX_STR, r"!", TOKEN_NAME_REGEX_STR); > +my $apitoken_id_regex = qr/${user_id_regex}\!${token_name_regex}/; > + > # TODO: use a client with native rust/proxmox-backup bindings to profit from > # API schema checks and types > my sub pbs_api_connect { > @@ -710,8 +725,8 @@ my sub pbs_api_connect { > > my $user = $scfg->{username} // 'root@pam'; > > - if (my $tokenid = PVE::AccessControl::pve_verify_tokenid($user, 1)) { > - $params->{apitoken} = "PBSAPIToken=${tokenid}:${password}"; > + if ($user =~ qr/^${apitoken_id_regex}$/) { > + $params->{apitoken} = "PBSAPIToken=${user}:${password}"; > } else { > $params->{password} = $password; > $params->{username} = $user; _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel