That worked like a champ. Here's my new code, with the old code commented out. From eap.c:
unsigned char *eap_regenerateid(REQUEST *request, unsigned char response_id)
{
//VALUE_PAIR *nas = NULL;
VALUE_PAIR *state = NULL;
unsigned char *id = NULL;#ifdef XXREMOVE
/* This check should be in the server code */
nas = pairfind(request->packet->vps, PW_NAS_IP_ADDRESS);
if (nas == NULL) {
nas = pairfind(request->packet->vps, PW_NAS_IDENTIFIER);
if (nas == NULL) {
radlog(L_ERR, "rlm_eap: Invalid RADIUS packet."
" Both NAS-IP-Address & NAS-Identifier "
"are missing");
return NULL;
}
}
#endif state = pairfind(request->packet->vps, PW_STATE);
if (state == NULL) {
radlog(L_INFO, "rlm_eap: NO State Attribute found.");
return NULL;
}
if (verify_state(state) != 0) {
radlog(L_ERR, "rlm_eap: State verification failed.");
return NULL;
}//id = (unsigned char *)malloc(1/*Length*/ + 1/*Id*/ + state->length + nas->length);
id = (unsigned char *)malloc(1/*Length*/ + 1/*Id*/ + state->length + sizeof(request->packet->src_ipaddr));
if (id == NULL) {
radlog(L_ERR, "rlm_eap: out of memory");
return NULL;
}
/*
* Generate unique-id to check for the reply
* id = Length + ID + State + (NAS-IP-Address | NAS-Identifier)
*/
//id[0] = (1 + 1 + state->length + nas->length) & 0xFF;
id[0] = (1 + 1 + state->length + sizeof(request->packet->src_ipaddr))
& 0xFF;
memcpy(id+1, &response_id, sizeof(unsigned char));
memcpy(id+2, state->strvalue, state->length);
//memcpy(id+2+state->length, nas->strvalue, nas->length);
memcpy(id+2+state->length, &request->packet->src_ipaddr,
sizeof(request->packet->src_ipaddr));return id; }
unsigned char *eap_generateid(REQUEST *request, unsigned char response_id)
{
//VALUE_PAIR *nas = NULL;
VALUE_PAIR *state = NULL;
unsigned char *id = NULL;#ifdef XXREMOVE
/* This check should be in the server code */
nas = pairfind(request->packet->vps, PW_NAS_IP_ADDRESS);
if (nas == NULL) {
nas = pairfind(request->packet->vps, PW_NAS_IDENTIFIER);
if (nas == NULL) {
radlog(L_ERR, "rlm_eap: Invalid RADIUS packet."
" Both NAS-IP-Address & NAS-Identifier "
"are missing");
return NULL;
}
}
#endif state = pairfind(request->reply->vps, PW_STATE);
if (state == NULL) {
radlog(L_INFO, "rlm_eap: NO State Attribute found.");
return NULL;
}//id = (unsigned char *)malloc(1/*Length*/ + 1/*Id*/ + state->length + nas->length);
id = (unsigned char *)malloc(1/*Length*/ + 1/*Id*/ + state->length + sizeof(request->packet->src_ipaddr));
if (id == NULL) {
radlog(L_ERR, "rlm_eap: out of memory");
return NULL;
}
/*
* Generate unique-id to check for the reply
* id = Length + ID + State + (NAS-IP-Address | NAS-Identifier)
*/
//id[0] = (1 + 1 + state->length + nas->length) & 0xFF;
id[0] = (1 + 1 + state->length + sizeof(request->packet->src_ipaddr))
& 0xFF;
memcpy(id+1, &response_id, sizeof(unsigned char));
memcpy(id+2, state->strvalue, state->length);
//memcpy(id+2+state->length, nas->strvalue, nas->length);
memcpy(id+2+state->length, &request->packet->src_ipaddr,
sizeof(request->packet->src_ipaddr));return id; }
Thanks, Dave
Alan DeKok wrote:
Dave Mason <[EMAIL PROTECTED]> wrote:
...This is an old post from January. At the time you agreed it was a bug and updated the CVS, but today I had a fresh look and I'm not sure it's completely resolved. First, here is the relevant code from rlm_preprocess.c:
That code should be OK.
I found that the strvalue is determined correctly but only the first 4 characters are used by the EAP code that compares the two NAS-IP-Addresses.
Then that's a bug in rlm_eap, NOT rlm_preprocess.
If you get in gdb and do a "p *nas" right before the pairadd, you see that the length is 4. This may be an attribute of PW_TYPE_IPADDR?
Yes. The length is supposed to be 4.
As I mentioned earlier, it would also be possible for eap_regenerateid to use the lvalue instead of the strvalue - one's as good as another as long as it works.
Looking at the cureent code in rlm_eap, I believe it to be incorrect. It should NOT be relying on the contents of the RADIUS packet to determine the 'session id/state' it uses internally to keep track of the EAP sessions. A NAS may send different attributes in different messages.
I would suggest changing rlm_eap/eap.c to use request->packet->src_ipaddr, instead of PW_NAS_IP_ADDRESS and PW_NAS_IDENTIFIER. That is, get rid of all of the logic related to the 'nas' variable in eap_regenerateid() and eap_generateid(), and change the final memcpy's from:
memcpy(id+2+state->length, nas->strvalue, nas->length);
to:
memcpy(id+2+state->length, request->packet->src_ipaddr, sizeof(request->packet->src_ipaddr));
If that works for you, I'll update the eap module in CVS.
Alan DeKok.
- List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
