applied On Fri, Dec 15, 2017 at 06:41:49AM +0100, Thomas Lamprecht wrote: > 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 <t.lampre...@proxmox.com> > --- > > changes v1 -> v2: > * fix forgotten change to import raise not raise_perm_exc now, was > unseen as it was imported by our users (AnyEvent) and thus I did > not saw an error... > > 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.11.0
_______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel