From: Alan T. DeKok Date: Tue, 15 Dec 2020 15:03:47 +0000 (-0500) Subject: make PAIR_LIST use maps X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=473614fd4ddfc26ba7a6b4a0670fc55d2fc04c15;p=thirdparty%2Ffreeradius-server.git make PAIR_LIST use maps we still require changes in rlm_files to use maps natively. Right now, it still converts them to VPs and compares the VPs. Once rlm_files supports maps, it can once again support regular expressions in the "users" file, and full list qualifiers anywhere. --- diff --git a/src/lib/server/users_file.c b/src/lib/server/users_file.c index a2ee0461e12..720186833e6 100644 --- a/src/lib/server/users_file.c +++ b/src/lib/server/users_file.c @@ -37,34 +37,6 @@ RCSID("$Id$") #include #include -/* - * Debug code. - */ -#if 0 -static void debug_pair_list(PAIR_LIST *pl) -{ - fr_pair_t *vp; - fr_cursor_t cursor; - - while(pl) { - printf("Pair list: %s\n", pl->name); - printf("** Check:\n"); - for(vp = fr_cursor_init(&cursor, &pl->check); vp; vp = fr_cursor_next(&cursor)) { - printf(" "); - fr_log(&default_log, L_DBG, __FILE__, __LINE__, "%pP", vp); - printf("\n"); - } - printf("** Reply:\n"); - for(vp = fr_cursor_init(&cursor, &pl->reply); vp; vp = fr_cursor_next(&cursor)) { - printf(" "); - fr_log(&default_log, L_DBG, __FILE__, __LINE__, "%pP", vp); - printf("\n"); - } - pl = pl->next; - } -} -#endif - /* * Free a PAIR_LIST */ @@ -74,37 +46,171 @@ void pairlist_free(PAIR_LIST **pl) *pl = NULL; } +static fr_table_num_sorted_t const check_cmp_op_table[] = { + { L("!*"), T_OP_CMP_FALSE }, + { L("!="), T_OP_NE }, + { L("!~"), T_OP_REG_NE }, + { L("+="), T_OP_ADD }, + { L(":="), T_OP_SET }, + { L("<"), T_OP_LT }, + { L("<="), T_OP_LE }, + { L("="), T_OP_EQ }, + { L("=*"), T_OP_CMP_TRUE }, + { L("=="), T_OP_CMP_EQ }, + { L("=~"), T_OP_REG_EQ }, + { L(">"), T_OP_GT }, + { L(">="), T_OP_GE } +}; +static size_t check_cmp_op_table_len = NUM_ELEMENTS(check_cmp_op_table); + +static const fr_sbuff_term_t name_terms = FR_SBUFF_TERMS( + L("\t"), + L("\n"), + L(" "), + L("#"), +); + +static fr_sbuff_parse_rules_t const rhs_term = { + .escapes = &(fr_sbuff_unescape_rules_t){ + .chr = '\\', + /* + * Allow barewords to contain whitespace + * if they're escaped. + */ + .subs = { + ['\t'] = '\t', + ['\n'] = '\n', + [' '] = ' ' + }, + .do_hex = true, + .do_oct = false + }, + .terminals = &FR_SBUFF_TERMS( + L("\t"), + L("\n"), + L(" "), + L("#"), + L(","), + ) +}; + +/* + * Caller saw a $INCLUDE at the start of a line. + */ +static int users_include(TALLOC_CTX *ctx, fr_dict_t const *dict, fr_sbuff_t *sbuff, PAIR_LIST **last, + char const *file, int lineno) +{ + size_t len; + char *newfile, *p, c; + fr_sbuff_marker_t name; + + *last = NULL; + fr_sbuff_advance(sbuff, 8); + + /* + * Skip spaces after the $INCLUDE. + */ + if (fr_sbuff_adv_past_allowed(sbuff, SIZE_MAX, sbuff_char_blank) == 0) { + ERROR("%s[%d]: Unexpected text after $INCLUDE", + file, lineno); + return -1; + } + + /* + * Remember when the name started, and skip over the name + * until spaces, comments, or LF + */ + fr_sbuff_marker(&name, sbuff); + len = fr_sbuff_adv_until(sbuff, SIZE_MAX, &name_terms, 0); + if (len == 0) { + fr_sbuff_marker_release(&name); + ERROR("%s[%d]: No filename after $INCLUDE ", + file, lineno); + return -1; + } + + /* + * If the input file is a relative path, try to copy the + * leading directory from it. If there's no leading + * directory, just use the $INCLUDE name as-is. + * + * If there is a leading directory in the input name, + * paste that as the directory to the $INCLUDE name. + * + * Otherwise the $INCLUDE name is an absolute path, use + * it as -is. + */ + c = *fr_sbuff_current(&name); + if (c != '/') { + p = strrchr(file, '/'); + + if (!p) goto copy_name; + + newfile = talloc_asprintf(NULL, "%.*s/%.*s", + (int) (p - file), file, + (int) len, fr_sbuff_current(&name)); + } else { + copy_name: + newfile = talloc_asprintf(NULL, "%.*s", (int) len, fr_sbuff_current(&name)); + } + fr_sbuff_marker_release(&name); + + /* + * Skip spaces and comments after the name. + */ + fr_sbuff_adv_past_allowed(sbuff, SIZE_MAX, sbuff_char_blank); + if (fr_sbuff_next_if_char(sbuff, '#')) { + (void) fr_sbuff_adv_to_chr(sbuff, SIZE_MAX, '\n'); + } + + /* + * There's no LF, but if we skip non-spaces and + * non-comments to find the LF, then there must be extra + * text after the filename. That's an error. + * + * Unless the line has EOF after the filename. in which + * case this error will get hit, too. + */ + if (!fr_sbuff_is_char(sbuff, '\n') && + (fr_sbuff_adv_to_chr(sbuff, SIZE_MAX, '\n') > 0)) { + ERROR("%s[%d]: Unexpected text after filename", + file, lineno); + talloc_free(newfile); + return -1; + } + + /* + * Read the $INCLUDEd file recursively. + */ + if (pairlist_read(ctx, dict, newfile, last, 0) != 0) { + ERROR("%s[%d]: Could not read included file %s: %s", + file, lineno, newfile, fr_syserror(errno)); + talloc_free(newfile); + return -1; + } + talloc_free(newfile); + + return 0; +} -#define FIND_MODE_NAME 0 -#define FIND_MODE_WANT_REPLY 1 -#define FIND_MODE_HAVE_REPLY 2 /* * Read the users file. Return a PAIR_LIST. */ int pairlist_read(TALLOC_CTX *ctx, fr_dict_t const *dict, char const *file, PAIR_LIST **list, int complain) { - FILE *fp; - int mode = FIND_MODE_NAME; - char entry[256]; - char buffer[8192]; - char const *ptr; - fr_pair_list_t check_tmp; - fr_pair_list_t reply_tmp; - PAIR_LIST *pl = NULL, *t; - PAIR_LIST **last = &pl; - int order = 0; - int lineno = 0; - int entry_lineno = 0; - fr_token_t parsecode; -#ifdef HAVE_REGEX_H - fr_pair_t *vp; - fr_cursor_t cursor; -#endif - char newfile[8192]; - - fr_pair_list_init(&check_tmp); - fr_pair_list_init(&reply_tmp); + char *q; + PAIR_LIST *pl = NULL; + PAIR_LIST **last = &pl; + int order = 0; + int lineno = 1; + map_t **map_tail; + FILE *fp; + fr_sbuff_t sbuff; + fr_sbuff_uctx_file_t fctx; + tmpl_rules_t lhs_rules, rhs_rules; + char buffer[8192]; + DEBUG2("Reading file %s", file); /* @@ -112,254 +218,408 @@ int pairlist_read(TALLOC_CTX *ctx, fr_dict_t const *dict, char const *file, PAIR * more useful... */ if ((fp = fopen(file, "r")) == NULL) { - if (!complain) - return -1; + if (!complain) return -1; + ERROR("Couldn't open %s for reading: %s", file, fr_syserror(errno)); return -1; } - /* - * Allocate the structure for one entry. - */ - MEM(t = talloc_zero(ctx, PAIR_LIST)); - fr_pair_list_init(&t->check); - fr_pair_list_init(&t->reply); + fr_sbuff_init_file(&sbuff, &fctx, buffer, sizeof(buffer), fp, SIZE_MAX); - /* - * Read the entire file into memory for speed. - */ - while (fgets(buffer, sizeof(buffer), fp) != NULL) { - lineno++; + lhs_rules = (tmpl_rules_t) { + .dict_def = dict, + .request_def = REQUEST_CURRENT, + .prefix = TMPL_ATTR_REF_PREFIX_AUTO, + .disallow_qualifiers = true, /* for now, until more tests are made */ - if (!feof(fp) && (strchr(buffer, '\n') == NULL)) { - fclose(fp); - ERROR("%s[%d]: line too long", file, lineno); + /* + * Otherwise the tmpl code returns 0 when asked + * to parse unknown names. So we say "please + * parse unknown names as unresolved attributes", + * and then do a second pass to complain that the + * thing isn't known. + */ + .allow_unresolved = true, + }; + rhs_rules = (tmpl_rules_t) { + .dict_def = dict, + .request_def = REQUEST_CURRENT, + .prefix = TMPL_ATTR_REF_PREFIX_YES, + .disallow_qualifiers = true, /* for now, until rlm_files supports it */ + }; + + while (true) { + size_t len; + bool comma; + bool leading_spaces; + PAIR_LIST *t; + + /* + * If the line is empty or has only comments, + * then we don't care about leading spaces. + */ + leading_spaces = (fr_sbuff_adv_past_allowed(&sbuff, SIZE_MAX, sbuff_char_blank) > 0); + if (fr_sbuff_next_if_char(&sbuff, '#')) { + (void) fr_sbuff_adv_to_chr(&sbuff, SIZE_MAX, '\n'); + } + if (fr_sbuff_next_if_char(&sbuff, '\n')) { + lineno++; + continue; + } + + /* + * We're trying to read a name. It MUST have + * been at the start of the line. So whatever + * this is, it's wrong. + */ + if (leading_spaces) { + ERROR("%s[%d]: Entry does not begin with a user name", + file, lineno); + fail: pairlist_free(&pl); + fclose(fp); return -1; } /* - * If the line contains nothing but whitespace, - * ignore it. + * $INCLUDE filename */ - ptr = buffer; - fr_skip_whitespace(ptr); + if (fr_sbuff_is_str(&sbuff, "$INCLUDE", 8)) { + if (users_include(ctx, dict, &sbuff, &t, file, lineno) < 0) goto fail; - if (*ptr == '#' || *ptr == '\n' || !*ptr) continue; - -parse_again: - if (mode == FIND_MODE_NAME) { /* - * The user's name MUST be the first text on the line. + * The file may have read no entries, one + * entry, or it may be a linked list of + * entries. Go to the end of the list. */ - if (isspace((int) buffer[0])) { - ERROR("%s[%d]: Entry does not begin with a user name", - file, lineno); - fclose(fp); - return -1; + *last = t; + while (*last) { + (*last)->order = order++; + last = &((*last)->next); } - /* - * Get the name. - */ - ptr = buffer; - getword(&ptr, entry, sizeof(entry), false); - entry_lineno = lineno; - - /* - * Include another file if we see - * $INCLUDE filename - */ - if (strcasecmp(entry, "$INCLUDE") == 0) { - fr_skip_whitespace(ptr); - - /* - * If it's an absolute pathname, - * then use it verbatim. - * - * If not, then make the $include - * files *relative* to the current - * file. - */ - if (FR_DIR_IS_RELATIVE(ptr)) { - char *p; - - strlcpy(newfile, file, - sizeof(newfile)); - p = strrchr(newfile, FR_DIR_SEP); - if (!p) { - p = newfile + strlen(newfile); - *p = FR_DIR_SEP; - } - getword(&ptr, p + 1, sizeof(newfile) - 1 - (p - newfile), false); - } else { - getword(&ptr, newfile, sizeof(newfile), false); - } - - if (pairlist_read(ctx, dict, newfile, last, 0) != 0) { - pairlist_free(&pl); - ERROR("%s[%d]: Could not open included file %s: %s", - file, lineno, newfile, fr_syserror(errno)); - fclose(fp); - return -1; - } - - /* - * t may be NULL, it may have one - * entry, or it may be a linked list - * of entries. Go to the end of the - * list. - */ - while (*last) { - (*last)->order = order++; - last = &((*last)->next); - } + if (fr_sbuff_next_if_char(&sbuff, '\n')) { + lineno++; continue; - } /* $INCLUDE ... */ + } /* - * Parse the check values + * The next character is not LF, but the + * function skipped to LF. So, by + * process of elimination, we must be at + * EOF. */ - fr_assert(check_tmp == NULL); - fr_assert(reply_tmp == NULL); - - parsecode = fr_pair_list_afrom_str(t, dict, ptr, &check_tmp); - if (parsecode == T_INVALID) { - pairlist_free(&pl); - PERROR("%s[%d]: Parse error (check) for entry %s", file, lineno, entry); - fclose(fp); - return -1; - } + break; + } /* else it wasn't $INCLUDE */ + + /* + * We MUST be either at a valid entry, OR at EOF. + */ + MEM(t = talloc_zero(ctx, PAIR_LIST)); + t->lineno = lineno; + t->order = order++; - if (parsecode != T_EOL) { - pairlist_free(&pl); - talloc_free(t); - ERROR("%s[%d]: Invalid text after check attributes for entry %s", - file, lineno, entry); - fclose(fp); - return -1; + /* + * Copy the name from the entry. + */ + len = fr_sbuff_out_abstrncpy_until(t, &q, &sbuff, SIZE_MAX, &name_terms, NULL); + if (len == 0) { + talloc_free(t); + break; + } + t->name = q; + map_tail = &t->check; + + lhs_rules.list_def = PAIR_LIST_CONTROL; + comma = false; + +check_item: + /* + * Skip spaces before the item, and allow the + * check list to end on comment or LF. + * + * Note that we _don't_ call map_afrom_sbuff() to + * skip spaces, as it will skip LF, too! + */ + (void) fr_sbuff_adv_past_allowed(&sbuff, SIZE_MAX, sbuff_char_blank); + if (fr_sbuff_is_char(&sbuff, '#')) goto check_item_comment; + if (fr_sbuff_is_char(&sbuff, '\n')) goto check_item_end; + + /* + * Try to parse the check item. + */ + if (map_afrom_sbuff(t, map_tail, &sbuff, check_cmp_op_table, check_cmp_op_table_len, + &lhs_rules, &rhs_rules, &rhs_term, sbuff_char_blank) < 0) { + ERROR("%s[%d]: Failed reading check pair: %s", + file, lineno, fr_strerror()); + fail_entry: + talloc_free(t); + goto fail; + } + + /* + * The map "succeeded", but no map was created. + * It must have hit a terminal character, OR EOF. + * + * Except we've already skipped spaces, tabs, + * comments, and LFs. So the only thing which is + * left is a comma. + */ + if (!*map_tail) { + if (fr_sbuff_is_char(&sbuff, ',')) { + ERROR("%s[%d]: Unexpected extra comma reading check pair", + file, lineno); + goto fail_entry; } -#ifdef HAVE_REGEX_H /* - * Do some more sanity checks. + * Otherwise map_afrom_sbuff() returned + * nothing, because there's no more + * input. */ - for (vp = fr_cursor_init(&cursor, &check_tmp); - vp; - vp = fr_cursor_next(&cursor)) { - if (((vp->op == T_OP_REG_EQ) || - (vp->op == T_OP_REG_NE)) && - (vp->vp_type != FR_TYPE_STRING)) { - pairlist_free(&pl); - talloc_free(t); - ERROR("%s[%d]: Cannot use regular expressions for non-string " - "attributes in entry %s", file, lineno, entry); - fclose(fp); - return -1; - } - } -#endif + add_entry: + *last = t; + break; + } + fr_assert((*map_tail)->lhs != NULL); + fr_assert((*map_tail)->rhs != NULL); + fr_assert((*map_tail)->next == NULL); + + if (!tmpl_is_attr((*map_tail)->lhs)) { + ERROR("%s[%d]: Unknown attribute '%s'", + file, lineno, (*map_tail)->lhs->name); + goto fail_entry; + } + + if (!tmpl_is_data((*map_tail)->rhs) && !tmpl_is_exec((*map_tail)->rhs) && + !tmpl_contains_xlat((*map_tail)->rhs)) { + ERROR("%s[%d]: Invalid RHS '%s' for check item", + file, lineno, (*map_tail)->rhs->name); + goto fail_entry; + } + + map_tail = &(*map_tail)->next; + + /* + * There can be spaces before any comma. + */ + (void) fr_sbuff_adv_past_allowed(&sbuff, SIZE_MAX, sbuff_char_blank); + + /* + * Allow a comma after this item. But remember + * if we had a comma. + */ + if (fr_sbuff_next_if_char(&sbuff, ',')) { + comma = true; + goto check_item; + } + comma = false; + + check_item_comment: + /* + * There wasn't a comma after the item, so the + * next thing MUST be a comment, LF, EOF. + * + * If there IS stuff before the LF, then it's + * unknown text. + */ + if (fr_sbuff_next_if_char(&sbuff, '#')) { + (void) fr_sbuff_adv_to_chr(&sbuff, SIZE_MAX, '\n'); + } + check_item_end: + if (fr_sbuff_next_if_char(&sbuff, '\n')) { /* - * The reply MUST be on a new line. + * The check item list ended with a comma. + * That's bad. */ - mode = FIND_MODE_WANT_REPLY; - continue; + if (comma) { + ERROR("%s[%d]: Invalid comma ending the check item list.", + file, lineno); + goto fail_entry; + } + + lineno++; + goto setup_reply; } /* - * We COULD have a reply, OR we could have a new entry. + * We didn't see SPACE LF or SPACE COMMENT LF. + * There's something else going on. */ - if (mode == FIND_MODE_WANT_REPLY) { - if (!isspace((int) buffer[0])) goto create_entry; + if (fr_sbuff_adv_to_chr(&sbuff, SIZE_MAX, '\n') > 0) { + ERROR("%s[%d]: Unexpected text after check items: %s", + file, lineno, fr_strerror()); + goto fail_entry; + } - mode = FIND_MODE_HAVE_REPLY; + /* + * The next character is not LF, but we + * skipped to LF above. So, by process + * of elimination, we must be at EOF. + */ + if (!fr_sbuff_is_char(&sbuff, '\n')) { + goto add_entry; } +setup_reply: /* - * mode == FIND_MODE_HAVE_REPLY + * Setup the reply items. */ + map_tail = &t->reply; + lhs_rules.list_def = PAIR_LIST_REPLY; + comma = false; +reply_item: /* - * The previous line ended with a comma, and then - * we have the start of a new entry! + * Reply items start with spaces. If there's no + * spaces, then the current entry is done. Add + * it to the list, and go back to reading the + * user name or $INCLUDE. */ - if (!isspace((int) buffer[0])) { - trailing_comma: - pairlist_free(&pl); - talloc_free(t); - ERROR("%s[%d]: Invalid comma after the reply attributes. Please delete it.", + if (fr_sbuff_adv_past_allowed(&sbuff, SIZE_MAX, sbuff_char_blank) == 0) { + if (comma) { + ERROR("%s[%d]: Unexpected trailing comma in previous line", + file, lineno); + goto fail_entry; + } + + /* + * The line doesn't begin with spaces. + * The list of reply items MUST be + * finished. Go look for an entry name. + * + * Note that we don't allow comments in + * the middle of the reply item list. Oh + * well. + */ + *last = t; + last = &(t->next); + continue; + + } else if (lineno == (t->lineno + 1)) { + fr_assert(comma == false); + + } else if (!comma) { + ERROR("%s[%d]: Missing comma in previous line", file, lineno); - fclose(fp); - return -1; + goto fail_entry; } /* - * Parse the reply values. If there's a trailing - * comma, keep parsing the reply values. + * SPACES COMMENT or SPACES LF means "end of + * reply item list" */ - parsecode = fr_pair_list_afrom_str(t, dict, buffer, &reply_tmp); - if (parsecode == T_COMMA) { - continue; + if (fr_sbuff_is_char(&sbuff, '#')) { + (void) fr_sbuff_adv_to_chr(&sbuff, SIZE_MAX, '\n'); + } + if (fr_sbuff_next_if_char(&sbuff, '\n')) { + lineno++; + goto add_entry; } +next_reply_item: /* - * We expect an EOL. Anything else is an error. + * Unlike check items, we don't skip spaces or + * comments here. All of the code paths which + * lead to here have already checked for those + * cases. */ - if (parsecode != T_EOL) { - pairlist_free(&pl); - talloc_free(t); - PERROR("%s[%d]: Parse error (reply) for entry %s", file, lineno, entry); - fclose(fp); - return -1; + if (map_afrom_sbuff(t, map_tail, &sbuff, map_assignment_op_table, map_assignment_op_table_len, + &lhs_rules, &rhs_rules, &rhs_term, sbuff_char_blank) < 0) { + ERROR("%s[%d]: Failed reading reply pair: %s", + file, lineno, fr_strerror()); + goto fail; } - create_entry: - t->check = check_tmp; - t->reply = reply_tmp; - t->lineno = entry_lineno; - t->order = order++; - fr_pair_list_init(&check_tmp); - fr_pair_list_init(&reply_tmp); + /* + * The map "succeeded", but no map was created. + * Maybe we hit a terminal string, or EOF. + * + * We can't have hit space/tab, as that was + * checked for at "reply_item", and again after + * map_afrom_sbuff(), if we actually got + * something. + * + * What's left is a comment, comma, LF, or EOF. + */ + if (!*map_tail) { + (void) fr_sbuff_adv_past_allowed(&sbuff, SIZE_MAX, sbuff_char_blank); + if (fr_sbuff_is_char(&sbuff, ',')) { + ERROR("%s[%d]: Unexpected extra comma reading reply pair", + file, lineno); + goto fail_entry; + } + + if (fr_sbuff_is_char(&sbuff, '#')) goto reply_item_comment; + if (fr_sbuff_is_char(&sbuff, '\n')) goto reply_item_end; + + /* + * We didn't read anything, but none of + * the terminal characters match. It must be EOF. + */ + goto add_entry; + } + fr_assert((*map_tail)->lhs != NULL); + fr_assert((*map_tail)->rhs != NULL); + fr_assert((*map_tail)->next == NULL); + + if (!tmpl_is_attr((*map_tail)->lhs)) { + ERROR("%s[%d]: Unknown attribute '%s'", + file, lineno, (*map_tail)->lhs->name); + goto fail_entry; + } + + if (!tmpl_is_data((*map_tail)->rhs) && !tmpl_is_exec((*map_tail)->rhs) && + !tmpl_contains_xlat((*map_tail)->rhs)) { + ERROR("%s[%d]: Invalid RHS '%s' for reply item", + file, lineno, (*map_tail)->rhs->name); + goto fail_entry; + } + + fr_assert(tmpl_list((*map_tail)->lhs) == PAIR_LIST_REPLY); - t->name = talloc_typed_strdup(t, entry); + map_tail = &(*map_tail)->next; - *last = t; - last = &(t->next); + (void) fr_sbuff_adv_past_allowed(&sbuff, SIZE_MAX, sbuff_char_blank); /* - * Allocate another one, just in case it's needed. + * Commas separate entries on the same line. And + * we allow spaces after commas, too. */ - MEM(t = talloc_zero(ctx, PAIR_LIST)); - fr_pair_list_init(&t->check); - fr_pair_list_init(&t->reply); + if (fr_sbuff_next_if_char(&sbuff, ',')) { + comma = true; + (void) fr_sbuff_adv_past_allowed(&sbuff, SIZE_MAX, sbuff_char_blank); + } else { + comma = false; + } /* - * Look for a name. If we came here because - * there were no reply attributes, then re-parse - * the current line, instead of reading another one. + * Comments or LF will end this particular line. + * + * Reading the next line will cause a complaint + * if this line ended with a comma. */ - mode = FIND_MODE_NAME; - if (feof(fp)) break; - if (!isspace((int) buffer[0])) goto parse_again; - } - - /* - * We're at EOF. If we're supposed to read more, that's - * an error. - */ - if (mode == FIND_MODE_HAVE_REPLY) goto trailing_comma; + reply_item_comment: + if (fr_sbuff_next_if_char(&sbuff, '#')) { + (void) fr_sbuff_adv_to_chr(&sbuff, SIZE_MAX, '\n'); + } + reply_item_end: + if (fr_sbuff_next_if_char(&sbuff, '\n')) { + lineno++; + goto reply_item; + } - /* - * We had an entry, but no reply attributes. That's OK. - */ - if (mode == FIND_MODE_WANT_REPLY) goto create_entry; + /* + * Not comment or LF, the content MUST be another + * pair. + */ + if (comma) goto next_reply_item; - /* - * We allocated one more than we need, so free this one - * now. - */ - talloc_free(t); + ERROR("%s[%d]: Unexpected text after reply pair: %s", + file, lineno, fr_sbuff_current(&sbuff)); + goto fail_entry; + } /* * Else we were looking for an entry. We didn't get one diff --git a/src/lib/server/users_file.h b/src/lib/server/users_file.h index 13309e72b28..70ab824f9b1 100644 --- a/src/lib/server/users_file.h +++ b/src/lib/server/users_file.h @@ -32,12 +32,13 @@ extern "C" { #endif #include +#include #include typedef struct pair_list { char const *name; - fr_pair_list_t check; - fr_pair_list_t reply; + map_t *check; + map_t *reply; int order; int lineno; struct pair_list *next; diff --git a/src/modules/rlm_attr_filter/rlm_attr_filter.c b/src/modules/rlm_attr_filter/rlm_attr_filter.c index 21d3f429f8c..39d7462a279 100644 --- a/src/modules/rlm_attr_filter/rlm_attr_filter.c +++ b/src/modules/rlm_attr_filter/rlm_attr_filter.c @@ -107,7 +107,7 @@ static int attr_filter_getfile(TALLOC_CTX *ctx, rlm_attr_filter_t *inst, char co int rcode; PAIR_LIST *attrs = NULL; PAIR_LIST *entry; - fr_pair_t *vp; + map_t *map; rcode = pairlist_read(ctx, dict_radius, filename, &attrs, 1); if (rcode < 0) { @@ -115,30 +115,40 @@ static int attr_filter_getfile(TALLOC_CTX *ctx, rlm_attr_filter_t *inst, char co } /* - * Walk through the 'attrs' file list. + * Walk through the 'attrs' file list. */ - - entry = attrs; - while (entry) { - entry->check = entry->reply; - entry->reply = NULL; - - for (vp = fr_cursor_init(&cursor, &entry->check); - vp; - vp = fr_cursor_next(&cursor)) { - /* - * If it's NOT a vendor attribute, - * and it's NOT a wire protocol - * and we ignore Fall-Through, - * then bitch about it, giving a good warning message. - */ - if (fr_dict_attr_is_top_level(vp->da) && (vp->da->attr > 1000)) { - WARN("[%s]:%d Check item \"%s\"\n\tfound in filter list for realm \"%s\".\n", - filename, entry->lineno, vp->da->name, entry->name); - } + for (entry = attrs; entry != NULL; entry = entry->next) { + /* + * We apply the rules in the reply items. + */ + if (entry->check) { + WARN("%s[%d] Check list is not empty for entry \"%s\".\n", + filename, entry->lineno, entry->name); } - entry = entry->next; + for (map = fr_cursor_init(&cursor, &entry->reply); + map; + map = fr_cursor_next(&cursor)) { + fr_dict_attr_t const *da; + + if (!tmpl_is_attr(map->lhs)) { + ERROR("%s[%d] Left side of filter %s is not an attribute", + filename, entry->lineno, map->lhs->name); + return -1; + } + da = tmpl_da(map->lhs); + + /* + * If it's NOT a vendor attribute, + * and it's NOT a wire protocol + * and we ignore Fall-Through, + * then bitch about it, giving a good warning message. + */ + if (fr_dict_attr_is_top_level(da) && (da->attr > 1000)) { + WARN("%s[%d] Check item \"%s\" was found in filter list for entry \"%s\".\n", + filename, entry->lineno, da->name, entry->name); + } + } } *pair_list = attrs; @@ -173,9 +183,7 @@ static unlang_action_t CC_HINT(nonnull(1,2)) attr_filter_common(rlm_rcode_t *p_r fr_radius_packet_t *packet) { rlm_attr_filter_t const *inst = talloc_get_type_abort_const(instance, rlm_attr_filter_t); - fr_pair_t *vp; - fr_cursor_t input, check, out; - fr_pair_t *input_item, *check_item; + fr_cursor_t out; fr_pair_list_t output; PAIR_LIST *pl; int found = 0; @@ -209,6 +217,10 @@ static unlang_action_t CC_HINT(nonnull(1,2)) attr_filter_common(rlm_rcode_t *p_r for (pl = inst->attrs; pl; pl = pl->next) { int fall_through = 0; int relax_filter = inst->relaxed; + map_t *map; + fr_pair_t *check_item, *input_item; + fr_pair_list_t check_list; + fr_cursor_t check, cursor; /* * If the current entry is NOT a default, @@ -217,18 +229,27 @@ static unlang_action_t CC_HINT(nonnull(1,2)) attr_filter_common(rlm_rcode_t *p_r */ if ((strcmp(pl->name, "DEFAULT") != 0) && (strcmp(keyname, pl->name) != 0)) { - continue; + continue; } RDEBUG2("Matched entry %s at line %d", pl->name, pl->lineno); found = 1; - for (check_item = fr_cursor_init(&check, &pl->check); - check_item; - check_item = fr_cursor_next(&check)) { + fr_pair_list_init(&check_list); + fr_cursor_init(&check, &check_list); + + for (map = fr_cursor_init(&cursor, &pl->reply); + map; + map = fr_cursor_next(&cursor)) { + if (map_to_vp(packet, &check_item, request, map, NULL) < 0) { + RPWARN("Failed parsing map %s for check item, skipping it", map->lhs->name); + continue; + } + if (check_item->da == attr_fall_through) { if (check_item->vp_uint32 == 1) { fall_through = 1; + fr_pair_list_free(&check_item); continue; } } else if (check_item->da == attr_relax_filter) { @@ -240,12 +261,14 @@ static unlang_action_t CC_HINT(nonnull(1,2)) attr_filter_common(rlm_rcode_t *p_r * the output list without checking it. */ if (check_item->op == T_OP_SET ) { - vp = fr_pair_copy(packet, check_item); - if (!vp) goto error; - - xlat_eval_pair(request, vp); - fr_cursor_append(&out, vp); + fr_cursor_append(&out, check_item); + continue; } + + /* + * Append the realized VP to the check list. + */ + fr_cursor_append(&check, check_item); } /* @@ -256,9 +279,9 @@ static unlang_action_t CC_HINT(nonnull(1,2)) attr_filter_common(rlm_rcode_t *p_r * only if it matches all rules that describe an * Idle-Timeout. */ - for (input_item = fr_cursor_init(&input, &packet->vps); + for (input_item = fr_cursor_init(&cursor, &packet->vps); input_item; - input_item = fr_cursor_next(&input)) { + input_item = fr_cursor_next(&cursor)) { pass = fail = 0; /* reset the pass,fail vars for each reply item */ /* @@ -290,13 +313,13 @@ static unlang_action_t CC_HINT(nonnull(1,2)) attr_filter_common(rlm_rcode_t *p_r * should copy unmatched attributes ('relaxed' mode). */ if (fail == 0 && (pass > 0 || relax_filter)) { + fr_pair_t *vp; + if (!pass) { RDEBUG3("Attribute \"%s\" allowed by relaxed mode", input_item->da->name); } - vp = fr_pair_copy(packet, input_item); - if (!vp) { - goto error; - } + vp = fr_cursor_remove(&check); + fr_assert(vp != NULL); fr_cursor_append(&out, vp); } } @@ -322,10 +345,6 @@ static unlang_action_t CC_HINT(nonnull(1,2)) attr_filter_common(rlm_rcode_t *p_r packet->vps = output; RETURN_MODULE_UPDATED; - -error: - fr_pair_list_free(&output); - RETURN_MODULE_FAIL; } #define RLM_AF_FUNC(_x, _y) static unlang_action_t CC_HINT(nonnull) mod_##_x(rlm_rcode_t *p_result, module_ctx_t const *mctx, request_t *request) \ diff --git a/src/modules/rlm_files/rlm_files.c b/src/modules/rlm_files/rlm_files.c index c54a251797b..4e7bdbaad79 100644 --- a/src/modules/rlm_files/rlm_files.c +++ b/src/modules/rlm_files/rlm_files.c @@ -75,16 +75,6 @@ fr_dict_attr_autoload_t rlm_files_dict_attr[] = { { NULL } }; -/* - * See if a fr_pair_t list contains Fall-Through = Yes - */ -static int fall_through(fr_pair_list_t const *vps) -{ - fr_pair_t *tmp; - tmp = fr_pair_find_by_da(vps, attr_fall_through); - - return tmp ? tmp->vp_uint32 : 0; -} static const CONF_PARSER module_config[] = { { FR_CONF_OFFSET("filename", FR_TYPE_FILE_INPUT, rlm_files_t, filename) }, @@ -105,7 +95,6 @@ static int pairlist_cmp(void const *a, void const *b) static int getusersfile(TALLOC_CTX *ctx, char const *filename, rbtree_t **ptree) { int rcode; - fr_pair_t *vp; PAIR_LIST *users = NULL; PAIR_LIST *entry, *next; PAIR_LIST *user_list, *default_list, **default_tail; @@ -126,6 +115,8 @@ static int getusersfile(TALLOC_CTX *ctx, char const *filename, rbtree_t **ptree) */ entry = users; while (entry) { + map_t *map; + fr_dict_attr_t const *da; fr_cursor_t cursor; /* @@ -135,14 +126,22 @@ static int getusersfile(TALLOC_CTX *ctx, char const *filename, rbtree_t **ptree) * and probably ':=' for server * configuration items. */ - for (vp = fr_cursor_init(&cursor, &entry->check); - vp; - vp = fr_cursor_next(&cursor)) { + for (map = fr_cursor_init(&cursor, &entry->check); + map; + map = fr_cursor_next(&cursor)) { + if (!tmpl_is_attr(map->lhs)) { + ERROR("%s[%d] Left side of check item %s is not an attribute", + filename, entry->lineno, map->lhs->name); + return -1; + + } + da = tmpl_da(map->lhs); + /* * Ignore attributes which are set * properly. */ - if (vp->op != T_OP_EQ) { + if (map->op != T_OP_EQ) { continue; } @@ -151,13 +150,13 @@ static int getusersfile(TALLOC_CTX *ctx, char const *filename, rbtree_t **ptree) * or it's a wire protocol, * ensure it has '=='. */ - if ((fr_dict_vendor_num_by_da(vp->da) != 0) || - (vp->da->attr < 0x100)) { - WARN("[%s]:%d Changing '%s =' to '%s =='\n\tfor comparing RADIUS attribute in check item list for user %s", + if ((fr_dict_vendor_num_by_da(da) != 0) || + (da->attr < 0x100)) { + WARN("%s[%d] Changing '%s =' to '%s =='\n\tfor comparing RADIUS attribute in check item list for user %s", filename, entry->lineno, - vp->da->name, vp->da->name, + da->name, da->name, entry->name); - vp->op = T_OP_CMP_EQ; + map->op = T_OP_CMP_EQ; continue; } } /* end of loop over check items */ @@ -169,9 +168,16 @@ static int getusersfile(TALLOC_CTX *ctx, char const *filename, rbtree_t **ptree) * It's a common enough mistake, that it's * worth doing. */ - for (vp = fr_cursor_init(&cursor, &entry->reply); - vp; - vp = fr_cursor_next(&cursor)) { + for (map = fr_cursor_init(&cursor, &entry->reply); + map; + map = fr_cursor_next(&cursor)) { + if (!tmpl_is_attr(map->lhs)) { + ERROR("%s[%d] Left side of reply item %s is not an attribute", + filename, entry->lineno, map->rhs->name); + return -1; + } + da = tmpl_da(map->lhs); + /* * If it's NOT a vendor attribute, * and it's NOT a wire protocol @@ -179,13 +185,22 @@ static int getusersfile(TALLOC_CTX *ctx, char const *filename, rbtree_t **ptree) * then bitch about it, giving a * good warning message. */ - if (fr_dict_attr_is_top_level(vp->da) && (vp->da->attr > 1000)) { - WARN("[%s]:%d Check item \"%s\"\n" - "\tfound in reply item list for user \"%s\".\n" - "\tThis attribute MUST go on the first line" - " with the other check items", filename, entry->lineno, vp->da->name, - entry->name); + if (fr_dict_attr_is_top_level(da) && (da->attr > 1000)) { + WARN("%s[%d] Check item \"%s\"\n" + "\tfound in reply item list for user \"%s\".\n" + "\tThis attribute MUST go on the first line" + " with the other check items", filename, entry->lineno, da->name, + entry->name); } + + /* + * If we allow list qualifiers in + * users_file.c, then this module also + * needs to be updated. Ensure via an + * assertion that they do not get out of + * sync. + */ + fr_assert(tmpl_list(map->lhs) == PAIR_LIST_REPLY); } entry = entry->next; @@ -212,6 +227,17 @@ static int getusersfile(TALLOC_CTX *ctx, char const *filename, rbtree_t **ptree) next = entry->next; entry->next = NULL; + /* + * @todo - loop over entry->reply, calling + * unlang_fixup_update() or unlang_fixup_filter() + * to double-check the maps. + * + * Those functions do normalization and sanity + * checks which are needed if this module is + * going to call an unlang function to *apply* + * the maps. + */ + /* * DEFAULT entries get their own list. */ @@ -255,6 +281,11 @@ static int getusersfile(TALLOC_CTX *ctx, char const *filename, rbtree_t **ptree) /* * Find the tail of this list, and add it * there. + * + * @todo - maybe use dlists here to avoid + * O(N^2) issues? But people who put 10K + * entries for the same username should + * really re-think their approach. */ while (user_list->next) user_list = user_list->next; @@ -296,17 +327,11 @@ static unlang_action_t file_common(rlm_rcode_t *p_result, rlm_files_t const *ins fr_radius_packet_t *packet, fr_radius_packet_t *reply) { char const *name; - fr_pair_list_t check_tmp, reply_tmp; PAIR_LIST const *user_pl, *default_pl; bool found = false; PAIR_LIST my_pl; char buffer[256]; - fr_pair_list_init(&check_tmp); - fr_pair_list_init(&reply_tmp); - fr_pair_list_init(&my_pl.check); - fr_pair_list_init(&my_pl.reply); - if (tmpl_expand(&name, buffer, sizeof(buffer), request, inst->key, NULL, NULL) < 0) { REDEBUG("Failed expanding key %s", inst->key->name); RETURN_MODULE_FAIL; @@ -323,9 +348,12 @@ static unlang_action_t file_common(rlm_rcode_t *p_result, rlm_files_t const *ins * Find the entry for the user. */ while (user_pl || default_pl) { - fr_cursor_t cursor; fr_pair_t *vp; + map_t *map; PAIR_LIST const *pl; + fr_pair_list_t list; + fr_cursor_t cursor, list_cursor; + bool fall_through = false; /* * Figure out which entry to match on. @@ -348,46 +376,86 @@ static unlang_action_t file_common(rlm_rcode_t *p_result, rlm_files_t const *ins default_pl = default_pl->next; } - MEM(fr_pair_list_copy(request, &check_tmp, &pl->check) >= 0); - for (vp = fr_cursor_init(&cursor, &check_tmp); - vp; - vp = fr_cursor_next(&cursor)) { - if (xlat_eval_pair(request, vp) < 0) { - RPWARN("Failed parsing expanded value for check item, skipping entry"); - fr_pair_list_free(&check_tmp); - continue; + fr_pair_list_init(&list); + fr_cursor_init(&list_cursor, &list); + + /* + * Realize the map to a list of VPs + * + * @todo convert the pl->check to fr_cond_t, and just use that! + */ + for (map = fr_cursor_init(&cursor, &pl->check); + map; + map = fr_cursor_next(&cursor)) { + if (map_to_vp(request, &vp, request, map, NULL) < 0) { + fr_pair_list_free(&list); + RPWARN("Failed parsing map for check item, skipping entry"); + break; } + VP_VERIFY(vp); + + /* + * @todo - handle an actual list. + * + * This short-cut SHOULD be OK, as the + * parser above ensures that the LHS of + * the map is an attribute, and not a + * list. + */ + + fr_assert(vp->next == NULL); + fr_cursor_append(&list_cursor, vp); + fr_cursor_tail(&list_cursor); } - if (paircmp(request, &packet->vps, &check_tmp) == 0) { - RDEBUG2("Found match \"%s\" one line %d of %s", pl->name, pl->lineno, filename); - found = true; + if (paircmp(request, &packet->vps, &list) != 0) { + fr_pair_list_free(&list); + continue; + } - /* ctx may be reply */ - if (pl->reply) { - MEM(fr_pair_list_copy(reply, &reply_tmp, &pl->reply) >= 0); - radius_pairmove(request, &reply->vps, &reply_tmp, true); - } - fr_pair_list_move(&request->control_pairs, &check_tmp); + RDEBUG2("Found match \"%s\" one line %d of %s", pl->name, pl->lineno, filename); + found = true; + fall_through = false; - reply_tmp = NULL; /* radius_pairmove() frees input attributes */ - fr_pair_list_free(&check_tmp); + /* + * Move the control items over, too. + */ + fr_pair_list_move(&request->control_pairs, &list); + fr_pair_list_free(&list); + + /* ctx may be reply */ + if (pl->reply) { + for (map = fr_cursor_init(&cursor, &pl->reply); + map; + map = fr_cursor_next(&cursor)) { + if (map->op == T_OP_CMP_FALSE) continue; + + if (map_to_vp(reply, &vp, request, map, NULL) < 0) { + RPWARN("Failed parsing map for reply item %s, skipping it", map->rhs->name); + break; + } - /* - * Fallthrough? - */ - if (!fall_through(&pl->reply)) break; + /* + * Check for Fall-Through in the + * reply list. If so, don't copy + * the attribute over to the reply + */ + if (vp->da == attr_fall_through) { + fall_through = vp->vp_bool; + fr_pair_list_free(&vp); + continue; + } + + radius_pairmove(request, &reply->vps, &vp, true); + } } - /* Ensure temporary check list is clear before next match */ - fr_pair_list_free(&check_tmp); + /* + * Fallthrough? + */ + if (!fall_through) break; } - /* - * Remove server internal parameters. - */ - fr_pair_delete_by_da(&reply->vps, attr_fall_through); - /* * See if we succeeded. */