From 53ea01935792f86054249de1247bb9607e1bbd65 Mon Sep 17 00:00:00 2001 From: xzh457 Date: Fri, 13 Sep 2024 17:19:48 +0800 Subject: [PATCH 1/2] add pid check Signed-off-by: xzh457 Change-Id: I363a37c3cbb41b4184193e735fbff72ca9a7ea27 --- .../init/include/softbus_server_stub.h | 1 - .../standard/init/src/softbus_server_stub.cpp | 38 +++++++------------ .../manager/src/trans_channel_manager.c | 30 +++++++++------ .../include/trans_tcp_direct_sessionconn.h | 2 + .../src/trans_tcp_direct_sessionconn.c | 32 +++++++++++++++- tests/core/frame/unittest/BUILD.gn | 1 + .../trans_channel_manager_test.cpp | 2 +- 7 files changed, 67 insertions(+), 39 deletions(-) diff --git a/core/frame/standard/init/include/softbus_server_stub.h b/core/frame/standard/init/include/softbus_server_stub.h index 85e3d5821..e15d33274 100644 --- a/core/frame/standard/init/include/softbus_server_stub.h +++ b/core/frame/standard/init/include/softbus_server_stub.h @@ -45,7 +45,6 @@ private: int32_t CheckOpenSessionPermission(const SessionParam *param); int32_t CheckChannelPermission(int32_t channelId, int32_t channelType); int32_t EvaluateQosInner(MessageParcel &data, MessageParcel &reply); - int32_t CheckPidByChannelId(pid_t callingPid, int32_t channelId, int32_t channelType); int32_t JoinLNNInner(MessageParcel &data, MessageParcel &reply); int32_t LeaveLNNInner(MessageParcel &data, MessageParcel &reply); diff --git a/core/frame/standard/init/src/softbus_server_stub.cpp b/core/frame/standard/init/src/softbus_server_stub.cpp index b9f99a534..6e363a311 100644 --- a/core/frame/standard/init/src/softbus_server_stub.cpp +++ b/core/frame/standard/init/src/softbus_server_stub.cpp @@ -31,6 +31,7 @@ #include "trans_event.h" #include "trans_network_statistics.h" #include "trans_session_manager.h" +#include "trans_tcp_direct_sessionconn.h" #ifdef SUPPORT_BUNDLENAME #include "bundle_mgr_proxy.h" @@ -71,6 +72,11 @@ int32_t SoftBusServerStub::CheckOpenSessionPermission(const SessionParam *param) return SOFTBUS_PERMISSION_DENIED; } + if (!CheckUidAndPid(param->sessionName, callingUid, callingPid)) { + COMM_LOGE(COMM_SVC, "Check Uid and Pid failed, sessionName=%{public}s", param->sessionName); + return SOFTBUS_TRANS_CHECK_PID_ERROR; + } + if (CheckTransSecLevel(param->sessionName, param->peerSessionName) != SOFTBUS_OK) { COMM_LOGE(COMM_SVC, "OpenSession sec level invalid"); return SOFTBUS_PERMISSION_DENIED; @@ -100,29 +106,6 @@ int32_t SoftBusServerStub::CheckChannelPermission(int32_t channelId, int32_t cha return SOFTBUS_OK; } -int32_t SoftBusServerStub::CheckPidByChannelId(pid_t callingPid, int32_t channelId, int32_t channelType) -{ - AppInfo *appInfo = (AppInfo *)SoftBusCalloc(sizeof(AppInfo)); - if (appInfo == NULL) { - COMM_LOGE(COMM_SVC, "malloc appInfo failed"); - return SOFTBUS_MALLOC_ERR; - } - int32_t ret = TransGetAppInfoByChanId(channelId, channelType, appInfo); - (void)memset_s(appInfo->sessionKey, sizeof(appInfo->sessionKey), 0, sizeof(appInfo->sessionKey)); - if (ret != SOFTBUS_OK) { - COMM_LOGE(COMM_SVC, "get AppInfo by channelId failed!"); - SoftBusFree(appInfo); - return ret == SOFTBUS_NOT_FIND ? SOFTBUS_OK : ret; - } - if (callingPid != appInfo->myData.pid) { - COMM_LOGE(COMM_SVC, "callingPid not match!"); - SoftBusFree(appInfo); - return SOFTBUS_TRANS_CHECK_PID_ERROR; - } - SoftBusFree(appInfo); - return SOFTBUS_OK; -} - static int32_t CheckAndRecordAccessToken(const char *permission) { uint32_t tokenCaller = IPCSkeleton::GetCallingTokenID(); @@ -611,6 +594,7 @@ int32_t SoftBusServerStub::NotifyAuthSuccessInner(MessageParcel &data, MessagePa COMM_LOGD(COMM_SVC, "enter"); int32_t channelId; int32_t channelType; + pid_t callingPid = OHOS::IPCSkeleton::GetCallingPid(); if (!data.ReadInt32(channelId)) { COMM_LOGE(COMM_SVC, "NotifyAuthSuccessInner read channel Id failed!"); return SOFTBUS_TRANS_PROXY_READINT_FAILED; @@ -619,6 +603,12 @@ int32_t SoftBusServerStub::NotifyAuthSuccessInner(MessageParcel &data, MessagePa COMM_LOGE(COMM_SVC, "NotifyAuthSuccessInner read channel type failed!"); return SOFTBUS_TRANS_PROXY_READINT_FAILED; } + int32_t ret = TransGetAndComparePid(callingPid, channelId, channelType); + if (ret != SOFTBUS_OK) { + COMM_LOGE(COMM_SVC, "callingPid not equal pid, callingPid=%{public}d, channelId=%{public}d", + callingPid, channelId); + return ret; + } int32_t retReply = NotifyAuthSuccess(channelId, channelType); if (!reply.WriteInt32(retReply)) { COMM_LOGE(COMM_SVC, "NotifyAuthSuccessInner write reply failed!"); @@ -761,7 +751,7 @@ int32_t SoftBusServerStub::SendMessageInner(MessageParcel &data, MessageParcel & return SOFTBUS_TRANS_PROXY_READINT_FAILED; } pid_t callingPid = OHOS::IPCSkeleton::GetCallingPid(); - if (CheckPidByChannelId(callingPid, channelId, channelType) != SOFTBUS_OK) { + if (TransGetAndComparePid(callingPid, channelId, channelType) != SOFTBUS_OK) { COMM_LOGE(COMM_SVC, "pid permission check failed!"); return SOFTBUS_PERMISSION_DENIED; } diff --git a/core/transmission/trans_channel/manager/src/trans_channel_manager.c b/core/transmission/trans_channel/manager/src/trans_channel_manager.c index 4890ad536..bffd6fd15 100644 --- a/core/transmission/trans_channel/manager/src/trans_channel_manager.c +++ b/core/transmission/trans_channel/manager/src/trans_channel_manager.c @@ -657,20 +657,26 @@ int32_t TransGetNameByChanId(const TransInfo *info, char *pkgName, char *session int32_t TransGetAndComparePid(pid_t pid, int32_t channelId, int32_t channelType) { + int32_t curChannelPid; + int32_t ret = SOFTBUS_OK; if ((ChannelType)channelType == CHANNEL_TYPE_TCP_DIRECT) { - TRANS_LOGD(TRANS_CTRL, "channel type is tcp direct!"); - return SOFTBUS_OK; + ret = TransGetPidByChanId(channelId, channelType, &curChannelPid); + if (ret != SOFTBUS_OK) { + TRANS_LOGE(TRANS_CTRL, "get pid by channelId failed, channelId=%{public}d", channelId); + return ret; + } + } else { + AppInfo appInfo; + ret = TransGetAppInfoByChanId(channelId, channelType, &appInfo); + (void)memset_s(appInfo.sessionKey, sizeof(appInfo.sessionKey), 0, sizeof(appInfo.sessionKey)); + if (ret != SOFTBUS_OK) { + TRANS_LOGE(TRANS_CTRL, "get appInfo by channelId failed, channelId=%{public}d", channelId); + return ret; + } + curChannelPid = appInfo.myData.pid; } - AppInfo appInfo; - int32_t ret = TransGetAppInfoByChanId(channelId, channelType, &appInfo); - (void)memset_s(appInfo.sessionKey, sizeof(appInfo.sessionKey), 0, sizeof(appInfo.sessionKey)); - if (ret != SOFTBUS_OK) { - TRANS_LOGE(TRANS_CTRL, "get appInfo by channelId failed, ret = %{public}d", ret); - return ret; - } - pid_t curChannelPid = appInfo.myData.pid; - if (pid != curChannelPid) { - TRANS_LOGE(TRANS_CTRL, "callingPid not equal curChannelPid, callingPid = %{public}d, pid = %{public}d", + if (pid != (pid_t)curChannelPid) { + TRANS_LOGE(TRANS_CTRL, "callingPid not equal curChannelPid, callingPid=%{public}d, pid=%{public}d", pid, curChannelPid); return SOFTBUS_TRANS_CHECK_PID_ERROR; } diff --git a/core/transmission/trans_channel/tcp_direct/include/trans_tcp_direct_sessionconn.h b/core/transmission/trans_channel/tcp_direct/include/trans_tcp_direct_sessionconn.h index a7bb0c585..a7b8ce510 100644 --- a/core/transmission/trans_channel/tcp_direct/include/trans_tcp_direct_sessionconn.h +++ b/core/transmission/trans_channel/tcp_direct/include/trans_tcp_direct_sessionconn.h @@ -132,6 +132,8 @@ void TransTdcChannelInfoDeathCallback(const char *pkgName, int32_t pid); int32_t TransTdcGetLocalIpAndConnectTypeById(int32_t channelId, char *localIp, uint32_t maxIpLen, int32_t *connectType); +int32_t TransGetPidByChanId(int32_t channelId, int32_t channelType, int32_t *pid); + #ifdef __cplusplus #if __cplusplus } diff --git a/core/transmission/trans_channel/tcp_direct/src/trans_tcp_direct_sessionconn.c b/core/transmission/trans_channel/tcp_direct/src/trans_tcp_direct_sessionconn.c index a3a5c8e38..95d198dcd 100644 --- a/core/transmission/trans_channel/tcp_direct/src/trans_tcp_direct_sessionconn.c +++ b/core/transmission/trans_channel/tcp_direct/src/trans_tcp_direct_sessionconn.c @@ -634,4 +634,34 @@ int32_t *GetChannelIdsByAuthIdAndStatus(int32_t *num, const AuthHandle *authHand } ReleaseSessionConnLock(); return result; -} \ No newline at end of file +} + +int32_t TransGetPidByChanId(int32_t channelId, int32_t channelType, int32_t *pid) +{ + if (pid == NULL) { + TRANS_LOGE(TRANS_CTRL, "pid is null"); + return SOFTBUS_INVALID_PARAM; + } + + if (g_tcpChannelInfoList == NULL) { + TRANS_LOGE(TRANS_CTRL, "tcp channel info list hasn't init."); + return SOFTBUS_INVALID_PARAM; + } + + if (SoftBusMutexLock(&(g_tcpChannelInfoList->lock)) != SOFTBUS_OK) { + TRANS_LOGE(TRANS_SVC, "lock failed"); + return SOFTBUS_LOCK_ERR; + } + + TcpChannelInfo *info = NULL; + LIST_FOR_EACH_ENTRY(info, &(g_tcpChannelInfoList->list), TcpChannelInfo, node) { + if (info->channelId == channelId && info->channelType == channelType) { + *pid = info->pid; + (void)SoftBusMutexUnlock(&(g_tcpChannelInfoList->lock)); + return SOFTBUS_OK; + } + } + (void)SoftBusMutexUnlock(&(g_tcpChannelInfoList->lock)); + TRANS_LOGE(TRANS_SVC, "can not find pid by channelId=%{public}d", channelId); + return SOFTBUS_TRANS_INVALID_CHANNEL_ID; +} diff --git a/tests/core/frame/unittest/BUILD.gn b/tests/core/frame/unittest/BUILD.gn index 70c462ad4..59d3c2ef7 100644 --- a/tests/core/frame/unittest/BUILD.gn +++ b/tests/core/frame/unittest/BUILD.gn @@ -44,6 +44,7 @@ ohos_unittest("SoftbusServerStubTest") { "$dsoftbus_root_path/core/transmission/common/include", "$dsoftbus_root_path/core/transmission/session/include", "$dsoftbus_root_path/core/transmission/trans_channel/manager/include", + "$dsoftbus_root_path/core/transmission/trans_channel/tcp_direct/include", "$dsoftbus_root_path/interfaces/kits/common", "$dsoftbus_root_path/interfaces/kits/discovery", "$dsoftbus_root_path/interfaces/kits/transport", diff --git a/tests/core/transmission/trans_channel/manager/trans_channel_manager_test/trans_channel_manager_test.cpp b/tests/core/transmission/trans_channel/manager/trans_channel_manager_test/trans_channel_manager_test.cpp index d4fcf8616..13a5d310a 100644 --- a/tests/core/transmission/trans_channel/manager/trans_channel_manager_test/trans_channel_manager_test.cpp +++ b/tests/core/transmission/trans_channel/manager/trans_channel_manager_test/trans_channel_manager_test.cpp @@ -788,7 +788,7 @@ HWTEST_F(TransChannelManagerTest, GetAuthAppInfo005, TestSize.Level1) HWTEST_F(TransChannelManagerTest, TransGetAndComparePid001, TestSize.Level1) { int32_t ret = TransGetAndComparePid(TRANS_TEST_PID, 1, CHANNEL_TYPE_TCP_DIRECT); - EXPECT_EQ(SOFTBUS_OK, ret); + EXPECT_EQ(SOFTBUS_INVALID_PARAM, ret); ret = TransGetAndComparePid(TRANS_TEST_PID, 1, CHANNEL_TYPE_AUTH); EXPECT_EQ(SOFTBUS_INVALID_PARAM, ret); } From dbc9cdfe7280f5644948b0bf7546cc1283d0e8ab Mon Sep 17 00:00:00 2001 From: xzh457 Date: Fri, 13 Sep 2024 18:30:14 +0800 Subject: [PATCH 2/2] add pid check Signed-off-by: xzh457 Change-Id: I101b2073a0d325ac4d30d597f2e15ce332d8ff43 --- .../unittest/softbus_server_stub_test.cpp | 32 ------------------- 1 file changed, 32 deletions(-) diff --git a/tests/core/frame/unittest/softbus_server_stub_test.cpp b/tests/core/frame/unittest/softbus_server_stub_test.cpp index f734a66df..60122e616 100644 --- a/tests/core/frame/unittest/softbus_server_stub_test.cpp +++ b/tests/core/frame/unittest/softbus_server_stub_test.cpp @@ -213,38 +213,6 @@ HWTEST_F(SoftbusServerStubTest, SoftbusServerStubTest002, TestSize.Level1) EXPECT_EQ(SOFTBUS_PERMISSION_DENIED, ret); } -/** - * @tc.name: SoftbusServerStubTest003 - * @tc.desc: Verify the CheckPidByChannelId function. - * @tc.type: FUNC - * @tc.require: - */ -HWTEST_F(SoftbusServerStubTest, SoftbusServerStubTest003, TestSize.Level1) -{ - sptr softBusServer = new OHOS::SoftBusServer(SOFTBUS_SERVER_SA_ID, true); - ASSERT_NE(nullptr, softBusServer); - NiceMock softbusServerStubMock; - pid_t callingPid = 0; - int32_t channelId = 0; - int32_t channelType = 0; - - EXPECT_CALL(softbusServerStubMock, TransGetAppInfoByChanId).WillRepeatedly(Return(SOFTBUS_INVALID_PARAM)); - int32_t ret = softBusServer->CheckPidByChannelId(callingPid, channelId, channelType); - EXPECT_EQ(SOFTBUS_INVALID_PARAM, ret); - - EXPECT_CALL(softbusServerStubMock, TransGetAppInfoByChanId).WillRepeatedly(Return(SOFTBUS_NOT_FIND)); - ret = softBusServer->CheckPidByChannelId(callingPid, channelId, channelType); - EXPECT_EQ(SOFTBUS_OK, ret); - - EXPECT_CALL(softbusServerStubMock, TransGetAppInfoByChanId).WillRepeatedly(Return(SOFTBUS_OK)); - ret = softBusServer->CheckPidByChannelId(callingPid, channelId, channelType); - EXPECT_EQ(SOFTBUS_OK, ret); - - callingPid = -1; - ret = softBusServer->CheckPidByChannelId(callingPid, channelId, channelType); - EXPECT_EQ(SOFTBUS_TRANS_CHECK_PID_ERROR, ret); -} - /** * @tc.name: SoftbusServerStubTest004 * @tc.desc: Verify the CheckAndRecordAccessToken function.