public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [PATCH access-control] csrf: drop leftover SHA-1 token fallback
@ 2026-10-23 18:57 Mark Atwood
  2026-11-02 14:06 ` Shannon Sterz
  0 siblings, 1 reply; 2+ messages in thread
From: Mark Atwood @ 2026-10-23 18:57 UTC (permalink / raw)
  To: pve-devel

verify_csrf_prevention_token still derives a SHA-1 based secret from the
www key and switches to it for tokens with a 27 character signature, as
issued by PVE 5 and older.

This fallback stopped working when pve-common commit ff39327 ("ticket:
remove fallback for SHA1-base64 CSRF prevention tokens") made
PVE::Ticket::verify_csrf_prevention_token always compute an
HMAC-SHA-256, whose 43 character signature can never match a 27
character one. Such tokens are therefore rejected either way, and the
build dependency on libpve-common-perl (>= 9.1.11) already pulls in that
change.

Remove the dead code, including the FIXME that asked for its removal
with PVE 7. This also drops the last SHA-1 use from CSRF handling.

Signed-off-by: Mark Atwood <mark@wolfssl.com>
---
 src/PVE/AccessControl.pm | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/src/PVE/AccessControl.pm b/src/PVE/AccessControl.pm
index 8879ab8..68c253b 100644
--- a/src/PVE/AccessControl.pm
+++ b/src/PVE/AccessControl.pm
@@ -322,12 +322,10 @@ sub pve_verify_tokenid {
 }
 
 my $csrf_prevention_secret;
-my $csrf_prevention_secret_legacy;
 my $get_csrfr_secret = sub {
     if (!$csrf_prevention_secret) {
         my $input = PVE::Tools::file_get_contents($pve_www_key_fn);
         $csrf_prevention_secret = Digest::SHA::hmac_sha256_base64($input);
-        $csrf_prevention_secret_legacy = Digest::SHA::sha1_base64($input);
     }
     return $csrf_prevention_secret;
 };
@@ -345,15 +343,6 @@ sub verify_csrf_prevention_token {
 
     my $secret = $get_csrfr_secret->();
 
-    # FIXME: remove with PVE 7 and/or refactor all into PVE::Ticket ?
-    if ($token =~ m/^([A-Z0-9]{8}):(\S+)\z/) {
-        my $sig = $2;
-        if (length($sig) == 27) {
-            # the legacy secret got populated by above get_csrfr_secret call
-            $secret = $csrf_prevention_secret_legacy;
-        }
-    }
-
     return PVE::Ticket::verify_csrf_prevention_token(
         $secret, $username, $token, -$auth_graceperiod, $ticket_lifetime, $noerr,
     );

base-commit: dff26b458651b4e1141ff0a41a296d2904b16228
-- 
2.55.0




^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH access-control] csrf: drop leftover SHA-1 token fallback
  2026-10-23 18:57 [PATCH access-control] csrf: drop leftover SHA-1 token fallback Mark Atwood
@ 2026-11-02 14:06 ` Shannon Sterz
  0 siblings, 0 replies; 2+ messages in thread
From: Shannon Sterz @ 2026-11-02 14:06 UTC (permalink / raw)
  To: Mark Atwood, pve-devel

On Fri Oct 23, 2026 at 8:57 PM CEST, Mark Atwood wrote:
> verify_csrf_prevention_token still derives a SHA-1 based secret from the
> www key and switches to it for tokens with a 27 character signature, as
> issued by PVE 5 and older.
>
> This fallback stopped working when pve-common commit ff39327 ("ticket:
> remove fallback for SHA1-base64 CSRF prevention tokens") made
> PVE::Ticket::verify_csrf_prevention_token always compute an
> HMAC-SHA-256, whose 43 character signature can never match a 27
> character one. Such tokens are therefore rejected either way, and the
> build dependency on libpve-common-perl (>= 9.1.11) already pulls in that
> change.
>
> Remove the dead code, including the FIXME that asked for its removal
> with PVE 7. This also drops the last SHA-1 use from CSRF handling.

-->8 snip 8<--

Hey Mark,

thanks for this patch, looks good from my perspective. Did a quick test
and everything still seems to work as expected. Consider this:

Reviewed-by: Shannon Sterz <s.sterz@proxmox.com>
Tested-by: Shannon Sterz <s.sterz@proxmox.com>




^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-11-02 14:06 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-23 18:57 [PATCH access-control] csrf: drop leftover SHA-1 token fallback Mark Atwood
2026-11-02 14:06 ` Shannon Sterz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal