Hi Stefan, I am using your patch [from Apr 21] to address the problem of the unknown vendor. I have also used the same approach for standard attributes we want to ignore [Microsoft Server 2008 defaults to sending the "class" attribute which we don't have support for]. The additional problem I was talking about was to do with the effect of trying to ignore the last attribute in the list. In this case, pair is returned but it still set to null. My solution to this - not necessarily the way forward, but at least a workaround - was to rename rc_avpair_gen() to old_rc_avpair_gen() and then wrap it a new version of rc_avpair_gen() which adds a dummy pair and then removes it when old_rc_avpair_gen() returns. For what it's worth, the code for (the new) rc_avpair_gen() is as follows: /* * * Function: rc_avpair_gen * * * there's a bug in rc_avpair_gen() [now renamed old_rc_avpair_gen()) which doesn't handle an attribute * correctly if it is the last attribute in the list and the vendor is unknown. The attribute is meant * to be ignored but what happens is that all other attributes also get ignored. * * The solution is to add a dummy pair which is then treated as the (already handled) last attribute. * Use this function as a wrapper for the old function. * * Returns: value_pair list or NULL on failure */ VALUE_PAIR * rc_avpair_gen(const rc_handle *rh, VALUE_PAIR *pair, unsigned char *ptr, int length, int vendorpec) { VALUE_PAIR *dummy_pair_p; VALUE_PAIR *temp_pair_p; VALUE_PAIR *pair_p; if (pair != NULL) { rc_log(LOG_ERR, "rc_avpair_gen (new): unexpected non-null pair input"); return NULL; } dummy_pair_p = malloc(sizeof(*dummy_pair_p)); if (dummy_pair_p == NULL) { rc_log(LOG_CRIT, "rc_avpair_gen (new): out of memory"); return NULL; } memset(dummy_pair_p, '\0', sizeof(*dummy_pair_p)); dummy_pair_p->next = NULL; strcpy(dummy_pair_p->name, "DUMMY"); dummy_pair_p->attribute = 0; dummy_pair_p->type = PW_TYPE_INTEGER; dummy_pair_p->vp_integer = 0; pair_p = old_rc_avpair_gen(rh, dummy_pair_p, ptr, length, vendorpec); if (pair_p != NULL) { /* remove the dummy pair_p from the end of the list */ temp_pair_p = pair_p; while ((temp_pair_p->next != dummy_pair_p) && (temp_pair_p->next != NULL)) { temp_pair_p = temp_pair_p->next; } if (temp_pair_p == NULL) { rc_log(LOG_ERR, "rc_avpair_gen (new): pair not found"); /* this should never happen - something is wrong with the coding logic. clean up as best we can */ while (pair_p != NULL) { temp_pair_p = pair_p->next; free(pair_p); pair_p = temp_pair_p; } } else { temp_pair_p->next = NULL; } free(dummy_pair_p); } return pair_p; } Stefan Karss wrote:
Hello Adam,
I guess my description was a bit confusing - "first recursion" means the first recursion instance in that bit of messy code ;-). So wherever an unknown vendor happens the reply pointer is NULL.
Try my patch and see if it helps. If I remember correctly someone on the list wanted to submit another patch that translates unknown attributes/vendors into a special syntax - but I'm not sure if it has already been submitted.
Rgds, Stefan
On Tue, Jun 15, 2010 at 10:12 AM, Adam Lewis <VNQM87@motorola.com> wrote:
I also seem to be having a problem with attributes that should be ignored but otherwise have no effect on the other attributes in a reply.
If an attribute from an unknown vendor happens to be the very last attribute in the list, rc_avpair_gen() ignores it by returning pair immediately. The only problem being pair is still set to the entry value of NULL and this ripples up through all the recursive calls, which means all the other attributes get ignored as well. Has anybody else seen this?
Adam.
Stefan Karss wrote:
Hello again,
when the RADIUS servers sends a reply packet with an attribute for an unknown vendor freeradius-client just skips over this when filling response VALUE_PAIR list. Unfortunately it doesn't for unknown attributes from known vendors - in that case rc_avpair_gen returns NULL in the first
recursion
which results in a NULL pointer for reply VALUE_PAIR.
The attached patch just skips over unknown attributes for known vendors just like the lib skips over attributes from unknown vendors.
Rgds, Stefan
- List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
-- View this message in context: http://old.nabble.com/Patch%3A-avpair.c---empty-reply-VALUE_PAIR-%280x0%29-f... Sent from the FreeRadius - Dev mailing list archive at Nabble.com.
- List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
- List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
-- View this message in context: http://old.nabble.com/Patch%3A-avpair.c---empty-reply-VALUE_PAIR-%280x0%29-f... Sent from the FreeRadius - Dev mailing list archive at Nabble.com.