Re: [pve-devel] [PATCH pve-storage] fix #6900: correctly detect PBS API tokens in storage plugin

[Robert Obkircher <r.obkircher@proxmox.com>]

On 11/11/25 13:24, Fabian Grünbichler wrote:
> On November 3, 2025 3:30 pm, Robert Obkircher wrote:
>> 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.
>>
>> Relax the token pattern to allow token names that start with numbers
>> or underscores. Also allow single character names, which are
>> technically allowed on the Rust side even though they can't be created
>> through the PBS Web UI.
>>
>> Signed-off-by: Robert Obkircher <r.obkircher@proxmox.com>
>> ---
>>   src/PVE/Storage/PBSPlugin.pm | 24 +++++++++++++++++++++++-
>>   1 file changed, 23 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/PVE/Storage/PBSPlugin.pm b/src/PVE/Storage/PBSPlugin.pm
>> index 5842004..892b4d5 100644
>> --- a/src/PVE/Storage/PBSPlugin.pm
>> +++ b/src/PVE/Storage/PBSPlugin.pm
>> @@ -14,6 +14,7 @@ use POSIX qw(mktime strftime ENOENT);
>>   use POSIX::strptime;
>>   
>>   use PVE::APIClient::LWP;
>> +use PVE::Auth::Plugin;
>>   use PVE::JSONSchema qw(get_standard_option);
>>   use PVE::Network;
>>   use PVE::PBSClient;
>> @@ -701,6 +702,27 @@ my sub snapshot_files_encrypted {
>>       return $any && $all;
>>   }
>>   
>> +# On the Rust side this is TOKEN_NAME_REGEX_STR: = SAFE_ID_REGEX_STR
>> +# which is = r"(?:[A-Za-z0-9_][A-Za-z0-9._\-]*)";
>> +our $token_subid_regex = qr/[A-Za-z0-9_][A-Za-z0-9\.\-_]*/;
>> +
>> +our $token_full_regex =
>> +    qr/((${PVE::Auth::Plugin::user_regex})\@(${PVE::Auth::Plugin::realm_regex}))!(${token_subid_regex})/;
> nit: these two don't need to be "our"
>
> did you verify the other two parts here are identical between PVE and
> PBS?
They were in fact slightly different. I sent a v3 where I ported all 
relevant regular expressions.

https://lore.proxmox.com/pve-devel/20251120121039.100300-1-r.obkircher@proxmox.com/

>
>> +
>> +# Similar to PVE::AccessControl::pve_verify_tokenid, except that this
>> +# also allows the subid to start with numbers or underscores.
>> +sub pbs_verify_tokenid {
> nit: and this could be a private helper, unless we expect a need to verify
> this outside as well?
>
>> +    my ($tokenid, $noerr) = @_;
>> +
>> +    if ($tokenid =~ /^${token_full_regex}$/) {
>> +        return wantarray ? ($tokenid, $2, $3, $4) : $tokenid;
>> +    }
>> +
>> +    die "value '$tokenid' does not look like a valid token ID\n" if !$noerr;
>> +
>> +    return undef;
>> +}
>> +
>>   # TODO: use a client with native rust/proxmox-backup bindings to profit from
>>   # API schema checks and types
>>   my sub pbs_api_connect {
>> @@ -710,7 +732,7 @@ my sub pbs_api_connect {
>>   
>>       my $user = $scfg->{username} // 'root@pam';
>>   
>> -    if (my $tokenid = PVE::AccessControl::pve_verify_tokenid($user, 1)) {
>> +    if (my $tokenid = pbs_verify_tokenid($user, 1)) {
>>           $params->{apitoken} = "PBSAPIToken=${tokenid}:${password}";
>>       } else {
>>           $params->{password} = $password;
>> -- 
>> 2.47.3
>>
>>
>>
>> _______________________________________________
>> pve-devel mailing list
>> pve-devel@lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>>
>>
>>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel