From: Arran Cudbard-Bell Date: Thu, 10 Nov 2011 19:08:48 +0000 (+0100) Subject: Deduplicate attribute name resolution code between valuepair.c and evaluate.c X-Git-Tag: release_3_0_0_beta0~512^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ffe221a7039f5c4447a43fe71a03976cecc95430;p=thirdparty%2Ffreeradius-server.git Deduplicate attribute name resolution code between valuepair.c and evaluate.c --- diff --git a/src/include/radiusd.h b/src/include/radiusd.h index c7911c4b991..50992a9f0cc 100644 --- a/src/include/radiusd.h +++ b/src/include/radiusd.h @@ -765,11 +765,11 @@ int radius_evaluate_condition(REQUEST *request, int modreturn, int depth, int radius_update_attrlist(REQUEST *request, CONF_SECTION *cs, VALUE_PAIR *input_vps, const char *name); void radius_pairmove(REQUEST *request, VALUE_PAIR **to, VALUE_PAIR *from); -int radius_get_vp(REQUEST *request, const char *name, VALUE_PAIR **vp_p); -int radius_get_vps(REQUEST *request, const char *name, - VALUE_PAIR **vps, const char **attr); - +VALUE_PAIR **radius_list(REQUEST *request, pair_lists_t list); +pair_lists_t radius_list_name(const char **name, pair_lists_t unknown); +int radius_ref_request(const char **name, REQUEST **request); +int radius_get_vp(REQUEST *request, const char *name, VALUE_PAIR **vp_p); #ifdef WITH_TLS /* diff --git a/src/main/evaluate.c b/src/main/evaluate.c index 92dd0fad4f0..cf4a209f8f1 100644 --- a/src/main/evaluate.c +++ b/src/main/evaluate.c @@ -1124,85 +1124,34 @@ int radius_update_attrlist(REQUEST *request, CONF_SECTION *cs, { CONF_ITEM *ci; VALUE_PAIR *newlist, *vp; - VALUE_PAIR **output_vps = NULL; - REQUEST *myrequest = request; + VALUE_PAIR **output_vps; + + int list; if (!request || !cs) return RLM_MODULE_INVALID; - /* - * If we are an inner tunnel session, permit the - * policy to update the outer lists directly. + /* + * Qualifiers not valid for this request */ - if (strncmp(name, "outer.", 6) == 0) { - if (!request->parent) return RLM_MODULE_NOOP; - - myrequest = request->parent; - name += 6; + if(!radius_ref_request(&name, &request)){ + RDEBUG("WARNING: List name refers to outer request" + " but not in a tunnel."); + return RLM_MODULE_NOOP; } - - if (strcmp(name, "request") == 0) { - output_vps = &myrequest->packet->vps; - - } else if (strcmp(name, "reply") == 0) { - output_vps = &myrequest->reply->vps; - -#ifdef WITH_PROXY - } else if (strcmp(name, "proxy-request") == 0) { - if (request->proxy) output_vps = &myrequest->proxy->vps; - - } else if (strcmp(name, "proxy-reply") == 0) { - if (request->proxy_reply) output_vps = &request->proxy_reply->vps; -#endif - - } else if (strcmp(name, "config") == 0) { - output_vps = &myrequest->config_items; - - } else if (strcmp(name, "control") == 0) { - output_vps = &myrequest->config_items; - -#ifdef WITH_COA - } else if (strcmp(name, "coa") == 0) { - if (!myrequest->coa) { - request_alloc_coa(myrequest); - myrequest->coa->proxy->code = PW_COA_REQUEST; - } - - if (myrequest->coa && - (myrequest->coa->proxy->code == PW_COA_REQUEST)) { - output_vps = &myrequest->coa->proxy->vps; - } - - } else if (strcmp(name, "coa-reply") == 0) { - if (myrequest->coa && /* match reply with request */ - (myrequest->coa->proxy->code == PW_COA_REQUEST) && - (myrequest->coa->proxy_reply)) { - output_vps = &myrequest->coa->proxy_reply->vps; - } - - } else if (strcmp(name, "disconnect") == 0) { - if (!myrequest->coa) { - request_alloc_coa(myrequest); - if (myrequest->coa) myrequest->coa->proxy->code = PW_DISCONNECT_REQUEST; - } - - if (myrequest->coa && - (myrequest->coa->proxy->code == PW_DISCONNECT_REQUEST)) { - output_vps = &myrequest->coa->proxy->vps; - } - - } else if (strcmp(name, "disconnect-reply") == 0) { - if (myrequest->coa && /* match reply with request */ - (myrequest->coa->proxy->code == PW_DISCONNECT_REQUEST) && - (myrequest->coa->proxy_reply)) { - output_vps = &myrequest->coa->proxy_reply->vps; - } -#endif - - } else { + + list = fr_str2int(pair_lists, name, PAIR_LIST_UNKNOWN); + + /* + * Bad list name name + */ + if (list == PAIR_LIST_UNKNOWN) { + RDEBUG("ERROR: Invalid list name"); return RLM_MODULE_INVALID; } - - if (!output_vps) return RLM_MODULE_NOOP; /* didn't update the list */ + + output_vps = radius_list(request, list); + + rad_assert(output_vps); newlist = paircopy(input_vps); if (!newlist) { diff --git a/src/main/valuepair.c b/src/main/valuepair.c index 534d82efa68..1787162311d 100644 --- a/src/main/valuepair.c +++ b/src/main/valuepair.c @@ -854,105 +854,145 @@ void debug_pair_list(VALUE_PAIR *vp) fflush(fr_log_fp); } -/** @brief Resolve attribute qualifiers to an attribute list +/** Resolve attribute pair_lists_t value to an attribute list. + * + * The value returned is a pointer to the pointer of the HEAD of the list + * in the REQUEST. If the head of the list changes, the pointer will still + * be valid. * - * @param request The current request - * @param name Attribute name including qualifiers - * @param vps Where to write the pointer to resolved list. Will be NULL if list name - * couldn't be resolved, or is invalid in the current context - * @param attribute Where to write pointer into name (name minus qualifiers) - * @return False if the list qualifiers were invalid, else true + * @param[in] request containing the target lists. + * @param[in] list pair_list_t value to resolve to VALUE_PAIR list. + * Will be NULL if list name couldn't be resolved. */ -int radius_get_vps(REQUEST *request, const char *name, - VALUE_PAIR **vps, const char **attribute){ - REQUEST *my_request = request; - - const char *p = name; - const char *q; - - *vps = NULL; - *attribute = p; - - pair_lists_t list; - - /* - * Allow for tunneled sessions. - */ - if (strncmp(p, "outer.", 6) == 0) { - if (!request->parent) return TRUE; - p += 6; - my_request = request->parent; - } - - q = strchr(p, ':'); - if (q == NULL) { - *vps = my_request->packet->vps; - *attribute = p; - return TRUE; - } - *attribute = q + 1; - - list = fr_substr2int(pair_lists, p, PAIR_LIST_UNKNOWN, q - p); +VALUE_PAIR **radius_list(REQUEST *request, pair_lists_t list) +{ switch (list) { - case PAIR_LIST_REQUEST: - *vps = my_request->packet->vps; + case PAIR_LIST_UNKNOWN: + default: break; + case PAIR_LIST_REQUEST: + return &request->packet->vps; + case PAIR_LIST_REPLY: - *vps = my_request->reply->vps; - break; + return &request->reply->vps; case PAIR_LIST_CONTROL: - *vps = my_request->config_items; - break; + return &request->config_items; #ifdef WITH_PROXY case PAIR_LIST_PROXY_REQUEST: - *vps = my_request->proxy->vps; - break; + return &request->proxy->vps; case PAIR_LIST_PROXY_REPLY: - *vps = my_request->proxy_reply->vps; - break; + return &request->proxy_reply->vps; #endif #ifdef WITH_COA case PAIR_LIST_COA: - if (my_request->coa && - (my_request->coa->proxy->code == PW_COA_REQUEST)) - *vps = my_request->coa->proxy->vps; - + if (request->coa && + (request->coa->proxy->code == PW_COA_REQUEST)) { + return &request->coa->proxy->vps; + } break; case PAIR_LIST_COA_REPLY: - if (my_request->coa && /* match reply with request */ - (my_request->coa->proxy->code == PW_COA_REQUEST) && - my_request->coa->proxy_reply) - *vps = my_request->coa->proxy_reply->vps; - + if (request->coa && /* match reply with request */ + (request->coa->proxy->code == PW_COA_REQUEST) && + request->coa->proxy_reply) { + return &request->coa->proxy_reply->vps; + } break; case PAIR_LIST_DM: - if (my_request->coa && - (my_request->coa->proxy->code == PW_DISCONNECT_REQUEST)) - *vps = my_request->coa->proxy->vps; - + if (request->coa && + (request->coa->proxy->code == PW_DISCONNECT_REQUEST)) { + return &request->coa->proxy->vps; + } break; case PAIR_LIST_DM_REPLY: - if (my_request->coa && /* match reply with request */ - (my_request->coa->proxy->code == PW_DISCONNECT_REQUEST) && - my_request->coa->proxy_reply) - *vps = my_request->coa->proxy->vps; + if (request->coa && /* match reply with request */ + (request->coa->proxy->code == PW_DISCONNECT_REQUEST) && + request->coa->proxy_reply) { + return &request->coa->proxy->vps; + } break; #endif - default: - return FALSE; + } + + return NULL; +} + +/** Resolve attribute name to a list. + * + * Check the name string for qualifiers that specify a list and return + * an pair_lists_t value for that list. This value may be passed to + * radius_list, along with the current request, to get a pointer to the + * actual list in the request. + * + * If qualifiers were consumed, write a new pointer into name to the + * char after the last qualifier to be consumed. + * + * radius_list_name should be called before passing a name string that + * may contain qualifiers to dict_attrbyname. + * + * @see dict_attrbyname + * + * @param[in,out] name of attribute. + * @param[in] unknown the list to return if no qualifiers were found. + * @return PAIR_LIST_UNKOWN if qualifiers couldn't be resolved to a list. + */ +pair_lists_t radius_list_name(const char **name, pair_lists_t unknown) +{ + const char *p = *name; + char *q; + + /* This should never be a NULL pointer or zero length string */ + rad_assert(name && *name); + q = strrchr(p, ':'); + if (!q) { + return unknown; } - return TRUE; + + *name = (q + 1); + + return fr_substr2int(pair_lists, p, PAIR_LIST_UNKNOWN, (q - p)); +} + +/** Resolve attribute name to a request. + * + * Check the name string for qualifiers that reference a parent request and + * write the pointer to this request to 'request'. + * + * If qualifiers were consumed, write a new pointer into name to the + * char after the last qualifier to be consumed. + * + * radius_ref_request should be called before radius_list_name. + * + * @see radius_list_name + * + * @param[in,out] current request. + * @param[in,out] name of attribute. + * @return FALSE if qualifiers found but not in a tunnel, else TRUE. + */ +int radius_ref_request(const char **name, REQUEST **request) +{ + rad_assert(name && *name); + rad_assert(request && *request); + + if (strncmp(*name, "outer.", 6) == 0) { + if (!(*request)->parent) { + return FALSE; + } + *request = (*request)->parent; + *name += 6; } + + return TRUE; +} -/** @brief Return a VP from the specified request +/** Return a VP from the specified request. * * @param request The current request * @param name Attribute name including qualifiers @@ -962,25 +1002,38 @@ int radius_get_vps(REQUEST *request, const char *name, */ int radius_get_vp(REQUEST *request, const char *name, VALUE_PAIR **vp_p) { - const char *attribute; + VALUE_PAIR **vps; + pair_lists_t list; - VALUE_PAIR *vps = NULL; - DICT_ATTR *da; + const DICT_ATTR *da; - int ret; - *vp_p = NULL; + + if (!radius_ref_request(&name, &request)) { + RDEBUG("WARNING: Attribute name refers to outer request" + " but not in a tunnel."); + return TRUE; /* Discuss, we don't actually know if + the attrname was valid... */ + } + + list = radius_list_name(&name, PAIR_LIST_REQUEST); + if (list == PAIR_LIST_UNKNOWN) { + RDEBUG("ERROR: Invalid list qualifier"); + return FALSE; + } + + da = dict_attrbyname(name); + if (!da) { + RDEBUG("ERROR: Attribute \"%s\" unknown", name); + return FALSE; + } - ret = radius_get_vps(request, name, &vps, &attribute); - if (!ret) return FALSE; /* not a valid attribute list */ - if (!vps) return TRUE; /* list is valid but not in current context */ - - da = dict_attrbyname(attribute); - if (!da) return FALSE; /* not a dictionary name */ - + vps = radius_list(request, list); + rad_assert(vps); + /* * May not may not be found, but it *is* a known name. */ - *vp_p = pairfind(vps, da->attr, da->vendor); + *vp_p = pairfind(*vps, da->attr, da->vendor); return TRUE; }