From 32b06d23e29a692f2821f39d02be0f9d092b1648 Mon Sep 17 00:00:00 2001 From: Chuck Rolke Date: Tue, 8 May 2018 10:37:35 -0400 Subject: [PATCH 01/12] DISPATCH-990: vhost hostname patterns management interface --- python/qpid_dispatch/management/qdrouter.json | 7 +++++++ src/policy.c | 16 ++++++++-------- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/python/qpid_dispatch/management/qdrouter.json b/python/qpid_dispatch/management/qdrouter.json index 83e11827aa..ee8a9435c9 100644 --- a/python/qpid_dispatch/management/qdrouter.json +++ b/python/qpid_dispatch/management/qdrouter.json @@ -1678,6 +1678,13 @@ "required": false, "create": true }, + "useVhostNamePatterns": { + "type": "boolean", + "default": false, + "description": "Use Vhost name patterns.", + "required": false, + "create": true + }, "policyDir": { "type": "path", "default": "", diff --git a/src/policy.c b/src/policy.c index a7e31cff52..915ca37bd3 100644 --- a/src/policy.c +++ b/src/policy.c @@ -61,10 +61,12 @@ struct qd_policy_t { qd_dispatch_t *qd; qd_log_source_t *log_source; void *py_policy_manager; + qd_parse_tree_t *hostname_tree; // configured settings int max_connection_limit; char *policyDir; bool enableVhostPolicy; + bool useVhostNamePatterns; // live statistics int connections_processed; int connections_denied; @@ -77,15 +79,11 @@ struct qd_policy_t { qd_policy_t *qd_policy(qd_dispatch_t *qd) { qd_policy_t *policy = NEW(qd_policy_t); + ZERO(policy); policy->qd = qd; policy->log_source = qd_log_source("POLICY"); policy->max_connection_limit = 65535; - policy->py_policy_manager = 0; - policy->policyDir = 0; - policy->enableVhostPolicy = false; - policy->connections_processed= 0; - policy->connections_denied = 0; - policy->connections_current = 0; + policy->hostname_tree = qd_parse_tree_new(QD_PARSE_TREE_ADDRESS); qd_log(policy->log_source, QD_LOG_TRACE, "Policy Initialized"); return policy; @@ -99,6 +97,7 @@ void qd_policy_free(qd_policy_t *policy) { if (policy->policyDir) free(policy->policyDir); + qd_parse_tree_free(policy->hostname_tree); free(policy); } @@ -114,8 +113,9 @@ qd_error_t qd_entity_configure_policy(qd_policy_t *policy, qd_entity_t *entity) policy->policyDir = qd_entity_opt_string(entity, "policyDir", 0); CHECK(); policy->enableVhostPolicy = qd_entity_opt_bool(entity, "enableVhostPolicy", false); CHECK(); - qd_log(policy->log_source, QD_LOG_INFO, "Policy configured maxConnections: %d, policyDir: '%s', access rules enabled: '%s'", - policy->max_connection_limit, policy->policyDir, (policy->enableVhostPolicy ? "true" : "false")); + policy->useVhostNamePatterns = qd_entity_opt_bool(entity, "useVhostNamePatterns", false); CHECK(); + qd_log(policy->log_source, QD_LOG_INFO, "Policy configured maxConnections: %d, policyDir: '%s', access rules enabled: '%s', use hostname patterns: '%s'", + policy->max_connection_limit, policy->policyDir, (policy->enableVhostPolicy ? "true" : "false"), (policy->useVhostNamePatterns ? "true" : "false")); return QD_ERROR_NONE; error: From 862b0214991a8d4a6145b5a50980d7613941b2e5 Mon Sep 17 00:00:00 2001 From: Chuck Rolke Date: Tue, 8 May 2018 13:46:45 -0400 Subject: [PATCH 02/12] DISPATCH-990: reduce line lengths --- src/policy.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/policy.c b/src/policy.c index 915ca37bd3..7de111c225 100644 --- a/src/policy.c +++ b/src/policy.c @@ -114,8 +114,15 @@ qd_error_t qd_entity_configure_policy(qd_policy_t *policy, qd_entity_t *entity) qd_entity_opt_string(entity, "policyDir", 0); CHECK(); policy->enableVhostPolicy = qd_entity_opt_bool(entity, "enableVhostPolicy", false); CHECK(); policy->useVhostNamePatterns = qd_entity_opt_bool(entity, "useVhostNamePatterns", false); CHECK(); - qd_log(policy->log_source, QD_LOG_INFO, "Policy configured maxConnections: %d, policyDir: '%s', access rules enabled: '%s', use hostname patterns: '%s'", - policy->max_connection_limit, policy->policyDir, (policy->enableVhostPolicy ? "true" : "false"), (policy->useVhostNamePatterns ? "true" : "false")); + qd_log(policy->log_source, QD_LOG_INFO, + "Policy configured maxConnections: %d, " + "policyDir: '%s'," + "access rules enabled: '%s', " + "use hostname patterns: '%s'", + policy->max_connection_limit, + policy->policyDir, + (policy->enableVhostPolicy ? "true" : "false"), + (policy->useVhostNamePatterns ? "true" : "false")); return QD_ERROR_NONE; error: From 10ad8cf370412ec424f92e1a3db0aee031f4afc4 Mon Sep 17 00:00:00 2001 From: Chuck Rolke Date: Thu, 10 May 2018 10:17:21 -0400 Subject: [PATCH 03/12] DISPATCH-990: Reorder config steps to init policy Policy manager needs some setup before vhosts are processed. --- .../qpid_dispatch_internal/management/config.py | 17 +++++++++++------ .../policy/policy_local.py | 5 +++++ .../policy/policy_manager.py | 8 ++++++++ src/policy.c | 2 ++ 4 files changed, 26 insertions(+), 6 deletions(-) diff --git a/python/qpid_dispatch_internal/management/config.py b/python/qpid_dispatch_internal/management/config.py index 303609e5c1..aad6bd271a 100644 --- a/python/qpid_dispatch_internal/management/config.py +++ b/python/qpid_dispatch_internal/management/config.py @@ -174,13 +174,21 @@ def configure(attributes): from qpid_dispatch_internal.display_name.display_name import DisplayNameService displayname_service = DisplayNameService() qd.qd_dispatch_register_display_name_service(dispatch, displayname_service) - policyDir = config.by_type('policy')[0]['policyDir'] - policyDefaultVhost = config.by_type('policy')[0]['defaultVhost'] + + # Configure policy and policy manager before vhosts + policyDir = config.by_type('policy')[0]['policyDir'] + policyDefaultVhost = config.by_type('policy')[0]['defaultVhost'] + useHostnamePatterns = config.by_type('policy')[0]['useVhostNamePatterns'] + for a in config.by_type("policy"): + configure(a) + agent.policy.set_default_vhost(policyDefaultVhost) + agent.policy.set_use_hostname_patterns(useHostnamePatterns) + # Remaining configuration for t in "sslProfile", "authServicePlugin", "listener", "connector", \ "router.config.address", "router.config.linkRoute", "router.config.autoLink", \ "router.config.exchange", "router.config.binding", \ - "policy", "vhost": + "vhost": for a in config.by_type(t): configure(a) if t == "sslProfile": @@ -201,6 +209,3 @@ def configure(attributes): pconfig = PolicyConfig(os.path.join(apath, i)) for a in pconfig.by_type("vhost"): agent.configure(a) - - # Set policy default application after all rulesets loaded - agent.policy.set_default_vhost(policyDefaultVhost) diff --git a/python/qpid_dispatch_internal/policy/policy_local.py b/python/qpid_dispatch_internal/policy/policy_local.py index b482b7f9ce..bee0499aa2 100644 --- a/python/qpid_dispatch_internal/policy/policy_local.py +++ b/python/qpid_dispatch_internal/policy/policy_local.py @@ -508,6 +508,11 @@ def __init__(self, manager): # open.hostname is not found in the rulesetdb self._default_vhost = "" + # _use_hostname_patterns + # holds policy setting. + # When true policy ruleset definitions are propagated to C code + self.use_hostname_patterns = False + # # Service interfaces # diff --git a/python/qpid_dispatch_internal/policy/policy_manager.py b/python/qpid_dispatch_internal/policy/policy_manager.py index 34deabb0a6..0437fdd78d 100644 --- a/python/qpid_dispatch_internal/policy/policy_manager.py +++ b/python/qpid_dispatch_internal/policy/policy_manager.py @@ -43,6 +43,7 @@ def __init__(self, agent): self._agent = agent self._policy_local = PolicyLocal(self) self.log_adapter = LogAdapter("POLICY") + self._use_hostname_patterns = False def log(self, level, text): info = traceback.extract_stack(limit=2)[0] # Caller frame info @@ -70,6 +71,13 @@ def log_warning(self, text): def get_agent(self): return self._agent + def get_use_hostname_patterns(self): + return self._use_hostname_patterns + + def set_use_hostname_patterns(self, v): + self._use_hostname_patterns = v + self._policy_local.use_hostname_patterns = v + # # Management interface to create a ruleset # diff --git a/src/policy.c b/src/policy.c index 7de111c225..e982077157 100644 --- a/src/policy.c +++ b/src/policy.c @@ -61,6 +61,7 @@ struct qd_policy_t { qd_dispatch_t *qd; qd_log_source_t *log_source; void *py_policy_manager; + sys_mutex_t *tree_lock; qd_parse_tree_t *hostname_tree; // configured settings int max_connection_limit; @@ -83,6 +84,7 @@ qd_policy_t *qd_policy(qd_dispatch_t *qd) policy->qd = qd; policy->log_source = qd_log_source("POLICY"); policy->max_connection_limit = 65535; + policy->tree_lock = sys_mutex(); policy->hostname_tree = qd_parse_tree_new(QD_PARSE_TREE_ADDRESS); qd_log(policy->log_source, QD_LOG_TRACE, "Policy Initialized"); From e1ae8d3022d680775a5f441ea677b1498453ee15 Mon Sep 17 00:00:00 2001 From: Chuck Rolke Date: Thu, 10 May 2018 15:41:21 -0400 Subject: [PATCH 04/12] DISPATCH-990: Add c code to support tree hostname lookups --- src/policy.c | 38 ++++++++++++++++++++++++++++++++++++++ src/policy.h | 19 +++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/src/policy.c b/src/policy.c index e982077157..c545f46159 100644 --- a/src/policy.c +++ b/src/policy.c @@ -831,3 +831,41 @@ bool qd_policy_approve_link_name(const char *username, } return false; } + + +// Add a hostname to the lookup parse_tree +void qd_policy_host_pattern_add(qd_policy_t *policy, const char *hostPattern) +{ + sys_mutex_lock(policy->tree_lock); + void *oldp = qd_parse_tree_add_pattern_str(policy->hostname_tree, hostPattern, (void*)hostPattern); + sys_mutex_unlock(policy->tree_lock); + if (oldp) { + qd_log(policy->log_source, QD_LOG_INFO, "vhost hostname pattern '%s' replaced existing pattern", hostPattern); + } +} + + +// Remove a hostname from the lookup parse_tree +void qd_policy_host_pattern_remove(qd_policy_t *policy, const char *hostPattern) +{ + sys_mutex_lock(policy->tree_lock); + void *oldp = qd_parse_tree_remove_pattern_str(policy->hostname_tree, hostPattern); + sys_mutex_unlock(policy->tree_lock); + if (!oldp) { + qd_log(policy->log_source, QD_LOG_INFO, "vhost hostname pattern '%s' for removal not found", hostPattern); + } +} + + +// Look up a hostname in the lookup parse_tree +const char *qd_policy_host_pattern_lookup(qd_policy_t *policy, const char *hostPattern) +{ + void *payload = 0; + sys_mutex_lock(policy->tree_lock); + bool matched = qd_parse_tree_retrieve_match_str(policy->hostname_tree, hostPattern, &payload); + sys_mutex_unlock(policy->tree_lock); + if (!matched) payload = 0; + qd_log(policy->log_source, QD_LOG_TRACE, "vhost hostname pattern '%s' lookup returned '%s'", + hostPattern, (payload ? (char *)payload : "null")); + return (const char *)payload; +} diff --git a/src/policy.h b/src/policy.h index 9bddd288bb..66f7ac1150 100644 --- a/src/policy.h +++ b/src/policy.h @@ -177,4 +177,23 @@ bool qd_policy_approve_link_name(const char *username, const char *proposed, bool isReceiver ); + +/** Add a hostname to the lookup parse_tree + * @param[in] policy qd_policy_t + * @param[in] hostPattern the hostname pattern with possible parse_tree wildcards + */ +void qd_policy_host_pattern_add(qd_policy_t *policy, const char *hostPattern); + +/** Remove a hostname from the lookup parse_tree + * @param[in] policy qd_policy_t + * @param[in] hostPattern the hostname pattern with possible parse_tree wildcards + */ +void qd_policy_host_pattern_remove(qd_policy_t *policy, const char *hostPattern); + +/** Look up a hostname in the lookup parse_tree + * @param[in] policy qd_policy_t + * @param[in] hostname a concrete vhost name + * @return the name of the ruleset whose hostname pattern matched this actual hostname + */ +const char *qd_policy_host_pattern_lookup(qd_policy_t *policy, const char *hostPattern); #endif From f90c2a805d663fb2ea15a0fec7c70c16ce0aa1aa Mon Sep 17 00:00:00 2001 From: Chuck Rolke Date: Fri, 11 May 2018 09:22:24 -0400 Subject: [PATCH 05/12] DISPATCH-990: Add/remove policy hostname tree entries Tie hostname lookup tree to life cycle of policy rulesets. --- python/qpid_dispatch_internal/dispatch.py | 4 ++++ .../qpid_dispatch_internal/policy/policy_local.py | 10 ++++++++++ src/dispatch.c | 15 +++++++++++++++ src/policy.c | 14 ++++++++------ src/policy.h | 6 +++--- 5 files changed, 40 insertions(+), 9 deletions(-) diff --git a/python/qpid_dispatch_internal/dispatch.py b/python/qpid_dispatch_internal/dispatch.py index 4c9123b0c5..609396ef98 100644 --- a/python/qpid_dispatch_internal/dispatch.py +++ b/python/qpid_dispatch_internal/dispatch.py @@ -78,6 +78,10 @@ def __init__(self, handle): self._prototype(self.qd_dispatch_policy_c_counts_alloc, c_long, [], check=False) self._prototype(self.qd_dispatch_policy_c_counts_free, None, [c_long], check=False) self._prototype(self.qd_dispatch_policy_c_counts_refresh, None, [c_long, py_object]) + self._prototype(self.qd_dispatch_policy_host_pattern_add, None, [self.qd_dispatch_p, c_char_p]) + self._prototype(self.qd_dispatch_policy_host_pattern_remove, None, [self.qd_dispatch_p, c_char_p]) + self._prototype(self.qd_dispatch_policy_host_pattern_lookup, c_char_p, [self.qd_dispatch_p, c_char_p]) + self._prototype(self.qd_dispatch_register_display_name_service, None, [self.qd_dispatch_p, py_object]) diff --git a/python/qpid_dispatch_internal/policy/policy_local.py b/python/qpid_dispatch_internal/policy/policy_local.py index bee0499aa2..cd620720c8 100644 --- a/python/qpid_dispatch_internal/policy/policy_local.py +++ b/python/qpid_dispatch_internal/policy/policy_local.py @@ -539,6 +539,13 @@ def create_ruleset(self, attributes): self.statsdb[name].update_ruleset(candidate) self._manager.log_info("Updated policy rules for vhost %s" % name) # TODO: ruleset lock + if self.use_hostname_patterns: + agent = self._manager.get_agent() + if name in self.rulesetdb: + # an update. remove existing hostname pattern + agent.qd.qd_dispatch_policy_host_pattern_remove(agent.dispatch, name) + # add new pattern + agent.qd.qd_dispatch_policy_host_pattern_add(agent.dispatch, name) self.rulesetdb[name] = {} self.rulesetdb[name].update(candidate) @@ -550,6 +557,9 @@ def policy_delete(self, name): if name not in self.rulesetdb: raise PolicyError("Policy '%s' does not exist" % name) # TODO: ruleset lock + if self.use_hostname_patterns: + agent = self._manager.get_agent() + agent.qd.qd_dispatch_policy_host_pattern_remove(agent.dispatch, name) del self.rulesetdb[name] # diff --git a/src/dispatch.c b/src/dispatch.c index 99469f0e9d..6f8247187f 100644 --- a/src/dispatch.c +++ b/src/dispatch.c @@ -270,6 +270,21 @@ void qd_dispatch_policy_c_counts_refresh(long ccounts, qd_entity_t *entity) qd_policy_c_counts_refresh(ccounts, entity); } +void qd_dispatch_policy_host_pattern_add(qd_dispatch_t *qd, char *hostPattern) +{ + qd_policy_host_pattern_add(qd->policy, hostPattern); +} + +void qd_dispatch_policy_host_pattern_remove(qd_dispatch_t *qd, char *hostPattern) +{ + qd_policy_host_pattern_remove(qd->policy, hostPattern); +} + +char * qd_dispatch_policy_host_pattern_lookup(qd_dispatch_t *qd, char *hostPattern) +{ + return qd_policy_host_pattern_lookup(qd->policy, hostPattern); +} + qd_error_t qd_dispatch_prepare(qd_dispatch_t *qd) { qd->server = qd_server(qd, qd->thread_count, qd->router_id, qd->sasl_config_path, qd->sasl_config_name); diff --git a/src/policy.c b/src/policy.c index c545f46159..77523ce55e 100644 --- a/src/policy.c +++ b/src/policy.c @@ -834,10 +834,10 @@ bool qd_policy_approve_link_name(const char *username, // Add a hostname to the lookup parse_tree -void qd_policy_host_pattern_add(qd_policy_t *policy, const char *hostPattern) +void qd_policy_host_pattern_add(qd_policy_t *policy, char *hostPattern) { sys_mutex_lock(policy->tree_lock); - void *oldp = qd_parse_tree_add_pattern_str(policy->hostname_tree, hostPattern, (void*)hostPattern); + void *oldp = qd_parse_tree_add_pattern_str(policy->hostname_tree, hostPattern, hostPattern); sys_mutex_unlock(policy->tree_lock); if (oldp) { qd_log(policy->log_source, QD_LOG_INFO, "vhost hostname pattern '%s' replaced existing pattern", hostPattern); @@ -846,7 +846,7 @@ void qd_policy_host_pattern_add(qd_policy_t *policy, const char *hostPattern) // Remove a hostname from the lookup parse_tree -void qd_policy_host_pattern_remove(qd_policy_t *policy, const char *hostPattern) +void qd_policy_host_pattern_remove(qd_policy_t *policy, char *hostPattern) { sys_mutex_lock(policy->tree_lock); void *oldp = qd_parse_tree_remove_pattern_str(policy->hostname_tree, hostPattern); @@ -858,14 +858,16 @@ void qd_policy_host_pattern_remove(qd_policy_t *policy, const char *hostPattern) // Look up a hostname in the lookup parse_tree -const char *qd_policy_host_pattern_lookup(qd_policy_t *policy, const char *hostPattern) +char * qd_policy_host_pattern_lookup(qd_policy_t *policy, char *hostPattern) { void *payload = 0; sys_mutex_lock(policy->tree_lock); bool matched = qd_parse_tree_retrieve_match_str(policy->hostname_tree, hostPattern, &payload); sys_mutex_unlock(policy->tree_lock); - if (!matched) payload = 0; + if (!matched) { + payload = 0; + } qd_log(policy->log_source, QD_LOG_TRACE, "vhost hostname pattern '%s' lookup returned '%s'", hostPattern, (payload ? (char *)payload : "null")); - return (const char *)payload; + return payload; } diff --git a/src/policy.h b/src/policy.h index 66f7ac1150..57a64fe5ad 100644 --- a/src/policy.h +++ b/src/policy.h @@ -182,18 +182,18 @@ bool qd_policy_approve_link_name(const char *username, * @param[in] policy qd_policy_t * @param[in] hostPattern the hostname pattern with possible parse_tree wildcards */ -void qd_policy_host_pattern_add(qd_policy_t *policy, const char *hostPattern); +void qd_policy_host_pattern_add(qd_policy_t *policy, char *hostPattern); /** Remove a hostname from the lookup parse_tree * @param[in] policy qd_policy_t * @param[in] hostPattern the hostname pattern with possible parse_tree wildcards */ -void qd_policy_host_pattern_remove(qd_policy_t *policy, const char *hostPattern); +void qd_policy_host_pattern_remove(qd_policy_t *policy, char *hostPattern); /** Look up a hostname in the lookup parse_tree * @param[in] policy qd_policy_t * @param[in] hostname a concrete vhost name * @return the name of the ruleset whose hostname pattern matched this actual hostname */ -const char *qd_policy_host_pattern_lookup(qd_policy_t *policy, const char *hostPattern); +char * qd_policy_host_pattern_lookup(qd_policy_t *policy, char *hostPattern); #endif From f17631cbe7a695fb4492a4437289d82a1a89a1c0 Mon Sep 17 00:00:00 2001 From: Chuck Rolke Date: Fri, 11 May 2018 11:10:49 -0400 Subject: [PATCH 06/12] DISPATCH-990: integrate the pattern match with vhost lookups --- .../qpid_dispatch_internal/policy/policy_local.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/python/qpid_dispatch_internal/policy/policy_local.py b/python/qpid_dispatch_internal/policy/policy_local.py index cd620720c8..ce47e211a8 100644 --- a/python/qpid_dispatch_internal/policy/policy_local.py +++ b/python/qpid_dispatch_internal/policy/policy_local.py @@ -607,15 +607,24 @@ def lookup_user(self, user, rhost, vhost_in, conn_name, conn_id): """ try: # choose rule set based on incoming vhost or default vhost + # or potential vhost found by pattern matching vhost = vhost_in - if vhost_in not in self.rulesetdb: + if self.use_hostname_patterns: + agent = self._manager.get_agent() + vhost = agent.qd.qd_dispatch_policy_host_pattern_lookup(agent.dispatch, vhost) + if vhost not in self.rulesetdb: if self.default_vhost_enabled(): vhost = self._default_vhost else: self._manager.log_info( "DENY AMQP Open for user '%s', rhost '%s', vhost '%s': " - "No policy defined for vhost" % (user, rhost, vhost)) + "No policy defined for vhost" % (user, rhost, vhost_in)) return "" + if vhost != vhost_in: + self._manager.log_debug( + "AMQP Open for user '%s', rhost '%s', vhost '%s': " + "proceeds using vhost '%s' ruleset" % (user, rhost, vhost_in, vhost)) + ruleset = self.rulesetdb[vhost] # look up the stats From 9c0a09d793795acbc165df37a45f8fa86329da0f Mon Sep 17 00:00:00 2001 From: Chuck Rolke Date: Mon, 14 May 2018 12:31:13 -0400 Subject: [PATCH 07/12] DISPATCH-990: Reject add of vhost that has pattern conflict. Add self test showing how conflicts are managed. --- python/qpid_dispatch_internal/dispatch.py | 2 +- .../policy/policy_local.py | 12 ++-- src/dispatch.c | 4 +- src/policy.c | 14 +++- src/policy.h | 8 ++- tests/parse_tree_tests.c | 67 +++++++++++++++++++ 6 files changed, 93 insertions(+), 14 deletions(-) diff --git a/python/qpid_dispatch_internal/dispatch.py b/python/qpid_dispatch_internal/dispatch.py index 609396ef98..c819122690 100644 --- a/python/qpid_dispatch_internal/dispatch.py +++ b/python/qpid_dispatch_internal/dispatch.py @@ -78,7 +78,7 @@ def __init__(self, handle): self._prototype(self.qd_dispatch_policy_c_counts_alloc, c_long, [], check=False) self._prototype(self.qd_dispatch_policy_c_counts_free, None, [c_long], check=False) self._prototype(self.qd_dispatch_policy_c_counts_refresh, None, [c_long, py_object]) - self._prototype(self.qd_dispatch_policy_host_pattern_add, None, [self.qd_dispatch_p, c_char_p]) + self._prototype(self.qd_dispatch_policy_host_pattern_add, ctypes.c_bool, [self.qd_dispatch_p, c_char_p]) self._prototype(self.qd_dispatch_policy_host_pattern_remove, None, [self.qd_dispatch_p, c_char_p]) self._prototype(self.qd_dispatch_policy_host_pattern_lookup, c_char_p, [self.qd_dispatch_p, c_char_p]) diff --git a/python/qpid_dispatch_internal/policy/policy_local.py b/python/qpid_dispatch_internal/policy/policy_local.py index ce47e211a8..6f8dbb3c2b 100644 --- a/python/qpid_dispatch_internal/policy/policy_local.py +++ b/python/qpid_dispatch_internal/policy/policy_local.py @@ -531,6 +531,11 @@ def create_ruleset(self, attributes): if len(warnings) > 0: for warning in warnings: self._manager.log_warning(warning) + # Reject if parse tree optimized name collision + if self.use_hostname_patterns: + agent = self._manager.get_agent() + if not agent.qd.qd_dispatch_policy_host_pattern_add(agent.dispatch, name): + raise PolicyError("Policy '%s' optimized pattern conflicts with existing pattern" % name) if name not in self.rulesetdb: if name not in self.statsdb: self.statsdb[name] = AppStats(name, self._manager, candidate) @@ -539,13 +544,6 @@ def create_ruleset(self, attributes): self.statsdb[name].update_ruleset(candidate) self._manager.log_info("Updated policy rules for vhost %s" % name) # TODO: ruleset lock - if self.use_hostname_patterns: - agent = self._manager.get_agent() - if name in self.rulesetdb: - # an update. remove existing hostname pattern - agent.qd.qd_dispatch_policy_host_pattern_remove(agent.dispatch, name) - # add new pattern - agent.qd.qd_dispatch_policy_host_pattern_add(agent.dispatch, name) self.rulesetdb[name] = {} self.rulesetdb[name].update(candidate) diff --git a/src/dispatch.c b/src/dispatch.c index 6f8247187f..3c2629a1ec 100644 --- a/src/dispatch.c +++ b/src/dispatch.c @@ -270,9 +270,9 @@ void qd_dispatch_policy_c_counts_refresh(long ccounts, qd_entity_t *entity) qd_policy_c_counts_refresh(ccounts, entity); } -void qd_dispatch_policy_host_pattern_add(qd_dispatch_t *qd, char *hostPattern) +bool qd_dispatch_policy_host_pattern_add(qd_dispatch_t *qd, char *hostPattern) { - qd_policy_host_pattern_add(qd->policy, hostPattern); + return qd_policy_host_pattern_add(qd->policy, hostPattern); } void qd_dispatch_policy_host_pattern_remove(qd_dispatch_t *qd, char *hostPattern) diff --git a/src/policy.c b/src/policy.c index 77523ce55e..dec645ee08 100644 --- a/src/policy.c +++ b/src/policy.c @@ -834,14 +834,22 @@ bool qd_policy_approve_link_name(const char *username, // Add a hostname to the lookup parse_tree -void qd_policy_host_pattern_add(qd_policy_t *policy, char *hostPattern) +bool qd_policy_host_pattern_add(qd_policy_t *policy, char *hostPattern) { sys_mutex_lock(policy->tree_lock); void *oldp = qd_parse_tree_add_pattern_str(policy->hostname_tree, hostPattern, hostPattern); sys_mutex_unlock(policy->tree_lock); if (oldp) { - qd_log(policy->log_source, QD_LOG_INFO, "vhost hostname pattern '%s' replaced existing pattern", hostPattern); + qd_log(policy->log_source, + QD_LOG_WARNING, + "vhost hostname pattern '%s' failed to replace optimized pattern '%s'", + hostPattern, oldp); + sys_mutex_lock(policy->tree_lock); + void *recovered = qd_parse_tree_add_pattern_str(policy->hostname_tree, (char *)oldp, oldp); + sys_mutex_unlock(policy->tree_lock); + assert (recovered && !strcmp((char *)recovered, hostPattern)); } + return oldp == 0; } @@ -852,7 +860,7 @@ void qd_policy_host_pattern_remove(qd_policy_t *policy, char *hostPattern) void *oldp = qd_parse_tree_remove_pattern_str(policy->hostname_tree, hostPattern); sys_mutex_unlock(policy->tree_lock); if (!oldp) { - qd_log(policy->log_source, QD_LOG_INFO, "vhost hostname pattern '%s' for removal not found", hostPattern); + qd_log(policy->log_source, QD_LOG_WARNING, "vhost hostname pattern '%s' for removal not found", hostPattern); } } diff --git a/src/policy.h b/src/policy.h index 57a64fe5ad..98500ec66d 100644 --- a/src/policy.h +++ b/src/policy.h @@ -179,10 +179,16 @@ bool qd_policy_approve_link_name(const char *username, ); /** Add a hostname to the lookup parse_tree + * Note that the parse_tree may store an 'optimised' pattern for a given + * pattern. Thus the patterns a user puts in may collide with existing + * patterns even though the text of the host patterns is different. + * This function does not allow new patterns with thier optimizations + * to overwrite existing patterns that may have been optimised. * @param[in] policy qd_policy_t * @param[in] hostPattern the hostname pattern with possible parse_tree wildcards + * @return True if the possibly optimised pattern was added to the lookup parse tree */ -void qd_policy_host_pattern_add(qd_policy_t *policy, char *hostPattern); +bool qd_policy_host_pattern_add(qd_policy_t *policy, char *hostPattern); /** Remove a hostname from the lookup parse_tree * @param[in] policy qd_policy_t diff --git a/tests/parse_tree_tests.c b/tests/parse_tree_tests.c index 5800b4a752..1dba5ba3e4 100644 --- a/tests/parse_tree_tests.c +++ b/tests/parse_tree_tests.c @@ -98,6 +98,72 @@ static char *test_add_and_match_str(void *context) return NULL; } +static char *test_usurpation_recovery_str(void *context) +{ + const char *A = "#"; + const char *B = "#.#.#.#"; + qd_parse_tree_t *node = qd_parse_tree_new(QD_PARSE_TREE_ADDRESS); + void *payload; + void *usurped; + void *deposed; + + // rightful owner is ensconsced + if (qd_parse_tree_add_pattern_str(node, A, (void *)A)) + return "Add returned existing value (1)"; + + // matches on A or B both return A + if (!qd_parse_tree_retrieve_match_str(node, A, &payload)) + return "Could not get pattern"; + + if (!payload || strcmp(A, (char *)payload)) + return "Got bad pattern"; + + if (!qd_parse_tree_retrieve_match_str(node, B, &payload)) + return "Could not get pattern"; + + if (!payload || strcmp(A, (char *)payload)) + return "Got bad pattern"; + + // usurper comes along + usurped = qd_parse_tree_add_pattern_str(node, B, (void *)B); + if (!usurped || strcmp(A, (char *)usurped)) + return "Usurper should have grabbed '#' optimized match"; + + // matches on A or B both return B + if (!qd_parse_tree_retrieve_match_str(node, A, &payload)) + return "Could not get pattern"; + + if (!payload || strcmp(B, (char *)payload)) + return "Got bad pattern"; + + if (!qd_parse_tree_retrieve_match_str(node, B, &payload)) + return "Could not get pattern"; + + if (!payload || strcmp(B, (char *)payload)) + return "Got bad pattern"; + + // Restore rightful owner + deposed = qd_parse_tree_add_pattern_str(node, usurped, usurped); + if (!deposed || strcmp(B, (char *)deposed)) + return "Failed to depose B"; + + // matches on A or B both return A + if (!qd_parse_tree_retrieve_match_str(node, A, &payload)) + return "Could not get pattern"; + + if (!payload || strcmp(A, (char *)payload)) + return "Got bad pattern"; + + if (!qd_parse_tree_retrieve_match_str(node, B, &payload)) + return "Could not get pattern"; + + if (!payload || strcmp(A, (char *)payload)) + return "Got bad pattern"; + + qd_parse_tree_free(node); + return NULL; +} + // for pattern match callback typedef struct { int count; @@ -633,6 +699,7 @@ int parse_tree_tests(void) TEST_CASE(test_add_remove, 0); TEST_CASE(test_add_and_match_str, 0); + TEST_CASE(test_usurpation_recovery_str, 0); TEST_CASE(test_normalization, 0); TEST_CASE(test_matches, 0); TEST_CASE(test_multiple_matches, 0); From 2b0f3d066428f5c13eed83848f017c39ca56f565 Mon Sep 17 00:00:00 2001 From: Chuck Rolke Date: Mon, 14 May 2018 12:57:20 -0400 Subject: [PATCH 08/12] DISPATCH-990: Fix locks so hostname adds are atomic --- src/policy.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/policy.c b/src/policy.c index dec645ee08..9c99c88b00 100644 --- a/src/policy.c +++ b/src/policy.c @@ -838,17 +838,16 @@ bool qd_policy_host_pattern_add(qd_policy_t *policy, char *hostPattern) { sys_mutex_lock(policy->tree_lock); void *oldp = qd_parse_tree_add_pattern_str(policy->hostname_tree, hostPattern, hostPattern); - sys_mutex_unlock(policy->tree_lock); if (oldp) { - qd_log(policy->log_source, - QD_LOG_WARNING, - "vhost hostname pattern '%s' failed to replace optimized pattern '%s'", - hostPattern, oldp); - sys_mutex_lock(policy->tree_lock); void *recovered = qd_parse_tree_add_pattern_str(policy->hostname_tree, (char *)oldp, oldp); - sys_mutex_unlock(policy->tree_lock); - assert (recovered && !strcmp((char *)recovered, hostPattern)); + assert (recovered); } + sys_mutex_unlock(policy->tree_lock); + if (oldp) + qd_log(policy->log_source, + QD_LOG_WARNING, + "vhost hostname pattern '%s' failed to replace optimized pattern '%s'", + hostPattern, oldp); return oldp == 0; } From 82cef3abff91857871ba7e9ca88177a39db0f7a0 Mon Sep 17 00:00:00 2001 From: Chuck Rolke Date: Mon, 14 May 2018 14:11:30 -0400 Subject: [PATCH 09/12] DISPATCH-990: Add hostname translation to settings lookup --- python/qpid_dispatch_internal/policy/policy_local.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/python/qpid_dispatch_internal/policy/policy_local.py b/python/qpid_dispatch_internal/policy/policy_local.py index 6f8dbb3c2b..4f33fc6b69 100644 --- a/python/qpid_dispatch_internal/policy/policy_local.py +++ b/python/qpid_dispatch_internal/policy/policy_local.py @@ -703,9 +703,16 @@ def lookup_settings(self, vhost_in, groupname, upolicy): """ try: vhost = vhost_in + if self.use_hostname_patterns: + agent = self._manager.get_agent() + vhost = agent.qd.qd_dispatch_policy_host_pattern_lookup(agent.dispatch, vhost) if vhost not in self.rulesetdb: if self.default_vhost_enabled(): vhost = self._default_vhost + if vhost != vhost_in: + self._manager.log_debug( + "AMQP Open lookup settings for user '%s', rhost '%s', vhost '%s': " + "proceeds using vhost '%s' ruleset" % (user, rhost, vhost_in, vhost)) if vhost not in self.rulesetdb: self._manager.log_info( From 415b4568ecac113f3ea4212507407dba03f7cdd1 Mon Sep 17 00:00:00 2001 From: Chuck Rolke Date: Mon, 14 May 2018 15:56:58 -0400 Subject: [PATCH 10/12] DISPATCH-990: Add hostname pattern self test Remove stray variables from log message. --- .../policy/policy_local.py | 4 +- tests/system_tests_policy.py | 75 +++++++++++++++++++ 2 files changed, 77 insertions(+), 2 deletions(-) diff --git a/python/qpid_dispatch_internal/policy/policy_local.py b/python/qpid_dispatch_internal/policy/policy_local.py index 4f33fc6b69..f651dcc293 100644 --- a/python/qpid_dispatch_internal/policy/policy_local.py +++ b/python/qpid_dispatch_internal/policy/policy_local.py @@ -711,8 +711,8 @@ def lookup_settings(self, vhost_in, groupname, upolicy): vhost = self._default_vhost if vhost != vhost_in: self._manager.log_debug( - "AMQP Open lookup settings for user '%s', rhost '%s', vhost '%s': " - "proceeds using vhost '%s' ruleset" % (user, rhost, vhost_in, vhost)) + "AMQP Open lookup settings for vhost '%s': " + "proceeds using vhost '%s' ruleset" % (vhost_in, vhost)) if vhost not in self.rulesetdb: self._manager.log_info( diff --git a/tests/system_tests_policy.py b/tests/system_tests_policy.py index 93c7d6ad73..e2f2797d19 100644 --- a/tests/system_tests_policy.py +++ b/tests/system_tests_policy.py @@ -745,5 +745,80 @@ def test_link_name_parse_tree_patterns(self): self.assertTrue(exception) +class PolicyHostamePatternTest(TestCase): + """ + Verify hostname pattern matching + """ + @classmethod + def setUpClass(cls): + """Start the router""" + super(PolicyHostamePatternTest, cls).setUpClass() + listen_port = cls.tester.get_port() + policy_config_path = os.path.join(DIR, 'policy-8') + config = Qdrouterd.Config([ + ('router', {'mode': 'standalone', 'id': 'QDR.Policy8'}), + ('listener', {'port': listen_port}), + ('policy', {'maxConnections': 2, 'policyDir': policy_config_path, 'enableVhostPolicy': 'true', 'useVhostNamePatterns': 'true'}) + ]) + + cls.router = cls.tester.qdrouterd('PolicyVhostNamePatternTest', config, wait=True) + try: + cls.router.wait_ready(timeout = 5) + except Exception, e: + pass + + def address(self): + return self.router.addresses[0] + + def run_qdmanage(self, cmd, input=None, expect=Process.EXIT_OK): + p = self.popen( + ['qdmanage'] + cmd.split(' ') + ['--bus', 'u1:password@' + self.address(), '--indent=-1', '--timeout', str(TIMEOUT)], + stdin=PIPE, stdout=PIPE, stderr=STDOUT, expect=expect) + out = p.communicate(input)[0] + try: + p.teardown() + except Exception, e: + raise Exception("%s\n%s" % (e, out)) + return out + + def disallowed_hostname(self): + return """ +{ + "hostname": "#.#.0.0", + "maxConnections": 3, + "maxConnectionsPerHost": 3, + "maxConnectionsPerUser": 3, + "allowUnknownUser": true, + "groups": { + "$default": { + "allowAnonymousSender": true, + "maxReceivers": 99, + "users": "*", + "maxSessionWindow": 1000000, + "maxFrameSize": 222222, + "sources": "public, private, $management", + "maxMessageSize": 222222, + "allowDynamicSource": true, + "remoteHosts": "*", + "maxSessions": 2, + "maxSenders": 22 + } + } +} +""" + + def test_hostname_pattern_00_hello(self): + rulesets = json.loads(self.run_qdmanage('query --type=vhost')) + self.assertEqual(len(rulesets), 1) + + def test_hostname_pattern_01_denied_add(self): + qdm_out = "" + try: + qdm_out = self.run_qdmanage('create --type=vhost --name=#.#.0.0 --stdin', input=self.disallowed_hostname()) + except Exception, e: + self.assertTrue("pattern conflicts" in e.message, msg=('Error running qdmanage %s' % e.message)) + self.assertFalse("222222" in qdm_out) + + if __name__ == '__main__': unittest.main(main_module()) From 8712cabdf8f37dddc7f8d1ffc4792fbd177f050d Mon Sep 17 00:00:00 2001 From: Chuck Rolke Date: Mon, 14 May 2018 16:14:03 -0400 Subject: [PATCH 11/12] DISPATCH-990: add policy dir to support self test --- tests/policy-8/management-access.json | 46 +++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 tests/policy-8/management-access.json diff --git a/tests/policy-8/management-access.json b/tests/policy-8/management-access.json new file mode 100644 index 0000000000..7d99c10c2d --- /dev/null +++ b/tests/policy-8/management-access.json @@ -0,0 +1,46 @@ +## +## Licensed to the Apache Software Foundation (ASF) under one +## or more contributor license agreements. See the NOTICE file +## distributed with this work for additional information +## regarding copyright ownership. The ASF licenses this file +## to you under the Apache License, Version 2.0 (the +## "License"); you may not use this file except in compliance +## with the License. You may obtain a copy of the License at +## +## http://www.apache.org/licenses/LICENSE-2.0 +## +## Unless required by applicable law or agreed to in writing, +## software distributed under the License is distributed on an +## "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +## KIND, either express or implied. See the License for the +## specific language governing permissions and limitations +## under the License +## + +[ + ["vhost", { + "hostname": "#.0.0", + "maxConnections": 50, + "maxConnectionsPerUser": 5, + "maxConnectionsPerHost": 20, + "allowUnknownUser": true, + "groups": { + "$default" : { + "users": "*", + "remoteHosts": "*", + "maxFrameSize": 222222, + "maxMessageSize": 222222, + "maxSessionWindow": 222222, + "maxSessions": 2, + "maxSenders": 22, + "maxReceivers": 22, + "allowDynamicSource": true, + "allowAnonymousSender": true, + "allowUserIdProxy": false, + "sources": "$management, _local/$displayname", + "targets": "$management, _local/$displayname" + } + } + } + ] +] From 63a56d6052e747a0043df621a67805989de5d255 Mon Sep 17 00:00:00 2001 From: Chuck Rolke Date: Tue, 15 May 2018 10:43:44 -0400 Subject: [PATCH 12/12] DISPATCH-990: Document name pattern match feature in old book. Rename and touch up configuration settings to make docs read better. --- doc/book/policy.adoc | 59 +++++++++++++++++++ python/qpid_dispatch/management/qdrouter.json | 4 +- .../management/config.py | 2 +- src/policy.c | 6 +- tests/system_tests_policy.py | 2 +- 5 files changed, 66 insertions(+), 7 deletions(-) diff --git a/doc/book/policy.adoc b/doc/book/policy.adoc index c2be257154..888611cc88 100644 --- a/doc/book/policy.adoc +++ b/doc/book/policy.adoc @@ -90,6 +90,64 @@ xref:example2[Example 2] illustrates how the default vhost feature can be used to apply a single vhost policy set of restrictions to any number of vhost connections. +=== Vhost Patterns + +Policy vhost names may be interpreted as literal host names or +as host name patterns. Vhost name patterns are a convenience +for letting a single policy rule cover a wide range of vhosts. + +Host name patterns consist of a series of host and domain name +labels and one or more tokens all concatenated with periods or dots. +A token can be one of the following: + +[options="header"] +|==== +| Token character | Match rule +| asterisk * | matches a single hostname label +| hash # | matches zero or more hostname labels +|==== + +Some simple examples show how given policy name patterns match +incoming connection vhost names. + +[options="header"] +|==== +| Policy pattern | Connection vhost | Policy match +| *.example.com | example.com | no +| *.example.com | www.example.com | yes +| *.example.com | srv2.www.example.com | no +| #.example.com | example.com | yes +| #.example.com | www.example.com | yes +| #.example.com | a.b.c.d.example.com | yes +| #.example.com | bighost.com | no +| www.*.test.example.com | www.test.example.com | no +| www.*.test.example.com | www.a.test.example.com | yes +| www.*.test.example.com | www.a.b.c.test.example.com | no +| www.#.test.example.com | www.test.example.com | yes +| www.#.test.example.com | www.a.test.example.com | yes +| www.#.test.example.com | www.a.b.c.test.example.com | yes +|==== + +Pattern matching applies the following precedence rules. + +[options="header"] +|==== +| Policy pattern | Precedence +| exact match | high +| asterisk * | medium +| hash # | low +|==== + +Policy vhost name patterns are optimised before they are used +in connection vhost name matching. As a result of this +optimisation the names stored for pattern match lookups are +not necessarily the same as the patterns specified in the +vhost policy hostname. The policy agent disallows vhost +name patterns that reduce to the same pattern as an existing name +pattern. For instance, name pattern _pass:[#.#.#.#.com]_ is reduced to _pass:[#.com]_. +Attempts to create a vhost name pattern whose optimised +name conflicts with an existing optimised name will be denied. + == Policy Schema Policy configuration is specified in two schema objects. @@ -123,6 +181,7 @@ created as needed. | enableVhostPolicy | false | Enable vhost policy connection denial, and resource limit enforcement. | policyDir | "" | Absolute path to a directory that holds vhost definition .json files. All vhost definitions in all .json files in this directory are processed. | defaultVhost | "$default" | Vhost rule set name to use for connections with a vhost that is otherwise not defined. Default vhost processing may be disabled either by erasing the definition of _defaultVhost_ or by not defining a _vhost_ object named _$default_. +| enableVhostNamePatterns | false | Enable vhost name patterns. When false vhost hostnames are treated as literal strings. When true vhost hostnames are treated as match patterns. |==== === Vhost Policy diff --git a/python/qpid_dispatch/management/qdrouter.json b/python/qpid_dispatch/management/qdrouter.json index ee8a9435c9..42f501fac9 100644 --- a/python/qpid_dispatch/management/qdrouter.json +++ b/python/qpid_dispatch/management/qdrouter.json @@ -1678,10 +1678,10 @@ "required": false, "create": true }, - "useVhostNamePatterns": { + "enableVhostNamePatterns": { "type": "boolean", "default": false, - "description": "Use Vhost name patterns.", + "description": "Enable vhost name patterns. When false vhost hostnames are treated as literal strings. When true vhost hostnames are treated as match patterns.", "required": false, "create": true }, diff --git a/python/qpid_dispatch_internal/management/config.py b/python/qpid_dispatch_internal/management/config.py index aad6bd271a..82814b525b 100644 --- a/python/qpid_dispatch_internal/management/config.py +++ b/python/qpid_dispatch_internal/management/config.py @@ -178,7 +178,7 @@ def configure(attributes): # Configure policy and policy manager before vhosts policyDir = config.by_type('policy')[0]['policyDir'] policyDefaultVhost = config.by_type('policy')[0]['defaultVhost'] - useHostnamePatterns = config.by_type('policy')[0]['useVhostNamePatterns'] + useHostnamePatterns = config.by_type('policy')[0]['enableVhostNamePatterns'] for a in config.by_type("policy"): configure(a) agent.policy.set_default_vhost(policyDefaultVhost) diff --git a/src/policy.c b/src/policy.c index 9c99c88b00..697ec1b811 100644 --- a/src/policy.c +++ b/src/policy.c @@ -67,7 +67,7 @@ struct qd_policy_t { int max_connection_limit; char *policyDir; bool enableVhostPolicy; - bool useVhostNamePatterns; + bool enableVhostNamePatterns; // live statistics int connections_processed; int connections_denied; @@ -115,7 +115,7 @@ qd_error_t qd_entity_configure_policy(qd_policy_t *policy, qd_entity_t *entity) policy->policyDir = qd_entity_opt_string(entity, "policyDir", 0); CHECK(); policy->enableVhostPolicy = qd_entity_opt_bool(entity, "enableVhostPolicy", false); CHECK(); - policy->useVhostNamePatterns = qd_entity_opt_bool(entity, "useVhostNamePatterns", false); CHECK(); + policy->enableVhostNamePatterns = qd_entity_opt_bool(entity, "enableVhostNamePatterns", false); CHECK(); qd_log(policy->log_source, QD_LOG_INFO, "Policy configured maxConnections: %d, " "policyDir: '%s'," @@ -124,7 +124,7 @@ qd_error_t qd_entity_configure_policy(qd_policy_t *policy, qd_entity_t *entity) policy->max_connection_limit, policy->policyDir, (policy->enableVhostPolicy ? "true" : "false"), - (policy->useVhostNamePatterns ? "true" : "false")); + (policy->enableVhostNamePatterns ? "true" : "false")); return QD_ERROR_NONE; error: diff --git a/tests/system_tests_policy.py b/tests/system_tests_policy.py index e2f2797d19..92555d03ee 100644 --- a/tests/system_tests_policy.py +++ b/tests/system_tests_policy.py @@ -758,7 +758,7 @@ def setUpClass(cls): config = Qdrouterd.Config([ ('router', {'mode': 'standalone', 'id': 'QDR.Policy8'}), ('listener', {'port': listen_port}), - ('policy', {'maxConnections': 2, 'policyDir': policy_config_path, 'enableVhostPolicy': 'true', 'useVhostNamePatterns': 'true'}) + ('policy', {'maxConnections': 2, 'policyDir': policy_config_path, 'enableVhostPolicy': 'true', 'enableVhostNamePatterns': 'true'}) ]) cls.router = cls.tester.qdrouterd('PolicyVhostNamePatternTest', config, wait=True)