From 01aeca4d3f001c2d3d0b533037907516408ed9d5 Mon Sep 17 00:00:00 2001 From: Yan Yan Date: Thu, 28 Jul 2022 00:30:45 +0000 Subject: [PATCH] Make Vpn more testable with a wrapper class This commit creates a wrapper class for NetworkAgent to make the Vpn code more testable. Bug: 192077544 Test: atest VpnTest Change-Id: Ia599fed8c874ea9622a1f56f686de293bcbfddb8 --- .../com/android/server/connectivity/Vpn.java | 112 +++++++++++++++--- 1 file changed, 95 insertions(+), 17 deletions(-) diff --git a/services/core/java/com/android/server/connectivity/Vpn.java b/services/core/java/com/android/server/connectivity/Vpn.java index baeaa505549bf..3e32a9638ef82 100644 --- a/services/core/java/com/android/server/connectivity/Vpn.java +++ b/services/core/java/com/android/server/connectivity/Vpn.java @@ -514,12 +514,8 @@ public class Vpn { @NonNull NetworkScore score, @NonNull NetworkAgentConfig config, @Nullable NetworkProvider provider) { - return new NetworkAgent(context, looper, logTag, nc, lp, score, config, provider) { - @Override - public void onNetworkUnwanted() { - // We are user controlled, not driven by NetworkRequest. - } - }; + return new VpnNetworkAgentWrapper( + context, looper, logTag, nc, lp, score, config, provider); } } @@ -1813,7 +1809,7 @@ public class Vpn { Log.wtf(TAG, "Failed to add restricted user to owner", e); } if (mNetworkAgent != null) { - mNetworkAgent.sendNetworkCapabilities(mNetworkCapabilities); + doSendNetworkCapabilities(mNetworkAgent, mNetworkCapabilities); } } setVpnForcedLocked(mLockdown); @@ -1843,7 +1839,7 @@ public class Vpn { Log.wtf(TAG, "Failed to remove restricted user to owner", e); } if (mNetworkAgent != null) { - mNetworkAgent.sendNetworkCapabilities(mNetworkCapabilities); + doSendNetworkCapabilities(mNetworkAgent, mNetworkCapabilities); } } setVpnForcedLocked(mLockdown); @@ -2077,7 +2073,7 @@ public class Vpn { return false; } boolean success = jniAddAddress(mInterface, address, prefixLength); - mNetworkAgent.sendLinkProperties(makeLinkProperties()); + doSendLinkProperties(mNetworkAgent, makeLinkProperties()); return success; } @@ -2086,7 +2082,7 @@ public class Vpn { return false; } boolean success = jniDelAddress(mInterface, address, prefixLength); - mNetworkAgent.sendLinkProperties(makeLinkProperties()); + doSendLinkProperties(mNetworkAgent, makeLinkProperties()); return success; } @@ -2100,8 +2096,11 @@ public class Vpn { // Make defensive copy since the content of array might be altered by the caller. mConfig.underlyingNetworks = (networks != null) ? Arrays.copyOf(networks, networks.length) : null; - mNetworkAgent.setUnderlyingNetworks((mConfig.underlyingNetworks != null) - ? Arrays.asList(mConfig.underlyingNetworks) : null); + doSetUnderlyingNetworks( + mNetworkAgent, + (mConfig.underlyingNetworks != null) + ? Arrays.asList(mConfig.underlyingNetworks) + : null); return true; } @@ -2910,7 +2909,7 @@ public class Vpn { return; // Link properties are already sent. } else { // Underlying networks also set in agentConnect() - networkAgent.setUnderlyingNetworks(Collections.singletonList(network)); + doSetUnderlyingNetworks(networkAgent, Collections.singletonList(network)); mNetworkCapabilities = new NetworkCapabilities.Builder(mNetworkCapabilities) .setUnderlyingNetworks(Collections.singletonList(network)) @@ -2920,7 +2919,7 @@ public class Vpn { lp = makeLinkProperties(); // Accesses VPN instance fields; must be locked } - networkAgent.sendLinkProperties(lp); + doSendLinkProperties(networkAgent, lp); } catch (Exception e) { Log.d(TAG, "Error in ChildOpened for token " + token, e); onSessionLost(token, e); @@ -2987,7 +2986,7 @@ public class Vpn { new NetworkCapabilities.Builder(mNetworkCapabilities) .setUnderlyingNetworks(Collections.singletonList(network)) .build(); - mNetworkAgent.setUnderlyingNetworks(Collections.singletonList(network)); + doSetUnderlyingNetworks(mNetworkAgent, Collections.singletonList(network)); } mTunnelIface.setUnderlyingNetwork(network); @@ -3384,7 +3383,7 @@ public class Vpn { null /*gateway*/, null /*iface*/, RTN_UNREACHABLE)); } if (mNetworkAgent != null) { - mNetworkAgent.sendLinkProperties(makeLinkProperties()); + doSendLinkProperties(mNetworkAgent, makeLinkProperties()); } } } @@ -4121,7 +4120,7 @@ public class Vpn { .setUids(createUserAndRestrictedProfilesRanges( mUserId, null /* allowedApplications */, excludedApps)) .build(); - mNetworkAgent.sendNetworkCapabilities(mNetworkCapabilities); + doSendNetworkCapabilities(mNetworkAgent, mNetworkCapabilities); } } } @@ -4198,6 +4197,85 @@ public class Vpn { return isCurrentIkev2VpnLocked(packageName) ? makeVpnProfileStateLocked() : null; } + /** Proxy to allow different testing setups */ + // TODO: b/240492694 Remove VpnNetworkAgentWrapper and this method when + // NetworkAgent#sendLinkProperties can be un-finalized. + private static void doSendLinkProperties( + @NonNull NetworkAgent agent, @NonNull LinkProperties lp) { + if (agent instanceof VpnNetworkAgentWrapper) { + ((VpnNetworkAgentWrapper) agent).doSendLinkProperties(lp); + } else { + agent.sendLinkProperties(lp); + } + } + + /** Proxy to allow different testing setups */ + // TODO: b/240492694 Remove VpnNetworkAgentWrapper and this method when + // NetworkAgent#sendNetworkCapabilities can be un-finalized. + private static void doSendNetworkCapabilities( + @NonNull NetworkAgent agent, @NonNull NetworkCapabilities nc) { + if (agent instanceof VpnNetworkAgentWrapper) { + ((VpnNetworkAgentWrapper) agent).doSendNetworkCapabilities(nc); + } else { + agent.sendNetworkCapabilities(nc); + } + } + + /** Proxy to allow different testing setups */ + // TODO: b/240492694 Remove VpnNetworkAgentWrapper and this method when + // NetworkAgent#setUnderlyingNetworks can be un-finalized. + private static void doSetUnderlyingNetworks( + @NonNull NetworkAgent agent, @NonNull List networks) { + if (agent instanceof VpnNetworkAgentWrapper) { + ((VpnNetworkAgentWrapper) agent).doSetUnderlyingNetworks(networks); + } else { + agent.setUnderlyingNetworks(networks); + } + } + + /** + * Proxy to allow testing + * + * @hide + */ + // TODO: b/240492694 Remove VpnNetworkAgentWrapper when NetworkAgent's methods can be + // un-finalized. + @VisibleForTesting + public static class VpnNetworkAgentWrapper extends NetworkAgent { + /** Create an VpnNetworkAgentWrapper */ + public VpnNetworkAgentWrapper( + @NonNull Context context, + @NonNull Looper looper, + @NonNull String logTag, + @NonNull NetworkCapabilities nc, + @NonNull LinkProperties lp, + @NonNull NetworkScore score, + @NonNull NetworkAgentConfig config, + @Nullable NetworkProvider provider) { + super(context, looper, logTag, nc, lp, score, config, provider); + } + + /** Update the LinkProperties */ + public void doSendLinkProperties(@NonNull LinkProperties lp) { + sendLinkProperties(lp); + } + + /** Update the NetworkCapabilities */ + public void doSendNetworkCapabilities(@NonNull NetworkCapabilities nc) { + sendNetworkCapabilities(nc); + } + + /** Set the underlying networks */ + public void doSetUnderlyingNetworks(@NonNull List networks) { + setUnderlyingNetworks(networks); + } + + @Override + public void onNetworkUnwanted() { + // We are user controlled, not driven by NetworkRequest. + } + } + /** * Proxy to allow testing *