Hello haproxy team.
This patch covers issue https://github.com/haproxy/haproxy/issues/3221.
Before working keyword "peers", which has bigger loggic than "userlist", I've
fixed "userlist" keyword first to check and make consensus with maintainers.
The parser for the "userlist" section did not use the standard keyword
registration mechanism. Instead, it relied on a series of strcmp() comparisons
to identify keywords such as "group" and "user".
This had two main drawbacks:
1. The keywords were not discoverable by the "-dKall" dump option, making it
difficult for users to see all available keywords for the section.
2. The implementation was inconsistent with the parsers for other sections,
which have been progressively refactored to use the standard cfg_kw_list
infrastructure.
This patch refactors the userlist parser to align it with the project's
standard conventions.
The parsing logic for the "group" and "user" keywords has been extracted from
the if/else block in cfg_parse_users() into two new dedicated functions:
- cfg_parse_users_group()
- cfg_parse_users_user()
These two keywords are now registered via a dedicated cfg_kw_list, making them
visible to the rest of the HAPorxy ecosystem, including the -dKall dump.
I hope that this change makes the userlist parser cleaner, and furthermore,
provides a clear pattern for refactoring other sections like "peers" or the
other keywords.
---
src/cfgparse.c | 259 ++++++++++++++++++++++++++++---------------------
1 file changed, 146 insertions(+), 113 deletions(-)
diff --git a/src/cfgparse.c b/src/cfgparse.c
index 5a7ec7405..08cd3e0d1 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -1389,153 +1389,184 @@ cfg_parse_users(const char *file, int linenum, char
**args, int kwm)
newul->next = userlist;
userlist = newul;
- } else if (strcmp(args[0], "group") == 0) { /* new group */
- int cur_arg;
- const char *err;
- struct auth_groups *ag;
+ } else {
+ const struct cfg_kw_list *kwl;
+ char *errmsg = NULL;
+ int index;
- if (!*args[1]) {
- ha_alert("parsing [%s:%d]: '%s' expects <name> as
arguments.\n",
- file, linenum, args[0]);
- err_code |= ERR_ALERT | ERR_FATAL;
- goto out;
+ list_for_each_entry(kwl, &cfg_keywords.list, list) {
+ for (index = 0; kwl->kw[index].kw; index++) {
+ if ((kwl->kw[index].section & CFG_USERLIST) &&
+ (strcmp(kwl->kw[index].kw, args[0]) ==
0)) {
+ err_code |=
kwl->kw[index].parse(args, CFG_USERLIST, NULL, NULL, file, linenum, &errmsg);
+ if (errmsg) {
+ ha_alert("parsing
[%s:%d] : %s\n", file, linenum, errmsg);
+ ha_free(&errmsg);
+ }
+ goto out;
+ }
+ }
}
- err = invalid_char(args[1]);
- if (err) {
- ha_alert("parsing [%s:%d]: character '%c' is not
permitted in '%s' name '%s'.\n",
- file, linenum, *err, args[0], args[1]);
- err_code |= ERR_ALERT | ERR_FATAL;
- goto out;
- }
+ ha_alert("parsing [%s:%d]: unknown keyword '%s' in '%s'
section\n", file, linenum, args[0], "users");
+ err_code |= ERR_ALERT | ERR_FATAL;
+ }
- if (!userlist)
- goto out;
+out:
+ return err_code;
+}
- for (ag = userlist->groups; ag; ag = ag->next)
- if (strcmp(ag->name, args[1]) == 0) {
- ha_warning("parsing [%s:%d]: ignoring
duplicated group '%s' in userlist '%s'.\n",
- file, linenum, args[1],
userlist->name);
- err_code |= ERR_ALERT;
- goto out;
- }
+int cfg_parse_users_group(char **args, int section_type, struct proxy
*curproxy, const struct proxy *defproxy, const char *file, int linenum, char
**err)
+{
+ int cur_arg;
+ const char *err_str;
+ struct auth_groups *ag;
+ int err_code = 0;
- ag = calloc(1, sizeof(*ag));
- if (!ag) {
- ha_alert("parsing [%s:%d]: out of memory.\n", file,
linenum);
- err_code |= ERR_ALERT | ERR_ABORT;
- goto out;
- }
+ if (!*args[1]) {
+ ha_alert("parsing [%s:%d]: '%s' expects <name> as arguments.\n",
+ file, linenum, args[0]);
+ err_code |= ERR_ALERT | ERR_FATAL;
+ goto out;
+ }
- ag->name = strdup(args[1]);
- if (!ag->name) {
- ha_alert("parsing [%s:%d]: out of memory.\n", file,
linenum);
- err_code |= ERR_ALERT | ERR_ABORT;
- free(ag);
+ err_str = invalid_char(args[1]);
+ if (err_str) {
+ ha_alert("parsing [%s:%d]: character '%c' is not permitted in
'%s' name '%s'.\n",
+ file, linenum, *err_str, args[0], args[1]);
+ err_code |= ERR_ALERT | ERR_FATAL;
+ goto out;
+ }
+
+ if (!userlist)
+ goto out;
+
+ for (ag = userlist->groups; ag; ag = ag->next)
+ if (strcmp(ag->name, args[1]) == 0) {
+ ha_warning("parsing [%s:%d]: ignoring duplicated group
'%s' in userlist '%s'.\n",
+ file, linenum, args[1], userlist->name);
+ err_code |= ERR_ALERT;
goto out;
}
- cur_arg = 2;
+ ag = calloc(1, sizeof(*ag));
+ if (!ag) {
+ ha_alert("parsing [%s:%d]: out of memory.\n", file, linenum);
+ err_code |= ERR_ALERT | ERR_ABORT;
+ goto out;
+ }
- while (*args[cur_arg]) {
- if (strcmp(args[cur_arg], "users") == 0) {
- if (ag->groupusers) {
- ha_alert("parsing [%s:%d]: 'users'
option already defined in '%s' name '%s'.\n",
- file, linenum, args[0],
args[1]);
- err_code |= ERR_ALERT | ERR_FATAL;
- free(ag->groupusers);
- free(ag->name);
- free(ag);
- goto out;
- }
- ag->groupusers = strdup(args[cur_arg + 1]);
- cur_arg += 2;
- continue;
- } else {
- ha_alert("parsing [%s:%d]: '%s' only supports
'users' option.\n",
- file, linenum, args[0]);
+ ag->name = strdup(args[1]);
+ if (!ag->name) {
+ ha_alert("parsing [%s:%d]: out of memory.\n", file, linenum);
+ err_code |= ERR_ALERT | ERR_ABORT;
+ free(ag);
+ goto out;
+ }
+
+ cur_arg = 2;
+
+ while (*args[cur_arg]) {
+ if (strcmp(args[cur_arg], "users") == 0) {
+ if (ag->groupusers) {
+ ha_alert("parsing [%s:%d]: 'users' option
already defined in '%s' name '%s'.\n",
+ file, linenum, args[0],
args[1]);
err_code |= ERR_ALERT | ERR_FATAL;
free(ag->groupusers);
free(ag->name);
free(ag);
goto out;
}
+ ag->groupusers = strdup(args[cur_arg + 1]);
+ cur_arg += 2;
+ continue;
+ } else {
+ ha_alert("parsing [%s:%d]: '%s' only supports 'users'
option.\n",
+ file, linenum, args[0]);
+ err_code |= ERR_ALERT | ERR_FATAL;
+ free(ag->groupusers);
+ free(ag->name);
+ free(ag);
+ goto out;
}
+ }
- ag->next = userlist->groups;
- userlist->groups = ag;
+ ag->next = userlist->groups;
+ userlist->groups = ag;
- } else if (strcmp(args[0], "user") == 0) { /* new user */
- struct auth_users *newuser;
- int cur_arg;
+out:
+ return err_code;
+}
- if (!*args[1]) {
- ha_alert("parsing [%s:%d]: '%s' expects <name> as
arguments.\n",
- file, linenum, args[0]);
- err_code |= ERR_ALERT | ERR_FATAL;
- goto out;
- }
- if (!userlist)
- goto out;
+int cfg_parse_users_user(char **args, int section_type, struct proxy
*curproxy, const struct proxy *defproxy, const char *file, int linenum, char
**err)
+{
+ struct auth_users *newuser;
+ int cur_arg;
+ int err_code = 0;
- for (newuser = userlist->users; newuser; newuser =
newuser->next)
- if (strcmp(newuser->user, args[1]) == 0) {
- ha_warning("parsing [%s:%d]: ignoring
duplicated user '%s' in userlist '%s'.\n",
- file, linenum, args[1],
userlist->name);
- err_code |= ERR_ALERT;
- goto out;
- }
+ if (!*args[1]) {
+ ha_alert("parsing [%s:%d]: '%s' expects <name> as arguments.\n",
+ file, linenum, args[0]);
+ err_code |= ERR_ALERT | ERR_FATAL;
+ goto out;
+ }
+ if (!userlist)
+ goto out;
- newuser = calloc(1, sizeof(*newuser));
- if (!newuser) {
- ha_alert("parsing [%s:%d]: out of memory.\n", file,
linenum);
- err_code |= ERR_ALERT | ERR_ABORT;
+ for (newuser = userlist->users; newuser; newuser = newuser->next)
+ if (strcmp(newuser->user, args[1]) == 0) {
+ ha_warning("parsing [%s:%d]: ignoring duplicated user
'%s' in userlist '%s'.\n",
+ file, linenum, args[1], userlist->name);
+ err_code |= ERR_ALERT;
goto out;
}
- newuser->user = strdup(args[1]);
+ newuser = calloc(1, sizeof(*newuser));
+ if (!newuser) {
+ ha_alert("parsing [%s:%d]: out of memory.\n", file, linenum);
+ err_code |= ERR_ALERT | ERR_ABORT;
+ goto out;
+ }
+
+ newuser->user = strdup(args[1]);
- newuser->next = userlist->users;
- userlist->users = newuser;
+ newuser->next = userlist->users;
+ userlist->users = newuser;
- cur_arg = 2;
+ cur_arg = 2;
- while (*args[cur_arg]) {
- if (strcmp(args[cur_arg], "password") == 0) {
+ while (*args[cur_arg]) {
+ if (strcmp(args[cur_arg], "password") == 0) {
#ifdef USE_LIBCRYPT
- if (!crypt("", args[cur_arg + 1])) {
- ha_alert("parsing [%s:%d]: the
encrypted password used for user '%s' is not supported by crypt(3).\n",
- file, linenum, newuser->user);
- err_code |= ERR_ALERT | ERR_FATAL;
- goto out;
- }
-#else
- ha_warning("parsing [%s:%d]: no crypt(3)
support compiled, encrypted passwords will not work.\n",
- file, linenum);
- err_code |= ERR_ALERT;
-#endif
- newuser->pass = strdup(args[cur_arg + 1]);
- cur_arg += 2;
- continue;
- } else if (strcmp(args[cur_arg], "insecure-password")
== 0) {
- newuser->pass = strdup(args[cur_arg + 1]);
- newuser->flags |= AU_O_INSECURE;
- cur_arg += 2;
- continue;
- } else if (strcmp(args[cur_arg], "groups") == 0) {
- newuser->u.groups_names = strdup(args[cur_arg +
1]);
- cur_arg += 2;
- continue;
- } else {
- ha_alert("parsing [%s:%d]: '%s' only supports
'password', 'insecure-password' and 'groups' options.\n",
- file, linenum, args[0]);
+ if (!crypt("", args[cur_arg + 1])) {
+ ha_alert("parsing [%s:%d]: the encrypted
password used for user '%s' is not supported by crypt(3).\n",
+ file, linenum, newuser->user);
err_code |= ERR_ALERT | ERR_FATAL;
goto out;
}
+#else
+ ha_warning("parsing [%s:%d]: no crypt(3) support
compiled, encrypted passwords will not work.\n",
+ file, linenum);
+ err_code |= ERR_ALERT;
+#endif
+ newuser->pass = strdup(args[cur_arg + 1]);
+ cur_arg += 2;
+ continue;
+ } else if (strcmp(args[cur_arg], "insecure-password") == 0) {
+ newuser->pass = strdup(args[cur_arg + 1]);
+ newuser->flags |= AU_O_INSECURE;
+ cur_arg += 2;
+ continue;
+ } else if (strcmp(args[cur_arg], "groups") == 0) {
+ newuser->u.groups_names = strdup(args[cur_arg + 1]);
+ cur_arg += 2;
+ continue;
+ } else {
+ ha_alert("parsing [%s:%d]: '%s' only supports
'password', 'insecure-password' and 'groups' options.\n",
+ file, linenum, args[0]);
+ err_code |= ERR_ALERT | ERR_FATAL;
+ goto out;
}
- } else {
- ha_alert("parsing [%s:%d]: unknown keyword '%s' in '%s'
section\n", file, linenum, args[0], "users");
- err_code |= ERR_ALERT | ERR_FATAL;
}
out:
@@ -4921,6 +4952,8 @@ REGISTER_CONFIG_SECTION("traces",
cfg_parse_traces, NULL);
static struct cfg_kw_list cfg_kws = {{ },{
{ CFG_GLOBAL, "default-path", cfg_parse_global_def_path },
+ { CFG_USERLIST, "group", cfg_parse_users_group },
+ { CFG_USERLIST, "user", cfg_parse_users_user },
{ /* END */ }
}};
--
2.48.1
