Login
[x]
Log in using an account from:
Fedora Account System
Red Hat Associate
Red Hat Customer
Or login using a Red Hat Bugzilla account
Forgot Password
Login:
Hide Forgot
Create an Account
Red Hat Bugzilla – Attachment 902128 Details for
Bug 883866
[RFE]: Access control for QMF functionality should be improved
[?]
New
Simple Search
Advanced Search
My Links
Browse
Requests
Reports
Current State
Search
Tabular reports
Graphical reports
Duplicates
Other Reports
User Changes
Plotly Reports
Bug Status
Bug Severity
Non-Defaults
|
Product Dashboard
Help
Page Help!
Bug Writing Guidelines
What's new
Browser Support Policy
5.0.4.rh83 Release notes
FAQ
Guides index
User guide
Web Services
Contact
Legal
This site requires JavaScript to be enabled to function correctly, please enable it.
[patch]
Patch proposal
bz883866.patch (text/plain), 21.51 KB, created by
Pavel Moravec
on 2014-06-04 10:27:15 UTC
(
hide
)
Description:
Patch proposal
Filename:
MIME Type:
Creator:
Pavel Moravec
Created:
2014-06-04 10:27:15 UTC
Size:
21.51 KB
patch
obsolete
>Index: cpp/src/qpid/broker/Queue.cpp >=================================================================== >--- cpp/src/qpid/broker/Queue.cpp (revision 1597555) >+++ cpp/src/qpid/broker/Queue.cpp (working copy) >@@ -21,6 +21,7 @@ > > #include "qpid/broker/Queue.h" > #include "qpid/broker/Broker.h" >+#include "qpid/broker/Connection.h" > #include "qpid/broker/AclModule.h" > #include "qpid/broker/QueueCursor.h" > #include "qpid/broker/QueueDepth.h" >@@ -73,6 +74,7 @@ > using qpid::management::ManagementObject; > using qpid::management::Manageable; > using qpid::management::Args; >+using qpid::management::getCurrentPublisher; > using std::string; > using std::for_each; > using std::mem_fun; >@@ -1392,12 +1394,17 @@ > Manageable::status_t Queue::ManagementMethod (uint32_t methodId, Args& args, string& etext) > { > Manageable::status_t status = Manageable::STATUS_UNKNOWN_METHOD; >+ AclModule* acl = broker->getAcl(); >+ std::string _userId = (getCurrentPublisher()?getCurrentPublisher()->getUserId():""); > > QPID_LOG (debug, "Queue::ManagementMethod [id=" << methodId << "]"); > > switch (methodId) { > case _qmf::Queue::METHOD_PURGE : > { >+ if ((acl)&&(!(acl->authorise(_userId, acl::ACT_PURGE, acl::OBJ_QUEUE, name, NULL)))) { >+ throw framing::UnauthorizedAccessException(QPID_MSG("ACL denied purge request from " << _userId)); >+ } > _qmf::ArgsQueuePurge& purgeArgs = (_qmf::ArgsQueuePurge&) args; > purge(purgeArgs.i_request, boost::shared_ptr<Exchange>(), &purgeArgs.i_filter); > status = Manageable::STATUS_OK; >@@ -1425,6 +1432,14 @@ > } > } > >+ if (acl) { >+ std::map<acl::Property, std::string> params; >+ params.insert(make_pair(acl::PROP_EXCHANGENAME, dest->getName())); >+ if (!acl->authorise(_userId, acl::ACT_REROUTE, acl::OBJ_QUEUE, name, ¶ms)) { >+ throw framing::UnauthorizedAccessException(QPID_MSG("ACL denied reroute request from " << _userId)); >+ } >+ } >+ > purge(rerouteArgs.i_request, dest, &rerouteArgs.i_filter); > status = Manageable::STATUS_OK; > } >Index: cpp/src/qpid/broker/Broker.h >=================================================================== >--- cpp/src/qpid/broker/Broker.h (revision 1597555) >+++ cpp/src/qpid/broker/Broker.h (working copy) >@@ -169,7 +169,7 @@ > const Connection* context); > Manageable::status_t setTimestampConfig(const bool receive, > const Connection* context); >- Manageable::status_t queueRedirect(const std::string& srcQueue, const std::string& tgtQueue); >+ Manageable::status_t queueRedirect(const std::string& srcQueue, const std::string& tgtQueue, const Connection* context); > void queueRedirectDestroy(boost::shared_ptr<Queue> srcQ, boost::shared_ptr<Queue> tgtQ, bool moveMsgs); > boost::shared_ptr<sys::Poller> poller; > std::auto_ptr<sys::Timer> timer; >@@ -291,11 +291,12 @@ > Return -1 if one of the queues does not exist, otherwise > the number of messages moved. > */ >- QPID_BROKER_EXTERN int32_t queueMoveMessages( >+ QPID_BROKER_EXTERN uint32_t queueMoveMessages( > const std::string& srcQueue, > const std::string& destQueue, > uint32_t qty, >- const qpid::types::Variant::Map& filter); >+ const qpid::types::Variant::Map& filter, >+ const Connection* context); > > QPID_BROKER_EXTERN const TransportInfo& getTransportInfo( > const std::string& name = TCP_TRANSPORT) const; >Index: cpp/src/qpid/broker/Broker.cpp >=================================================================== >--- cpp/src/qpid/broker/Broker.cpp (revision 1597555) >+++ cpp/src/qpid/broker/Broker.cpp (working copy) >@@ -544,10 +544,7 @@ > _qmf::ArgsBrokerQueueMoveMessages& moveArgs= > dynamic_cast<_qmf::ArgsBrokerQueueMoveMessages&>(args); > QPID_LOG (debug, "Broker::queueMoveMessages()"); >- if (queueMoveMessages(moveArgs.i_srcQueue, moveArgs.i_destQueue, moveArgs.i_qty, moveArgs.i_filter) >= 0) >- status = Manageable::STATUS_OK; >- else >- return Manageable::STATUS_PARAMETER_INVALID; >+ status = queueMoveMessages(moveArgs.i_srcQueue, moveArgs.i_destQueue, moveArgs.i_qty, moveArgs.i_filter, getCurrentPublisher()); > break; > } > case _qmf::Broker::METHOD_SETLOGLEVEL : >@@ -612,7 +609,7 @@ > string srcQueue(dynamic_cast<_qmf::ArgsBrokerQueueRedirect&>(args).i_sourceQueue); > string tgtQueue(dynamic_cast<_qmf::ArgsBrokerQueueRedirect&>(args).i_targetQueue); > QPID_LOG (debug, "Broker::queueRedirect source queue:" << srcQueue << " to target queue " << tgtQueue); >- status = queueRedirect(srcQueue, tgtQueue); >+ status = queueRedirect(srcQueue, tgtQueue, getCurrentPublisher()); > break; > } > default: >@@ -1088,7 +1085,8 @@ > > > Manageable::status_t Broker::queueRedirect(const std::string& srcQueue, >- const std::string& tgtQueue) >+ const std::string& tgtQueue, >+ const Connection* context) > { > Queue::shared_ptr srcQ(queues.find(srcQueue)); > if (!srcQ) { >@@ -1136,6 +1134,12 @@ > return Manageable::STATUS_USER; > } > >+ if (acl) { >+ std::map<acl::Property, std::string> params; >+ params.insert(make_pair(acl::PROP_QUEUENAME, tgtQ->getName())); >+ if (!acl->authorise((context)?context->getUserId():"", acl::ACT_REDIRECT, acl::OBJ_QUEUE, srcQ->getName(), ¶ms)) >+ throw framing::UnauthorizedAccessException(QPID_MSG("ACL denied redirect request from " << ((context)?context->getUserId():"(uknown)"))); >+ } > // Start the backup overflow partnership > srcQ->setRedirectPeer(tgtQ, true); > tgtQ->setRedirectPeer(srcQ, false); >@@ -1167,6 +1171,13 @@ > return Manageable::STATUS_USER; > } > >+ if (acl) { >+ std::map<acl::Property, std::string> params; >+ params.insert(make_pair(acl::PROP_QUEUENAME, tgtQ->getName())); >+ if (!acl->authorise((context)?context->getUserId():"", acl::ACT_REDIRECT, acl::OBJ_QUEUE, srcQ->getName(), ¶ms)) >+ throw framing::UnauthorizedAccessException(QPID_MSG("ACL denied redirect request from " << ((context)?context->getUserId():"(uknown)"))); >+ } >+ > queueRedirectDestroy(srcQ, tgtQ, true); > > return Manageable::STATUS_OK; >@@ -1260,20 +1271,29 @@ > else throw NoSuchTransportException(QPID_MSG("Unsupported transport type: " << transport)); > } > >-int32_t Broker::queueMoveMessages( >+uint32_t Broker::queueMoveMessages( > const std::string& srcQueue, > const std::string& destQueue, > uint32_t qty, >- const Variant::Map& filter) >+ const Variant::Map& filter, >+ const Connection* context) > { > Queue::shared_ptr src_queue = queues.find(srcQueue); > if (!src_queue) >- return -1; >+ return Manageable::STATUS_PARAMETER_INVALID; >+ > Queue::shared_ptr dest_queue = queues.find(destQueue); > if (!dest_queue) >- return -1; >+ return Manageable::STATUS_PARAMETER_INVALID; > >- return (int32_t) src_queue->move(dest_queue, qty, &filter); >+ if (acl) { >+ std::map<acl::Property, std::string> params; >+ params.insert(make_pair(acl::PROP_QUEUENAME, dest_queue->getName())); >+ if (!acl->authorise((context)?context->getUserId():"", acl::ACT_MOVE, acl::OBJ_QUEUE, src_queue->getName(), ¶ms)) >+ throw framing::UnauthorizedAccessException(QPID_MSG("ACL denied move request from " << ((context)?context->getUserId():"(uknown)"))); >+ } >+ src_queue->move(dest_queue, qty, &filter); >+ return Manageable::STATUS_OK; > } > > >Index: cpp/src/qpid/broker/AclModule.h >=================================================================== >--- cpp/src/qpid/broker/AclModule.h (revision 1597555) >+++ cpp/src/qpid/broker/AclModule.h (working copy) >@@ -61,6 +61,9 @@ > ACT_DELETE, > ACT_PURGE, > ACT_UPDATE, >+ ACT_MOVE, >+ ACT_REDIRECT, >+ ACT_REROUTE, > ACTIONSIZE }; // ACTIONSIZE must be last in list > > // Property used in ACL authorize interface >@@ -74,6 +77,7 @@ > PROP_TYPE, > PROP_ALTERNATE, > PROP_QUEUENAME, >+ PROP_EXCHANGENAME, > PROP_SCHEMAPACKAGE, > PROP_SCHEMACLASS, > PROP_POLICYTYPE, >@@ -100,6 +104,7 @@ > SPECPROP_TYPE = PROP_TYPE, > SPECPROP_ALTERNATE = PROP_ALTERNATE, > SPECPROP_QUEUENAME = PROP_QUEUENAME, >+ SPECPROP_EXCHANGENAME = PROP_EXCHANGENAME, > SPECPROP_SCHEMAPACKAGE = PROP_SCHEMAPACKAGE, > SPECPROP_SCHEMACLASS = PROP_SCHEMACLASS, > SPECPROP_POLICYTYPE = PROP_POLICYTYPE, >@@ -200,28 +205,34 @@ > return ""; > } > static inline Action getAction(const std::string& str) { >- if (str.compare("consume") == 0) return ACT_CONSUME; >- if (str.compare("publish") == 0) return ACT_PUBLISH; >- if (str.compare("create") == 0) return ACT_CREATE; >- if (str.compare("access") == 0) return ACT_ACCESS; >- if (str.compare("bind") == 0) return ACT_BIND; >- if (str.compare("unbind") == 0) return ACT_UNBIND; >- if (str.compare("delete") == 0) return ACT_DELETE; >- if (str.compare("purge") == 0) return ACT_PURGE; >- if (str.compare("update") == 0) return ACT_UPDATE; >+ if (str.compare("consume") == 0) return ACT_CONSUME; >+ if (str.compare("publish") == 0) return ACT_PUBLISH; >+ if (str.compare("create") == 0) return ACT_CREATE; >+ if (str.compare("access") == 0) return ACT_ACCESS; >+ if (str.compare("bind") == 0) return ACT_BIND; >+ if (str.compare("unbind") == 0) return ACT_UNBIND; >+ if (str.compare("delete") == 0) return ACT_DELETE; >+ if (str.compare("purge") == 0) return ACT_PURGE; >+ if (str.compare("update") == 0) return ACT_UPDATE; >+ if (str.compare("move") == 0) return ACT_MOVE; >+ if (str.compare("redirect") == 0) return ACT_REDIRECT; >+ if (str.compare("reroute") == 0) return ACT_REROUTE; > throw qpid::Exception(str); > } > static inline std::string getActionStr(const Action a) { > switch (a) { >- case ACT_CONSUME: return "consume"; >- case ACT_PUBLISH: return "publish"; >- case ACT_CREATE: return "create"; >- case ACT_ACCESS: return "access"; >- case ACT_BIND: return "bind"; >- case ACT_UNBIND: return "unbind"; >- case ACT_DELETE: return "delete"; >- case ACT_PURGE: return "purge"; >- case ACT_UPDATE: return "update"; >+ case ACT_CONSUME: return "consume"; >+ case ACT_PUBLISH: return "publish"; >+ case ACT_CREATE: return "create"; >+ case ACT_ACCESS: return "access"; >+ case ACT_BIND: return "bind"; >+ case ACT_UNBIND: return "unbind"; >+ case ACT_DELETE: return "delete"; >+ case ACT_PURGE: return "purge"; >+ case ACT_UPDATE: return "update"; >+ case ACT_MOVE: return "move"; >+ case ACT_REDIRECT: return "redirect"; >+ case ACT_REROUTE: return "reroute"; > default: assert(false); // should never get here > } > return ""; >@@ -236,6 +247,7 @@ > if (str.compare("type") == 0) return PROP_TYPE; > if (str.compare("alternate") == 0) return PROP_ALTERNATE; > if (str.compare("queuename") == 0) return PROP_QUEUENAME; >+ if (str.compare("exchangename") == 0) return PROP_EXCHANGENAME; > if (str.compare("schemapackage") == 0) return PROP_SCHEMAPACKAGE; > if (str.compare("schemaclass") == 0) return PROP_SCHEMACLASS; > if (str.compare("policytype") == 0) return PROP_POLICYTYPE; >@@ -259,6 +271,7 @@ > case PROP_TYPE: return "type"; > case PROP_ALTERNATE: return "alternate"; > case PROP_QUEUENAME: return "queuename"; >+ case PROP_EXCHANGENAME: return "exchangename"; > case PROP_SCHEMAPACKAGE: return "schemapackage"; > case PROP_SCHEMACLASS: return "schemaclass"; > case PROP_POLICYTYPE: return "policytype"; >@@ -283,6 +296,7 @@ > if (str.compare("type") == 0) return SPECPROP_TYPE; > if (str.compare("alternate") == 0) return SPECPROP_ALTERNATE; > if (str.compare("queuename") == 0) return SPECPROP_QUEUENAME; >+ if (str.compare("exchangename") == 0) return SPECPROP_EXCHANGENAME; > if (str.compare("schemapackage") == 0) return SPECPROP_SCHEMAPACKAGE; > if (str.compare("schemaclass") == 0) return SPECPROP_SCHEMACLASS; > if (str.compare("policytype") == 0) return SPECPROP_POLICYTYPE; >@@ -315,6 +329,7 @@ > case SPECPROP_TYPE: return "type"; > case SPECPROP_ALTERNATE: return "alternate"; > case SPECPROP_QUEUENAME: return "queuename"; >+ case SPECPROP_EXCHANGENAME: return "exchangename"; > case SPECPROP_SCHEMAPACKAGE: return "schemapackage"; > case SPECPROP_SCHEMACLASS: return "schemaclass"; > case SPECPROP_POLICYTYPE: return "policytype"; >@@ -413,12 +428,22 @@ > p4->insert(PROP_MAXQUEUESIZE); > p4->insert(PROP_MAXQUEUECOUNT); > >+ propSetPtr p5(new propSet); >+ p5->insert(PROP_QUEUENAME); >+ >+ propSetPtr p6(new propSet); >+ p6->insert(PROP_EXCHANGENAME); >+ >+ > actionMapPtr a1(new actionMap); >- a1->insert(actionPair(ACT_ACCESS, p0)); >- a1->insert(actionPair(ACT_CREATE, p4)); >- a1->insert(actionPair(ACT_PURGE, p0)); >- a1->insert(actionPair(ACT_DELETE, p0)); >- a1->insert(actionPair(ACT_CONSUME, p0)); >+ a1->insert(actionPair(ACT_ACCESS, p0)); >+ a1->insert(actionPair(ACT_CREATE, p4)); >+ a1->insert(actionPair(ACT_PURGE, p0)); >+ a1->insert(actionPair(ACT_DELETE, p0)); >+ a1->insert(actionPair(ACT_CONSUME, p0)); >+ a1->insert(actionPair(ACT_MOVE, p5)); >+ a1->insert(actionPair(ACT_REDIRECT, p5)); >+ a1->insert(actionPair(ACT_REROUTE, p6)); > > map->insert(objectPair(OBJ_QUEUE, a1)); > >@@ -431,12 +456,12 @@ > > // == Method == > >- propSetPtr p5(new propSet); >- p5->insert(PROP_SCHEMAPACKAGE); >- p5->insert(PROP_SCHEMACLASS); >+ propSetPtr p7(new propSet); >+ p7->insert(PROP_SCHEMAPACKAGE); >+ p7->insert(PROP_SCHEMACLASS); > > actionMapPtr a4(new actionMap); >- a4->insert(actionPair(ACT_ACCESS, p5)); >+ a4->insert(actionPair(ACT_ACCESS, p7)); > > map->insert(objectPair(OBJ_METHOD, a4)); > } >Index: cpp/src/tests/acl.py >=================================================================== >--- cpp/src/tests/acl.py (revision 1597555) >+++ cpp/src/tests/acl.py (working copy) >@@ -27,7 +27,9 @@ > from qmf.console import Session > from qpid.datatypes import Message > import qpid.messaging >+from qpidtoollibs import BrokerAgent > >+ > class ACLFile: > def __init__(self, policy='data_dir/policy.acl'): > self.f = open(policy,'w') >@@ -40,6 +42,14 @@ > > class ACLTests(TestBase010): > >+ # required for testing QMF methods >+ def get_messaging_connection(self, user, passwd): >+ parms = {'username':user, 'password':passwd, 'sasl_mechanisms':'PLAIN'} >+ brokerurl="%s:%s" %(self.broker.host, self.broker.port) >+ connection = qpid.messaging.Connection(brokerurl, **parms) >+ connection.open() >+ return connection >+ > # For connection limit tests this function > # throws if the connection won't start > # returns a connection that the caller can close if he likes. >@@ -796,10 +806,16 @@ > aclf.write('acl deny bob@QPID create queue name=q1 durable=true\n') > aclf.write('acl deny bob@QPID create queue name=q2 exclusive=true policytype=ring\n') > aclf.write('acl deny bob@QPID access queue name=q3\n') >- aclf.write('acl deny bob@QPID purge queue name=q3\n') > aclf.write('acl deny bob@QPID delete queue name=q4\n') > aclf.write('acl deny bob@QPID create queue name=q5 maxqueuesize=1000 maxqueuecount=100\n') > aclf.write('acl deny bob@QPID create queue name=q6 paging=true\n') >+ aclf.write('acl deny bob@QPID purge queue name=q7\n') >+ aclf.write('acl deny all move queue name=q7\n') >+ aclf.write('acl deny all move queue name=q8 queuename=q7\n') >+ aclf.write('acl deny all redirect queue name=q7\n') >+ aclf.write('acl deny all redirect queue name=q8 queuename=q7\n') >+ aclf.write('acl deny all reroute queue name=q7\n') >+ aclf.write('acl deny all reroute queue name=q8 exchangename=amq.fanout\n') > aclf.write('acl allow all all') > aclf.close() > >@@ -881,20 +897,92 @@ > self.assertEqual(403,e.args[0].error_code) > session = self.get_session('bob','bob') > >+ # some queues needs to be created for testing purge / move / reroute / redirect >+ session.queue_declare(queue="q7") >+ session.queue_declare(queue="q8") >+ session.queue_declare(queue="q9") > try: >- session.queue_purge(queue="q3") >- self.fail("ACL should deny queue purge request for q3"); >+ session.queue_purge(queue="q7") >+ self.fail("ACL should deny queue purge request for q7"); > except qpid.session.SessionException, e: > self.assertEqual(403,e.args[0].error_code) > session = self.get_session('bob','bob') > > try: >- session.queue_purge(queue="q4") >+ session.queue_purge(queue="q8") > except qpid.session.SessionException, e: > if (403 == e.args[0].error_code): >- self.fail("ACL should allow queue purge request for q4"); >+ self.fail("ACL should allow queue purge request for q8"); > >+ # as we use QMF methods, it is easier to use BrokerAgent from messaging.connection and not use session object as above >+ broker_agent = BrokerAgent(self.get_messaging_connection('bob','bob')) >+ > try: >+ broker_agent.queueMoveMessages("q7", "q8", 0) >+ self.fail("ACL should deny queue move request from q7 to q8"); >+ except Exception, e: >+ print "type:%s: args[0]: %s, args: %s" %(type(e).__name__, e.args[0], e.args) >+ self.assertTrue("'error_code': 7," in e.args[0]) >+ broker_agent = BrokerAgent(self.get_messaging_connection('bob','bob')) >+ >+ try: >+ broker_agent.queueMoveMessages("q8", "q9", 0) >+ except Exception, e: >+ if ("'error_code': 7," in e.args[0]): >+ self.fail("ACL should allow queue move request from q8 to q9"); >+ >+ try: >+ broker_agent.queueMoveMessages("q9", "q8", 0) >+ except Exception, e: >+ if ("'error_code': 7," in e.args[0]): >+ self.fail("ACL should allow queue move request from q9 to q8"); >+ >+ try: >+ broker_agent.Redirect("q7", "q8") >+ self.fail("ACL should deny queue redirect request from q7 to q8"); >+ except Exception, e: >+ self.assertTrue("'error_code': 7," in e.args[0]) >+ broker_agent = BrokerAgent(self.get_messaging_connection('bob','bob')) >+ >+ try: >+ broker_agent.Redirect("q8", "q9") >+ except Exception, e: >+ if ("'error_code': 7," in e.args[0]): >+ self.fail("ACL should allow queue redirect request from q8 to q9"); >+ >+ try: >+ broker_agent.Redirect("q9", "q8") >+ except Exception, e: >+ if ("'error_code': 7," in e.args[0]): >+ self.fail("ACL should allow queue redirect request from q9 to q8"); >+ >+ try: >+ broker_agent.getQueue('q7').reroute(0, False, "amq.fanout") >+ self.fail("ACL should deny queue reroute request from q7 to amq.fanout"); >+ except Exception, e: >+ self.assertTrue("'error_code': 7," in e.args[0]) >+ broker_agent = BrokerAgent(self.get_messaging_connection('bob','bob')) >+ >+ try: >+ broker_agent.getQueue('q8').reroute(0, False, "amq.fanout") >+ self.fail("ACL should deny queue reroute request from q8 to amq.fanout"); >+ except Exception, e: >+ self.assertTrue("'error_code': 7," in e.args[0]) >+ broker_agent = BrokerAgent(self.get_messaging_connection('bob','bob')) >+ >+ try: >+ broker_agent.getQueue('q8').reroute(0, False, "amq.direct") >+ except Exception, e: >+ if ("'error_code': 7," in e.args[0]): >+ self.fail("ACL should allow queue reroute request from q8 to amq.direct"); >+ >+ try: >+ broker_agent.getQueue('q9').reroute(0, False, "amq.fanout") >+ except Exception, e: >+ if ("'error_code': 7," in e.args[0]): >+ self.fail("ACL should allow queue reroute request from q9 to amq.fanout"); >+ >+ try: > session.queue_delete(queue="q4") > self.fail("ACL should deny queue delete request for q4"); > except qpid.session.SessionException, e:
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
Attachments on
bug 883866
:
899491
| 902128