-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
On 25/11/15 22:47, Russ Allbery wrote:
> Robert Bradley <robert.brad...@it.ox.ac.uk> writes:
>
>> I'm currently in the process of performing an upgrade of our
>> Webauth cluster from Webauth 3.7.4 (Debian squeeze) to 4.6.1
>> (Debian jessie). This process has gone very well despite the
>> large jump in versions and the switch to Template Toolkit for
>> templating. However, I have run into a small problem with login
>> timeouts. At present, if a user leaves the login page for over 5
>> minutes before entering their username and password, they get a
>> message saying that they took too long to log in. When I try the
>> same thing with the new Webauth 4 servers though, the user is
>> successfully logged in. I can replicate this easily even when
>> setting "WebKdcTokenMaxTTL 30s" and "WebKdcLoginTimeLimit 30s" in
>> Apache to deliberately reduce the timeouts.
>
>> There is probably some misunderstanding on my part here, but I
>> was wondering if anyone else had seen this?
>
> WebKdcTokenMaxTTL should be controlling this....
> (WebKdcLoginTimeLimit is something different.)
>
> Oh.
>
> So, this patch might help:
>
> --- a/modules/webkdc/mod_webkdc.c +++
> b/modules/webkdc/mod_webkdc.c @@ -990,9 +990,11 @@
> parse_request_token(MWK_REQ_CTXT *rc, const char *token, /* Copy
> the token and do some additional checks. */ *rt =
> &data->token.request; expiration = (*rt)->creation +
> rc->sconf->token_max_ttl; - if (expiration < time(NULL)) + if
> (expiration < time(NULL)) { set_errorResponse(rc,
> WA_PEC_REQUEST_TOKEN_STALE, "request token was stale", mwk_func,
> false); + return MWK_ERROR; + } return MWK_OK; }
>
> Looks like I introduced that bug in WebAuth 4.0.0 and no one ever
> noticed. (Note that there's no actual private data in the login
> request, so it's not clear that there's any important security
> reason to use an aggressive timeout here.)
>
Thanks! That patch solves the problem perfectly, although I
personally agree that having a time limit to login isn't all that
important. Is this likely to be added to the Debian packages at some
point?
Incidentally, while I was testing, I also saw several complaints from
CGI::param in my Apache logs:
FastCGI: server "/usr/share/webkdc/cgi/login.fcgi" stderr: CGI::param
called in list context from package WebLogin line 1615, this can lead
to vulnerabilities. See the warning in "Fetching the value or values
of a single named parameter" at /usr/share/perl5/CGI.pm line 436.
The following patch seems to silence the log noise, assuming that line
wraps do not break it. However, it's worth double-checking it to make
sure I'm not forcing too much to be scalar:
- ----
diff --git a/perl/lib/WebLogin.pm b/perl/lib/WebLogin.pm
index 43c523f..4274cf6 100644
- --- a/perl/lib/WebLogin.pm
+++ b/perl/lib/WebLogin.pm
@@ -194,7 +194,7 @@ sub cgiapp_prerun {
# Store the CPT if one was already generated, so that we have one
place to
# check.
- - $self->param ('CPT', $self->query->param ('CPT'));
+ $self->param ('CPT', scalar $self->query->param ('CPT'));
# Work around a bug in CGI that doesn't always set the script name.
$self->query->{'.script_name'} = $ENV{SCRIPT_NAME};
@@ -432,7 +432,7 @@ sub print_headers {
# Set the test cookie unless it's already set.
unless ($q->cookie ($self->param ('test_cookie'))) {
- - my $cookie = $q->cookie (-name => $self->param
('test_cookie'),
+ my $cookie = $q->cookie (-name => scalar $self->param
('test_cookie'),
-value => 'True',
-secure => $secure,
-httponly => 1);
@@ -1077,8 +1077,8 @@ sub print_remuser_redirect {
$self->template_params ({err_msg => $errmsg});
return $self->print_error_page;
} else {
- - $uri .= "?RT=" . $self->fix_token ($q->param ('RT')) .
- - ";ST=" . $self->fix_token ($q->param ('ST'));
+ $uri .= "?RT=" . $self->fix_token (scalar $q->param ('RT')) .
+ ";ST=" . $self->fix_token (scalar $q->param ('ST'));
print STDERR "redirecting to $uri\n" if $self->param ('debug');
return $self->redirect ($uri);
}
@@ -1608,19 +1608,19 @@ sub setup_kdc_request {
my $q = $self->query;
# Set up the parameters to the WebKDC request.
- - $self->{request}->service_token ($self->fix_token ($q->param ('ST')))
+ $self->{request}->service_token ($self->fix_token (scalar
$q->param ('ST')))
if $q->param ('ST');
- - $self->{request}->request_token ($self->fix_token ($q->param ('RT')))
+ $self->{request}->request_token ($self->fix_token (scalar
$q->param ('RT')))
if $q->param ('RT');
- - $self->{request}->pass ($q->param ('password'))
+ $self->{request}->pass (scalar $q->param ('password'))
if $q->param ('password');
- - $self->{request}->otp ($q->param ('otp'))
+ $self->{request}->otp (scalar $q->param ('otp'))
if $q->param ('otp');
- - $self->{request}->otp_type ($q->param ('factor_type'))
+ $self->{request}->otp_type (scalar $q->param ('factor_type'))
if $q->param ('factor_type');
- - $self->{request}->authz_subject ($q->param ('authz_subject'))
+ $self->{request}->authz_subject (scalar $q->param ('authz_subject'))
if $q->param ('authz_subject');
- - $self->{request}->login_state ($q->param ('LS'))
+ $self->{request}->login_state (scalar $q->param ('LS'))
if $q->param ('LS');
# For the initial login page and password change page, we may
need to map
@@ -1641,7 +1641,7 @@ sub setup_kdc_request {
}
$q->param ('username', $username);
}
- - $self->{request}->user ($q->param ('username')) if $q->param
('username');
+ $self->{request}->user (scalar $q->param ('username')) if
$q->param ('username');
# Check for replays or rate limiting of failed authentications
for the
# initial login page, the multifactor login page, and the
multifactor_send
- ----
- --
Dr Robert Bradley
Identity and Access Management, IT Services, University of Oxford
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
iQIcBAEBCAAGBQJWVzaeAAoJEJRhp8p2r+O++pUP/1lTBCgiIGUJlD1OmQA9bPaw
JXmN6tIS2zoBAg9l81+lZ0P2nivyHFsL0e3aOAj52yq0gfkdfnb6oahHzhtlNM+d
G88bamQucGoymznnYoWx3+PMHHFINCOqR4pILwLQx7sSM12J2ejlF+pTHQgaPnfI
AX880ygjoRHiUyGS9ZKnB6IZ6i5aVRnDX3puKEZPMEOrtY4bCQpI/1Rx1FVLYHA5
JE3ffpQA92+36qEOyBFnyH5vMbXWhdL6uay0xhg8TxVN80JFNhn92xGmIHpS1bBN
9eiEv8PZt+HpEYEvQXE2j0ZverQstCSZzkuyMAD2Vj2xJvT0FB9FsqneymGVQz4B
SNe385yq19BjwhCha1eE8ikuh8jh1K/0atc33ohT0AS4Cmyz6qgcphSk43ENhfZ+
aM/TF3ZNAX9snbtG/94kUppkGNCeMiVoQYdizIbm3bS4LazJ/LsoIckAVA5WMCDb
DpMzz+n1sIhAIGqrc0jk4yZPMTO6FS8EjU8ArsXVCYoAwEc5nGK/cjma8jU9yPCh
Ts9wdQBLHjzTSSxpKrnjgZ6cMFcX9/AWRGIB4dJjzXbz8FcU7Zsk9voVIaS1DCGv
rcg75GGdl8QJDZzIykuQV4fBXCXIPWBYk41DHwXQeypugL4BLqtFH0LZ0RzBAdNl
Ugs4hGHZTYXWgSrzW6FL
=dBD8
-----END PGP SIGNATURE-----
