Open reidsunderland opened 5 months ago
The 'l' in all the patterns are a long gone transition thing... if we are changing them the patterns should just be [rs]. ( in about 2017 we decided to rename the telemetry messages from "log" messages to "report" messages, and the xl and xr messages were meant to be publication channels for telemetry messages returned by consumers.
If we are looking at this. I notice a weakness in all the patterns based on usernames that are substrings of other user names. If we have user A and user AB, then user A will be able to use any of user AB eschanges. I think the patterns should likely be changed to add a trailing underscore.... qUSERNAME. ... x[rs]USERNAME. ... etc... This may involve some incompatibility with existing deplorements.
I just re-read the relevant documentation with @mshak2 and this intent here looks correct... the corresponding documentaiton is wrong and needs to be updated also.
Role can be one of:
subscriber A subscriber is a user that can only subscribe to data and report messages. Not permitted to inject data. Each subscriber gets an xs_
named exchange on the pump. If a user is named Acme, the corresponding exchange will be xs_Acme. This exchange is where an sr_subscribe process will send its report messages. By convention/default, the anonymous user is created on all pumps to permit subscription without a specific account.
should likely be xr_
discussion:
observations:
If reports are posted to xs_user, then subscriber and source permissions differ only in the terminal $. Is that bad?
data messages and report messages are posted to the same exchange. Is that bad?
question raised: why bother with reports at all? A recurring user complaint is routing is opaque. With fully implemented reports, the entire routing process, and all subscribers will be fully visible to data producers. Not just initial data injection, but each hop through the data pumps, and their consumption by end users. *Users have asked for this repeatedly over time, and still complain about it with current deployments (because reports are not implemented.)
Assumptions:
Options:
if you have a user MSC, and a second user MSC-DEV.... MSC has access to all queues of MSC-DEV because MSC is a substring of MSC-DEV.
for the username security/confusion...
old: ^q_USERNAME. new: ^qUSERNAME[\.].
Add a trailing . or underscore... to username to match defaults... @junhu3 points out that this vulnerability is actually added flexibility to define hierarchical groups of users. Admins should just ensure that usernames are never substrings of eachother.
@reidsunderland points out that a part of the motivation for analysts to avoid using declare is that some hand-crafted permissions are present on some brokers that cannot be reproduced using the tool. He suggests a configuration option to override whatever defaults we agree on. Me guessing at a format:
declare subscriber hoho { "read": "^q_AVALANCHE.*|^xs_AVALANCHE.*" , "write": "^q_AVALANCHE.*|^xs_AVALANCHE.*", "configure": "^q_AVALANCHE.*|^xs_AVALANCHE.*" }
alternate (more using... rabbitmq syntax:
declare user { "user": "AVALANCHE", "vhost":"/", .... "role": "source" ...
-- if "role" is missing, default to "subscriber".
@reidsunderland will check if the syntax used by rabbitmq gui matches this? or maybe we use whatever it uses...
wondering if monitoring perms could be delegated somehow...
note: for vhost... that is controlled by the "admin" setting. it sets the broker-url we are administering. sr3 does not support multiple brokers/vhosts being administered in a single account/config tree. You can connect to other brokers, but only admin one broker or cluster in one tree. having admin "amqps://admin@localhost/my_vhost" would make all the users belong in "my_vhost" in theory... provided the vhost support is working still (implemented in #384 ... but no unit testing and no usage, so could have broken in the mean time.)
v
The RabbitMQ permissions set by sr3:
https://github.com/MetPX/sarracenia/blob/68394078a4dca11eda7e13a4a304e81bf0883319/sarracenia/rabbitmq_admin.py#L106-L127
seem a little bit wrong, after discussing with @petersilva, @andreleblanc11 and @junhu3.
^q_USERNAME.*\|^xs_USERNAME.*
^q_USERNAME.*\|^xs_USERNAME.*
^q_USERNAME.*\|^x[lrs]_USERNAME.*\|^x.*public$
^q_USERNAME.*
^q_USERNAME.*\|^xs_USERNAME$
^q_USERNAME.*\|^x[lrs]_USERNAME.*\|^x.*public$
History: the
l
inx[lrs]
was forlog
, which was renamed toreport
. We keep it for backwards compatibility.I think the correct permissions should probably be:
^q_USERNAME.*\|^x[lrs]_USERNAME.*
^q_USERNAME.*\|^x[lrs]_USERNAME.*
^q_USERNAME.*\|^x[lrs]_USERNAME.*\|^x.*public$
^q_USERNAME.*\|^x[lr]_USERNAME$
^q_USERNAME.*\|^x[lr]_USERNAME$
^q_USERNAME.*\|^x[lrs]_USERNAME.*\|^x.*public$