From bcc0f5b14fbcf63c06cce1cd04cfa0e8282d2762 Mon Sep 17 00:00:00 2001
From: Hungming Chen <nuccachen@google.com>
Date: Mon, 7 Mar 2022 14:13:49 +0800
Subject: [PATCH] BpfHandler: only allow to tag INET/INET6 socket

This is a follow up commit for the family validation {INET, INET6}.
The protocol validation {TCP, UDP} has been added in previous
commit.

The TrafficController socket destroy listener only monitors
on the group {INET_TCP, INET_UDP, INET6_TCP, INET6_UDP}.
Tagging listener unsupported socket causes that the tag can't
be removed from tag map automatically. Eventually, the tag map
run out of space because of dead tag entries.

See TrafficController::makeSkDestroyListener in
packages/modules/Connectivity/service/native/TrafficController.cpp

Also address the comments from previous commit.
- Remove the useless else-statment in tagSocket protocol validation.
- Make the socket cookie query and test into one line in
  BpfHandlerTest#TestTagSocketWithUnsupportedProtocol

Bug: 223094609
Test: atest BpfHandlerTest
Change-Id: I0f571fc00caa01c86399f0dbb593e8a40ad94bbd
---
 netd/BpfHandler.cpp     | 34 +++++++++++++++++++++++-----------
 netd/BpfHandlerTest.cpp | 10 ++++++++--
 2 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/netd/BpfHandler.cpp b/netd/BpfHandler.cpp
index 1e47ea3e91..f3dfb57cde 100644
--- a/netd/BpfHandler.cpp
+++ b/netd/BpfHandler.cpp
@@ -146,20 +146,32 @@ int BpfHandler::tagSocket(int sockFd, uint32_t tag, uid_t chargeUid, uid_t realU
     // The socket destroy listener only monitors on the group {INET_TCP, INET_UDP, INET6_TCP,
     // INET6_UDP}. Tagging listener unsupported socket causes that the tag can't be removed from
     // tag map automatically. Eventually, the tag map may run out of space because of dead tag
-    // entries.
-    // See TrafficController::makeSkDestroyListener in
+    // entries. Note that although tagSocket() of net client has already denied the family which
+    // is neither AF_INET nor AF_INET6, the family validation is still added here just in case.
+    // See tagSocket in system/netd/client/NetdClient.cpp and
+    // TrafficController::makeSkDestroyListener in
     // packages/modules/Connectivity/service/native/TrafficController.cpp
     // TODO: remove this once the socket destroy listener can detect more types of socket destroy.
-    int socketProto;
-    socklen_t intSize = sizeof(socketProto);
-    if (getsockopt(sockFd, SOL_SOCKET, SO_PROTOCOL, &socketProto, &intSize)) {
-        ALOGE("Failed to getsockopt: %s, fd: %d", strerror(errno), sockFd);
+    int socketFamily;
+    socklen_t familyLen = sizeof(socketFamily);
+    if (getsockopt(sockFd, SOL_SOCKET, SO_DOMAIN, &socketFamily, &familyLen)) {
+        ALOGE("Failed to getsockopt SO_DOMAIN: %s, fd: %d", strerror(errno), sockFd);
         return -errno;
-    } else {
-        if (socketProto != IPPROTO_UDP && socketProto != IPPROTO_TCP) {
-            ALOGE("Unsupported protocol: %d", socketProto);
-            return -EPROTONOSUPPORT;
-        }
+    }
+    if (socketFamily != AF_INET && socketFamily != AF_INET6) {
+        ALOGE("Unsupported family: %d", socketFamily);
+        return -EAFNOSUPPORT;
+    }
+
+    int socketProto;
+    socklen_t protoLen = sizeof(socketProto);
+    if (getsockopt(sockFd, SOL_SOCKET, SO_PROTOCOL, &socketProto, &protoLen)) {
+        ALOGE("Failed to getsockopt SO_PROTOCOL: %s, fd: %d", strerror(errno), sockFd);
+        return -errno;
+    }
+    if (socketProto != IPPROTO_UDP && socketProto != IPPROTO_TCP) {
+        ALOGE("Unsupported protocol: %d", socketProto);
+        return -EPROTONOSUPPORT;
     }
 
     uint64_t sock_cookie = getSocketCookie(sockFd);
diff --git a/netd/BpfHandlerTest.cpp b/netd/BpfHandlerTest.cpp
index 66a2f803a3..cd6b565653 100644
--- a/netd/BpfHandlerTest.cpp
+++ b/netd/BpfHandlerTest.cpp
@@ -188,11 +188,17 @@ TEST_F(BpfHandlerTest, TestTagInvalidSocket) {
     expectMapEmpty(mFakeCookieTagMap);
 }
 
+TEST_F(BpfHandlerTest, TestTagSocketWithUnsupportedFamily) {
+    int packetSocket = socket(AF_PACKET, SOCK_DGRAM | SOCK_CLOEXEC, 0);
+    EXPECT_LE(0, packetSocket);
+    EXPECT_NE(NONEXISTENT_COOKIE, getSocketCookie(packetSocket));
+    EXPECT_EQ(-EAFNOSUPPORT, mBh.tagSocket(packetSocket, TEST_TAG, TEST_UID, TEST_UID));
+}
+
 TEST_F(BpfHandlerTest, TestTagSocketWithUnsupportedProtocol) {
     int rawSocket = socket(AF_INET, SOCK_RAW | SOCK_CLOEXEC, IPPROTO_RAW);
     EXPECT_LE(0, rawSocket);
-    uint64_t sockCookie = getSocketCookie(rawSocket);
-    EXPECT_NE(NONEXISTENT_COOKIE, sockCookie);
+    EXPECT_NE(NONEXISTENT_COOKIE, getSocketCookie(rawSocket));
     EXPECT_EQ(-EPROTONOSUPPORT, mBh.tagSocket(rawSocket, TEST_TAG, TEST_UID, TEST_UID));
 }