2

In the onBind() method of a service I'd like to check whether the caller has a particular permission. For that I need to find the identity of the caller.

I expected the following code to return the caller package name but instead I get the package name of the service. What am I doing wrong?

Binder.getCallingUid()
String pkg = getPackageManager().getNameForUid(uid);
Daniel
  • 2,380
  • 29
  • 44
  • 1
    are you aware that `onBind` is called once (per service) and not during `Binder` transaction? you need to call `Binder.getCallingUid()` during `Binder` transaction, not inside `onBind`, the docs say: '''Return the Linux uid assigned to the process that sent you the current transaction that is being processed. This uid can be used with higher-level system services to determine its identity and check permissions. **If the current thread is not currently executing an incoming transaction, then its own uid is returned.**''' – pskink Feb 10 '16 at 16:27
  • "are you aware that onBind is called once (per service) and not during Binder transaction?" -- I thought it was called once per client. My idea was to simply return `null` when a client does not hold the expected permission. – Daniel Feb 10 '16 at 17:01
  • 1
    you need to do that in `Binder#onTransact` method, this is a good place for any remote checks – pskink Feb 10 '16 at 17:58
  • The description of `Binder#onTransact` mentions unmarshalling transactions. Is it safe to use it just for checking permissions and returning false when the permissions are not okay, and returning the default implementation when all is well? – Daniel Feb 11 '16 at 15:36
  • i think so, but the docs do not say what is returned boolean for (try to return `false` and see what happens) or you would try to throw `RemoteException` saying "dear client, nonono, you have no permissions to call me" – pskink Feb 11 '16 at 16:03
  • I did try it. When I return `false` from `onTransact`, the function call on the client returns `null`. – Daniel Feb 11 '16 at 16:24
  • how do you call from the client? using AIDL or just calling `IBinder#transact` ? – pskink Feb 11 '16 at 16:27
  • "how do you call from the client?" --- Using AIDL, I don't call `transact()` directly. I checked the source code: the boolean from `onTransact` is returned by `transact` on the client. I haven't checked it, but I believe that when the boolean is false the client AIDL function call always returns null. – Daniel Feb 11 '16 at 16:34
  • so actually i dont understand: if you see AIDL generated sources `mRemote.transact` is called but its return boolean is ignored, so how can you get it? – pskink Feb 11 '16 at 17:18
  • "if you see AIDL generated sources mRemote.transact is called but its return boolean is ignored" --- Indeed, interesting. On the client-side, the generated code has an if with `(0!=_reply.readInt())`; when false, null is returned. On the server-side, this value is set by each *method* to 1 on success and 0 on failure, and always returns true. However, if onTransact returns false like I'm doing, this part is skipped and in the source code we can see a `reply.setDataPosition(0);`. Not sure on what this function does underneath, but if it sets position 0 w/ zero, then it triggers the client null. – Daniel Feb 11 '16 at 18:10
  • ok i got it: `mRemote.transact` is called inside `try {` and after this call `_reply.readException();` is failing since `_reply` is an empty `Parcel` thus `_result = _reply.read*();` is not executed and as the result `_result` is null, see: http://pastebin.com/Fdw4eKEg – pskink Feb 11 '16 at 18:35
  • the line `_reply.readException();` in generated code was quite interesting: i took a look at Parcel.java code and it seems that what you need is to call `reply.writeException(e)` and return from `onTransact` (no matter true/false), only limited exceptions are supported (see `Parcel#writeException` code): `SecurityException` (i think this is the best one), `BadParcelableException`, `IllegalArgumentException`, `NullPointerException`, `IllegalStateException` and `NetworkOnMainThreadException` and of course call your AIDL method in `try...catch` statement – pskink Feb 12 '16 at 03:32
  • Ah, [I see](https://android.googlesource.com/platform/frameworks/base/+/0e2d281/core/java/android/os/Parcel.java#1313). Initially was using `Context#checkCallingPermission(permission)` inside `onTransact` and manually throwing the exception you suggest. Then changed to `Context#enforceCallingPermission(permission, message)`, which does most of the work and throws `SecurityException` when the caller does not have the necessary permission. – Daniel Feb 12 '16 at 09:13
  • actually i was wrong **twice**, first throwing an Exception is "not supported yet" (i got that message when trying to call `throw new RemoteException` from `onTransact`, btw no Exception is supported) so you have to use `reply.writeException(e)`, second: when you use it you have to return **true** which now make sense: i noticed that if you return false the "reply" `Parcel` is not copied back to the client so the client gets an empty `Parcel`and when you return true it is copied back to the client so it can call `readException` (see generated code) and finally throw it on the client side – pskink Feb 12 '16 at 09:25
  • se the service code can look like: http://pastebin.com/jMXhysbb, of course testing for no permissions is fake: i make it for one particular AIDL method – pskink Feb 12 '16 at 09:31
  • The main branch of the [source code](https://android.googlesource.com/platform/frameworks/base/+/master/core/java/android/os/Parcel.java#1538) also includes `UnsupportedOperationException` and `ServiceSpecificException`. When the latter is thrown, there is an additional instruction at the end of the method: `writeInt(((ServiceSpecificException)e).errorCode);`. (These are all runtime exceptions so I prefer not to catch them on the client.) – Daniel Feb 12 '16 at 09:56
  • I tried throwing [`IllegalArgumentException("test test")`](http://pastebin.com/PZ40cv0R) inside `onTransact` and it was correctly [received on the client side](http://pastebin.com/qPvuRZqL). – Daniel Feb 12 '16 at 09:57
  • wow! you are right: actually you **CAN** throw some Exceptions, its seems that you can throw only those that are supported by `writeException`, so no need for calling `reply.writeException()`, thanx for trying `IllegalArgumentException` – pskink Feb 12 '16 at 10:09
  • other exceptions result in the following in the logcat: `JavaBinder E *** Uncaught remote exception! (Exceptions are not yet supported across processes.)` this is what i got when throwing `RemoteException` and couple of others, so i though that `onTransact` cannot throw any exceptions – pskink Feb 12 '16 at 10:13

1 Answers1

0

There's no need to do this type of operation. If you want to protect binding, starting or stopping your Service then add the android:permission attribute to your manifest declaration of the Service:

<service android:name=".MyService"
         android:permission="com.example.myapp.permission.SERVICE_USER" >
    <intent-filter>
    ...
    </intent-filter>
</service>

The system will automatically ensure that any app trying to use your service holds the specified permission. You only use the enforce/check permission APIs inside of binder exposed calls. You may find this article helpful: http://po.st/d6eTXj.

Larry Schiefer
  • 15,687
  • 2
  • 27
  • 33
  • The service returns multiple AIDL interfaces according to the action received in `onBind`. The permissions for each of those interfaces are different. If I add the permission(s) to the manifest then all AIDL interfaces are forced to use the same permission(s). – Daniel Feb 10 '16 at 17:04
  • I would suggest breaking it into separate `Service` implementations, one for each AIDL exposed interface. That will allow this to work and have a clean separation of responsibilities. You cannot check the UID/PID of the caller in `onBind()` because it is happening on the main thread of your app's process rather than a binder thread. Alternatively, expose a single AIDL defined interface but use different permissions to control access to specific APIs. – Larry Schiefer Feb 10 '16 at 17:07
  • Look into the example [PackageValidator](https://android.googlesource.com/platform/development/+/67d3c4a/samples/Support4Demos/src/com/example/android/supportv4/media/PackageValidator.java) where the app is allowing specific binders to perform action based on the permissions granted in an xml file. – waqaslam Oct 10 '18 at 09:51