[Asterisk-code-review] manager: prevent file access outside of config dir (asterisk[certified/18.9])
Benjamin Keith Ford
asteriskteam at digium.com
Thu Dec 1 11:54:22 CST 2022
Benjamin Keith Ford has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/19646 )
Change subject: manager: prevent file access outside of config dir
......................................................................
manager: prevent file access outside of config dir
Add live_dangerously flag to manager and use this flag to
determine if a configuation file outside of AST_CONFIG_DIR
should be read.
ASTERISK-30176
Change-Id: I46b26af4047433b49ae5c8a85cb8cda806a07404
---
M configs/samples/asterisk.conf.sample
A doc/UPGRADE-staging/manager_config_live_dangerously.txt
M include/asterisk/manager.h
M main/manager.c
M main/options.c
5 files changed, 85 insertions(+), 4 deletions(-)
Approvals:
Benjamin Keith Ford: Looks good to me, approved; Approved for Submit
Friendly Automation: Verified
diff --git a/configs/samples/asterisk.conf.sample b/configs/samples/asterisk.conf.sample
index efb3386..97f1e77 100644
--- a/configs/samples/asterisk.conf.sample
+++ b/configs/samples/asterisk.conf.sample
@@ -95,10 +95,13 @@
; documented in extensions.conf.sample.
; Default gosub.
;live_dangerously = no ; Enable the execution of 'dangerous' dialplan
- ; functions from external sources (AMI,
- ; etc.) These functions (such as SHELL) are
- ; considered dangerous because they can allow
- ; privilege escalation.
+ ; functions and configuration file access from
+ ; external sources (AMI, etc.) These functions
+ ; (such as SHELL) are considered dangerous
+ ; because they can allow privilege escalation.
+ ; Configuration files are considered dangerous
+ ; if they exist outside of the Asterisk
+ ; configuration directory.
; Default no
;entityid=00:11:22:33:44:55 ; Entity ID.
; This is in the form of a MAC address.
diff --git a/doc/UPGRADE-staging/manager_config_live_dangerously.txt b/doc/UPGRADE-staging/manager_config_live_dangerously.txt
new file mode 100644
index 0000000..56f39f9
--- /dev/null
+++ b/doc/UPGRADE-staging/manager_config_live_dangerously.txt
@@ -0,0 +1,8 @@
+Subject: AMI (Asterisk Manager Interface)
+
+Previously, GetConfig and UpdateConfig were able to access files outside of
+the Asterisk configuration directory. Now this access is put behind the
+live_dangerously configuration option in asterisk.conf, which is disabled by
+default. If access to configuration files outside of the Asterisk configuation
+directory is required via AMI, then the live_dangerously configuration option
+must be set to yes.
diff --git a/include/asterisk/manager.h b/include/asterisk/manager.h
index 1f68cd1..6d860d8 100644
--- a/include/asterisk/manager.h
+++ b/include/asterisk/manager.h
@@ -356,6 +356,18 @@
*/
void astman_send_list_complete_end(struct mansession *s);
+/*!
+ * \brief Enable/disable the inclusion of 'dangerous' configurations outside
+ * of the ast_config_AST_CONFIG_DIR
+ *
+ * This function can globally enable/disable the loading of configuration files
+ * outside of ast_config_AST_CONFIG_DIR.
+ *
+ * \param new_live_dangerously If true, enable the access of files outside
+ * ast_config_AST_CONFIG_DIR from astman.
+ */
+void astman_live_dangerously(int new_live_dangerously);
+
void __attribute__((format(printf, 2, 3))) astman_append(struct mansession *s, const char *fmt, ...);
/*! \brief Determine if a manager session ident is authenticated */
diff --git a/main/manager.c b/main/manager.c
index a895e3f..d2ec703 100644
--- a/main/manager.c
+++ b/main/manager.c
@@ -1498,6 +1498,11 @@
/*! \brief The \ref stasis_subscription for forwarding the Security topic to the AMI topic */
static struct stasis_forward *security_topic_forwarder;
+/*!
+ * \brief Set to true (non-zero) to globally allow all dangerous AMI actions to run
+ */
+static int live_dangerously;
+
#ifdef TEST_FRAMEWORK
/*! \brief The \ref stasis_subscription for forwarding the Test topic to the AMI topic */
static struct stasis_forward *test_suite_forwarder;
@@ -3617,6 +3622,29 @@
return 0;
}
+void astman_live_dangerously(int new_live_dangerously)
+{
+ if (new_live_dangerously && !live_dangerously)
+ {
+ ast_log(LOG_WARNING, "Manager Configuration load protection disabled.\n");
+ }
+
+ if (!new_live_dangerously && live_dangerously)
+ {
+ ast_log(LOG_NOTICE, "Manager Configuration load protection enabled.\n");
+ }
+ live_dangerously = new_live_dangerously;
+}
+
+static int restrictedFile(const char *filename)
+{
+ if (!live_dangerously && !strncasecmp(filename, "/", 1) &&
+ strncasecmp(filename, ast_config_AST_CONFIG_DIR, strlen(ast_config_AST_CONFIG_DIR))) {
+ return 1;
+ }
+ return 0;
+}
+
static int action_getconfig(struct mansession *s, const struct message *m)
{
struct ast_config *cfg;
@@ -3635,6 +3663,11 @@
return 0;
}
+ if (restrictedFile(fn)) {
+ astman_send_error(s, m, "File requires escalated priveledges");
+ return 0;
+ }
+
cfg = ast_config_load2(fn, "manager", config_flags);
if (cfg == CONFIG_STATUS_FILEMISSING) {
astman_send_error(s, m, "Config file not found");
@@ -3764,6 +3797,11 @@
return 0;
}
+ if (restrictedFile(fn)) {
+ astman_send_error(s, m, "File requires escalated priveledges");
+ return 0;
+ }
+
if (!(cfg = ast_config_load2(fn, "manager", config_flags))) {
astman_send_error(s, m, "Config file not found");
return 0;
@@ -4115,6 +4153,10 @@
astman_send_error(s, m, "Filename not specified");
return 0;
}
+ if (restrictedFile(sfn) || restrictedFile(dfn)) {
+ astman_send_error(s, m, "File requires escalated priveledges");
+ return 0;
+ }
if (!(cfg = ast_config_load2(sfn, "manager", config_flags))) {
astman_send_error(s, m, "Config file not found");
return 0;
diff --git a/main/options.c b/main/options.c
index f46aa74..15fd704 100644
--- a/main/options.c
+++ b/main/options.c
@@ -476,6 +476,7 @@
}
if (!ast_opt_remote) {
pbx_live_dangerously(live_dangerously);
+ astman_live_dangerously(live_dangerously);
}
option_debug += option_debug_new;
--
To view, visit https://gerrit.asterisk.org/c/asterisk/+/19646
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: certified/18.9
Gerrit-Change-Id: I46b26af4047433b49ae5c8a85cb8cda806a07404
Gerrit-Change-Number: 19646
Gerrit-PatchSet: 2
Gerrit-Owner: Friendly Automation
Gerrit-Reviewer: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-CC: Michael Bradeen <mbradeen at sangoma.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20221201/a4139a10/attachment-0001.html>
More information about the asterisk-code-review
mailing list