Fri 30 Nov 2007 12:27:44 AM UTC, original submission:
The code in upap_rauthnak() is as follows:
static void upap_rauthnak(upap_state u, u_char inp,
int id, int len)
{
....
/*
* Parse message.
*/
if (len < sizeof (u_char)) {
UPAPDEBUG((LOG_INFO,
"pap_rauthnak: rcvd short packet.\n"));
return;
}
...
u->us_clientstate = UPAPCS_BADAUTH;
UPAPDEBUG((LOG_ERR, "PAP authentication failed\n"));
auth_withpeer_fail(u->us_unit, PPP_PAP);
}
Unfortunately, at the above point "len" is the length of the PAP packet MINUS the four-byte NAK header. There's no requirement for the server to send anything more in this packet. I assume some servers do provide an explanatory message (no user, bad password etc), but the one we're working with doesn't.
So it drops a good packet and ignores it. It doesn't set the client state or call auth_withpeer_fail() and that means it doesn't record WHY the link dropped out in the error callbacks and status messages.
This isn't a showstopper as auth_withpeer_fail() then does:
* We've failed to authenticate ourselves to our peer.
* He'll probably take the link down, and there's not much
* we can do except wait for that.
I guess most peers tear the link down. Ours doesn't (it has bugs as well, but they're not ours to fix).
The same function in the PPPD 2.4.4 code is as follows:
static void
upap_rauthnak(u, inp, id, len)
upap_state *u;
u_char *inp;
int id;
int len;
{
u_char msglen;
char *msg;
if (u->us_clientstate != UPAPCS_AUTHREQ) /* XXX */
return;
/*
* Parse message.
*/
if (len < 1) {
UPAPDEBUG(("pap_rauthnak: ignoring missing msg-length."));
} else {
GETCHAR(msglen, inp);
if (msglen > 0) {
len -= sizeof (u_char);
if (len < msglen) {
UPAPDEBUG(("pap_rauthnak: rcvd short packet."));
return;
}
msg = (char *) inp;
PRINTMSG(msg, msglen);
}
}
u->us_clientstate = UPAPCS_BADAUTH;
error("PAP authentication failed");
auth_withpeer_fail(u->us_unit, PPP_PAP);
}
BTW the current PPPD 2.4.4 code calls "lcp_close()" in its auth_withpeer_fail() code, as does mine now.
I don't have CVS access, I'm not up to date on the latest source and I've diverged from the CVS base code too far for me to be able to fix or change it.
|