From 35ca6a7b2f4a597a2cde9a422e625eef9f96f4a6 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Tue, 15 May 2018 14:10:32 +0200 Subject: [PATCH 1/5] settings: Add reference feature Similar to the `also` keyword in ipsec.conf, the new syntax allows adding one or more references to other sections, which means all the settings and subsections defined there are inherited (values may be overridden, even with an empty value to clear it). It's important to note that all subsections are inherited, so if this is used to reference a connection in swanctl.conf all auth rounds and children are inherited. There is currently no syntax to limit the inclusion level or clear inherited sections (but as mentioned, settings in those inherited sections may be overridden). Another property is that inherited settings or sections always follow explicitly defined entries in the current section when they are enumerated. This is relevant if the order is important (e.g. for auth rounds if `round` is not specified). References are evaluated dynamically at runtime, so referring to sections later in the config file or included via other files is no problem. The colon used as separator to reference other sections may be used in section names by writing :: (e.g. for Windows log file paths). This is based on a patch originally written in 2016. --- src/libstrongswan/settings/settings.c | 355 +++++++++++------- src/libstrongswan/settings/settings.h | 26 +- src/libstrongswan/settings/settings_lexer.l | 60 ++- src/libstrongswan/settings/settings_parser.y | 45 ++- src/libstrongswan/settings/settings_types.c | 90 ++++- src/libstrongswan/settings/settings_types.h | 33 +- .../tests/suites/test_settings.c | 348 +++++++++++++++++ 7 files changed, 800 insertions(+), 157 deletions(-) diff --git a/src/libstrongswan/settings/settings.c b/src/libstrongswan/settings/settings.c index a4c5060fa..4d1f74344 100644 --- a/src/libstrongswan/settings/settings.c +++ b/src/libstrongswan/settings/settings.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2010-2014 Tobias Brunner + * Copyright (C) 2010-2018 Tobias Brunner * Copyright (C) 2008 Martin Willi * HSR Hochschule fuer Technik Rapperswil * @@ -73,6 +73,7 @@ struct private_settings_t { /** * Print a format key, but consume already processed arguments + * Note that key and start point into the same string */ static bool print_key(char *buf, int len, char *start, char *key, va_list args) { @@ -80,6 +81,11 @@ static bool print_key(char *buf, int len, char *start, char *key, va_list args) char *pos = start; bool res; + if (!args) + { + return snprintf(buf, len, "%s", key) < len; + } + va_copy(copy, args); while (TRUE) { @@ -114,6 +120,25 @@ static bool print_key(char *buf, int len, char *start, char *key, va_list args) return res; } +/** + * Check if the given section is contained in the given array. + */ +static bool has_section(array_t *array, section_t *section) +{ + section_t *current; + int i; + + for (i = 0; i < array_count(array); i++) + { + array_get(array, i, ¤t); + if (current == section) + { + return TRUE; + } + } + return FALSE; +} + /** * Find a section by a given key, using buffered key, reusable buffer. * If "ensure" is TRUE, the sections are created if they don't exist. @@ -160,15 +185,34 @@ static section_t *find_section_buffered(section_t *section, } /** - * Find all sections via a given key considering fallbacks, using buffered key, + * Forward declaration + */ +static array_t *find_sections(private_settings_t *this, section_t *section, + char *key, va_list args, array_t **sections); + +/** + * Resolve the given reference. Not thread-safe. + */ +static void resolve_reference(private_settings_t *this, section_ref_t *ref, + array_t **sections) +{ + find_sections(this, this->top, ref->name, NULL, sections); +} + +/** + * Find all sections via a given key considering references, using buffered key, * reusable buffer. */ -static void find_sections_buffered(section_t *section, char *start, char *key, - va_list args, char *buf, int len, array_t **sections) +static void find_sections_buffered(private_settings_t *this, section_t *section, + char *start, char *key, va_list args, + char *buf, int len, bool ignore_refs, + array_t **sections) { - section_t *found = NULL, *fallback; + section_t *found = NULL, *reference; + array_t *references; + section_ref_t *ref; char *pos; - int i; + int i, j; if (!section) { @@ -184,7 +228,7 @@ static void find_sections_buffered(section_t *section, char *start, char *key, return; } if (pos) - { /* restore so we can follow fallbacks */ + { /* restore so we can follow references */ *pos = '.'; } if (!strlen(buf)) @@ -199,26 +243,39 @@ static void find_sections_buffered(section_t *section, char *start, char *key, { if (pos) { - find_sections_buffered(found, start, pos+1, args, buf, len, - sections); + find_sections_buffered(this, found, start, pos+1, args, buf, len, + FALSE, sections); } - else + else if (!has_section(*sections, found)) { + /* ignore if already added to avoid loops */ array_insert_create(sections, ARRAY_TAIL, found); - for (i = 0; i < array_count(found->fallbacks); i++) + /* add all sections that are referenced here (also resolves + * references in parent sections of the referenced section) */ + for (i = 0; i < array_count(found->references); i++) { - array_get(found->fallbacks, i, &fallback); - array_insert_create(sections, ARRAY_TAIL, fallback); + array_get(found->references, i, &ref); + resolve_reference(this, ref, sections); } } } - if (section->fallbacks) + if (!ignore_refs && section != found && section->references) { - for (i = 0; i < array_count(section->fallbacks); i++) + /* find matching sub-sections relative to the referenced sections */ + for (i = 0; i < array_count(section->references); i++) { - array_get(section->fallbacks, i, &fallback); - find_sections_buffered(fallback, start, key, args, buf, len, - sections); + array_get(section->references, i, &ref); + references = NULL; + resolve_reference(this, ref, &references); + for (j = 0; j < array_count(references); j++) + { + array_get(references, j, &reference); + /* ignore references in this referenced section, they were + * resolved via resolve_reference() */ + find_sections_buffered(this, reference, start, key, args, + buf, len, TRUE, sections); + } + array_destroy(references); } } } @@ -245,101 +302,46 @@ static section_t *ensure_section(private_settings_t *this, section_t *section, } /** - * Find a section by a given key with its fallbacks (not thread-safe!). - * Sections are returned in depth-first order (array is allocated). NULL is - * returned if no sections are found. + * Find a section by a given key with resolved references (not thread-safe!). + * The array is allocated. NULL is returned if no sections are found. */ static array_t *find_sections(private_settings_t *this, section_t *section, - char *key, va_list args) + char *key, va_list args, array_t **sections) { char buf[128], keybuf[512]; - array_t *sections = NULL; if (snprintf(keybuf, sizeof(keybuf), "%s", key) >= sizeof(keybuf)) { return NULL; } - find_sections_buffered(section, keybuf, keybuf, args, buf, - sizeof(buf), §ions); - return sections; -} - -/** - * Check if the given fallback section already exists - */ -static bool fallback_exists(section_t *section, section_t *fallback) -{ - if (section == fallback) - { - return TRUE; - } - else if (section->fallbacks) - { - section_t *existing; - int i; - - for (i = 0; i < array_count(section->fallbacks); i++) - { - array_get(section->fallbacks, i, &existing); - if (existing == fallback) - { - return TRUE; - } - } - } - return FALSE; -} - -/** - * Ensure that the section with the given key exists and add the given fallback - * section (thread-safe). - */ -static void add_fallback_to_section(private_settings_t *this, - section_t *section, const char *key, va_list args, - section_t *fallback) -{ - char buf[128], keybuf[512]; - section_t *found; - - if (snprintf(keybuf, sizeof(keybuf), "%s", key) >= sizeof(keybuf)) - { - return; - } - this->lock->write_lock(this->lock); - found = find_section_buffered(section, keybuf, keybuf, args, buf, - sizeof(buf), TRUE); - if (!fallback_exists(found, fallback)) - { - /* to ensure sections referred to as fallback are not purged, we create - * the array there too */ - if (!fallback->fallbacks) - { - fallback->fallbacks = array_create(0, 0); - } - array_insert_create(&found->fallbacks, ARRAY_TAIL, fallback); - } - this->lock->unlock(this->lock); + find_sections_buffered(this, section, keybuf, keybuf, args, buf, + sizeof(buf), FALSE, sections); + return *sections; } /** * Find the key/value pair for a key, using buffered key, reusable buffer - * If "ensure" is TRUE, the sections (and key/value pair) are created if they - * don't exist. - * Fallbacks are only considered if "ensure" is FALSE. + * There are two modes: 1. To find a key at an exact location and create the + * sections (and key/value pair) if necessary, don't pass an array for sections. + * 2. To find a key and follow references pass a pointer to an array to store + * visited sections. NULL is returned in this case if the key is not found. */ -static kv_t *find_value_buffered(section_t *section, char *start, char *key, - va_list args, char *buf, int len, bool ensure) +static kv_t *find_value_buffered(private_settings_t *this, section_t *section, + char *start, char *key, va_list args, + char *buf, int len, bool ignore_refs, + array_t **sections) { - int i; - char *pos; - kv_t *kv = NULL; section_t *found = NULL; + kv_t *kv = NULL; + section_ref_t *ref; + array_t *references; + char *pos; + int i, j; - if (section == NULL) + if (!section) { return NULL; } - pos = strchr(key, '.'); if (pos) { @@ -348,7 +350,7 @@ static kv_t *find_value_buffered(section_t *section, char *start, char *key, { return NULL; } - /* restore so we can retry for fallbacks */ + /* restore so we can follow references */ *pos = '.'; if (!strlen(buf)) { @@ -357,7 +359,7 @@ static kv_t *find_value_buffered(section_t *section, char *start, char *key, else if (array_bsearch(section->sections, buf, settings_section_find, &found) == -1) { - if (ensure) + if (!sections) { found = settings_section_create(strdup(buf)); settings_section_add(section, found, NULL); @@ -365,44 +367,133 @@ static kv_t *find_value_buffered(section_t *section, char *start, char *key, } if (found) { - kv = find_value_buffered(found, start, pos+1, args, buf, len, - ensure); - } - if (!kv && !ensure && section->fallbacks) - { - for (i = 0; !kv && i < array_count(section->fallbacks); i++) - { - array_get(section->fallbacks, i, &found); - kv = find_value_buffered(found, start, key, args, buf, len, - ensure); - } + kv = find_value_buffered(this, found, start, pos+1, args, buf, len, + FALSE, sections); } } else { + if (sections) + { + array_insert_create(sections, ARRAY_TAIL, section); + } if (!print_key(buf, len, start, key, args)) { return NULL; } if (array_bsearch(section->kv, buf, settings_kv_find, &kv) == -1) { - if (ensure) + if (!sections) { kv = settings_kv_create(strdup(buf), NULL); settings_kv_add(section, kv, NULL); } - else if (section->fallbacks) + } + } + if (!kv && !ignore_refs && sections && section->references) + { + /* find key relative to the referenced sections */ + for (i = 0; !kv && i < array_count(section->references); i++) + { + array_get(section->references, i, &ref); + references = NULL; + resolve_reference(this, ref, &references); + for (j = 0; !kv && j < array_count(references); j++) { - for (i = 0; !kv && i < array_count(section->fallbacks); i++) + array_get(references, j, &found); + /* ignore if already added to avoid loops */ + if (!has_section(*sections, found)) { - array_get(section->fallbacks, i, &found); - kv = find_value_buffered(found, start, key, args, buf, len, - ensure); + /* ignore references in this referenced section, they were + * resolved via resolve_reference() */ + kv = find_value_buffered(this, found, start, key, args, + buf, len, TRUE, sections); + } + } + array_destroy(references); + } + } + return kv; +} + +/** + * Remove the key/value pair for a key, using buffered key, reusable buffer + */ +static void remove_value_buffered(private_settings_t *this, section_t *section, + char *start, char *key, va_list args, + char *buf, int len) +{ + section_t *found = NULL; + kv_t *kv = NULL, *ordered = NULL; + char *pos; + int idx, i; + + if (!section) + { + return; + } + pos = strchr(key, '.'); + if (pos) + { + *pos = '\0'; + pos++; + } + if (!print_key(buf, len, start, key, args)) + { + return; + } + if (!strlen(buf)) + { + found = section; + } + if (pos) + { + if (array_bsearch(section->sections, buf, settings_section_find, + &found) != -1) + { + remove_value_buffered(this, found, start, pos, args, buf, len); + } + } + else + { + idx = array_bsearch(section->kv, buf, settings_kv_find, &kv); + if (idx != -1) + { + array_remove(section->kv, idx, NULL); + for (i = 0; i < array_count(section->kv_order); i++) + { + array_get(section->kv_order, i, &ordered); + if (kv == ordered) + { + array_remove(section->kv_order, i, NULL); + settings_kv_destroy(kv, this->contents); } } } } - return kv; +} + +/* + * Described in header + */ +void settings_remove_value(settings_t *settings, char *key, ...) +{ + private_settings_t *this = (private_settings_t*)settings; + char buf[128], keybuf[512]; + va_list args; + + if (snprintf(keybuf, sizeof(keybuf), "%s", key) >= sizeof(keybuf)) + { + return; + } + va_start(args, key); + + this->lock->read_lock(this->lock); + remove_value_buffered(this, this->top, keybuf, keybuf, args, buf, + sizeof(buf)); + this->lock->unlock(this->lock); + + va_end(args); } /** @@ -412,6 +503,7 @@ static char *find_value(private_settings_t *this, section_t *section, char *key, va_list args) { char buf[128], keybuf[512], *value = NULL; + array_t *sections = NULL; kv_t *kv; if (snprintf(keybuf, sizeof(keybuf), "%s", key) >= sizeof(keybuf)) @@ -419,13 +511,14 @@ static char *find_value(private_settings_t *this, section_t *section, return NULL; } this->lock->read_lock(this->lock); - kv = find_value_buffered(section, keybuf, keybuf, args, buf, sizeof(buf), - FALSE); + kv = find_value_buffered(this, section, keybuf, keybuf, args, + buf, sizeof(buf), FALSE, §ions); if (kv) { value = kv->value; } this->lock->unlock(this->lock); + array_destroy(sections); return value; } @@ -443,8 +536,8 @@ static void set_value(private_settings_t *this, section_t *section, return; } this->lock->write_lock(this->lock); - kv = find_value_buffered(section, keybuf, keybuf, args, buf, sizeof(buf), - TRUE); + kv = find_value_buffered(this, section, keybuf, keybuf, args, + buf, sizeof(buf), FALSE, NULL); if (kv) { settings_kv_set(kv, strdupnull(value), this->contents); @@ -761,12 +854,12 @@ METHOD(settings_t, create_section_enumerator, enumerator_t*, private_settings_t *this, char *key, ...) { enumerator_data_t *data; - array_t *sections; + array_t *sections = NULL; va_list args; this->lock->read_lock(this->lock); va_start(args, key); - sections = find_sections(this, this->top, key, args); + sections = find_sections(this, this->top, key, args, §ions); va_end(args); if (!sections) @@ -793,13 +886,17 @@ CALLBACK(kv_filter, bool, while (orig->enumerate(orig, &kv)) { - if (seen->get(seen, kv->key) || !kv->value) + if (seen->get(seen, kv->key)) + { + continue; + } + seen->put(seen, kv->key, kv->key); + if (!kv->value) { continue; } *key = kv->key; *value = kv->value; - seen->put(seen, kv->key, kv->key); return TRUE; } return FALSE; @@ -818,12 +915,12 @@ METHOD(settings_t, create_key_value_enumerator, enumerator_t*, private_settings_t *this, char *key, ...) { enumerator_data_t *data; - array_t *sections; + array_t *sections = NULL; va_list args; this->lock->read_lock(this->lock); va_start(args, key); - sections = find_sections(this, this->top, key, args); + sections = find_sections(this, this->top, key, args, §ions); va_end(args); if (!sections) @@ -845,14 +942,20 @@ METHOD(settings_t, add_fallback, void, { section_t *section; va_list args; + char buf[512]; - /* find/create the fallback */ + /* find/create the section */ va_start(args, fallback); - section = ensure_section(this, this->top, fallback, args); + section = ensure_section(this, this->top, key, args); va_end(args); va_start(args, fallback); - add_fallback_to_section(this, this->top, key, args, section); + if (vsnprintf(buf, sizeof(buf), fallback, args) < sizeof(buf)) + { + this->lock->write_lock(this->lock); + settings_reference_add(section, strdup(buf), TRUE); + this->lock->unlock(this->lock); + } va_end(args); } diff --git a/src/libstrongswan/settings/settings.h b/src/libstrongswan/settings/settings.h index e25c9da38..814cf32e5 100644 --- a/src/libstrongswan/settings/settings.h +++ b/src/libstrongswan/settings/settings.h @@ -1,5 +1,5 @@ /* - * Copyright (C) 2010 Tobias Brunner + * Copyright (C) 2010-2018 Tobias Brunner * Copyright (C) 2008 Martin Willi * HSR Hochschule fuer Technik Rapperswil * @@ -288,15 +288,9 @@ struct settings_t { * 'section-one.two' will result in a lookup for the same section/key * in 'section-two'. * - * @note Lookups are depth-first and currently strictly top-down. - * For instance, if app.sec had lib1.sec as fallback and lib1 had lib2 as - * fallback the keys/sections in lib2.sec would not be considered. But if - * app had lib3 as fallback the contents of lib3.sec would (as app is passed - * during the initial lookup). In the last example the order during - * enumerations would be app.sec, lib1.sec, lib3.sec. - * * @note Additional arguments will be applied to both section format - * strings so they must be compatible. + * strings so they must be compatible. And they are evaluated immediately, + * so arguments can't contain dots. * * @param section section for which a fallback is configured, printf style * @param fallback fallback section, printf style @@ -413,4 +407,18 @@ settings_t *settings_create(char *file); */ settings_t *settings_create_string(char *settings); +/** + * Remove the given key/value. + * + * Compared to setting a key to NULL, which makes it appear to be unset (i.e. + * default values will apply) this removes the given key (if found) and + * references/fallbacks will apply when looking for that key. This is mainly + * usefuls for the unit tests. + * + * @param settings settings to remove key/value from + * @param key key including sections, printf style format + * @param ... argument list for key + */ +void settings_remove_value(settings_t *settings, char *key, ...); + #endif /** SETTINGS_H_ @}*/ diff --git a/src/libstrongswan/settings/settings_lexer.l b/src/libstrongswan/settings/settings_lexer.l index fa1ecac10..9cde11900 100644 --- a/src/libstrongswan/settings/settings_lexer.l +++ b/src/libstrongswan/settings/settings_lexer.l @@ -1,6 +1,6 @@ %{ /* - * Copyright (C) 2014 Tobias Brunner + * Copyright (C) 2014-2018 Tobias Brunner * HSR Hochschule fuer Technik Rapperswil * * This program is free software; you can redistribute it and/or modify it @@ -49,6 +49,8 @@ static void include_files(parser_helper_t *ctx); /* type of our extra data */ %option extra-type="parser_helper_t*" +/* state used to scan names */ +%x nam /* state used to scan values */ %x val /* state used to scan include file patterns */ @@ -56,14 +58,20 @@ static void include_files(parser_helper_t *ctx); /* state used to scan quoted strings */ %x str +/* pattern for section/key names */ +NAME [^#{}:,="\r\n\t ] + %% [\t ]*#[^\r\n]* /* eat comments */ [\t\r ]+ /* eat whitespace */ -\n|#.*\n return NEWLINE; /* also eats comments at the end of a line */ +\n|#.*\n /* eat newlines and comments at the end of a line */ "{" | -"}" return yytext[0]; +"}" | +"," return yytext[0]; + +":" return REFS; "=" { yy_push_state(val, yyscanner); @@ -80,16 +88,49 @@ static void include_files(parser_helper_t *ctx); return STRING_ERROR; } -[^#{}="\r\n\t ]+ { - yylval->s = strdup(yytext); - return NAME; +{NAME} { + yyextra->string_init(yyextra); + yyextra->string_add(yyextra, yytext); + yy_push_state(nam, yyscanner); +} + +{ + "::" { + yyextra->string_add(yyextra, yytext+1); + } + + {NAME}+ { + yyextra->string_add(yyextra, yytext); + } + + <> | + .|[\r\n] { + if (*yytext) + { + switch (yytext[0]) + { + case '\n': + /* put the newline back to fix the line numbers */ + unput('\n'); + yy_set_bol(0); + break; + default: + /* these are parsed outside of this start condition */ + unput(yytext[0]); + break; + } + } + yy_pop_state(yyscanner); + yylval->s = yyextra->string_get(yyextra); + return NAME; + } } { \r /* just ignore these */ [\t ]+ <> | - [#}\n] { + [#}\n] { if (*yytext) { switch (yytext[0]) @@ -107,15 +148,16 @@ static void include_files(parser_helper_t *ctx); } } yy_pop_state(yyscanner); + return NEWLINE; } - "\"" { + "\"" { yyextra->string_init(yyextra); yy_push_state(str, yyscanner); } /* same as above, but allow more characters */ - [^#}"\r\n\t ]+ { + [^#}"\r\n\t ]+ { yylval->s = strdup(yytext); return NAME; } diff --git a/src/libstrongswan/settings/settings_parser.y b/src/libstrongswan/settings/settings_parser.y index 2ab9ea723..7e72a90c9 100644 --- a/src/libstrongswan/settings/settings_parser.y +++ b/src/libstrongswan/settings/settings_parser.y @@ -1,6 +1,6 @@ %{ /* - * Copyright (C) 2014 Tobias Brunner + * Copyright (C) 2014-2018 Tobias Brunner * HSR Hochschule fuer Technik Rapperswil * * This program is free software; you can redistribute it and/or modify it @@ -49,6 +49,7 @@ static section_t *push_section(parser_helper_t *ctx, char *name); static section_t *pop_section(parser_helper_t *ctx); static void add_section(parser_helper_t *ctx, section_t *section); static void add_setting(parser_helper_t *ctx, kv_t *kv); +static void add_references(parser_helper_t *ctx, array_t *references); /** * Make sure to call lexer with the proper context @@ -78,20 +79,24 @@ static int yylex(YYSTYPE *lvalp, parser_helper_t *ctx) char *s; struct section_t *sec; struct kv_t *kv; + array_t *refs; } %token NAME STRING +%token REFS ":" %token NEWLINE STRING_ERROR /* ...and other symbols */ %type value valuepart %type section_start section %type setting +%type references /* properly destroy string tokens that are strdup()ed on error */ %destructor { free($$); } NAME STRING value valuepart /* properly destroy parse results on error */ %destructor { pop_section(ctx); settings_section_destroy($$, NULL); } section_start section %destructor { settings_kv_destroy($$, NULL); } setting +%destructor { array_destroy_function($$, (void*)free, NULL); } references /* there are two shift/reduce conflicts because of the "NAME = NAME" and * "NAME {" ambiguity, and the "NAME =" rule) */ @@ -133,9 +138,24 @@ section_start: $$ = push_section(ctx, $NAME); } | - NAME NEWLINE '{' + NAME ":" references '{' { $$ = push_section(ctx, $NAME); + add_references(ctx, $references); + array_destroy($references); + } + ; + +references: + NAME + { + $$ = array_create(0, 0); + array_insert($$, ARRAY_TAIL, $1); + } + | references ',' NAME + { + array_insert($1, ARRAY_TAIL, $3); + $$ = $1; } ; @@ -238,6 +258,27 @@ static void add_setting(parser_helper_t *ctx, kv_t *kv) settings_kv_add(section, kv, NULL); } +/** + * Adds the given references to the section on top of the stack + */ +static void add_references(parser_helper_t *ctx, array_t *references) +{ + array_t *sections = (array_t*)ctx->context; + section_t *section; + enumerator_t *refs; + char *ref; + + array_get(sections, ARRAY_TAIL, §ion); + + refs = array_create_enumerator(references); + while (refs->enumerate(refs, &ref)) + { + settings_reference_add(section, ref, FALSE); + array_remove_at(references, refs); + } + refs->destroy(refs); +} + /** * Parse the given file and add all sections and key/value pairs to the * given section. diff --git a/src/libstrongswan/settings/settings_types.c b/src/libstrongswan/settings/settings_types.c index 1c2d61de7..625b70409 100644 --- a/src/libstrongswan/settings/settings_types.c +++ b/src/libstrongswan/settings/settings_types.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2010-2014 Tobias Brunner + * Copyright (C) 2010-2018 Tobias Brunner * HSR Hochschule fuer Technik Rapperswil * * This program is free software; you can redistribute it and/or modify it @@ -69,6 +69,12 @@ static void kv_destroy(kv_t *kv, int idx, array_t *contents) settings_kv_destroy(kv, contents); } +static void ref_destroy(section_ref_t *ref, int idx, void *ctx) +{ + free(ref->name); + free(ref); +} + /* * Described in header */ @@ -78,7 +84,7 @@ void settings_section_destroy(section_t *this, array_t *contents) array_destroy(this->sections_order); array_destroy_function(this->kv, (void*)kv_destroy, contents); array_destroy(this->kv_order); - array_destroy(this->fallbacks); + array_destroy_function(this->references, (void*)ref_destroy, NULL); free(this->name); free(this); } @@ -129,6 +135,35 @@ void settings_kv_add(section_t *section, kv_t *kv, array_t *contents) } } +/* + * Described in header + */ +void settings_reference_add(section_t *section, char *name, bool permanent) +{ + section_ref_t *ref; + int i; + + for (i = 0; i < array_count(section->references); i++) + { + array_get(section->references, i, &ref); + if (ref->permanent && !permanent) + { /* add it before any permanent references */ + break; + } + if (ref->permanent == permanent && streq(name, ref->name)) + { + free(name); + return; + } + } + + INIT(ref, + .name = name, + .permanent = permanent, + ); + array_insert_create(§ion->references, i, ref); +} + /* * Add a section to the given parent, optionally remove settings/subsections * not found when extending an existing section @@ -167,14 +202,28 @@ void settings_section_add(section_t *parent, section_t *section, static bool section_purge(section_t *this, array_t *contents) { section_t *current; + section_ref_t *ref; int i, idx; array_destroy_function(this->kv, (void*)kv_destroy, contents); this->kv = NULL; array_destroy(this->kv_order); this->kv_order = NULL; - /* we ensure sections used as fallback, or configured with fallbacks (or - * having any such subsections) are not removed */ + /* remove non-permanent references */ + for (i = array_count(this->references) - 1; i >= 0; i--) + { + array_get(this->references, i, &ref); + if (!ref->permanent) + { + array_remove(this->references, i, NULL); + ref_destroy(ref, 0, NULL); + } + } + if (!array_count(this->references)) + { + array_destroy(this->references); + this->references = NULL; + } for (i = array_count(this->sections_order) - 1; i >= 0; i--) { array_get(this->sections_order, i, ¤t); @@ -187,7 +236,9 @@ static bool section_purge(section_t *this, array_t *contents) settings_section_destroy(current, contents); } } - return !this->fallbacks && !array_count(this->sections); + /* we ensure sections configured with permanent references (or having any + * such subsections) are not removed */ + return !this->references && !array_count(this->sections); } /* @@ -198,14 +249,15 @@ void settings_section_extend(section_t *base, section_t *extension, { enumerator_t *enumerator; section_t *section; + section_ref_t *ref; kv_t *kv; array_t *sections = NULL, *kvs = NULL; int idx; if (purge) - { /* remove sections and settings in base not found in extension, the - * others are removed too (from the _order list) so they can be inserted - * in the order found in extension */ + { /* remove sections, settings in base not found in extension, the others + * are removed too (from the _order list) so they can be inserted in the + * order found in extension, non-permanent references are removed */ enumerator = array_create_enumerator(base->sections_order); while (enumerator->enumerate(enumerator, (void**)§ion)) { @@ -245,6 +297,18 @@ void settings_section_extend(section_t *base, section_t *extension, array_sort(kvs, settings_kv_sort, NULL); } } + + enumerator = array_create_enumerator(base->references); + while (enumerator->enumerate(enumerator, (void**)&ref)) + { + if (ref->permanent) + { /* permanent references are ignored */ + continue; + } + array_remove_at(base->references, enumerator); + ref_destroy(ref, 0, NULL); + } + enumerator->destroy(enumerator); } while (array_remove(extension->sections_order, 0, §ion)) @@ -278,6 +342,16 @@ void settings_section_extend(section_t *base, section_t *extension, array_remove(extension->kv, idx, NULL); settings_kv_add(base, kv, contents); } + + while (array_remove(extension->references, 0, &ref)) + { + if (ref->permanent) + { /* ignore permanent references in the extension */ + continue; + } + settings_reference_add(base, strdup(ref->name), FALSE); + ref_destroy(ref, 0, NULL); + } array_destroy(sections); array_destroy(kvs); } diff --git a/src/libstrongswan/settings/settings_types.h b/src/libstrongswan/settings/settings_types.h index 82bcb230a..8163a0134 100644 --- a/src/libstrongswan/settings/settings_types.h +++ b/src/libstrongswan/settings/settings_types.h @@ -1,5 +1,5 @@ /* - * Copyright (C) 2010-2014 Tobias Brunner + * Copyright (C) 2010-2018 Tobias Brunner * HSR Hochschule fuer Technik Rapperswil * * This program is free software; you can redistribute it and/or modify it @@ -24,6 +24,7 @@ #define SETTINGS_TYPES_H_ typedef struct kv_t kv_t; +typedef struct section_ref_t section_ref_t; typedef struct section_t section_t; #include "collections/array.h" @@ -44,6 +45,23 @@ struct kv_t { char *value; }; +/** + * Section reference. + */ +struct section_ref_t { + + /** + * Name of the referenced section. + */ + char *name; + + /** + * TRUE for permanent references that were added programmatically via + * add_fallback() and are not removed during reloads/purges. + */ + bool permanent; +}; + /** * Section containing subsections and key value pairs. */ @@ -55,9 +73,9 @@ struct section_t { char *name; /** - * Fallback sections, as section_t. + * Referenced sections, as section_ref_t. */ - array_t *fallbacks; + array_t *references; /** * Subsections, as section_t. @@ -115,6 +133,15 @@ void settings_kv_set(kv_t *kv, char *value, array_t *contents); */ void settings_kv_add(section_t *section, kv_t *kv, array_t *contents); +/** + * Add a reference to another section. + * + * @param section section to which to add the reference + * @param name name of the referenced section (adopted) + * @param permanent whether the reference is not removed during reloads + */ +void settings_reference_add(section_t *section, char *name, bool permanent); + /** * Create a section with the given name. * diff --git a/src/libstrongswan/tests/suites/test_settings.c b/src/libstrongswan/tests/suites/test_settings.c index 0759f7013..39c59bbf2 100644 --- a/src/libstrongswan/tests/suites/test_settings.c +++ b/src/libstrongswan/tests/suites/test_settings.c @@ -1037,6 +1037,50 @@ START_TEST(test_add_fallback) } END_TEST +START_TEST(test_fallback_resolution) +{ + linked_list_t *keys, *values; + + settings->destroy(settings); + create_settings(chunk_from_str( + "base {\n" + " sub {\n" + " key1 = val1\n" + " key2 = val2\n" + " key5 = val5\n" + " subsub {\n" + " subkey1 = subval1\n" + " }\n" + " }\n" + "}\n" + "other {\n" + " sub {\n" + " key3 = val3\n" + " key4 = val4\n" + " }\n" + "}\n" + "main {\n" + " sub {\n" + " key4=\n" + " key5 = \n" + " }\n" + "}")); + + settings->add_fallback(settings, "other", "base"); + settings->add_fallback(settings, "main.sub", "other.sub"); + + verify_string("val1", "main.sub.key1"); + verify_string("val3", "main.sub.key3"); + verify_null("main.sub.key4"); + verify_null("main.sub.key5"); + verify_string("subval1", "main.sub.subsub.subkey1"); + + keys = linked_list_create_with_items("key3", "key1", "key2", NULL); + values = linked_list_create_with_items("val3", "val1", "val2", NULL); + verify_key_values(keys, values, "main.sub"); +} +END_TEST + START_TEST(test_add_fallback_printf) { settings->add_fallback(settings, "%s.sub1", "sub", "main"); @@ -1051,6 +1095,264 @@ START_TEST(test_add_fallback_printf) } END_TEST +START_TEST(test_references) +{ + linked_list_t *keys, *values; + + create_settings(chunk_from_str( + "main {\n" + " sub1 {\n" + " key1 = sub1val1\n" + " key2 = sub1val2\n" + " key4 = sub1val4\n" + " subsub {\n" + " subkey1 = sub1subsubval1\n" + " subkey2 = sub1subsubval2\n" + " }\n" + " subsub1 {\n" + " subkey1 = sub1subsub1val1\n" + " }\n" + " }\n" + " sub2 : main.sub1 {\n" + " key2 = sub2val2\n" + " key3 = sub2val3\n" + " key4 =\n" + " subsub {\n" + " subkey1 = sub2subsubval1\n" + " subkey3 = sub2subsubval3\n" + " }\n" + " }\n" + "}")); + + verify_string("sub1val1", "main.sub2.key1"); + verify_string("sub2val2", "main.sub2.key2"); + verify_string("sub2val3", "main.sub2.key3"); + verify_null("main.sub2.key4"); + verify_string("sub2subsubval1", "main.sub2.subsub.subkey1"); + verify_string("sub1subsubval2", "main.sub2.subsub.subkey2"); + verify_string("sub2subsubval3", "main.sub2.subsub.subkey3"); + verify_string("sub1subsub1val1", "main.sub2.subsub1.subkey1"); + + keys = linked_list_create_with_items("subsub", "subsub1", NULL); + verify_sections(keys, "main.sub2"); + + keys = linked_list_create_with_items("key2", "key3", "key1", NULL); + values = linked_list_create_with_items("sub2val2", "sub2val3", "sub1val1", NULL); + verify_key_values(keys, values, "main.sub2"); + + keys = linked_list_create_with_items("subkey1", "subkey3", "subkey2", NULL); + values = linked_list_create_with_items("sub2subsubval1", "sub2subsubval3", "sub1subsubval2", NULL); + verify_key_values(keys, values, "main.sub2.subsub"); +} +END_TEST + +START_TEST(test_references_templates) +{ + create_settings(chunk_from_str( + "sub-def {\n" + " key1 = sub1val1\n" + " key2 = sub1val2\n" + " subsub {\n" + " subkey1 = sub1subsubval1\n" + " }\n" + "}\n" + "subsub-def {\n" + " subkey1 = sub1subval1\n" + " subkey2 = sub1subval1\n" + "}\n" + "main {\n" + " sub1 : sub-def {\n" + " key1 = mainsub1val1\n" + " subsub : subsub-def {\n" + " subkey1 = mainsub1subval1\n" + " }\n" + " subsub1 {\n" + " subkey1 = mainsub1sub1val1\n" + " }\n" + " }\n" + " sub2 : sub-def {\n" + " key2 = mainsub2val2\n" + " key3 = mainsub2val3\n" + " subsub {\n" + " subkey3 = mainsub2subsubval3\n" + " }\n" + " }\n" + "}")); + + verify_string("mainsub1val1", "main.sub1.key1"); + verify_string("sub1val2", "main.sub1.key2"); + verify_string("mainsub1subval1", "main.sub1.subsub.subkey1"); + verify_string("sub1subval1", "main.sub1.subsub.subkey2"); + verify_string("mainsub1sub1val1", "main.sub1.subsub1.subkey1"); + verify_string("sub1val1", "main.sub2.key1"); + verify_string("mainsub2val2", "main.sub2.key2"); + verify_string("mainsub2val3", "main.sub2.key3"); + verify_string("sub1subsubval1", "main.sub2.subsub.subkey1"); + verify_null("main.sub2.subsub.subkey2"); + verify_string("mainsub2subsubval3", "main.sub2.subsub.subkey3"); +} +END_TEST + +START_TEST(test_references_order) +{ + linked_list_t *keys, *values; + + create_settings(chunk_from_str( + "main {\n" + " sub1 {\n" + " key1 = sub1val1\n" + " key2 = sub1val2\n" + " subsub1 {\n" + " }\n" + " }\n" + " sub2 {\n" + " key2 = sub2val2\n" + " key3 = sub2val3\n" + " subsub2 {\n" + " }\n" + " }\n" + " sub3 : main.sub1, main.sub2 {\n" + " key3 = sub3val3\n" + " }\n" + " sub4 : main.sub2, main.sub1 {\n" + " key3 = sub4val3\n" + " }\n" + "}")); + + verify_string("sub1val2", "main.sub3.key2"); + verify_string("sub3val3", "main.sub3.key3"); + verify_string("sub2val2", "main.sub4.key2"); + verify_string("sub4val3", "main.sub4.key3"); + + /* the order of referenced keys/subsections depends on the reference + * statement's order */ + keys = linked_list_create_with_items("subsub1", "subsub2", NULL); + verify_sections(keys, "main.sub3"); + + keys = linked_list_create_with_items("subsub2", "subsub1", NULL); + verify_sections(keys, "main.sub4"); + + /* local keys are always enumerated first */ + keys = linked_list_create_with_items("key3", "key1", "key2", NULL); + values = linked_list_create_with_items("sub3val3", "sub1val1", "sub1val2", NULL); + verify_key_values(keys, values, "main.sub3"); + + keys = linked_list_create_with_items("key3", "key2", "key1", NULL); + values = linked_list_create_with_items("sub4val3", "sub2val2", "sub1val1", NULL); + verify_key_values(keys, values, "main.sub4"); +} +END_TEST + +START_TEST(test_references_resolution) +{ + linked_list_t *keys, *values; + + create_settings(chunk_from_str( + "sec-a {\n" + " sub1 {\n" + " a1 = val-a1\n" + " key = sec-a-val1\n" + " sub-a {\n" + " }\n" + " }\n" + "}\n" + "sec-b : sec-a {\n" + " sub1 {\n" + " b1 = val-b1\n" + " key = sec-b-val1\n" + " sub-b1 {\n" + " }\n" + " }\n" + " sub2 {\n" + " b2 = val-b2\n" + " key = sec-b-val2\n" + " sub-b2 {\n" + " }\n" + " }\n" + "}\n" + "sec-c : sec-b {\n" + " sub2 : sec-b.sub1 {\n" + " c2 = val-c2\n" + " key = sec-c-val2\n" + " sub-c2 {\n" + " }\n" + " }\n" + "}")); + + verify_string("sec-c-val2", "sec-c.sub2.key"); + settings_remove_value(settings, "sec-c.sub2.key"); + verify_string("sec-b-val1", "sec-c.sub2.key"); + settings_remove_value(settings, "sec-b.sub1.key"); + verify_string("sec-a-val1", "sec-c.sub2.key"); + settings_remove_value(settings, "sec-a.sub1.key"); + verify_string("sec-b-val2", "sec-c.sub2.key"); + settings_remove_value(settings, "sec-b.sub2.key"); + verify_null("sec-c.sub2.key"); + + keys = linked_list_create_with_items("sub-c2", "sub-b1", "sub-a", "sub-b2", NULL); + verify_sections(keys, "sec-c.sub2"); + + keys = linked_list_create_with_items("c2", "b1", "a1", "b2", NULL); + values = linked_list_create_with_items("val-c2", "val-b1", "val-a1", "val-b2", NULL); + verify_key_values(keys, values, "sec-c.sub2"); +} +END_TEST + +START_TEST(test_references_fallback) +{ + linked_list_t *keys, *values; + +#define test_references_fallback_base_settings \ + "lib {\n" \ + " key1 = libval1\n" \ + " keylib = libval\n" \ + " sub {\n" \ + " key1 = libsubval1\n" \ + " }\n" \ + " libsub {\n" \ + " }\n" \ + "}\n" \ + "other {\n" \ + " key1 = otherval1\n" \ + " keyother = otherval\n" \ + " sub {\n" \ + " key1 = othersubval1\n" \ + " }\n" \ + " othersub {\n" \ + " }\n" \ + "}\n" + + create_settings(chunk_from_str( + test_references_fallback_base_settings "app : other {}")); + + /* references have precedence over fallbacks */ + settings->add_fallback(settings, "app", "lib"); + verify_string("otherval1", "app.key1"); + verify_string("libval", "app.keylib"); + verify_string("othersubval1", "app.sub.key1"); + + keys = linked_list_create_with_items("sub", "othersub", "libsub", NULL); + verify_sections(keys, "app"); + + keys = linked_list_create_with_items("key1", "keyother", "keylib", NULL); + values = linked_list_create_with_items("otherval1", "otherval", "libval", NULL); + verify_key_values(keys, values, "app"); + + /* fallbacks are unaffected when reloading configs with references */ + ck_assert(settings->load_string_section(settings, + test_references_fallback_base_settings "app {}", FALSE, "")); + verify_string("libval1", "app.key1"); + verify_string("libval", "app.keylib"); + verify_string("libsubval1", "app.sub.key1"); + + ck_assert(settings->load_string_section(settings, + test_references_fallback_base_settings "app : other {}", FALSE, "")); + verify_string("otherval1", "app.key1"); + verify_string("libval", "app.keylib"); + verify_string("othersubval1", "app.sub.key1"); +} +END_TEST + START_SETUP(setup_string_config) { create_settings(chunk_from_str( @@ -1115,6 +1417,37 @@ START_TEST(test_valid) ck_assert(chunk_write(contents, path, 0022, TRUE)); ck_assert(settings->load_files(settings, path, FALSE)); verify_string("a setting with = and { character", "equals"); + + contents = chunk_from_str( + "ref { key = value }\nvalid:ref {}"); + ck_assert(chunk_write(contents, path, 0022, TRUE)); + ck_assert(settings->load_files(settings, path, FALSE)); + verify_string("value", "valid.key"); + + contents = chunk_from_str( + "ref { key = value }\nvalid\n:\nref {}"); + ck_assert(chunk_write(contents, path, 0022, TRUE)); + ck_assert(settings->load_files(settings, path, FALSE)); + verify_string("value", "valid.key"); + + contents = chunk_from_str( + "ref { key = value }\nother { key1 = value1 }\nvalid\n:\nref\n\t,\nother {}"); + ck_assert(chunk_write(contents, path, 0022, TRUE)); + ck_assert(settings->load_files(settings, path, FALSE)); + verify_string("value", "valid.key"); + verify_string("value1", "valid.key1"); + + contents = chunk_from_str( + "c::\\Logfiles\\charon.log { dmn = 1 }"); + ck_assert(chunk_write(contents, path, 0022, TRUE)); + ck_assert(settings->load_files(settings, path, FALSE)); + verify_string("1", "%s.dmn", "c:\\Logfiles\\charon.log"); + + contents = chunk_from_str( + "section { c::\\Logfiles\\charon.log = 1 }"); + ck_assert(chunk_write(contents, path, 0022, TRUE)); + ck_assert(settings->load_files(settings, path, FALSE)); + verify_string("1", "section.%s", "c:\\Logfiles\\charon.log"); } END_TEST @@ -1157,6 +1490,11 @@ START_TEST(test_invalid) "\"unexpected\" = string"); ck_assert(chunk_write(contents, path, 0022, TRUE)); ck_assert(!settings->load_files(settings, path, FALSE)); + + contents = chunk_from_str( + "incorrect :: ref {}"); + ck_assert(chunk_write(contents, path, 0022, TRUE)); + ck_assert(!settings->load_files(settings, path, FALSE)); } END_TEST @@ -1331,9 +1669,19 @@ Suite *settings_suite_create() tc = tcase_create("fallback"); tcase_add_checked_fixture(tc, setup_fallback_config, teardown_config); tcase_add_test(tc, test_add_fallback); + tcase_add_test(tc, test_fallback_resolution); tcase_add_test(tc, test_add_fallback_printf); suite_add_tcase(s, tc); + tc = tcase_create("references"); + tcase_add_checked_fixture(tc, NULL, teardown_config); + tcase_add_test(tc, test_references); + tcase_add_test(tc, test_references_templates); + tcase_add_test(tc, test_references_order); + tcase_add_test(tc, test_references_resolution); + tcase_add_test(tc, test_references_fallback); + suite_add_tcase(s, tc); + tc = tcase_create("strings"); tcase_add_checked_fixture(tc, setup_string_config, teardown_config); tcase_add_test(tc, test_strings); From ca3c7b7ea65f155451ad63315bb01080ffb880d2 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Tue, 15 May 2018 17:04:23 +0200 Subject: [PATCH 2/5] settings-test: Add option to use the frontend to display the settings This resolves references and redefined values. It currently doesn't work properly if section names contain dots. --- scripts/settings-test.c | 107 +++++++++++++++++++++++++++++++++------- 1 file changed, 88 insertions(+), 19 deletions(-) diff --git a/scripts/settings-test.c b/scripts/settings-test.c index 2169552ac..336da09f0 100644 --- a/scripts/settings-test.c +++ b/scripts/settings-test.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2014 Tobias Brunner + * Copyright (C) 2014-2018 Tobias Brunner * HSR Hochschule fuer Technik Rapperswil * * This program is free software; you can redistribute it and/or modify it @@ -29,21 +29,32 @@ bool settings_parser_parse_file(void *this, char *name); /** - * Recursively print the section and all subsections/settings + * Produce indentation for the given level */ -static void print_section(section_t *section, int level) +static void get_indent(char indent[BUF_LEN], int level) { - section_t *sub; - kv_t *kv; int i; - char indent[256]; - for (i = 0; i < level * 2 && i < sizeof(indent) - 2; i += 2) + for (i = 0; i < level * 2 && i < BUF_LEN - 2; i += 2) { indent[i ] = ' '; indent[i+1] = ' '; } indent[i] = '\0'; +} + +/** + * Recursively print the section and all subsections/settings + */ +static void print_section(section_t *section, int level) +{ + section_t *sub; + section_ref_t *ref; + kv_t *kv; + char indent[BUF_LEN]; + int i, j; + + get_indent(indent, level); for (i = 0; i < array_count(section->kv_order); i++) { @@ -53,12 +64,53 @@ static void print_section(section_t *section, int level) for (i = 0; i < array_count(section->sections_order); i++) { array_get(section->sections_order, i, &sub); - printf("%s%s {\n", indent, sub->name); + printf("%s%s", indent, sub->name); + if (array_count(sub->references)) + { + for (j = 0; j < array_count(sub->references); j++) + { + array_get(sub->references, j, &ref); + printf("%s%s", j == 0 ? " : " : ", ", ref->name); + } + } + printf(" {\n", indent); print_section(sub, level + 1); printf("%s}\n", indent); } } +/** + * Recursively print a given section and all subsections/settings + * FIXME: Doesn't work properly if any of the keys contain dots + */ +static void print_settings_section(settings_t *settings, char *section, + int level) +{ + enumerator_t *enumerator; + char indent[BUF_LEN], buf[BUF_LEN], *key, *value; + + get_indent(indent, level); + + enumerator = settings->create_key_value_enumerator(settings, section); + while (enumerator->enumerate(enumerator, &key, &value)) + { + printf("%s%s = %s\n", indent, key, value); + + } + enumerator->destroy(enumerator); + + enumerator = settings->create_section_enumerator(settings, section); + while (enumerator->enumerate(enumerator, &key)) + { + printf("%s%s {\n", indent, key); + snprintf(buf, sizeof(buf), "%s%s%s", section, + strlen(section) ? "." : "", key); + print_settings_section(settings, buf, level + 1); + printf("%s}\n", indent); + } + enumerator->destroy(enumerator); +} + static void usage(FILE *out, char *name) { fprintf(out, "Test strongswan.conf parser\n\n"); @@ -66,6 +118,7 @@ static void usage(FILE *out, char *name) fprintf(out, "Options:\n"); fprintf(out, " -h, --help print this help.\n"); fprintf(out, " -d, --debug enables debugging of the parser.\n"); + fprintf(out, " -r, --resolve displays the settings with references/redefines resolved.\n"); fprintf(out, " -f, --file=FILE config file to load (default STDIN).\n"); fprintf(out, "\n"); } @@ -73,12 +126,7 @@ static void usage(FILE *out, char *name) int main(int argc, char *argv[]) { char *file = NULL; - - /* don't load strongswan.conf */ - library_init("", "settings-test"); - atexit(library_deinit); - - dbg_default_set_level(3); + bool resolve = FALSE; while (true) { @@ -86,9 +134,10 @@ int main(int argc, char *argv[]) {"help", no_argument, NULL, 'h' }, {"debug", no_argument, NULL, 'd' }, {"file", required_argument, NULL, 'f' }, + {"resolve", no_argument, NULL, 'r' }, {0,0,0,0 }, }; - switch (getopt_long(argc, argv, "hdf:", long_opts, NULL)) + switch (getopt_long(argc, argv, "hdf:r", long_opts, NULL)) { case EOF: break; @@ -101,6 +150,9 @@ int main(int argc, char *argv[]) case 'f': file = optarg; continue; + case 'r': + resolve = TRUE; + continue; default: usage(stderr, argv[0]); return 1; @@ -108,15 +160,32 @@ int main(int argc, char *argv[]) break; } + /* don't load strongswan.conf */ + library_init("", "settings-test"); + atexit(library_deinit); + + dbg_default_set_level(3); + if (file) { - section_t *root = settings_section_create(strdup("root")); + if (resolve) + { + settings_t *settings = settings_create(file); - settings_parser_parse_file(root, file); + print_settings_section(settings, "", 0); - print_section(root, 0); + settings->destroy(settings); + } + else + { + section_t *root = settings_section_create(strdup("root")); - settings_section_destroy(root, NULL); + settings_parser_parse_file(root, file); + + print_section(root, 0); + + settings_section_destroy(root, NULL); + } } else { From 089d5f9765c88c95e588737ff902a59e3f186d3b Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Tue, 22 May 2018 10:51:50 +0200 Subject: [PATCH 3/5] settings: Properly lock when extending sections or adding fallbacks There was a potential chance for a race condition if the ensured section was purged for some reason before using it later. This also changes the behavior for NULL/empty strings via load_string* with merge == FALSE, which now purges the config/section. --- src/libstrongswan/settings/settings.c | 106 +++++++++++------- .../tests/suites/test_settings.c | 77 ++++++++++--- 2 files changed, 129 insertions(+), 54 deletions(-) diff --git a/src/libstrongswan/settings/settings.c b/src/libstrongswan/settings/settings.c index 4d1f74344..1224c7417 100644 --- a/src/libstrongswan/settings/settings.c +++ b/src/libstrongswan/settings/settings.c @@ -281,24 +281,19 @@ static void find_sections_buffered(private_settings_t *this, section_t *section, } /** - * Ensure that the section with the given key exists (thread-safe). + * Ensure that the section with the given key exists (not thread-safe). */ static section_t *ensure_section(private_settings_t *this, section_t *section, const char *key, va_list args) { char buf[128], keybuf[512]; - section_t *found; if (snprintf(keybuf, sizeof(keybuf), "%s", key) >= sizeof(keybuf)) { return NULL; } - /* we might have to change the tree */ - this->lock->write_lock(this->lock); - found = find_section_buffered(section, keybuf, keybuf, args, buf, - sizeof(buf), TRUE); - this->lock->unlock(this->lock); - return found; + return find_section_buffered(section, keybuf, keybuf, args, buf, + sizeof(buf), TRUE); } /** @@ -944,37 +939,32 @@ METHOD(settings_t, add_fallback, void, va_list args; char buf[512]; - /* find/create the section */ + this->lock->write_lock(this->lock); va_start(args, fallback); section = ensure_section(this, this->top, key, args); va_end(args); va_start(args, fallback); - if (vsnprintf(buf, sizeof(buf), fallback, args) < sizeof(buf)) + if (section && vsnprintf(buf, sizeof(buf), fallback, args) < sizeof(buf)) { - this->lock->write_lock(this->lock); settings_reference_add(section, strdup(buf), TRUE); - this->lock->unlock(this->lock); } va_end(args); + this->lock->unlock(this->lock); } /** * Load settings from files matching the given file pattern or from a string. - * All sections and values are added relative to "parent". * All files (even included ones) have to be loaded successfully. - * If merge is FALSE the contents of parent are replaced with the parsed - * contents, otherwise they are merged together. */ -static bool load_internal(private_settings_t *this, section_t *parent, - char *pattern, bool merge, bool string) +static section_t *load_internal(char *pattern, bool string) { section_t *section; bool loaded; if (pattern == NULL || !pattern[0]) - { /* TODO: Clear parent if merge is FALSE? */ - return TRUE; + { + return settings_section_create(NULL); } section = settings_section_create(NULL); @@ -983,61 +973,101 @@ static bool load_internal(private_settings_t *this, section_t *parent, if (!loaded) { settings_section_destroy(section, NULL); - return FALSE; + section = NULL; } + return section; +} - this->lock->write_lock(this->lock); - settings_section_extend(parent, section, this->contents, !merge); +/** + * Add sections and values in "section" relative to "parent". + * If merge is FALSE the contents of parent are replaced with the parsed + * contents, otherwise they are merged together. + * + * Releases the write lock and destroys the given section. + * If parent is NULL this is all that happens. + */ +static bool extend_section(private_settings_t *this, section_t *parent, + section_t *section, bool merge) +{ + if (parent) + { + settings_section_extend(parent, section, this->contents, !merge); + } this->lock->unlock(this->lock); - settings_section_destroy(section, NULL); - return TRUE; + return parent != NULL; } METHOD(settings_t, load_files, bool, private_settings_t *this, char *pattern, bool merge) { - return load_internal(this, this->top, pattern, merge, FALSE); + section_t *section; + + section = load_internal(pattern, FALSE); + if (!section) + { + return FALSE; + } + + this->lock->write_lock(this->lock); + return extend_section(this, this->top, section, merge); } METHOD(settings_t, load_files_section, bool, private_settings_t *this, char *pattern, bool merge, char *key, ...) { - section_t *section; + section_t *section, *parent; va_list args; - va_start(args, key); - section = ensure_section(this, this->top, key, args); - va_end(args); - + section = load_internal(pattern, FALSE); if (!section) { return FALSE; } - return load_internal(this, section, pattern, merge, FALSE); + + this->lock->write_lock(this->lock); + + va_start(args, key); + parent = ensure_section(this, this->top, key, args); + va_end(args); + + return extend_section(this, parent, section, merge); } METHOD(settings_t, load_string, bool, private_settings_t *this, char *settings, bool merge) { - return load_internal(this, this->top, settings, merge, TRUE); + section_t *section; + + section = load_internal(settings, TRUE); + if (!section) + { + return FALSE; + } + + this->lock->write_lock(this->lock); + return extend_section(this, this->top, section, merge); } METHOD(settings_t, load_string_section, bool, private_settings_t *this, char *settings, bool merge, char *key, ...) { - section_t *section; + section_t *section, *parent; va_list args; - va_start(args, key); - section = ensure_section(this, this->top, key, args); - va_end(args); - + section = load_internal(settings, TRUE); if (!section) { return FALSE; } - return load_internal(this, section, settings, merge, TRUE); + + this->lock->write_lock(this->lock); + + va_start(args, key); + parent = ensure_section(this, this->top, key, args); + va_end(args); + + return extend_section(this, parent, section, merge); } METHOD(settings_t, destroy, void, diff --git a/src/libstrongswan/tests/suites/test_settings.c b/src/libstrongswan/tests/suites/test_settings.c index 39c59bbf2..6259c9ed0 100644 --- a/src/libstrongswan/tests/suites/test_settings.c +++ b/src/libstrongswan/tests/suites/test_settings.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2014 Tobias Brunner + * Copyright (C) 2014-2018 Tobias Brunner * HSR Hochschule fuer Technik Rapperswil * * This program is free software; you can redistribute it and/or modify it @@ -452,9 +452,10 @@ static void verify_sections(linked_list_t *verifier, char *parent) enumerator = settings->create_section_enumerator(settings, parent); ver = verifier->create_enumerator(verifier); - while (enumerator->enumerate(enumerator, §ion) && - ver->enumerate(ver, ¤t)) + while (enumerator->enumerate(enumerator, §ion)) { + ck_assert_msg(ver->enumerate(ver, ¤t), + "no more sections expected, found %s", section); ck_assert_str_eq(section, current); verifier->remove_at(verifier, ver); } @@ -498,10 +499,11 @@ static void verify_key_values(linked_list_t *keys, linked_list_t *values, enumerator = settings->create_key_value_enumerator(settings, parent); enum_keys = keys->create_enumerator(keys); enum_values = values->create_enumerator(values); - while (enumerator->enumerate(enumerator, &key, &value) && - enum_keys->enumerate(enum_keys, ¤t_key) && - enum_values->enumerate(enum_values, ¤t_value)) + while (enumerator->enumerate(enumerator, &key, &value)) { + ck_assert_msg(enum_keys->enumerate(enum_keys, ¤t_key), + "no more key/value expected, found %s = %s", key, value); + ck_assert(enum_values->enumerate(enum_values, ¤t_value)); ck_assert_str_eq(current_key, key); ck_assert_str_eq(current_value, value); keys->remove_at(keys, enum_keys); @@ -519,8 +521,8 @@ START_TEST(test_key_value_enumerator) { linked_list_t *keys, *values; - keys = linked_list_create_with_items("key1", "key2", "empty", "key3", NULL); - values = linked_list_create_with_items("val1", "with space", "", "string with\nnewline", NULL); + keys = linked_list_create_with_items("key1", "key2", "empty", "key3", "key4", "key5", NULL); + values = linked_list_create_with_items("val1", "with space", "", "string with\nnewline", "multi line\nstring", "escaped newline", NULL); verify_key_values(keys, values, "main"); keys = linked_list_create_with_items("key", "key2", "subsub", NULL); @@ -894,7 +896,6 @@ START_TEST(test_load_string) } END_TEST - START_TEST(test_load_string_section) { char *content = @@ -914,13 +915,6 @@ START_TEST(test_load_string_section) ck_assert(settings->load_string_section(settings, include_content2, TRUE, "main.sub1")); verify_include(); - /* invalid strings are a failure */ - ck_assert(!settings->load_string_section(settings, "conf {", TRUE, "")); - /* NULL or empty strings are OK though */ - ck_assert(settings->load_string_section(settings, "", TRUE, "")); - ck_assert(settings->load_string_section(settings, NULL, TRUE, "")); - verify_include(); - ck_assert(settings->load_string_section(settings, include_content2, FALSE, "main")); verify_null("main.key1"); verify_string("v2", "main.key2"); @@ -934,6 +928,56 @@ START_TEST(test_load_string_section) } END_TEST +START_TEST(test_load_string_section_null) +{ + linked_list_t *keys, *values; + + char *content = + "main {\n" + " key1 = val1\n" + " key2 = val2\n" + " none = x\n" + " sub1 {\n" + " include = value\n" + " key2 = value2\n" + " }\n" + "}"; + + settings = settings_create_string(content); + + ck_assert(settings->load_string_section(settings, include_content1, TRUE, "")); + ck_assert(settings->load_string_section(settings, include_content2, TRUE, "main.sub1")); + verify_include(); + + /* invalid strings are a failure */ + ck_assert(!settings->load_string_section(settings, "conf {", TRUE, "")); + /* NULL or empty strings are OK though when merging */ + ck_assert(settings->load_string_section(settings, "", TRUE, "")); + ck_assert(settings->load_string_section(settings, NULL, TRUE, "")); + verify_include(); + + /* they do purge the settings if merge is not TRUE */ + ck_assert(settings->load_string_section(settings, "", FALSE, "main")); + verify_null("main.key1"); + verify_null("main.sub1.key2"); + + keys = linked_list_create_with_items(NULL); + verify_sections(keys, "main"); + + keys = linked_list_create_with_items(NULL); + values = linked_list_create_with_items(NULL); + verify_key_values(keys, values, "main"); + + keys = linked_list_create_with_items("main", NULL); + verify_sections(keys, ""); + + ck_assert(settings->load_string_section(settings, NULL, FALSE, "")); + + keys = linked_list_create_with_items(NULL); + verify_sections(keys, ""); +} +END_TEST + START_SETUP(setup_fallback_config) { create_settings(chunk_from_str( @@ -1664,6 +1708,7 @@ Suite *settings_suite_create() tcase_add_checked_fixture(tc, setup_include_config, teardown_config); tcase_add_test(tc, test_load_string); tcase_add_test(tc, test_load_string_section); + tcase_add_test(tc, test_load_string_section_null); suite_add_tcase(s, tc); tc = tcase_create("fallback"); From 61c3870bef5e0ebf792b18553b86c95ed5acf121 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Thu, 31 May 2018 11:29:37 +0200 Subject: [PATCH 4/5] conf: Document reference syntax --- conf/strongswan.conf.5.head.in | 76 ++++++++++++++++++++++++++++-- src/swanctl/swanctl.conf.5.head.in | 4 +- 2 files changed, 75 insertions(+), 5 deletions(-) diff --git a/conf/strongswan.conf.5.head.in b/conf/strongswan.conf.5.head.in index 23454e758..c9db45355 100644 --- a/conf/strongswan.conf.5.head.in +++ b/conf/strongswan.conf.5.head.in @@ -32,15 +32,20 @@ and key/value pairs: .PP Values must be terminated by a newline. .PP -Comments are possible using the \fB#\fP-character, but be careful: The parser -implementation is currently limited and does not like brackets in comments. +Comments are possible using the \fB#\fP-character. .PP Section names and keys may contain any printable character except: .PP .EX - . { } # \\n \\t space + . , : { } = " # \\n \\t space .EE .PP +In rare circumstances \fB.\fP characters may be used in section names (e.g. for +log file names in a filelog section), but should generally be avoided. +To use \fB:\fP characters in section names (e.g. for Windows log file paths) +they may be written as \fB::\fP, which the parser replaces with a single +\fB:\fP. + An example file in this format might look like this: .PP .EX @@ -60,6 +65,71 @@ An example file in this format might look like this: .PP Indentation is optional, you may use tabs or spaces. + +.SH REFERENCING OTHER SECTIONS +It is possible to inherit settings and sections from another section. This +feature is mainly useful in swanctl.conf (which uses the same file format). +The syntax is as follows: +.PP +.EX + section := name : references { settings } + references := absname[, absname]* + absname := name[.name]* +.EE +.PP +All key/value pairs and all subsections of the referenced sections will be +inherited by the section that references them via their absolute name. Values +may be overridden in the section or any of its sub-sections (use an empty +assignment to clear a value so its default value, if any, will apply). It is +currently not possible to limit the inclusion level or clear/remove inherited +sub-sections. + +If the order is important (e.g. for auth rounds in a connection, if \fIround\fR +is not used), it should be noted that inherited settings/sections will follow +those defined in the current section (if multiple sections are referenced, their +settings are enumerated left to right). + +References are evaluated dynamically at runtime, so referring to sections later +in the config file or included via other files is no problem. + +Here is an example of how this might look like: +.PP +.EX + conn-defaults { + # default settings for all conns (e.g. a cert, or IP pools) + } + eap-defaults { + # defaults if eap is used (e.g. a remote auth round) + } + child-defaults { + # defaults for child configs (e.g. traffic selectors) + } + connections { + conn-a : conn-defaults, eap-defaults { + # set/override stuff specific to this connection + children { + child-a : child-defaults { + # set/override stuff specific to this child + } + } + } + conn-b : conn-defaults { + # set/override stuff specific to this connection + children { + child-b : child-defaults { + # set/override stuff specific to this child + } + } + } + conn-c : connections.conn-a { + # everything is inherited, including everything conn-a + # already inherits from the sections it and its + # sub-section reference + } + } +.EE +.PP + .SH INCLUDING FILES Using the .B include diff --git a/src/swanctl/swanctl.conf.5.head.in b/src/swanctl/swanctl.conf.5.head.in index 5742d2593..a14225df0 100644 --- a/src/swanctl/swanctl.conf.5.head.in +++ b/src/swanctl/swanctl.conf.5.head.in @@ -6,8 +6,8 @@ swanctl.conf is the configuration file used by the .BR swanctl (8) tool to load configurations and credentials into the strongSwan IKE daemon. -For a description of the basic file syntax, including how to split the -configuration in multiple files by including other files, refer to +For a description of the basic file syntax, including how to reference sections +or split the configuration in multiple files by including other files, refer to .BR strongswan.conf (5). .SH TIME FORMATS From b9745618cdae3637c10783291fab7f2ed4cb48b2 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Thu, 31 May 2018 11:36:15 +0200 Subject: [PATCH 5/5] daemon: Allow configuration of logfile path as value Some characters are not allowed in section names, this way they can still be used in paths of log files. --- conf/options/charon-logging.opt | 4 ++++ src/libcharon/daemon.c | 22 +++++++++++++--------- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/conf/options/charon-logging.opt b/conf/options/charon-logging.opt index 2bbb5dce4..31ebf6c32 100644 --- a/conf/options/charon-logging.opt +++ b/conf/options/charon-logging.opt @@ -5,6 +5,10 @@ charon.filelog {} charon.filelog. { # } is the full path to the log file. +charon.filelog..path = + Optional path to the log file. Overrides the section name, may be used + if the path contains characters that aren't allowed in section names. + charon.filelog..default = 1 Default loglevel. diff --git a/src/libcharon/daemon.c b/src/libcharon/daemon.c index e4b819710..c02ab28f2 100644 --- a/src/libcharon/daemon.c +++ b/src/libcharon/daemon.c @@ -480,25 +480,29 @@ static void load_sys_logger(private_daemon_t *this, char *facility, /** * Load the given file logger configured in strongswan.conf */ -static void load_file_logger(private_daemon_t *this, char *filename, +static void load_file_logger(private_daemon_t *this, char *section, linked_list_t *current_loggers) { file_logger_t *file_logger; debug_t group; level_t def; bool add_ms, ike_name, flush_line, append; - char *time_format; + char *time_format, *filename; time_format = lib->settings->get_str(lib->settings, - "%s.filelog.%s.time_format", NULL, lib->ns, filename); + "%s.filelog.%s.time_format", NULL, lib->ns, section); add_ms = lib->settings->get_bool(lib->settings, - "%s.filelog.%s.time_add_ms", FALSE, lib->ns, filename); + "%s.filelog.%s.time_add_ms", FALSE, lib->ns, section); ike_name = lib->settings->get_bool(lib->settings, - "%s.filelog.%s.ike_name", FALSE, lib->ns, filename); + "%s.filelog.%s.ike_name", FALSE, lib->ns, section); flush_line = lib->settings->get_bool(lib->settings, - "%s.filelog.%s.flush_line", FALSE, lib->ns, filename); + "%s.filelog.%s.flush_line", FALSE, lib->ns, section); append = lib->settings->get_bool(lib->settings, - "%s.filelog.%s.append", TRUE, lib->ns, filename); + "%s.filelog.%s.append", TRUE, lib->ns, section); + time_format = lib->settings->get_str(lib->settings, + "%s.filelog.%s.time_format", NULL, lib->ns, section); + filename = lib->settings->get_str(lib->settings, + "%s.filelog.%s.path", section, lib->ns, section); file_logger = add_file_logger(this, filename, current_loggers); if (!file_logger) @@ -510,12 +514,12 @@ static void load_file_logger(private_daemon_t *this, char *filename, file_logger->open(file_logger, flush_line, append); def = lib->settings->get_int(lib->settings, "%s.filelog.%s.default", 1, - lib->ns, filename); + lib->ns, section); for (group = 0; group < DBG_MAX; group++) { file_logger->set_level(file_logger, group, lib->settings->get_int(lib->settings, "%s.filelog.%s.%N", def, - lib->ns, filename, debug_lower_names, group)); + lib->ns, section, debug_lower_names, group)); } charon->bus->add_logger(charon->bus, &file_logger->logger); }