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

binder: Cached PERMISSION_DENIED results from SecurityPolicies.hasPermission() can be confusing #10950

Open
jdcormie opened this issue Feb 23, 2024 · 0 comments

Comments

@jdcormie
Copy link
Member

jdcormie commented Feb 23, 2024

Context: The gRPC framework assumes that a SecurityPolicys verdict for a given peer UID will not change over the lifetime of any process with that UID. But Android runtime permissions can be granted or revoked by the user at any time so the hasPermissions() policy has the confusing behavior that calls may still fail even after the client requests and obtains the required permission. And on the flip side, clients can only safely require that servers hold install-time permissions which cannot change.

The official Android advice is pretty clear that apps should check/request/obtain before attempting a permission controlled operation. So while I think the current behavior is technically fine, developers would be happier if our access control decisions reflected the current state of all grants.

Idea 1: keep the cache but listen for grant changes and invalidate. I'm not aware of an Android API that lets you get notified when a permission is granted to another app. So this may be infeasible.

Idea 2: don't cache SecurityPolicy outcomes that depend on grants. hasPermissions() costs two round trips to system_server. Paying that once per transport setup may not be that expensive compared to everything else going on.

Idea 3: stop caching negative SecurityPolicy outcomes. If devs are following the check/request/obtain rules, PERMISSION_DENIED should only happen during development when performance doesn't really matter. Repeated successful transport setup results in cache hits and remains just as fast as today.

(From b/326200530)

@sergiitk sergiitk added this to the Next milestone Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants