Return-Path: X-Original-To: apmail-qpid-commits-archive@www.apache.org Delivered-To: apmail-qpid-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 290011942D for ; Tue, 5 Apr 2016 22:15:45 +0000 (UTC) Received: (qmail 94962 invoked by uid 500); 5 Apr 2016 22:15:45 -0000 Delivered-To: apmail-qpid-commits-archive@qpid.apache.org Received: (qmail 94927 invoked by uid 500); 5 Apr 2016 22:15:45 -0000 Mailing-List: contact commits-help@qpid.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@qpid.apache.org Delivered-To: mailing list commits@qpid.apache.org Received: (qmail 94918 invoked by uid 99); 5 Apr 2016 22:15:45 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 05 Apr 2016 22:15:44 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id C4E49DFB90; Tue, 5 Apr 2016 22:15:44 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: chug@apache.org To: commits@qpid.apache.org Message-Id: X-Mailer: ASF-Git Admin Mailer Subject: qpid-dispatch git commit: DISPATCH-257: Fix policy link counting * Add self test to verify sender/recevier link denial by count. * Fix code to account for sender/receiver links properly. Router 'incoming links' are 'senders' for policy. * Better log me Date: Tue, 5 Apr 2016 22:15:44 +0000 (UTC) Repository: qpid-dispatch Updated Branches: refs/heads/master f46b3afa9 -> 15de0870d DISPATCH-257: Fix policy link counting * Add self test to verify sender/recevier link denial by count. * Fix code to account for sender/receiver links properly. Router 'incoming links' are 'senders' for policy. * Better log messages. Project: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/repo Commit: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/commit/15de0870 Tree: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/tree/15de0870 Diff: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/diff/15de0870 Branch: refs/heads/master Commit: 15de0870d5430b69f94205bae33c0fc6e7d4a14f Parents: f46b3af Author: Chuck Rolke Authored: Tue Apr 5 18:07:56 2016 -0400 Committer: Chuck Rolke Committed: Tue Apr 5 18:07:56 2016 -0400 ---------------------------------------------------------------------- .../policy/policy_local.py | 9 +- src/container.c | 38 ++++----- src/policy.c | 16 ++-- tests/CMakeLists.txt | 1 + tests/policy-3/test-sender-receiver-limits.json | 26 ++++++ tests/system_tests_policy.py | 89 ++++++++++++++++++++ 6 files changed, 150 insertions(+), 29 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/15de0870/python/qpid_dispatch_internal/policy/policy_local.py ---------------------------------------------------------------------- diff --git a/python/qpid_dispatch_internal/policy/policy_local.py b/python/qpid_dispatch_internal/policy/policy_local.py index 89e5016..c1bea13 100644 --- a/python/qpid_dispatch_internal/policy/policy_local.py +++ b/python/qpid_dispatch_internal/policy/policy_local.py @@ -679,10 +679,11 @@ class PolicyLocal(object): @return: """ try: - facts = self._connections[conn_id] - stats = self.statsdb[facts.app] - stats.disconnect(facts.conn_name, facts.user, facts.host) - del self._connections[conn_id] + if conn_id in self._connections: + facts = self._connections[conn_id] + stats = self.statsdb[facts.app] + stats.disconnect(facts.conn_name, facts.user, facts.host) + del self._connections[conn_id] except Exception, e: self._manager.log_trace( "Policy internal error closing connection id %s. %s" % (conn_id, str(e))) http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/15de0870/src/container.c ---------------------------------------------------------------------- diff --git a/src/container.c b/src/container.c index 254e1ad..54b1138 100644 --- a/src/container.c +++ b/src/container.c @@ -423,11 +423,11 @@ int pn_event_handler(void *handler_context, void *conn_context, pn_event_t *even if (qd_link && qd_link->node) { if (qd_conn->policy_settings) { if (qd_link->direction == QD_OUTGOING) { - qd_conn->n_senders--; - assert(qd_conn->n_senders >= 0); - } else { qd_conn->n_receivers--; assert(qd_conn->n_receivers >= 0); + } else { + qd_conn->n_senders--; + assert(qd_conn->n_senders >= 0); } } qd_log(container->log_source, QD_LOG_NOTICE, @@ -454,7 +454,7 @@ int pn_event_handler(void *handler_context, void *conn_context, pn_event_t *even if (!qd_policy_approve_amqp_receiver_link(pn_link, qd_conn)) { break; } - qd_conn->n_senders++; + qd_conn->n_receivers++; } setup_outgoing_link(container, pn_link); } else { @@ -462,7 +462,7 @@ int pn_event_handler(void *handler_context, void *conn_context, pn_event_t *even if (!qd_policy_approve_amqp_sender_link(pn_link, qd_conn)) { break; } - qd_conn->n_receivers++; + qd_conn->n_senders++; } setup_incoming_link(container, pn_link); } @@ -480,22 +480,22 @@ int pn_event_handler(void *handler_context, void *conn_context, pn_event_t *even if (node) node->ntype->link_detach_handler(node->context, qd_link, dt); else if (qd_link->pn_link == pn_link) { - - // - // This policy stuff is in the wrong place. This else clause typically does not run. - // - if (qd_conn->policy_settings) { - if (pn_link_is_sender(pn_link)) { - qd_conn->n_senders--; - assert (qd_conn->n_senders >= 0); - } else { - qd_conn->n_receivers--; - assert (qd_conn->n_receivers >= 0); - } + pn_link_close(pn_link); + } + if (qd_conn->policy_counted && qd_conn->policy_settings) { + if (pn_link_is_sender(pn_link)) { + qd_conn->n_receivers--; + qd_log(container->log_source, QD_LOG_TRACE, + "Closed receiver link %s. n_receivers: %d", + pn_link_name(pn_link), qd_conn->n_receivers); + assert (qd_conn->n_receivers >= 0); } else { - // no policy - links not counted + qd_conn->n_senders--; + qd_log(container->log_source, QD_LOG_TRACE, + "Closed sender link %s. n_senders: %d", + pn_link_name(pn_link), qd_conn->n_senders); + assert (qd_conn->n_senders >= 0); } - pn_link_close(pn_link); } if (qd_link->close_sess_with_link && qd_link->pn_sess && pn_link_state(pn_link) == (PN_LOCAL_CLOSED | PN_REMOTE_CLOSED)) http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/15de0870/src/policy.c ---------------------------------------------------------------------- diff --git a/src/policy.c b/src/policy.c index df6ddd4..4d63753 100644 --- a/src/policy.c +++ b/src/policy.c @@ -254,7 +254,8 @@ void qd_policy_socket_close(void *context, const qd_connection_t *conn) } if (policy->max_connection_limit > 0) { const char *hostname = qdpn_connector_name(conn->pn_cxtr); - qd_log(policy->log_source, QD_LOG_DEBUG, "Connection '%s' closed. N connections=%d", hostname, n_connections); + qd_log(policy->log_source, QD_LOG_DEBUG, "Connection '%s' closed with resources n_sessions=%d, n_senders=%d, n_receivers=%d. Total connections=%d.", + hostname, conn->n_sessions, conn->n_senders, conn->n_receivers, n_connections); } } @@ -426,6 +427,7 @@ bool qd_policy_approve_amqp_session(pn_session_t *ssn, qd_connection_t *qd_conn) } } } + // Approved return true; } @@ -603,7 +605,7 @@ bool qd_policy_approve_amqp_sender_link(pn_link_t *pn_link, qd_connection_t *qd_ } else { // max sender limit not specified } - // Deny sender link based on target + // Approve sender link based on target const char * target = pn_terminus_get_address(pn_link_remote_target(pn_link)); bool lookup; if (target && *target) { @@ -623,13 +625,14 @@ bool qd_policy_approve_amqp_sender_link(pn_link_t *pn_link, qd_connection_t *qd_ // This happens all the time with anonymous relay lookup = qd_conn->policy_settings->allowAnonymousSender; qd_log(qd_conn->server->qd->policy->log_source, QD_LOG_TRACE, - "Approve anonymous sender for user '%s': %s", - qd_conn->user_id, (lookup ? "ALLOW" : "DENY")); + "Approve anonymous relay sender link for user '%s': %s", + qd_conn->user_id, (lookup ? "ALLOW" : "DENY")); if (!lookup) { _qd_policy_deny_amqp_receiver_link(pn_link, qd_conn); return false; } } + // Approved return true; } @@ -647,7 +650,7 @@ bool qd_policy_approve_amqp_receiver_link(pn_link_t *pn_link, qd_connection_t *q } else { // max receiver limit not specified } - // Deny receiver link based on source + // Approve receiver link based on source bool dynamic_src = pn_terminus_is_dynamic(pn_link_remote_source(pn_link)); if (dynamic_src) { bool lookup = qd_conn->policy_settings->allowDynamicSrc; @@ -677,11 +680,12 @@ bool qd_policy_approve_amqp_receiver_link(pn_link_t *pn_link, qd_connection_t *q // A receiver with no remote source. qd_log(qd_conn->server->qd->policy->log_source, QD_LOG_TRACE, "Approve receiver link '' for user '%s': DENY", - qd_conn->user_id); + qd_conn->user_id); _qd_policy_deny_amqp_receiver_link(pn_link, qd_conn); return false; } + // Approved return true; } http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/15de0870/tests/CMakeLists.txt ---------------------------------------------------------------------- diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 123884e..d4c55c3 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -106,6 +106,7 @@ file(COPY ${CMAKE_CURRENT_SOURCE_DIR}/policy-1/management-access.json DESTINATI file(COPY ${CMAKE_CURRENT_SOURCE_DIR}/policy-1/policy-boardwalk.json DESTINATION ${CMAKE_CURRENT_BINARY_DIR}/policy-1/) file(COPY ${CMAKE_CURRENT_SOURCE_DIR}/policy-1/policy-safari.json DESTINATION ${CMAKE_CURRENT_BINARY_DIR}/policy-1/) file(COPY ${CMAKE_CURRENT_SOURCE_DIR}/policy-2/policy-photoserver-sasl.sasldb DESTINATION ${CMAKE_CURRENT_BINARY_DIR}/policy-2) +file(COPY ${CMAKE_CURRENT_SOURCE_DIR}/policy-3/test-sender-receiver-limits.json DESTINATION ${CMAKE_CURRENT_BINARY_DIR}/policy-3) # following install() functions will be called only if you do a make "install" install(FILES ${SYSTEM_TEST_FILES} http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/15de0870/tests/policy-3/test-sender-receiver-limits.json ---------------------------------------------------------------------- diff --git a/tests/policy-3/test-sender-receiver-limits.json b/tests/policy-3/test-sender-receiver-limits.json new file mode 100644 index 0000000..2a5b367 --- /dev/null +++ b/tests/policy-3/test-sender-receiver-limits.json @@ -0,0 +1,26 @@ +[ +# Ruleset with differing number of senders and receivers +# so tests can determine that correct limit is matched. + ["policyRuleset", { + "applicationName": "0.0.0.0", + "maxConnections": 50, + "maxConnPerUser": 2, + "maxConnPerHost": 4, + "connectionAllowDefault": true, + "settings": { + "default" : { + "maxFrameSize": 222222, + "maxMessageSize": 222222, + "maxSessionWindow": 222222, + "maxSessions": 2, + "maxSenders": 2, + "maxReceivers": 4, + "allowDynamicSrc": true, + "allowAnonymousSender": true, + "sources": "*", + "targets": "*" + } + } + } + ] +] http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/15de0870/tests/system_tests_policy.py ---------------------------------------------------------------------- diff --git a/tests/system_tests_policy.py b/tests/system_tests_policy.py index e040a61..176d3c1 100644 --- a/tests/system_tests_policy.py +++ b/tests/system_tests_policy.py @@ -109,5 +109,94 @@ class LoadPolicyFromFolder(TestCase): rulesets = json.loads(self.run_qdmanage('query --type=policyRuleset')) self.assertEqual(len(rulesets), 3) +class SenderReceiverLimits(TestCase): + """ + Verify that specifying a policy folder from the router conf file + effects loading the policies in that folder. + This test relies on qdmanage utility. + """ + @classmethod + def setUpClass(cls): + """Start the router""" + super(SenderReceiverLimits, cls).setUpClass() + policy_config_path = os.path.join(cls.top_dir, 'policy-3') + config = Qdrouterd.Config([ + ('container', {'workerThreads': 4, 'containerName': 'Qpid.Dispatch.Router.Policy3'}), + ('router', {'mode': 'standalone', 'routerId': 'QDR.Policy'}), + ('listener', {'port': cls.tester.get_port()}), + ('policy', {'maximumConnections': 2, 'policyFolder': policy_config_path, 'enableAccessRules': 'true'}) + ]) + + cls.router = cls.tester.qdrouterd('SenderReceiverLimits', config, wait=True) + + def address(self): + return self.router.addresses[0] + + def test_verify_n_receivers(self): + n = 4 + addr = self.address() + + # connection should be ok + denied = False + try: + br1 = BlockingConnection(addr) + except ConnectionException: + denied = True + + self.assertFalse(denied) # assert if connections that should open did not open + + # n receivers OK + try: + r1 = br1.create_receiver(address="****YES_1of4***") + r2 = br1.create_receiver(address="****YES_20f4****") + r3 = br1.create_receiver(address="****YES_3of4****") + r4 = br1.create_receiver(address="****YES_4of4****") + except Exception: + denied = True + + self.assertFalse(denied) # n receivers should have worked + + # receiver n+1 should be denied + try: + r5 = br1.create_receiver("****NO****") + except Exception: + denied = True + + self.assertTrue(denied) # receiver n+1 should have failed + + br1.close() + + def test_verify_n_senders(self): + n = 2 + addr = self.address() + + # connection should be ok + denied = False + try: + bs1 = BlockingConnection(addr) + except ConnectionException: + denied = True + + self.assertFalse(denied) # assert if connections that should open did not open + + # n senders OK + try: + s1 = bs1.create_sender(address="****YES_1of2****") + s2 = bs1.create_sender(address="****YES_2of2****") + except Exception: + denied = True + + self.assertFalse(denied) # n senders should have worked + + # receiver n+1 should be denied + try: + s3 = bs1.create_sender("****NO****") + except Exception: + denied = True + + self.assertTrue(denied) # sender n+1 should have failed + + bs1.close() + if __name__ == '__main__': unittest.main(main_module()) --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org For additional commands, e-mail: commits-help@qpid.apache.org