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

[Bug] Data race in RoleManagerImpl.HasLink() breaks Enforcer when calling Enforce() #1318

Open
tomasger opened this issue Sep 22, 2023 · 1 comment

Comments

@tomasger
Copy link

Describe the bug

Setup:

  • Initialized Casbin Enforcer or Synced Casbin Enforcer
  • AddNamedMatchingFunc() used (e.g. keyMatch2, keyMatch4)

The bug is reproduced when calling Enforce() with the same parameters concurrently.

It boils down to this path being executed concurrently:
GenerateGFunction()->HasLink()->defer rm.removeRole().

Race explanation inside HasLink() function with goroutines A and B:

  • Enforce A: Enter HasLink(). Role does not exist, is added to map temporarily && defer rm.removeRole() called
  • Enforce A: hasLinkHelper iterates over roles via sync.Map.Range() recursively
  • Enforce B: Temp role exists, fetch it.
  • Enforce A: Exit function, removeRole() called.
  • Enforce B: hasLinkHelper iterates over roles but the temp role no longer exists
  • Enforce A: allow: true
  • Enforce B: allow: false

Furthermore once this happens, the calculation is cached in GenerateGFunction and also in getAndStoreMatcherExpression(), all further Enforce() calls for that set of parameters will incorrectly return a cached false response.

To Reproduce

This behaviour does not occur all the time, but by disabling cache inside GenerateGFunction it is possible to reproduce it reliably:

  1. Clone https://github.com/Metrika-Inc/casbin-race/
  2. Run the test: make test
    • rerun if not reproduced on first time
  3. Observe the panic due to Enforce() returning false in the test

Check README in the Metrika-Inc/casbin-race repository for more details.

Expected behavior

Enforcer always returns true when correct policies are in place.

Actual behavior

Enforcer sometimes returns false even when correct policies are in place and it may cache the result to continue returning false incorrectly.

Environment

  • OS: macOS and Linux

Possible solutions

It seems that feasible solutions may be having a mutex on HasLink() or not storing temporary roles in a shared map and having a temporary map defined inside the scope of the function.

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

No branches or pull requests

3 participants