Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Verify acl the right way #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

soundstorm
Copy link

Somehow MongoDB was fixed but other implementations not. Cost me a few hours to figure out.
The plugin is still deprecated but as I had a existing database which needed to be migrated, I'm stuck with it as the go plugin is currently not compilable on the compat-branch.

mosquitto_topic_matches_sub is not meant for ACL checking, so leads to errors.
@soundstorm
Copy link
Author

soundstorm commented Mar 30, 2020

I've also adjusted (for myself) be-mysql.c:341 to sprintf(query, conf->aclquery, u, acc, acc); and the aclquery to SELECT topic FROM acls WHERE username = '%s' AND (rw & %d) = %d while updating all rw values with +4 for compatibility with MOSQ_ACL_SUBSCRIBE

@kmihaylov
Copy link

@soundstorm thanks for this PR. I just saw somewhere that the other implementations were left to be fixed later. Would you please tell me how the ACL values are changed due to the latest fixes? It seems that they were not so minor as I thought, both im mosquitto_auth_plug and in mosquitto itself.
ACL values were 1 for read-only and 2 for read-write access.
Thank you!

@dmsherazi
Copy link

I've also adjusted (for myself) be-mysql.c:341 to sprintf(query, conf->aclquery, u, acc, acc); and the aclquery to SELECT topic FROM acls WHERE username = '%s' AND (rw & %d) = %d while updating all rw values with +4 for compatibility with MOSQ_ACL_SUBSCRIBE

I am having a situation as you had, where my existing setup is with vresion 1.14.7 for mosquitto and was using jpmens/mosquitto-auth-plug plugin. I have to update the server and in order to be safe for future I would like to shift to mosquitto 2.x and the go-auth but I guess that will have a lot of changes required.
So my mid step would be to update to 1.6.X and use oen of forks pmens/mosquitto-auth-plug that is updated.

Do I need to change the plugin acl checks to return true for all acl chhecks where acl check for 1 or 2 is valid or should I change the db column rw as you suggested. Replacing 1 with 5 and 2 with 6 or 7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants