Re: Missing nas->strvalue in add_nas_attr, bad EAP request ID
Dave Mason <[EMAIL PROTECTED]> wrote: > I checked the new one and it looks good. Thanks. > One a semi-related note, I tried doing a make on today's CVS with > disable-shared as a config option. I thought I could do an nm on > radiusd and see symbol definitions for EAP subtype functions, like > eapmd5_alloc, but they werent there. Is that expected? Yes. Static libs in rlm_eap need fixing... > PS: While I'm here, it looks like I still won't get to that RLM_MODULE > fix for EAP for a while. I still need to do it but I keep getting other > work dumped my way. Probably you can relate to that. :) Oh, yes. I just turned down an offer to go to Vienna to give a talk, because I'm too busy. Alan DeKok. - List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
Re: Missing nas->strvalue in add_nas_attr, bad EAP request ID
Hi Alan, I checked the new one and it looks good. One a semi-related note, I tried doing a make on today's CVS with disable-shared as a config option. I thought I could do an nm on radiusd and see symbol definitions for EAP subtype functions, like eapmd5_alloc, but they werent there. Is that expected? I checked the .libs directories for the subtypes and the .a libraries there look good. Functions in rlm_eap such as eap_authenticate are linked into radiusd. Regards, Dave PS: While I'm here, it looks like I still won't get to that RLM_MODULE fix for EAP for a while. I still need to do it but I keep getting other work dumped my way. Probably you can relate to that. :) Dave Mason <[EMAIL PROTECTED]> wrote: I just grabbed the June 26 CVS and eap.c looks like the original unpatched version. Yes, because I said I "just" added it. The snapshot from today should have the fix. Alan DeKok. - List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
Re: Missing nas->strvalue in add_nas_attr, bad EAP request ID
Dave Mason <[EMAIL PROTECTED]> wrote: > I just grabbed the June 26 CVS and eap.c looks like the original > unpatched version. Yes, because I said I "just" added it. The snapshot from today should have the fix. Alan DeKok. - List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
Re: Missing nas->strvalue in add_nas_attr, bad EAP request ID
I just grabbed the June 26 CVS and eap.c looks like the original unpatched version. Dave Alan DeKok wrote: Dave Mason <[EMAIL PROTECTED]> wrote: That worked like a champ. Here's my new code, with the old code commented out. From eap.c: ... I've added that patch to eap.c. Please double-check that it works before 0.9 is released. Alan DeKok. -- Dave Mason (817)481-4412 x139 voice, (817)481-4461 fax, [EMAIL PROTECTED] Transat Technologies180 State St, Suite 240, Southlake, TX 76092 Integrating 3GSM and WLANhttp://www.transat-tech.com - List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
Re: Missing nas->strvalue in add_nas_attr, bad EAP request ID
Dave Mason <[EMAIL PROTECTED]> wrote: > That worked like a champ. Here's my new code, with the old code > commented out. From eap.c: ... I've added that patch to eap.c. Please double-check that it works before 0.9 is released. Alan DeKok. - List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
Re: Missing nas->strvalue in add_nas_attr, bad EAP request ID
Hi Alan,
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->leng
Re: Missing nas->strvalue in add_nas_attr, bad EAP request ID
Dave Mason <[EMAIL PROTECTED]> wrote: > Hi - any thoughts on this? I'm curious if there's a bug here or if > everything is as intended. It's a bug. I'll update rlm_preprocess in the CVS head. Alan DeKok. - List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
Re: Missing nas->strvalue in add_nas_attr, bad EAP request ID
Hi - any thoughts on this? I'm curious if there's a bug here or if everything is as intended. Regards, Dave Dave Mason wrote: Hi, I'm working on a new EAP type, and using the supplied radclient for testing. I didnt add a NAS-IP-Address attribute to the Access Request (and radclient doesnt add one for you), so the add_nas_attr function in rlm_preprocess.c adds one to the request. Notice that no strvalue is added for NAS-IP-Address, only an lvalue. Later, the same function also makes a Client-IP-Address attribute with both an lvalue and strvalue. In the EAP code that matches a new EAP response with a request sent previously, the nas->strvalue is used as part of the identifier in eap_generateid and eap_regenerateid. This code is at the bottom of eap.c. These functions check the strvalue, not the lvalue as I would expect. For my case, the strvalue is all 0s, though the lvalue is set. They match up OK but only because they both have the default 0 value. Is either or both of those a problem? Dave - List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
