[Asterisk-code-review] res_rtp_asterisk: implement ACL mechanism for ICE and STUN addresses. (asterisk[16])
George Joseph
asteriskteam at digium.com
Fri Mar 20 08:40:25 CDT 2020
George Joseph has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/13973 )
Change subject: res_rtp_asterisk: implement ACL mechanism for ICE and STUN addresses.
......................................................................
res_rtp_asterisk: implement ACL mechanism for ICE and STUN addresses.
A pure blacklist is not good enough, we need a whitelist mechanism as
well, and the simplest way to do that is to re-use existing ACL
infrastructure.
This makes it simpler to blacklist say an entire block (/24) except a
smaller block (eg, a /29 or even a /32). Normally you'd need to
recursively split the block, so if you want to blacklist a /24 except
for a /29 you'd end up with a blacklit for a /25, /26, /27 and /28. I
feel that having an ACL instead of a blacklist only is clearer.
Change-Id: Id57a8df51fcfd3bd85ea67c489c85c6c3ecd7b30
Signed-off-by: Jaco Kroon <jaco at uls.co.za>
---
M configs/samples/rtp.conf.sample
A doc/CHANGES-staging/res_rtp_asterisk_cli.txt
M res/res_rtp_asterisk.c
3 files changed, 127 insertions(+), 103 deletions(-)
Approvals:
George Joseph: Looks good to me, approved; Approved for Submit
diff --git a/configs/samples/rtp.conf.sample b/configs/samples/rtp.conf.sample
index a94707e..f002449 100644
--- a/configs/samples/rtp.conf.sample
+++ b/configs/samples/rtp.conf.sample
@@ -69,12 +69,14 @@
; the wildcard 0.0.0.0 address. e.g., A PJSIP endpoint binding RTP to a
; specific address using the bind_rtp_to_media_address and media_address
; options. Or the PJSIP endpoint specifies an explicit transport that binds
-; to a specific IP address.
+; to a specific IP address. Blacklisting is done via ACL infrastructure
+; so it's possible to whitelist as well.
;
-; e.g. stun_blacklist = 192.168.1.0/255.255.255.0
-; stun_blacklist = 10.32.77.0/255.255.255.0
+; stun_acl = named_acl
+; stun_deny = 0.0.0.0/0
+; stun_permit = 1.2.3.4/32
;
-; stun_blacklist =
+; For historic reasons stun_blacklist is an alias for stun_deny.
;
; Hostname or address for the TURN server to be used as a relay. The port
; number is optional. If omitted the default value of 3478 will be used.
@@ -90,17 +92,19 @@
; Password used to authenticate with TURN relay server.
; turnpassword=
;
-; Subnets to exclude from ICE host, srflx and relay discovery. This is useful
-; to optimize the ICE process where a system has multiple host address ranges
-; and/or physical interfaces and certain of them are not expected to be used
-; for RTP. For example, VPNs and local interconnections may not be suitable or
-; necessary for ICE. Multiple subnets may be listed. If left unconfigured,
-; all discovered host addresses are used.
+; An ACL can be used to determine which discovered addresses to include for
+; ICE, srflx and relay discovery. This is useful to optimize the ICE process
+; where a system has multiple host address ranges and/or physical interfaces
+; and certain of them are not expected to be used for RTP. For example, VPNs
+; and local interconnections may not be suitable or necessary for ICE. Multiple
+; subnets may be listed. If left unconfigured, all discovered host addresses
+; are used.
;
-; e.g. ice_blacklist = 192.168.1.0/255.255.255.0
-; ice_blacklist = 10.32.77.0/255.255.255.0
+; ice_acl = named_acl
+; ice_deny = 0.0.0.0/0
+; ice_permit = 1.2.3.4/32
;
-; ice_blacklist =
+; For historic reasons ice_blacklist is an alias for ice_deny.
;
; The MTU to use for DTLS packet fragmentation. This option is set to 1200
; by default. The minimum MTU is 256.
diff --git a/doc/CHANGES-staging/res_rtp_asterisk_cli.txt b/doc/CHANGES-staging/res_rtp_asterisk_cli.txt
new file mode 100644
index 0000000..7b5516d
--- /dev/null
+++ b/doc/CHANGES-staging/res_rtp_asterisk_cli.txt
@@ -0,0 +1,18 @@
+Subject: res_rtp_asterisk
+
+The blacklist mechanism in res_rtp_asterisk for ICE and STUN was converted to
+an ACL mechanism.
+
+As such six now options are now available:
+
+ice_deny
+ice_permit
+ice_acl
+stun_deny
+stun_permit
+stun_acl
+
+These options have their obvious meanings as used elsewhere.
+
+Backwards compatibility was maintained by adding {stun,ice}_blacklist as
+aliases for {stun,ice}_deny.
diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c
index 366539f..05a993f 100644
--- a/res/res_rtp_asterisk.c
+++ b/res/res_rtp_asterisk.c
@@ -84,6 +84,7 @@
#include "asterisk/data_buffer.h"
#ifdef HAVE_PJPROJECT
#include "asterisk/res_pjproject.h"
+#include "asterisk/security_events.h"
#endif
#define MAX_TIMESTAMP_SKEW 640
@@ -212,13 +213,15 @@
static int turnport = DEFAULT_TURN_PORT;
static pj_str_t turnusername;
static pj_str_t turnpassword;
+static struct stasis_subscription *acl_change_sub = NULL;
-static struct ast_ha *ice_blacklist = NULL; /*!< Blacklisted ICE networks */
-static ast_rwlock_t ice_blacklist_lock = AST_RWLOCK_INIT_VALUE;
+/*! ACL for ICE addresses */
+static struct ast_acl_list *ice_acl = NULL;
+static ast_rwlock_t ice_acl_lock = AST_RWLOCK_INIT_VALUE;
-/*! Blacklisted networks for STUN requests */
-static struct ast_ha *stun_blacklist = NULL;
-static ast_rwlock_t stun_blacklist_lock = AST_RWLOCK_INIT_VALUE;
+/*! ACL for STUN requests */
+static struct ast_acl_list *stun_acl = NULL;
+static ast_rwlock_t stun_acl_lock = AST_RWLOCK_INIT_VALUE;
/*! \brief Pool factory used by pjlib to allocate memory. */
static pj_caching_pool cachingpool;
@@ -3424,6 +3427,21 @@
}
#ifdef HAVE_PJPROJECT
+static void acl_change_stasis_cb(void *data, struct stasis_subscription *sub, struct stasis_message *message);
+
+/*!
+ * \internal
+ * \brief Resets and ACL to empty state.
+ *
+ * \return Nothing
+ */
+static void rtp_unload_acl(ast_rwlock_t *lock, struct ast_acl_list **acl)
+{
+ ast_rwlock_wrlock(lock);
+ *acl = ast_free_acl_list(*acl);
+ ast_rwlock_unlock(lock);
+}
+
/*!
* \internal
* \brief Checks an address against the ICE blacklist
@@ -3435,17 +3453,17 @@
*/
static int rtp_address_is_ice_blacklisted(const pj_sockaddr_t *address)
{
- char buf[PJ_INET6_ADDRSTRLEN];
struct ast_sockaddr saddr;
- int result = 1;
+ int result = 0;
- ast_sockaddr_parse(&saddr, pj_sockaddr_print(address, buf, sizeof(buf), 0), 0);
-
- ast_rwlock_rdlock(&ice_blacklist_lock);
- if (!ice_blacklist || (ast_apply_ha(ice_blacklist, &saddr) == AST_SENSE_ALLOW)) {
- result = 0;
+ if (ast_sockaddr_from_pj_sockaddr(&saddr, address) < 0) {
+ ast_log(LOG_ERROR, "Failed to convert pj_sockddr_t to ast_sockaddr - ICE blacklisting (default)\n");
+ return 1;
}
- ast_rwlock_unlock(&ice_blacklist_lock);
+
+ ast_rwlock_rdlock(&ice_acl_lock);
+ result |= ast_apply_acl_nolog(ice_acl, &saddr) == AST_SENSE_DENY;
+ ast_rwlock_unlock(&ice_acl_lock);
return result;
}
@@ -3464,14 +3482,11 @@
*/
static int stun_address_is_blacklisted(const struct ast_sockaddr *addr)
{
- int result = 1;
+ int result = 0;
- ast_rwlock_rdlock(&stun_blacklist_lock);
- if (!stun_blacklist
- || ast_apply_ha(stun_blacklist, addr) == AST_SENSE_ALLOW) {
- result = 0;
- }
- ast_rwlock_unlock(&stun_blacklist_lock);
+ ast_rwlock_rdlock(&stun_acl_lock);
+ result |= ast_apply_acl_nolog(stun_acl, addr) == AST_SENSE_DENY;
+ ast_rwlock_unlock(&stun_acl_lock);
return result;
}
@@ -8868,75 +8883,16 @@
AST_CLI_DEFINE(handle_cli_rtcp_set_stats, "Enable/Disable RTCP stats"),
};
-#ifdef HAVE_PJPROJECT
-/*!
- * \internal
- * \brief Clear the configured blacklist.
- * \since 13.16.0
- *
- * \param lock R/W lock protecting the blacklist
- * \param blacklist List to clear
- *
- * \return Nothing
- */
-static void blacklist_clear(ast_rwlock_t *lock, struct ast_ha **blacklist)
-{
- ast_rwlock_wrlock(lock);
- ast_free_ha(*blacklist);
- *blacklist = NULL;
- ast_rwlock_unlock(lock);
-}
-
-/*!
- * \internal
- * \brief Load the blacklist configuration.
- * \since 13.16.0
- *
- * \param cfg Raw config file options.
- * \param option_name Blacklist option name
- * \param lock R/W lock protecting the blacklist
- * \param blacklist List to load
- *
- * \return Nothing
- */
-static void blacklist_config_load(struct ast_config *cfg, const char *option_name,
- ast_rwlock_t *lock, struct ast_ha **blacklist)
-{
- struct ast_variable *var;
-
- ast_rwlock_wrlock(lock);
- for (var = ast_variable_browse(cfg, "general"); var; var = var->next) {
- if (!strcasecmp(var->name, option_name)) {
- struct ast_ha *na;
- int ha_error = 0;
-
- na = ast_append_ha("d", var->value, *blacklist, &ha_error);
- if (!na) {
- ast_log(LOG_WARNING, "Invalid %s value: %s\n",
- option_name, var->value);
- } else {
- *blacklist = na;
- }
- if (ha_error) {
- ast_log(LOG_ERROR,
- "Bad %s configuration value line %d: %s\n",
- option_name, var->lineno, var->value);
- }
- }
- }
- ast_rwlock_unlock(lock);
-}
-#endif
-
-static int rtp_reload(int reload)
+static int rtp_reload(int reload, int by_external_config)
{
struct ast_config *cfg;
const char *s;
- struct ast_flags config_flags = { reload ? CONFIG_FLAG_FILEUNCHANGED : 0 };
+ struct ast_flags config_flags = { (reload && !by_external_config) ? CONFIG_FLAG_FILEUNCHANGED : 0 };
#ifdef HAVE_PJPROJECT
struct ast_variable *var;
struct ast_ice_host_candidate *candidate;
+ int acl_subscription_flag = 0;
#endif
cfg = ast_config_load2("rtp.conf", "rtp", config_flags);
@@ -8972,8 +8928,6 @@
turnusername = pj_str(NULL);
turnpassword = pj_str(NULL);
host_candidate_overrides_clear();
- blacklist_clear(&ice_blacklist_lock, &ice_blacklist);
- blacklist_clear(&stun_blacklist_lock, &stun_blacklist);
#endif
#if defined(HAVE_OPENSSL) && (OPENSSL_VERSION_NUMBER >= 0x10001000L) && !defined(OPENSSL_NO_SRTP)
@@ -9107,11 +9061,45 @@
}
AST_RWLIST_UNLOCK(&host_candidates);
- /* Read ICE blacklist configuration lines */
- blacklist_config_load(cfg, "ice_blacklist", &ice_blacklist_lock, &ice_blacklist);
+ ast_rwlock_wrlock(&ice_acl_lock);
+ ast_rwlock_wrlock(&stun_acl_lock);
- /* Read STUN blacklist configuration lines */
- blacklist_config_load(cfg, "stun_blacklist", &stun_blacklist_lock, &stun_blacklist);
+ ice_acl = ast_free_acl_list(ice_acl);
+ stun_acl = ast_free_acl_list(stun_acl);
+
+ for (var = ast_variable_browse(cfg, "general"); var; var = var->next) {
+ const char* sense = NULL;
+ struct ast_acl_list **acl = NULL;
+ if (strncasecmp(var->name, "ice_", 4) == 0) {
+ sense = var->name + 4;
+ acl = &ice_acl;
+ } else if (strncasecmp(var->name, "stun_", 5) == 0) {
+ sense = var->name + 5;
+ acl = &stun_acl;
+ } else {
+ continue;
+ }
+
+ if (strcasecmp(sense, "blacklist") == 0) {
+ sense = "deny";
+ }
+
+ if (strcasecmp(sense, "acl") && strcasecmp(sense, "permit") && strcasecmp(sense, "deny")) {
+ continue;
+ }
+
+ ast_append_acl(sense, var->value, acl, NULL, &acl_subscription_flag);
+ }
+ ast_rwlock_unlock(&ice_acl_lock);
+ ast_rwlock_unlock(&stun_acl_lock);
+
+ if (acl_subscription_flag && !acl_change_sub) {
+ acl_change_sub = stasis_subscribe(ast_security_topic(), acl_change_stasis_cb, NULL);
+ stasis_subscription_accept_message_type(acl_change_sub, ast_named_acl_change_type());
+ stasis_subscription_set_filter(acl_change_sub, STASIS_SUBSCRIPTION_FILTER_SELECTIVE);
+ } else if (!acl_subscription_flag && acl_change_sub) {
+ acl_change_sub = stasis_unsubscribe_and_join(acl_change_sub);
+ }
#endif
#if defined(HAVE_OPENSSL) && (OPENSSL_VERSION_NUMBER >= 0x10001000L) && !defined(OPENSSL_NO_SRTP)
if ((s = ast_variable_retrieve(cfg, "general", "dtls_mtu"))) {
@@ -9136,7 +9124,7 @@
static int reload_module(void)
{
- rtp_reload(1);
+ rtp_reload(1, 0);
return 0;
}
@@ -9154,6 +9142,16 @@
ast_pjproject_caching_pool_destroy(&cachingpool);
pj_shutdown();
}
+
+static void acl_change_stasis_cb(void *data, struct stasis_subscription *sub, struct stasis_message *message)
+{
+ if (stasis_message_type(message) != ast_named_acl_change_type()) {
+ return;
+ }
+
+ /* There is no simple way to just reload the ACLs, so just execute a forced reload. */
+ rtp_reload(1, 1);
+}
#endif
static int load_module(void)
@@ -9234,7 +9232,7 @@
return AST_MODULE_LOAD_DECLINE;
}
- rtp_reload(0);
+ rtp_reload(0, 0);
return AST_MODULE_LOAD_SUCCESS;
}
@@ -9254,6 +9252,10 @@
host_candidate_overrides_clear();
pj_thread_register_check();
rtp_terminate_pjproject();
+
+ acl_change_sub = stasis_unsubscribe_and_join(acl_change_sub);
+ rtp_unload_acl(&ice_acl_lock, &ice_acl);
+ rtp_unload_acl(&stun_acl_lock, &stun_acl);
#endif
return 0;
--
To view, visit https://gerrit.asterisk.org/c/asterisk/+/13973
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: Id57a8df51fcfd3bd85ea67c489c85c6c3ecd7b30
Gerrit-Change-Number: 13973
Gerrit-PatchSet: 3
Gerrit-Owner: Jaco Kroon <jaco at uls.co.za>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20200320/b02dde83/attachment-0001.html>
More information about the asterisk-code-review
mailing list