Skip to content

Commit 56beb88

Browse files
relkochtagvisor-bot
authored andcommitted
SCM_CREDENTIALS: use PID from ucred rather than caller's PID
Previously, the caller-provided PID was used for the CAP_SYS_ADMIN check, but wasn't actually passed along. FUTURE_COPYBARA_INTEGRATE_REVIEW=#13590 from relkochta:relkochta/fix-scmcred cd39eb9 PiperOrigin-RevId: 940099397
1 parent bcc0b39 commit 56beb88

3 files changed

Lines changed: 149 additions & 4 deletions

File tree

pkg/sentry/socket/control/control.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,13 @@ func NewSCMCredentials(t *kernel.Task, cred linux.ControlMessageCredentials) (SC
7070
if err != nil {
7171
return nil, err
7272
}
73-
tg := t.ThreadGroup()
74-
if kernel.ThreadID(cred.PID) != tg.ID() && !t.HasCapabilityIn(linux.CAP_SYS_ADMIN, t.PIDNamespace().UserNamespace()) {
73+
if kernel.ThreadID(cred.PID) != t.ThreadGroup().ID() && !t.HasCapabilityIn(linux.CAP_SYS_ADMIN, t.PIDNamespace().UserNamespace()) {
7574
return nil, linuxerr.EPERM
7675
}
76+
tg := t.PIDNamespace().ThreadGroupWithID(kernel.ThreadID(cred.PID))
77+
if tg == nil {
78+
return nil, linuxerr.ESRCH
79+
}
7780
return &scmCredentials{tg.PIDNamespacedIDs(), kuid, kgid}, nil
7881
}
7982

test/syscalls/linux/BUILD

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3816,10 +3816,13 @@ cc_library(
38163816
],
38173817
deps = select_gtest() + [
38183818
":unix_domain_socket_test_util",
3819+
"//test/util:capability_util",
3820+
"//test/util:cleanup",
38193821
"//test/util:socket_util",
38203822
"//test/util:test_util",
38213823
"//test/util:thread_util",
38223824
"@com_google_absl//absl/strings",
3825+
"@com_google_absl//absl/time",
38233826
],
38243827
alwayslink = 1,
38253828
)

test/syscalls/linux/socket_unix_cmsg.cc

Lines changed: 141 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,22 @@
1616

1717
#include <errno.h>
1818
#include <net/if.h>
19-
#include <stdio.h>
19+
#include <signal.h>
2020
#include <sys/ioctl.h>
2121
#include <sys/socket.h>
2222
#include <sys/types.h>
2323
#include <sys/un.h>
24+
#include <sys/wait.h>
25+
#include <unistd.h>
2426

2527
#include <vector>
2628

2729
#include "gtest/gtest.h"
28-
#include "absl/strings/string_view.h"
30+
#include "absl/time/clock.h"
31+
#include "absl/time/time.h"
2932
#include "test/syscalls/linux/unix_domain_socket_test_util.h"
33+
#include "test/util/cleanup.h"
34+
#include "test/util/linux_capability_util.h"
3035
#include "test/util/socket_util.h"
3136
#include "test/util/test_util.h"
3237
#include "test/util/thread_util.h"
@@ -722,6 +727,140 @@ TEST_P(UnixSocketPairCmsgTest, BasicCredPass) {
722727
EXPECT_EQ(sent_creds.gid, received_creds.gid);
723728
}
724729

730+
// A privileged sender (CAP_SYS_ADMIN) may attach SCM_CREDENTIALS naming the PID
731+
// of a different, live process. The receiver must observe that process's PID.
732+
TEST_P(UnixSocketPairCmsgTest, BasicCredPassDifferentPID) {
733+
SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN)));
734+
735+
auto sockets = ASSERT_NO_ERRNO_AND_VALUE(NewSocketPair());
736+
737+
pid_t child = fork();
738+
if (child == 0) {
739+
while (1) {
740+
absl::SleepFor(absl::Seconds(30));
741+
}
742+
_exit(0);
743+
}
744+
ASSERT_THAT(child, SyscallSucceeds());
745+
746+
auto cleanup = Cleanup([&child] {
747+
ASSERT_THAT(kill(child, SIGKILL), SyscallSucceeds());
748+
ASSERT_THAT(RetryEINTR(waitpid)(child, nullptr, 0),
749+
SyscallSucceedsWithValue(child));
750+
});
751+
752+
char sent_data[20];
753+
RandomizeBuffer(sent_data, sizeof(sent_data));
754+
755+
struct ucred sent_creds;
756+
sent_creds.pid = child;
757+
ASSERT_THAT(sent_creds.uid = getuid(), SyscallSucceeds());
758+
ASSERT_THAT(sent_creds.gid = getgid(), SyscallSucceeds());
759+
760+
ASSERT_NO_FATAL_FAILURE(
761+
SendCreds(sockets->first_fd(), sent_creds, sent_data, sizeof(sent_data)));
762+
763+
SetSoPassCred(sockets->second_fd());
764+
765+
char received_data[20];
766+
struct ucred received_creds;
767+
ASSERT_NO_FATAL_FAILURE(RecvCreds(sockets->second_fd(), &received_creds,
768+
received_data, sizeof(received_data)));
769+
770+
EXPECT_EQ(0, memcmp(sent_data, received_data, sizeof(sent_data)));
771+
// The received PID must be the child's, not the sender's.
772+
EXPECT_EQ(received_creds.pid, child);
773+
EXPECT_NE(received_creds.pid, getpid());
774+
EXPECT_EQ(received_creds.uid, sent_creds.uid);
775+
EXPECT_EQ(received_creds.gid, sent_creds.gid);
776+
}
777+
778+
// A sender without CAP_SYS_ADMIN should not be able to send SCM_CREDENTIALS
779+
// messages with a different PID.
780+
TEST_P(UnixSocketPairCmsgTest, CredPassDifferentPIDUnprivileged) {
781+
// Drop CAP_SYS_ADMIN
782+
AutoCapability cap(CAP_SYS_ADMIN, false);
783+
784+
auto sockets = ASSERT_NO_ERRNO_AND_VALUE(NewSocketPair());
785+
786+
pid_t child = fork();
787+
if (child == 0) {
788+
while (1) {
789+
absl::SleepFor(absl::Seconds(30));
790+
}
791+
_exit(0);
792+
}
793+
ASSERT_THAT(child, SyscallSucceeds());
794+
795+
auto cleanup = Cleanup([&child] {
796+
ASSERT_THAT(kill(child, SIGKILL), SyscallSucceeds());
797+
ASSERT_THAT(RetryEINTR(waitpid)(child, nullptr, 0),
798+
SyscallSucceedsWithValue(child));
799+
});
800+
801+
char sent_data[20];
802+
RandomizeBuffer(sent_data, sizeof(sent_data));
803+
804+
struct ucred sent_creds;
805+
sent_creds.pid = child;
806+
ASSERT_THAT(sent_creds.uid = getuid(), SyscallSucceeds());
807+
ASSERT_THAT(sent_creds.gid = getgid(), SyscallSucceeds());
808+
struct msghdr msg = {};
809+
char control[CMSG_SPACE(sizeof(struct ucred))];
810+
msg.msg_control = control;
811+
msg.msg_controllen = sizeof(control);
812+
813+
struct cmsghdr* cmsg = CMSG_FIRSTHDR(&msg);
814+
cmsg->cmsg_level = SOL_SOCKET;
815+
cmsg->cmsg_type = SCM_CREDENTIALS;
816+
cmsg->cmsg_len = CMSG_LEN(sizeof(struct ucred));
817+
memcpy(CMSG_DATA(cmsg), &sent_creds, sizeof(struct ucred));
818+
819+
struct iovec iov;
820+
iov.iov_base = sent_data;
821+
iov.iov_len = sizeof(sent_data);
822+
msg.msg_iov = &iov;
823+
msg.msg_iovlen = 1;
824+
825+
ASSERT_THAT(RetryEINTR(sendmsg)(sockets->first_fd(), &msg, 0),
826+
SyscallFailsWithErrno(EPERM));
827+
}
828+
829+
// Sending SCM_CREDENTIALS naming a PID that does not exist fails with ESRCH.
830+
TEST_P(UnixSocketPairCmsgTest, CredPassNonexistentPID) {
831+
SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN)));
832+
833+
auto sockets = ASSERT_NO_ERRNO_AND_VALUE(NewSocketPair());
834+
835+
char sent_data[20];
836+
RandomizeBuffer(sent_data, sizeof(sent_data));
837+
838+
struct ucred sent_creds;
839+
sent_creds.pid = -1;
840+
ASSERT_THAT(sent_creds.uid = getuid(), SyscallSucceeds());
841+
ASSERT_THAT(sent_creds.gid = getgid(), SyscallSucceeds());
842+
843+
struct msghdr msg = {};
844+
char control[CMSG_SPACE(sizeof(struct ucred))];
845+
msg.msg_control = control;
846+
msg.msg_controllen = sizeof(control);
847+
848+
struct cmsghdr* cmsg = CMSG_FIRSTHDR(&msg);
849+
cmsg->cmsg_level = SOL_SOCKET;
850+
cmsg->cmsg_type = SCM_CREDENTIALS;
851+
cmsg->cmsg_len = CMSG_LEN(sizeof(struct ucred));
852+
memcpy(CMSG_DATA(cmsg), &sent_creds, sizeof(struct ucred));
853+
854+
struct iovec iov;
855+
iov.iov_base = sent_data;
856+
iov.iov_len = sizeof(sent_data);
857+
msg.msg_iov = &iov;
858+
msg.msg_iovlen = 1;
859+
860+
ASSERT_THAT(RetryEINTR(sendmsg)(sockets->first_fd(), &msg, 0),
861+
SyscallFailsWithErrno(ESRCH));
862+
}
863+
725864
TEST_P(UnixSocketPairCmsgTest, SendNullCredsBeforeSoPassCredRecvEnd) {
726865
auto sockets = ASSERT_NO_ERRNO_AND_VALUE(NewSocketPair());
727866

0 commit comments

Comments
 (0)