[Asterisk-code-review] pbx_variables: Add variable registration and validation (asterisk[master])

N A asteriskteam at digium.com
Sat Jan 8 19:43:25 CST 2022


N A has uploaded this change for review. ( https://gerrit.asterisk.org/c/asterisk/+/17792 )


Change subject: pbx_variables: Add variable registration and validation
......................................................................

pbx_variables: Add variable registration and validation

Adds a mechanism for modules to register the special
variables they use with the PBX core. This allows us
to do a few things:

First, a CLI command is added to allow the user to see
all special Asterisk variables, along with a description
of what each variable is for. This includes core Asterisk
variables and those registered by modules.

Secondly, at runtime the variable setter now is aware
of what variables names are reserved and in use by
Asterisk, and it can throw a warning if a user tries to
set one of these variables. This operation would currently
fail, but would do so silently since the variable would
be saved to the channel; however, it can never be retrieved
as the variable substituter will match a built-in first,
and thus the stored value is not usable.

Consequently, more robust variable validation is now
possible and users are warned about confirmed or possible
conflicts with variable set operations.

ASTERISK-29849 #close

Change-Id: Iad3b8d9833c7d9debe04aca59260d7316c3ad28c
---
M apps/app_morsecode.c
M include/asterisk/chanvars.h
M include/asterisk/pbx.h
M main/pbx_variables.c
4 files changed, 335 insertions(+), 4 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/92/17792/1

diff --git a/apps/app_morsecode.c b/apps/app_morsecode.c
index f9b2119..166c646 100644
--- a/apps/app_morsecode.c
+++ b/apps/app_morsecode.c
@@ -281,6 +281,17 @@
 	return res;
 }
 
+struct {
+	char name[20];
+	char description[64];
+} morsevars[] =
+{
+	{ "MORSEDITLEN", "Value in ms for length of dit" },
+	{ "MORSETONE", "Pitch (in Hz) of tone, default is 800" },
+	{ "MORSESPACETONE", "Pitch (in Hz) of spaces, default is 0" },
+	{ "MORSETYPE", "Code type to use (AMERICAN or INTERNATIONAL (default))" },
+};
+
 static int unload_module(void)
 {
 	return ast_unregister_application(app_morsecode);
@@ -288,6 +299,7 @@
 
 static int load_module(void)
 {
+	AST_REGISTER_VARIABLES(AST_VAR_MODULE_WRITE, morsevars);
 	return ast_register_application_xml(app_morsecode, morsecode_exec);
 }
 
diff --git a/include/asterisk/chanvars.h b/include/asterisk/chanvars.h
index 1a303c5..932a3d2 100644
--- a/include/asterisk/chanvars.h
+++ b/include/asterisk/chanvars.h
@@ -25,12 +25,78 @@
 
 #include "asterisk/linkedlists.h"
 
+/*!
+ * \brief special variable types
+ * \note These only apply to special Asterisk variables (i.e. all caps)
+ */
+enum ast_variable_type {
+	AST_VAR_SYSTEM,				/*!< Special variables that pertain to the entire Asterisk system (R/O) */
+	AST_VAR_CHANNEL_BUILTIN,	/*!< Special variables that pertain to a specific channel (R/O) */
+	AST_VAR_MODULE, 			/*!< Special variables used by individual modules (R/O) */
+	AST_VAR_MODULE_WRITE,		/*!< Special variables used by individual modules, that a user may write */
+};
+
 struct ast_var_t {
 	AST_LIST_ENTRY(ast_var_t) entries;
 	char *value;
 	char name[0];
 };
 
+/*!
+ * \brief Determines whether a variable name is reserved by Asterisk
+ *
+ * \param name Name of variable
+ *
+ * \retval 1 reserved
+ * \retval 0 not reserved
+ */
+int ast_varname_reserved(const char *name);
+
+/*!
+ * \brief Determines whether a variable may be edited by users
+ *
+ * \param name Name of variable
+ *
+ * \retval 1 user editable
+ * \retval 0 not user editable (read only)
+ */
+int ast_variable_editable(const char *name);
+
+/*!
+ * \brief Determines whether a user variable assignment should be made
+ *
+ * \param name Name of variable
+ *
+ * \note This function is for user variable assignments only, not internal Asterisk assignments.
+ *       This variable will log any warnings as necessary and return whether or not assignment
+ *       should proceed.
+ *
+ * \retval -1 Do not set variable
+ * \retval 0 Okay to set variable
+ */
+int ast_bad_variable_assignment(const char *name);
+
+/*!
+ * \brief Register a special variable
+ *
+ * \param vartype The type of special variable
+ * \param name Name of variable
+ * \param description Description of variable
+ *
+ * \note This only applies to special Asterisk variables. No applicability to user globals or chan vars.
+ *
+ * \retval 0 Success
+ * \retval -1 Failure
+ */
+int ast_register_variable(int vartype, const char *name, char *description);
+
+#define AST_REGISTER_VARIABLES(typ, array) { \
+	int x; \
+	for (x = 0; x < ARRAY_LEN(array); x++) { \
+		ast_register_variable(typ, array[x].name, array[x].description); \
+	} \
+}
+
 AST_LIST_HEAD_NOLOCK(varshead, ast_var_t);
 
 struct varshead *ast_var_list_create(void);
