• R/O
  • HTTP
  • SSH

system-netd: Commit


Commit MetaInfo

Revision402e75fe80807042880ef5f0bc5db23bc84c5b3e (tree)
Zeit2017-11-16 05:42:47
AutorLorenzo Colitti <lorenzo@goog...>
CommiterDan Pasanen

Log Message

Don't allow seamless handover to networks requiring permissions.

Currently, implicitly-marked sockets continue to work when the
network changes permission. This makes it so that UDP sockets
connected on a foreground network will continue to work even if
the network moves into the background (e.g., when the linger
timer fires on cell data with mobile data always on).

Instead, make it so that sockets implicitly marked to a network
become unroutable when the network starts requiring permissions.
Explicitly-marked sockets will continue to be routed on the
network, as usual.

This is consistent with what we do for TCP: when a network
changes permissions, all implicitly-marked sockets on that
network are closed using SOCK_DESTROY.

This change should not affect any other behaviour because:

- Netd only ever implicitly marks sockets to the default network

or to a bypassable VPN that applies to the caller.

- In both cases, at the time of marking, the network does not

require permissions because:
- VPNs don't support permissions.
- The default network never requires any permissions:
- ConnectivityService's mDefaultRequest specifies
- The only case where a NOT_RESTRICTED network can require a
permission is if it's a background network, and the default
network is, by definition, never a background network.

- VPNs can't change permissions.
- If the network is still the default network, the lack of this

implicit rule doesn't matter.

Therefore, the only case where this rule can alter routing is if
a socket is implicitly marked on the default network and that
network, after ceasing to be the default, changes permissions.

Bug: 64103722
Test: builds
Test: manually observed IP rules while changing network permissions
Change-Id: I255a9d216c50aa47bb951be9bd6cce59a12c6165
Merged-In: I255a9d216c50aa47bb951be9bd6cce59a12c6165
(cherry picked from commit 050085a56162dff203979e8c62cb57449f5f7a26)

Ändern Zusammenfassung


--- a/server/RouteController.cpp
+++ b/server/RouteController.cpp
@@ -590,8 +590,7 @@ WARN_UNUSED_RESULT int modifyOutputInterfaceRules(const char* interface, uint32_
590590 // This is for sockets that have not explicitly requested a particular network, but have been
591591 // bound to one when they called connect(). This ensures that sockets connected on a particular
592592 // network stay on that network even if the default network changes.
593-WARN_UNUSED_RESULT int modifyImplicitNetworkRule(unsigned netId, uint32_t table,
594- Permission permission, bool add) {
593+WARN_UNUSED_RESULT int modifyImplicitNetworkRule(unsigned netId, uint32_t table, bool add) {
595594 Fwmark fwmark;
596595 Fwmark mask;
@@ -601,8 +600,8 @@ WARN_UNUSED_RESULT int modifyImplicitNetworkRule(unsigned netId, uint32_t table,
601600 fwmark.explicitlySelected = false;
602601 mask.explicitlySelected = true;
604- fwmark.permission = permission;
605- mask.permission = permission;
603+ fwmark.permission = PERMISSION_NONE;
604+ mask.permission = PERMISSION_NONE;
607606 return modifyIpRule(add ? RTM_NEWRULE : RTM_DELRULE, RULE_PRIORITY_IMPLICIT_NETWORK, table,
608607 fwmark.intValue, mask.intValue);
@@ -766,7 +765,31 @@ WARN_UNUSED_RESULT int modifyPhysicalNetwork(unsigned netId, const char* interfa
766765 add)) {
767766 return ret;
768767 }
769- return modifyImplicitNetworkRule(netId, table, permission, add);
769+ // Only set implicit rules for networks that don't require permissions.
770+ //
771+ // This is so that if the default network ceases to be the default network and then switches
772+ // from requiring no permissions to requiring permissions, we ensure that apps only use the
773+ // network if they explicitly select it. This is consistent with destroySocketsLackingPermission
774+ // - it closes all sockets on the network except sockets that are explicitly selected.
775+ //
776+ // The lack of this rule only affects the special case above, because:
777+ // - The only cases where we implicitly bind a socket to a network are the default network and
778+ // the bypassable VPN that applies to the app, if any.
779+ // - This rule doesn't affect VPNs because they don't support permissions at all.
780+ // - The default network doesn't require permissions. While we support doing this, the framework
781+ // never does it (partly because we'd end up in the situation where we tell apps that there is
782+ // a default network, but they can't use it).
783+ // - If the network is still the default network, the presence or absence of this rule does not
784+ // matter.
785+ //
786+ // Therefore, for the lack of this rule to affect a socket, the socket has to have been
787+ // implicitly bound to a network because at the time of connect() it was the default, and that
788+ // network must no longer be the default, and must now require permissions.
789+ if (permission == PERMISSION_NONE) {
790+ return modifyImplicitNetworkRule(netId, table, add);
791+ }
792+ return 0;
770793 }
772795 WARN_UNUSED_RESULT int modifyRejectNonSecureNetworkRule(const UidRanges& uidRanges, bool add) {
Show on old repository browser