Unsupported Vendor Specific Attribute results in incorrect behavior of rc_avpair_gen()

26 Jan 2015

      Hi,

I’m using freeradius client library 1.1.6 with radius server version 2.2.6.  The “users” file on server has an entry for user “abcd” as below:

abcd  Cleartext-Password := "xyz"
     Service-Type = Framed-Management,
    Management-Privilege-level  = 4,
    Vendor-Specific            = "14817",

The dictionary file on client does not have entry for Vendor-Specific attribute value “14817” so this is an unsupported vendor-specific attribute

I’ve found that if an unsupported Vendor-Specific attribute is last in the list of attributes then rc_avpair_gen function (avpair.c) returns NULL below:

Sendserver.c: rc_send_server: line 367
                if (length > 0) {
                                data->receive_pairs = rc_avpair_gen(rh, NULL, recv_auth->data,  ←- data->receive_pairs is NULL
                                    length, 0);
                } else {
                                data->receive_pairs = NULL;
                }

The problem lies in rc_avpair_gen function. As Vendor-Specific is last and unsupported so “pair” will be NULL before below fragment returns

                if (attribute == PW_VENDOR_SPECIFIC) {
                                if (attrlen < 4) {
                                                rc_log(LOG_ERR, "rc_avpair_gen: received VSA "
                                                    "attribute with invalid length");
                                                goto shithappens;
                                }
                                memcpy(&lvalue, ptr, 4);
                                vendorpec = ntohl(lvalue);
                                if (rc_dict_getvend(rh, vendorpec) == NULL) {
                                                /* Warn and skip over the unknown VSA */
                                                rc_log(LOG_WARNING, "rc_avpair_gen: received VSA "
                                                    "attribute with unknown Vendor-Id %d", vendorpec);
                                                return pair; ←- This will be NULL
                                }
                                /* Process recursively */
                                return rc_avpair_gen(rh, pair, ptr + 4, attrlen - 4,
                                    vendorpec);
                }

As a result, below fragment will return NULL as well. Thus recursion unwinds by returning “pair” NULL even if valid attributes are also present

                /* Advance to the next attribute and process recursively */
                if (length != attrlen) {
                                pair = rc_avpair_gen(rh, pair, ptr + attrlen, length - attrlen,
                                    vendorpec);
                                if (pair == NULL)  ←-- Recursion unwinds and all invocations return NULL
                                                return NULL;
                }

Please advise how it can be addressed? Why is check [if (pair == NULL) return NULL;] needed above? Can’t we just continue with the function and return from the end of the function?

Thanks

Amol Lad
Software Architect
4RF Limited
DDI:         +64 4 498 7024
Tel:          +64 4 499 6000
Mobile:    +64 21 257 6610
Fax:         +64 4 473 4447
Web:       www.4rf.com
Email:     amol.lad@4rf.com

The information in this email communication (inclusive of attachments) is confidential to 4RF Limited and the intended recipient(s). If you are not the intended recipient(s), please note that any use, disclosure, distribution or copying of this information or any part thereof is strictly prohibited and that the author accepts no liability for the consequences of any action taken on the basis of the information provided. If you have received this email in error, please notify the sender immediately by return email and then delete all instances of this email from your system. 4RF Limited will not accept responsibility for any consequences associated with the use of this email (including, but not limited to, damages sustained as a result of any viruses and/or any action or lack of action taken in reliance on it).