[PATCH common 1/2] ticket: accept RSA tickets signed with SHA-256

[Mark Atwood <mark@wolfssl.com>]

Crypt::OpenSSL::RSA signs and verifies with SHA-1 unless told otherwise,
so all RSA tickets (login, TFA, VNC, tunnel) are RSA PKCS#1 v1.5
signatures over SHA-1. NIST SP 800-131A Rev. 2 no longer allows SHA-1 for
creating digital signatures. FIPS providers for OpenSSL 3 can enforce
this, for example wolfProvider, or the OpenSSL FIPS provider with its
signature digest check enabled. With such a provider no ticket can be
issued and nobody can log in.

As a first step, make verification accept tickets signed with SHA-256 in
addition to SHA-1. Once all nodes of a cluster run a version with this
change, the signing side can switch to SHA-256 without locking users out
of nodes that were not yet upgraded.

The hash is set explicitly before each attempt, since callers cache and
reuse the key object. A provider that does not allow SHA-1 raises an
error from verify() instead of returning false, so the call is wrapped
in eval and such a ticket is treated like any other invalid ticket.

Add a test that signs tickets directly with Crypt::OpenSSL::RSA using
either hash, and checks that they are accepted and that modified,
expired, foreign-key or wrong-secret tickets are rejected.

Signed-off-by: Mark Atwood <mark@wolfssl.com>
---
 src/PVE/Ticket.pm   | 15 +++++++++-
 test/Makefile       |  1 +
 test/ticket_test.pl | 68 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 83 insertions(+), 1 deletion(-)
 create mode 100755 test/ticket_test.pl

diff --git a/src/PVE/Ticket.pm b/src/PVE/Ticket.pm
index 764c02d..f700f39 100644
--- a/src/PVE/Ticket.pm
+++ b/src/PVE/Ticket.pm
@@ -79,8 +79,21 @@ sub verify_rsa_ticket {
         my $sig = $2;
 
         my $full = defined($secret_data) ? "$plain:$secret_data" : $plain;
+        my $raw_sig = decode_base64($sig);
+
+        # Older versions sign with SHA-1, so during a rolling upgrade a cluster has tickets
+        # with either hash. A provider that refuses SHA-1 raises an error rather than
+        # returning false, which simply makes such a ticket invalid.
+        my $valid;
+        for my $use_hash ('use_sha256_hash', 'use_sha1_hash') {
+            $valid = eval {
+                $rsa_pub->$use_hash();
+                $rsa_pub->verify($full, $raw_sig);
+            };
+            last if $valid;
+        }
 
-        if ($rsa_pub->verify($full, decode_base64($sig))) {
+        if ($valid) {
             if ($plain =~ m/^\Q$prefix\E:(?:(\S+):)?([A-Z0-9]{8})$/) {
                 my $data = $1; # Note: not all tickets contains data
                 my $timestamp = $2;
diff --git a/test/Makefile b/test/Makefile
index 1bce185..7eac9ea 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -16,6 +16,7 @@ TESTS = lock_file.test			\
 	section_config_property_isolation_test.test \
 	file-test.test \
 	cmd-test.test \
+	ticket_test.test \
 
 all:
 
diff --git a/test/ticket_test.pl b/test/ticket_test.pl
new file mode 100755
index 0000000..37e30b4
--- /dev/null
+++ b/test/ticket_test.pl
@@ -0,0 +1,68 @@
+#!/usr/bin/perl
+
+# Tests for signing and verifying RSA tickets with PVE::Ticket
+
+use v5.36;
+
+use lib '../src';
+
+use Crypt::OpenSSL::RSA;
+use MIME::Base64;
+use Test::More;
+
+use PVE::Ticket;
+
+my $rsa_priv = Crypt::OpenSSL::RSA->generate_key(2048);
+my $rsa_pub = Crypt::OpenSSL::RSA->new_public_key($rsa_priv->get_public_key_string());
+
+# sign like a node would, with an explicitly chosen hash, independent of PVE::Ticket
+sub sign_ticket($priv, $hash, $plain, $secret_data = undef) {
+    my $full = defined($secret_data) ? "$plain:$secret_data" : $plain;
+    my $use_hash = "use_${hash}_hash";
+    $priv->$use_hash();
+    return $plain . "::" . encode_base64($priv->sign($full), '');
+}
+
+my $now = sprintf("%08X", time());
+my $old = sprintf("%08X", time() - 7200);
+
+for my $hash (qw(sha256 sha1)) {
+    my $ticket = sign_ticket($rsa_priv, $hash, "PVE:root\@pam:$now");
+    my ($data, $age) = PVE::Ticket::verify_rsa_ticket($rsa_pub, 'PVE', $ticket, undef, -1, 60, 1);
+    is($data, 'root@pam', "$hash signed ticket is accepted");
+    ok(defined($age) && $age >= 0 && $age < 60, "$hash signed ticket has plausible age");
+
+    $ticket = sign_ticket($rsa_priv, $hash, "PVEVNC:$now", 'vnc-secret');
+    ok(
+        PVE::Ticket::verify_rsa_ticket($rsa_pub, 'PVEVNC', $ticket, 'vnc-secret', -20, 40, 1),
+        "$hash signed ticket with secret data is accepted",
+    );
+    ok(
+        !PVE::Ticket::verify_rsa_ticket($rsa_pub, 'PVEVNC', $ticket, 'other', -20, 40, 1),
+        "$hash signed ticket with wrong secret data is rejected",
+    );
+
+    $ticket = sign_ticket($rsa_priv, $hash, "PVE:root\@pam:$now");
+    (my $tampered = $ticket) =~ s/^PVE:root\@pam:/PVE:admin\@pam:/;
+    ok(
+        !PVE::Ticket::verify_rsa_ticket($rsa_pub, 'PVE', $tampered, undef, -1, 60, 1),
+        "$hash signed ticket with modified data is rejected",
+    );
+    eval { PVE::Ticket::verify_rsa_ticket($rsa_pub, 'PVE', $tampered, undef, -1, 60) };
+    like($@, qr/invalid PVE ticket/, "$hash signed modified ticket raises error without noerr");
+
+    $ticket = sign_ticket($rsa_priv, $hash, "PVE:root\@pam:$old");
+    ok(
+        !PVE::Ticket::verify_rsa_ticket($rsa_pub, 'PVE', $ticket, undef, -1, 60, 1),
+        "$hash signed expired ticket is rejected",
+    );
+
+    my $other_priv = Crypt::OpenSSL::RSA->generate_key(2048);
+    $ticket = sign_ticket($other_priv, $hash, "PVE:root\@pam:$now");
+    ok(
+        !PVE::Ticket::verify_rsa_ticket($rsa_pub, 'PVE', $ticket, undef, -1, 60, 1),
+        "$hash signed ticket from another key is rejected",
+    );
+}
+
+done_testing();
-- 
2.55.0