Set correct owner UID for VPN agentConnect()

This commit changes agentConnect to set the owner UID as the mOwnerUid
field instead of the Binder.getCallingUid().

Binder.getCallingUid() can return incorrect results for platform VPNs,
as agentConnect() is called under a clean calling UID.

Additionally, this relaxes the ownerUid sanitization check to allow a
VPN network's owner to see it's own ownership information.

Vpn.mOwnerUid is guaranteed to be correct, as all VPNs MUST have called
prepareInternal() at some previous point, which sets mOwnerUid as the
package's UID (or SYSTEM_UID if this is legacy VPN).

Bug: 150135470
Test: CTS tests showing ownership information
Change-Id: Ic979dad73983d722365849fbfb0becfd432b894c
This commit is contained in:
Benedict Wong
2020-06-08 11:55:38 -07:00
parent 551b6e094b
commit bf004e9830
2 changed files with 28 additions and 5 deletions

View File

@@ -900,9 +900,17 @@ public final class NetworkCapabilities implements Parcelable {
* <p>For NetworkCapability instances being sent from ConnectivityService, this value MUST be
* reset to Process.INVALID_UID unless all the following conditions are met:
*
* <p>The caller is the network owner, AND one of the following sets of requirements is met:
*
* <ol>
* <li>The destination app is the network owner
* <li>The destination app has the ACCESS_FINE_LOCATION permission granted
* <li>The described Network is a VPN
* </ol>
*
* <p>OR:
*
* <ol>
* <li>The calling app is the network owner
* <li>The calling app has the ACCESS_FINE_LOCATION permission granted
* <li>The user's location toggle is on
* </ol>
*
@@ -928,7 +936,16 @@ public final class NetworkCapabilities implements Parcelable {
/**
* Retrieves the UID of the app that owns this network.
*
* <p>For user privacy reasons, this field will only be populated if:
* <p>For user privacy reasons, this field will only be populated if the following conditions
* are met:
*
* <p>The caller is the network owner, AND one of the following sets of requirements is met:
*
* <ol>
* <li>The described Network is a VPN
* </ol>
*
* <p>OR:
*
* <ol>
* <li>The calling app is the network owner
@@ -936,8 +953,8 @@ public final class NetworkCapabilities implements Parcelable {
* <li>The user's location toggle is on
* </ol>
*
* Instances of NetworkCapabilities sent to apps without the appropriate permissions will
* have this field cleared out.
* Instances of NetworkCapabilities sent to apps without the appropriate permissions will have
* this field cleared out.
*/
public int getOwnerUid() {
return mOwnerUid;

View File

@@ -1698,6 +1698,12 @@ public class ConnectivityService extends IConnectivityManager.Stub
return newNc;
}
// Allow VPNs to see ownership of their own VPN networks - not location sensitive.
if (nc.hasTransport(TRANSPORT_VPN)) {
// Owner UIDs already checked above. No need to re-check.
return newNc;
}
Binder.withCleanCallingIdentity(
() -> {
if (!mLocationPermissionChecker.checkLocationPermission(