diff --git a/include/asterisk/pbx.h b/include/asterisk/pbx.h
index f5f0691..4ada3e1 100644
--- a/include/asterisk/pbx.h
+++ b/include/asterisk/pbx.h
@@ -1386,6 +1386,18 @@
 int pbx_builtin_setvar_helper(struct ast_channel *chan, const char *name, const char *value);
 
 /*!
+ * \brief Same as pbx_builtin_setvar_helper, but accepts a restricted flag
+ *
+ * \param restricted 0 for (unrestricted) variable assignments made by Asterisk internally and 1 for (restricted)
+ *        variable assignments made by applications, functions, etc. processing user variable assignments.
+ *
+ * \note Will lock the channel.  May also be used to set a channel dialplan function to a particular value.
+ * \see ast_func_write
+ * \return -1 if the dialplan function fails to be set
+ */
+int pbx_builtin_setvar_helper_restricted(struct ast_channel *chan, const char *name, const char *value, int restricted);
+
+/*!
  * \brief Retrieve the value of a builtin variable or variable from the channel variable stack.
  * \note Will lock the channel.
  */
diff --git a/main/pbx_variables.c b/main/pbx_variables.c
index 7a85989..d3bf8d0 100644
--- a/main/pbx_variables.c
+++ b/main/pbx_variables.c
@@ -109,6 +109,190 @@
 AST_RWLOCK_DEFINE_STATIC(globalslock);
 static struct varshead globals = AST_LIST_HEAD_NOLOCK_INIT_VALUE;
 
+AST_RWLOCK_DEFINE_STATIC(systemvarslock);
+AST_RWLOCK_DEFINE_STATIC(specialchanvarslock);
+AST_RWLOCK_DEFINE_STATIC(modvarslock);
+AST_RWLOCK_DEFINE_STATIC(modeditvarslock);
+static struct varshead systemvars = AST_LIST_HEAD_NOLOCK_INIT_VALUE;
+static struct varshead specialchanvars = AST_LIST_HEAD_NOLOCK_INIT_VALUE;
+static struct varshead modvars = AST_LIST_HEAD_NOLOCK_INIT_VALUE;
+static struct varshead modeditvars = AST_LIST_HEAD_NOLOCK_INIT_VALUE;
+
+static int ast_variable_register_helper(struct varshead *headp, int add, const char *name, char *value)
+{
+	struct ast_var_t *newvariable;
+	const char *nametail = name;
+	int res = 0;
+
+	AST_LIST_TRAVERSE_SAFE_BEGIN(headp, newvariable, entries) {
+		if (!strcmp(ast_var_name(newvariable), nametail)) {
+			/* there is already such a variable */
+			if (!value) { /* we're not adding a variable at the moment */
+				res = 1; /* var exists */
+				break;
+			}
+			if (!add && value) { /* delete an existing var */
+				ast_debug(1, "Deregistering special variable '%s'\n", name);
+				AST_LIST_REMOVE_CURRENT(entries);
+				ast_var_delete(newvariable);
+				break;
+			}
+		}
+	}
+	AST_LIST_TRAVERSE_SAFE_END;
+
+	if (res && value) {
+		ast_log(LOG_WARNING, "Failed to catch duplicate registration of '%s'\n", name); /* should never happen */
+		return -1;
+	} else if (value && (newvariable = ast_var_assign(name, value))) {
+		ast_debug(1, "Registering special variable '%s'\n", name);
+		AST_LIST_INSERT_HEAD(headp, newvariable, entries);
+	}
+
+	return res;
+}
+
+int ast_varname_reserved(const char *name)
+{
+	char *tmp = (char*) name;
+	/* Asterisk broadly reserves all uppercase + underscore var names for use by Asterisk */
+	while (*tmp) {
+		if (tmp[0] != '_' && !isupper(tmp[0])) {
+			return 0;
+		}
+		tmp++;
+	}
+	return 1;
+}
+
+int ast_variable_editable(const char *name)
+{
+	int res;
+
+	if (!ast_varname_reserved(name)) {
+		return 1; /* if not a special reserved format (all uppercase/underscore), okay */
+	}
+
+	/* okay, if it's reserved, then it has to be registered as editable */
+	ast_rwlock_wrlock(&modeditvarslock);
+	res = ast_variable_register_helper(&modeditvars, 1, name, NULL);
+	ast_rwlock_unlock(&modeditvarslock);
+
+	return res;
+}
+
+int ast_bad_variable_assignment(const char *name)
+{
+	int res = 0;
+
+	if (ast_variable_editable(name)) {
+		return 0; /* if not a special reserved format (all uppercase/underscore), okay */
+	}
+
+	/* variable name is reserved, but is it actually in use? */
+	ast_rwlock_wrlock(&systemvarslock);
+	ast_rwlock_wrlock(&specialchanvarslock);
+	ast_rwlock_wrlock(&modvarslock);
+	res |= ast_variable_register_helper(&systemvars, 1, name, NULL);
+	res |= ast_variable_register_helper(&specialchanvars, 1, name, NULL);
+	res |= ast_variable_register_helper(&modvars, 1, name, NULL);
+	ast_rwlock_unlock(&systemvarslock);
+	ast_rwlock_unlock(&specialchanvarslock);
+	ast_rwlock_unlock(&modvarslock);
+
+	if (res) {
+		ast_log(LOG_WARNING, "Read-only variable name '%s' is reserved by Asterisk and cannot be set\n", name);
+		return -1; /* user cannot set this variable, reject */
+	}
+
+	ast_log(LOG_WARNING, "Variable name '%s' is reserved by Asterisk (all uppercase) and may pose a conflict\n", name);
+	return 0; /* warn the user, but don't outright reject setting the variable */
+}
+
+int ast_register_variable(int vartype, const char *name, char *description)
+{
+	int res = 0;
+
+	ast_rwlock_wrlock(&systemvarslock);
+	ast_rwlock_wrlock(&specialchanvarslock);
+	ast_rwlock_wrlock(&modvarslock);
+	ast_rwlock_wrlock(&modeditvarslock);
+
+	/* is the variable already registered? */
+	res |= ast_variable_register_helper(&systemvars, 1, name, NULL);
+	res |= ast_variable_register_helper(&specialchanvars, 1, name, NULL);
+	res |= ast_variable_register_helper(&modvars, 1, name, NULL);
+	res |= ast_variable_register_helper(&modeditvars, 1, name, NULL);
+
+	if (res) { /* yeah, it is */
+		res = -1;
+		ast_log(LOG_WARNING, "Special variable '%s' is already registered.\n", name);
+	} else if (ast_strlen_zero(description)) {
+		res = -1;
+		ast_log(LOG_WARNING, "Registration of variable '%s' failed: missing description\n", name);
+	} if (vartype == AST_VAR_SYSTEM) {
+		res = ast_variable_register_helper(&systemvars, 1, name, description);
+	} else if (vartype == AST_VAR_CHANNEL_BUILTIN) {
+		res = ast_variable_register_helper(&specialchanvars, 1, name, description);
+	} else if (vartype == AST_VAR_MODULE) {
+		res = ast_variable_register_helper(&modvars, 1, name, description);
+	} else if (vartype == AST_VAR_MODULE_WRITE) {
+		res = ast_variable_register_helper(&modeditvars, 1, name, description);
+	} else {
+		res = -1;
+		ast_log(LOG_WARNING, "Registration of variable '%s' failed: invalid type\n", name);
+	}
+
+	ast_rwlock_unlock(&systemvarslock);
+	ast_rwlock_unlock(&specialchanvarslock);
+	ast_rwlock_unlock(&modvarslock);
+	ast_rwlock_unlock(&modeditvarslock);
+
+	return res;
+}
+
+static int print_specialvars(int fd, ast_rwlock_t *lock, struct varshead *headp, char *catname, char *edit)
+{
+	int c = 0;
+	struct ast_var_t *newvariable;
+
+	ast_rwlock_wrlock(lock);
+	AST_LIST_TRAVERSE_SAFE_BEGIN(headp, newvariable, entries) {
+		ast_cli(fd, "%-12s %-20s %-8s %s\n", catname, ast_var_name(newvariable), edit, ast_var_value(newvariable));
+		c++;
+	}
+	AST_LIST_TRAVERSE_SAFE_END;
+	ast_rwlock_unlock(lock);
+
+	return c;
+}
+
+/*! \brief CLI support for listing special variables in a parseable way */
+static char *handle_show_specialvars(struct ast_cli_entry *e, int cmd, struct ast_cli_args *a)
+{
+	int i = 0;
+
+	switch (cmd) {
+	case CLI_INIT:
+		e->command = "dialplan show specialvars";
+		e->usage =
+			"Usage: dialplan show specialvars\n"
+			"       List available special variables\n";
+		return NULL;
+	case CLI_GENERATE:
+		return NULL;
+	}
+
+	ast_cli(a->fd, "%-12s %-20s %-8s %s\n", "Type", "Variable Name", "Editable", "Description");
+	i += print_specialvars(a->fd, &systemvarslock, &systemvars, "System", "No");
+	i += print_specialvars(a->fd, &specialchanvarslock, &specialchanvars, "Channel", "No");
+	i += print_specialvars(a->fd, &modvarslock, &modvars, "Module", "No");
+	i += print_specialvars(a->fd, &modeditvarslock, &modeditvars, "Module", "Yes");
+	ast_cli(a->fd, "\n    -- %d special variable(s) registered\n", i);
+
+	return CLI_SUCCESS;
+}
+
 /*!
  * \brief extract offset:length from variable name.
  * \return 1 if there is a offset:length part, which is
@@ -938,7 +1122,7 @@
 	if (a->argc != e->args + 2)
 		return CLI_SHOWUSAGE;
 
-	pbx_builtin_setvar_helper(NULL, a->argv[3], a->argv[4]);
+	pbx_builtin_setvar_helper_restricted(NULL, a->argv[3], a->argv[4], 1);
 	ast_cli(a->fd, "\n    -- Global variable '%s' set to '%s'\n", a->argv[3], a->argv[4]);
 
 	return CLI_SUCCESS;
@@ -972,7 +1156,7 @@
 		return CLI_FAILURE;
 	}
 
-	pbx_builtin_setvar_helper(chan, var_name, var_value);
+	pbx_builtin_setvar_helper_restricted(chan, var_name, var_value, 1);
 
 	chan = ast_channel_unref(chan);
 
@@ -1063,6 +1247,11 @@
 		return;
 	}
 
+	/* this function is only called by userland functions, so always check */
+	if (ast_bad_variable_assignment(name)) {
+		return;
+	}
+
 	if (chan) {
 		ast_channel_lock(chan);
 		headp = ast_channel_varshead(chan);
@@ -1085,6 +1274,11 @@
 
 int pbx_builtin_setvar_helper(struct ast_channel *chan, const char *name, const char *value)
 {
+	return pbx_builtin_setvar_helper_restricted(chan, name, value, 0);
+}
+
+int pbx_builtin_setvar_helper_restricted(struct ast_channel *chan, const char *name, const char *value, int restricted)
+{
 	struct ast_var_t *newvariable;
 	struct varshead *headp;
 	const char *nametail = name;
@@ -1097,6 +1291,10 @@
 		return ast_func_write(chan, function, value);
 	}
 
+	if (restricted && ast_bad_variable_assignment(name)) {
+		return -1;
+	}
+
 	if (chan) {
 		ast_channel_lock(chan);
 		headp = ast_channel_varshead(chan);
@@ -1162,7 +1360,7 @@
 		ast_log(LOG_WARNING, "Please avoid unnecessary spaces on variables as it may lead to unexpected results ('%s' set to '%s').\n", name, mydata);
 	}
 
-	pbx_builtin_setvar_helper(chan, name, value);
+	pbx_builtin_setvar_helper_restricted(chan, name, value, 1);
 
 	return 0;
 }
@@ -1190,7 +1388,7 @@
 	for (x = 0; x < args.argc; x++) {
 		AST_NONSTANDARD_APP_ARGS(pair, args.pair[x], '=');
 		if (pair.argc == 2) {
-			pbx_builtin_setvar_helper(chan, pair.name, pair.value);
+			pbx_builtin_setvar_helper_restricted(chan, pair.name, pair.value, 1);
 			if (strchr(pair.name, ' '))
 				ast_log(LOG_WARNING, "Please avoid unnecessary spaces on variables as it may lead to unexpected results ('%s' set to '%s').\n", pair.name, pair.value);
 		} else if (!chan) {
@@ -1213,9 +1411,49 @@
 	ast_rwlock_unlock(&globalslock);
 }
 
+struct pbx_builtin_system_vars {
+	char name[20];
+	char description[64];
+} builtinsystemvars[] =
+{
+	{ "EPOCH", "Current unix style epoch" },
+	{ "SYSTEMNAME", "value of systemname option from asterisk.conf" },
+	{ "ASTCACHEDIR", "value of astcachedir option from asterisk.conf" },
+	{ "ASTETCDIR", "value of astetcdir option from asterisk.conf" },
+	{ "ASTMODDIR", "value of astmoddir option from asterisk.conf" },
+	{ "ASTVARLIBDIR", "value of astvarlib option from asterisk.conf" },
+	{ "ASTDBDIR", "value of astdbdir option from asterisk.conf" },
+	{ "ASTKEYDIR", "value of astkeydir option from asterisk.conf" },
+	{ "ASTDATADIR", "value of astdatadir option from asterisk.conf" },
+	{ "ASTAGIDIR", "value of astagidir option from asterisk.conf" },
+	{ "ASTSPOOLDIR", "value of astspooldir option from asterisk.conf" },
+	{ "ASTRUNDIR", "value of astrundir option from asterisk.conf" },
+	{ "ASTLOGDIR", "value of astlogdir option from asterisk.conf" },
+	{ "ASTSBINDIR", "value of astsbindir option from asterisk.conf" },
+	{ "ENTITYID", "Global Entity ID set automatically, or from asterisk.conf" },
+};
+
+struct {
+	char name[20];
+	char description[64];
+} builtinchannelvars[] =
+{
+	{ "CALLINGPRES", "Caller ID presentation for incoming calls (PRI channels)" },
+	{ "CALLINGANI2", "Caller ANI2 (PRI channels)" },
+	{ "CALLINGTON", "Caller Type of Number (PRI channels)" },
+	{ "CALLINGTNS", "Transit Network Selector (PRI channels)" },
+	{ "EXTEN", "Current extension" },
+	{ "CONTEXT", "Current context" },
+	{ "PRIORITY", "Current priority" },
+	{ "CHANNEL", "Current channel name" },
+	{ "UNIQUEID", "Current call unique identifier" },
+	{ "HANGUPCAUSE", "Asterisk cause of hangup (inbound/outbound)" },
+};
+
 static struct ast_cli_entry vars_cli[] = {
 	AST_CLI_DEFINE(handle_show_globals, "Show global dialplan variables"),
 	AST_CLI_DEFINE(handle_show_chanvar, "Show channel variables"),
+	AST_CLI_DEFINE(handle_show_specialvars, "Show special Asterisk variables"),
 	AST_CLI_DEFINE(handle_set_global, "Set global dialplan variable"),
 	AST_CLI_DEFINE(handle_set_chanvar, "Set a channel variable"),
 };
@@ -1232,6 +1470,9 @@
 {
 	int res = 0;
 
+	AST_REGISTER_VARIABLES(AST_VAR_SYSTEM, builtinsystemvars);
+	AST_REGISTER_VARIABLES(AST_VAR_CHANNEL_BUILTIN, builtinchannelvars);
+
 	res |= ast_cli_register_multiple(vars_cli, ARRAY_LEN(vars_cli));
 	res |= ast_register_application2("Set", pbx_builtin_setvar, NULL, NULL, NULL);
 	res |= ast_register_application2("MSet", pbx_builtin_setvar_multiple, NULL, NULL, NULL);

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/17792
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: Iad3b8d9833c7d9debe04aca59260d7316c3ad28c
Gerrit-Change-Number: 17792
Gerrit-PatchSet: 1
Gerrit-Owner: N A <mail at interlinked.x10host.com>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20220108/0e53326a/attachment-0001.html>


More information about the asterisk-code-review mailing list