From 21f36fed3cfa110849b1a203c09d273d57d09a17 Mon Sep 17 00:00:00 2001 From: Thomas Lamprecht Date: Fri, 15 Dec 2017 06:41:49 +0100 Subject: [PATCH] ticket: raise UNAUTHORIZED not FORBIDDEN in verify subs In the ticket and CSRF prevention token verification methods we used a raise_perm exception to tell our caller about a failure of such a verification. raise_perm uses HTTP_FORBIDDEN (403) as code. Earlier, all such exceptions or die's where caught when the anyevent http server called the auth_handler method and transformed to HTTP_UNAUTHORIZED (401). With commit d8327719e353198a1dffad88c246fee065054a6b from pve-http-server we gained the ability to tell a client about a server internal 5XX error, so that clients do not get wrongly logged out if we have a internal error. This resulted also in the effect that the exceptions of the verify_rsa_ticket and verify_csrf_prevention_token sub methods where passed to the client. If an old, now invalid, ticket was sent to the server a client got 403 (FORBIDDEN) instead of the 401 (UNAUTHORIZED) - which he was used to, and thus meant that he did some wrong doing, instead of knowing that he just needs to login. As we are not yet logged in here, and thus cannot possibly know if the call is forbidden or not, HTTP_FORBIDDEN seems the wrong code. Change it to HTTP_UNAUTHORIZED, which restores it to the code we told API clients since ever and is the correct one here. Also RFC 2068 section 10.4.4 [1] defines that for the afformentioned verify methods FORBIDDEN was not really correct: > 403 Forbidden > > The server understood the request, but is refusing to fulfill it. > Authorization will not help and the request SHOULD NOT be > repeated. [...] With a invalid ticket or CSRF prevention token we have a authorization problem for the current call, not a permission problem (we may have, but we can't tell yet). [1] https://tools.ietf.org/html/rfc2068#section-10.4.4 Signed-off-by: Thomas Lamprecht --- src/PVE/Ticket.pm | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/PVE/Ticket.pm b/src/PVE/Ticket.pm index 68ea285..e9f8e3f 100644 --- a/src/PVE/Ticket.pm +++ b/src/PVE/Ticket.pm @@ -9,10 +9,12 @@ use MIME::Base64; use Digest::SHA; use Time::HiRes qw(gettimeofday); -use PVE::Exception qw(raise_perm_exc); +use PVE::Exception qw(raise); Crypt::OpenSSL::RSA->import_random_seed(); +use constant HTTP_UNAUTHORIZED => 401; + sub assemble_csrf_prevention_token { my ($secret, $username) = @_; @@ -38,7 +40,8 @@ sub verify_csrf_prevention_token { ($age < $max_age); } - raise_perm_exc("Permission denied - invalid csrf token") if !$noerr; + raise("Permission denied - invalid csrf token\n", code => HTTP_UNAUTHORIZED) + if !$noerr; return undef; } @@ -90,7 +93,8 @@ sub verify_rsa_ticket { } } - raise_perm_exc("permission denied - invalid $prefix ticket") if !$noerr; + raise("permission denied - invalid $prefix ticket\n", code => HTTP_UNAUTHORIZED) + if !$noerr; return undef; } -- 2.39.2