If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

PulseGuardian: Don't limit "usernames" to alphanumeric values

RESOLVED FIXED

Status

Webtools
Pulse
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: jonasfj, Assigned: mcote)

Tracking

Trunk
x86_64
Linux
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

3 years ago
I fully understand how we cannot allow slashes in pulse usernames, as this conflicts with the permission model (well, we could but it would be complicated to implement).

But restricting usernames to alphanumeric usernames is too strict. Usernames are not just used for consumers, but also appear in exchange names that are shared between users.
So having some form of separator is useful.

From testing with "rabbitmqctl" it seems RabbitMQ is somewhat liberal about what you can name your users. But I was unable to call "list_users" after adding the user: "#¤=)(&%~><1 -.?+@£$½", he he :)
So maybe it makes sense to be a little restrictive, nevertheless I had no problems creating and using a user called "my_user-test.net".
So underscore, dash and dot seems to be safe. They are conveniently also the most useful separators.

Perhaps usernames should just match /^[a-Z][a-Z0-9._-]*$/ as is a common requirement for identifiers in various places.

Examples of usernames I want to use:
  taskcluster-queue            (publish only)
  taskcluster-scheduler        (publish and consume)
  taskcluster-auth             (future possible use, probably publish)
  taskcluster-events           (consume only)
  taskcluster-index            (consume only)
  taskcluster-aws-provisioner  (future use planned)
  jonasfj-garbage              (for trashing when I'm debugging stuff)
  taskclusterdev-<names>       (equivalent set of users, for automated tests and devs)
(I'm sure we'll introduce additional components as we move forward).

As pulse usernames is also exchange namespaces/prefixes ("exchange/<username>/..."), one cannot easily change usernames. And it thus, seems wise to pick a reasonably sane pattern for usernames before starting to create a lot of them :)

Note, having a user for each taskcluster component provides some really nice isolation and security. And it allows us to be less paranoid about security during the initial development and deployment of a new component.

It'll cost a bit of configuration overhead for us, but long term I'm sure the isolation will serve us well. For example taskcluster-events will create an exclusive non-durable auto-delete queue on RabbitMQ whenever a user opens a websocket to listen for messages.
So it'll potentially have hundreds, maybe even a few thousand queues; either way, it's definitely something we want to isolate.
(Reporter)

Updated

3 years ago
Blocks: 1065248
(Assignee)

Updated

3 years ago
Assignee: nobody → mcote
Status: NEW → ASSIGNED
(Assignee)

Comment 1

3 years ago
https://github.com/mozilla/pulseguardian/commit/a1673572192526ba651401a4537e1c2d6e5cab97

I'll arrange to get that deployed shortly (I'll set this as a blocker so you can track the deployment).  Note that I have to now use re.escape() on the username, so, a user named "a.b-c_d" would get permissions like "queue/a\.b\-c\_d/*" in Rabbit (the escaping seems a little gratuitous but appears to work).
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
(Assignee)

Updated

3 years ago
Blocks: 1080726
You need to log in before you can comment on or make changes to this bug.