Merge changes I6eb6d92b,I638e29fd,I2348b7a3

* changes:
  Add a convenience method to update a network's capabilities.
  Disallow NetworkAgents from changing the owner UID.
  Observe mOwnerUID in NetworkCapabilities#equals.

core/java/android/net/NetworkCapabilities.java

@@ -975,6 +975,10 @@ public final class NetworkCapabilities implements Parcelable {
         return mOwnerUid;
     }
 
+    private boolean equalsOwnerUid(@NonNull final NetworkCapabilities nc) {
+        return mOwnerUid == nc.mOwnerUid;
+    }
+
     /**
      * UIDs of packages that are administrators of this network, or empty if none.
      *
@@ -1684,6 +1688,7 @@ public final class NetworkCapabilities implements Parcelable {
                 && equalsTransportInfo(that)
                 && equalsUids(that)
                 && equalsSSID(that)
+                && equalsOwnerUid(that)
                 && equalsPrivateDnsBroken(that)
                 && equalsRequestor(that)
                 && equalsAdministratorUids(that);
@@ -1697,17 +1702,18 @@ public final class NetworkCapabilities implements Parcelable {
                 + ((int) (mUnwantedNetworkCapabilities >> 32) * 7)
                 + ((int) (mTransportTypes & 0xFFFFFFFF) * 11)
                 + ((int) (mTransportTypes >> 32) * 13)
-                + (mLinkUpBandwidthKbps * 17)
-                + (mLinkDownBandwidthKbps * 19)
+                + mLinkUpBandwidthKbps * 17
+                + mLinkDownBandwidthKbps * 19
                 + Objects.hashCode(mNetworkSpecifier) * 23
-                + (mSignalStrength * 29)
-                + Objects.hashCode(mUids) * 31
-                + Objects.hashCode(mSSID) * 37
-                + Objects.hashCode(mTransportInfo) * 41
-                + Objects.hashCode(mPrivateDnsBroken) * 43
-                + Objects.hashCode(mRequestorUid) * 47
-                + Objects.hashCode(mRequestorPackageName) * 53
-                + Arrays.hashCode(mAdministratorUids) * 59;
+                + mSignalStrength * 29
+                + mOwnerUid * 31
+                + Objects.hashCode(mUids) * 37
+                + Objects.hashCode(mSSID) * 41
+                + Objects.hashCode(mTransportInfo) * 43
+                + Objects.hashCode(mPrivateDnsBroken) * 47
+                + Objects.hashCode(mRequestorUid) * 53
+                + Objects.hashCode(mRequestorPackageName) * 59
+                + Arrays.hashCode(mAdministratorUids) * 61;
     }
 
     @Override

services/core/java/com/android/server/ConnectivityService.java

@@ -2831,7 +2831,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
                             log(nai.toShortString() + " changed underlying networks to "
                                     + Arrays.toString(nai.declaredUnderlyingNetworks));
                         }
-                        updateCapabilities(nai.getCurrentScore(), nai, nai.networkCapabilities);
+                        updateCapabilitiesForNetwork(nai);
                         notifyIfacesChangedForNetworkStats();
                     }
                 }
@@ -2855,8 +2855,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
                     if (probePrivateDnsCompleted) {
                         if (nai.networkCapabilities.isPrivateDnsBroken() != privateDnsBroken) {
                             nai.networkCapabilities.setPrivateDnsBroken(privateDnsBroken);
-                            final int oldScore = nai.getCurrentScore();
-                            updateCapabilities(oldScore, nai, nai.networkCapabilities);
+                            updateCapabilitiesForNetwork(nai);
                         }
                         // Only show the notification when the private DNS is broken and the
                         // PRIVATE_DNS_BROKEN notification hasn't shown since last valid.
@@ -2871,8 +2870,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
                         // done yet. In either case, the networkCapabilities should be updated to
                         // reflect the new status.
                         nai.networkCapabilities.setPrivateDnsBroken(false);
-                        final int oldScore = nai.getCurrentScore();
-                        updateCapabilities(oldScore, nai, nai.networkCapabilities);
+                        updateCapabilitiesForNetwork(nai);
                         nai.networkAgentConfig.hasShownBroken = false;
                     }
                     break;
@@ -2893,7 +2891,6 @@ public class ConnectivityService extends IConnectivityManager.Stub
                     final NetworkAgentInfo nai = getNetworkAgentInfoForNetId(netId);
                     // If captive portal status has changed, update capabilities or disconnect.
                     if (nai != null && (visible != nai.lastCaptivePortalDetected)) {
-                        final int oldScore = nai.getCurrentScore();
                         nai.lastCaptivePortalDetected = visible;
                         nai.everCaptivePortalDetected |= visible;
                         if (nai.lastCaptivePortalDetected &&
@@ -2904,7 +2901,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
                             teardownUnneededNetwork(nai);
                             break;
                         }
-                        updateCapabilities(oldScore, nai, nai.networkCapabilities);
+                        updateCapabilitiesForNetwork(nai);
                     }
                     if (!visible) {
                         // Only clear SIGN_IN and NETWORK_SWITCH notifications here, or else other
@@ -2988,7 +2985,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
                     nai.networkAgentConfig.hasShownBroken = false;
                 }
             } else if (partialConnectivityChanged) {
-                updateCapabilities(nai.getCurrentScore(), nai, nai.networkCapabilities);
+                updateCapabilitiesForNetwork(nai);
             }
             updateInetCondition(nai);
             // Let the NetworkAgent know the state of its network
@@ -3656,7 +3653,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
                 nri.mSatisfier = null;
                 if (!wasBackgroundNetwork && nai.isBackgroundNetwork()) {
                     // Went from foreground to background.
-                    updateCapabilities(nai.getCurrentScore(), nai, nai.networkCapabilities);
+                    updateCapabilitiesForNetwork(nai);
                 }
             }
 
@@ -4820,7 +4817,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
         ensureRunningOnConnectivityServiceThread();
         for (NetworkAgentInfo nai : mNetworkAgentInfos.values()) {
             if (nai.supportsUnderlyingNetworks()) {
-                updateCapabilities(nai.getCurrentScore(), nai, nai.networkCapabilities);
+                updateCapabilitiesForNetwork(nai);
             }
         }
     }
@@ -6358,6 +6355,11 @@ public class ConnectivityService extends IConnectivityManager.Stub
      */
     private void processCapabilitiesFromAgent(NetworkAgentInfo nai, NetworkCapabilities nc) {
         nai.declaredMetered = !nc.hasCapability(NET_CAPABILITY_NOT_METERED);
+        if (nai.networkCapabilities.getOwnerUid() != nc.getOwnerUid()) {
+            Log.e(TAG, nai.toShortString() + ": ignoring attempt to change owner from "
+                    + nai.networkCapabilities.getOwnerUid() + " to " + nc.getOwnerUid());
+            nc.setOwnerUid(nai.networkCapabilities.getOwnerUid());
+        }
     }
 
     /** Modifies |caps| based on the capabilities of the specified underlying networks. */
@@ -6571,6 +6573,11 @@ public class ConnectivityService extends IConnectivityManager.Stub
         }
     }
 
+    /** Convenience method to update the capabilities for a given network. */
+    private void updateCapabilitiesForNetwork(NetworkAgentInfo nai) {
+        updateCapabilities(nai.getCurrentScore(), nai, nai.networkCapabilities);
+    }
+
     /**
      * Returns whether VPN isolation (ingress interface filtering) should be applied on the given
      * network.
@@ -6856,8 +6863,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
             teardownUnneededNetwork(oldNetwork);
         } else {
             // Put the network in the background.
-            updateCapabilities(oldNetwork.getCurrentScore(), oldNetwork,
-                    oldNetwork.networkCapabilities);
+            updateCapabilitiesForNetwork(oldNetwork);
         }
     }
 

tests/net/java/com/android/server/ConnectivityServiceTest.java

@@ -1957,7 +1957,7 @@ public class ConnectivityServiceTest {
     }
 
     @Test
-    public void testOwnerUidChangeBug() throws Exception {
+    public void testOwnerUidCannotChange() throws Exception {
         // Owner UIDs are not visible without location permission.
         setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION,
                 Manifest.permission.ACCESS_FINE_LOCATION);
@@ -1972,39 +1972,19 @@ public class ConnectivityServiceTest {
         waitForIdle();
 
         // Send ConnectivityService an update to the mWiFiNetworkAgent's capabilities that changes
-        // its owner UID.
+        // the owner UID and an unrelated capability.
         NetworkCapabilities agentCapabilities = mWiFiNetworkAgent.getNetworkCapabilities();
         assertEquals(originalOwnerUid, agentCapabilities.getOwnerUid());
         agentCapabilities.setOwnerUid(42);
-        mWiFiNetworkAgent.setNetworkCapabilities(agentCapabilities, true);
-        waitForIdle();
-
-        // Check that the owner UID is not updated.
-        NetworkCapabilities nc = mCm.getNetworkCapabilities(mWiFiNetworkAgent.getNetwork());
-        assertEquals(originalOwnerUid, nc.getOwnerUid());
-
-        // Make an unrelated change to the capabilities.
         assertFalse(agentCapabilities.hasCapability(NET_CAPABILITY_NOT_CONGESTED));
         agentCapabilities.addCapability(NET_CAPABILITY_NOT_CONGESTED);
         mWiFiNetworkAgent.setNetworkCapabilities(agentCapabilities, true);
         waitForIdle();
 
-        // Check that both the capability change and the owner UID have been modified.
-        // The owner UID is -1 because it is visible only to the UID that owns the network.
-        nc = mCm.getNetworkCapabilities(mWiFiNetworkAgent.getNetwork());
-        assertEquals(-1, nc.getOwnerUid());
-        assertTrue(nc.hasCapability(NET_CAPABILITY_NOT_CONGESTED));
-
-        // Set the owner back to originalOwnerUid, update the capabilities, and check that it is
-        // visible again.
-        // TODO: should this even be possible?
-        agentCapabilities.setOwnerUid(originalOwnerUid);
-        agentCapabilities.removeCapability(NET_CAPABILITY_NOT_CONGESTED);
-        mWiFiNetworkAgent.setNetworkCapabilities(agentCapabilities, true);
-        waitForIdle();
-
-        nc = mCm.getNetworkCapabilities(mWiFiNetworkAgent.getNetwork());
+        // Check that the capability change has been applied but the owner UID is not modified.
+        NetworkCapabilities nc = mCm.getNetworkCapabilities(mWiFiNetworkAgent.getNetwork());
         assertEquals(originalOwnerUid, nc.getOwnerUid());
+        assertTrue(nc.hasCapability(NET_CAPABILITY_NOT_CONGESTED));
     }
 
     @Test