retry count not working properly
walterdejong opened this issue · 5 comments
The USAGE file says:
retry = # - allow a number of retries before continuing to the next
authentication module
but this is not entirely true.
Even with retry=1 I can endlessly keep trying to authenticate against our radius server.
Walter de Jong wrote:
The |USAGE| file says:
|retry = # - allow a number of retries before continuing to the next
authentication module
|but this is not entirely true.
Even with |retry=1| I can endlessly keep trying to authenticate against
our radius server.
In what circumstance? Infinite Access-Challenge?
What happens?
I think there are a number of issues at play here that lead to unexpected behavior.
In the code, retry is put into conf->retries, which is passed to talk_radius() as tries. There it becomes server_tries, so I suspect that the "retry" parameter controls the number of retries for an unresponsive server, before moving on to the next server. But the text in USAGE suggests otherwise.
This probably works in conjunction with "infinite Access-Challenge";
* If we get an access challenge, then do a response, for as many
* challenges as we receive.
*/
while (response->code == PW_ACCESS_CHALLENGE) {
...
retval = talk_radius(&config, request, response, resp2challenge, NULL, 1);
And there in that last line, tries is always 1. It is clearly not conf->retries. Even if it were, the while loop does not exit after a number of retries.
I think there have been some mixups in the code about the precise meaning of tries/retries.
Anyway, it does not work as advertised. The net result for the user can look like this (with retry=1):
Failed authentication for user SURFsara/walter. Invalid response to a challenge. Enter a response from your token with serial number **** a
Failed authentication for user SURFsara/walter. Invalid response to a challenge. Enter a response from your token with serial number **** a
Failed authentication for user SURFsara/walter. Invalid response to a challenge. Enter a response from your token with serial number **** a
Failed authentication for user SURFsara/walter. Invalid response to a challenge. Enter a response from your token with serial number **** a
Failed authentication for user SURFsara/walter. Invalid response to a challenge. Enter a response from your token with serial number **** a
Failed authentication for user SURFsara/walter. Invalid response to a challenge. Enter a response from your token with serial number **** a
Failed authentication for user SURFsara/walter. Invalid response to a challenge. Enter a response from your token with serial number **** a
The response "a" is clearly invalid, but this (retarded) server does not Reject. Instead it chooses to Challenge again. The retry parameter does not help me here, although the USAGE seems to imply that it should.
The server is a proprietary server, that I can not change. Nor does it have a parameter to limit the auth attempts ...
Walter de Jong wrote:
I think there are a number of issues at play here that lead to
unexpected behavior.In the code, |retry| is put into |conf->retries|, which is passed to
|talk_radius()| as |tries|. There it becomes |server_tries|, so I
suspect that the "|retry|" parameter controls the number of retries for
an unresponsive server, before moving on to the next server.
It controls how many times one packet is retransmitted to one server.
But the
text in |USAGE| suggests otherwise.
It's not clear.
And there in that last line, |tries| is always |1|. It is clearly not
|conf->retries|. Even if it were, the |while| loop does not exit after a
number of retries.
Because an Access-Challenge isn't a failure.
I think there have been some mixups in the code about the precise
meaning of tries/retries.
Anyway, it does not work as advertised. The net result for the user can
look like this (with |retry=1|):|Failed authentication for user SURFsara/walter. Invalid response to a challenge. Enter a response from your token with serial number **** a
Failed authentication for user SURFsara/walter. Invalid response to a challenge. Enter a response from your token with serial number **** a
Failed authentication for user SURFsara/walter. Invalid response to a challenge. Enter a response from your token with serial number **** a
Failed authentication for user SURFsara/walter. Invalid response to a challenge. Enter a response from your token with serial number **** a
Failed authentication for user SURFsara/walter. Invalid response to a challenge. Enter a response from your token with serial number **** a
Failed authentication for user SURFsara/walter. Invalid response to a challenge. Enter a response from your token with serial number **** a
Failed authentication for user SURFsara/walter. Invalid response to a challenge. Enter a response from your token with serial number **** a
|The response "a" is clearly invalid, but this (retarded) server does not
Reject.
Then fix the retarded server. An Access-Challenge isn't a failure.
Instead it chooses to Challenge again. The |retry| parameter
does not help me here, although the |USAGE| seems to imply that it should.The server is a proprietary server, that I can not change. Nor does it
have a parameter to limit the auth attempts ...
Not really my problem. It's broken. It is indescribably stupid of it
to do challenges forever.
You can submit a patch to allow a limited number of Access-Challenge
packets. I would suggest a default limit of "1". But that is a
completely different configuration parameter than "retries"
OK, so the documentation for parameter retry is incorrect.
The other thing is a different discussion, but now that we're at it ...
I get your point that an Access-Challenge isn't a failure. Will file a bug report with the vendor of the appliance.
The server may be broken but the while loop is not so great either. It relies on the correctness of the server, too much, in my opinion.
Anyway, I have a patch lying around that limits the number of challenges. Will dust it off and submit it.
fixed by commit 50146dc
thanks && greets