PATCH: radius: bad_hook program caught signal 15

[Russ Allbery]

Toon van der Pas <toon at hout.vanvergehaald.nl> writes:

> I think I have a patch for the radius authentication client.  I noticed
> that a failed authorisation (because of an invalid password) resulted in
> the log message "bad_hook program caught signal 15".  Because this is
> not a very elegant way of handling an invalid password, I looked in the
> code. What I found is this snippet in radius.c:

> retval = rad_auth(&radconfig, authinfo->username, authinfo->password);
> if (retval == -1)
>     die("user %s password doesn't match", authinfo->username);
> else if (retval == -2)
>     /* couldn't talk to the radius server..  output logged above. */
>     exit(1);
> else if (retval != 0)
>     die("unexpected return code from authentication function: %d",
>         retval);

> The call to die() must be responsible for the signal 15 report.

Signal 15 is SIGTERM.  All that die() does is print out an error to stderr
and then exit(1).  So the only difference between this version and your
patched version is that it calls exit(1) instead of exit(0).  exit(1) is
what the other authenticators do if the password doesn't match.

So I don't think this change is correct and something else must be going
on.

We need to know why this program is catching signal 15, or if it's not,
why nnrpd thinks it is.  The relevant code in nnrpd is this, from perm.c:

    do {
        tmp = waitpid(prog->pid, &status, 0);
    } while ((tmp >= 0 || (tmp < 0 && errno == EINTR)) &&
      !WIFEXITED(status) && !WIFSIGNALED(status));
    if (WIFSIGNALED(status)) {
        ubuf[0] = '\0';
        syslog(L_NOTICE, "%s bad_hook program caught signal %d", ClientHost,
          WTERMSIG(status));
    } else if (WIFEXITED(status)) {
        if (WEXITSTATUS(status) != 0) {
            ubuf[0] = '\0';
            syslog(L_TRACE, "%s bad_hook program exited with status %d",
              ClientHost, WEXITSTATUS(status));
        }
    } else {
        syslog(L_ERROR, "%s bad_hook waitpid failed: %m", ClientHost);
        ubuf[0] = '\0';
    }

(We should probably also define some exit statuses for authenticators that
make it clear whether some sort of system error occurred or if the user
just got the password wrong, but that's future work.)

> This change caused the log messages to change from...

> Feb  8 12:55:15 news nnrpd[20272]: bbned23-32-100.dsl.hccnet.nl auth starting authenticator radius -f /usr/local/news/etc/radius.conf
> Feb  8 12:55:16 news nnrpd[20272]: bbned23-32-100.dsl.hccnet.nl bad_hook program caught signal 15

> ...into...

> Feb  8 14:12:07 news nnrpd[30312]: bbned23-32-100.dsl.hccnet.nl auth starting authenticator radius -f /usr/local/news/etc/radius.conf
> Feb  8 14:12:07 news nnrpd[30312]: bbned23-32-100.dsl.hccnet.nl auth_err user toon password doesn't match

Given that the only difference here is in the exit() call, it's very
mystifying that it would have this effect.

-- 
Russ Allbery (rra at stanford.edu)             <http://www.eyrie.org/~eagle/>

    Please send questions to the list rather than mailing me directly.
     <http://www.eyrie.org/~eagle/faqs/questions.html> explains why.