diff --git a/handler/mac/crash_report_exception_handler.cc b/handler/mac/crash_report_exception_handler.cc index b3b9605f..313d4e3b 100644 --- a/handler/mac/crash_report_exception_handler.cc +++ b/handler/mac/crash_report_exception_handler.cc @@ -29,6 +29,7 @@ #include "util/mach/exc_client_variants.h" #include "util/mach/exception_behaviors.h" #include "util/mach/mach_extensions.h" +#include "util/mach/mach_message.h" #include "util/mach/scoped_task_suspend.h" #include "util/misc/tri_state.h" #include "util/misc/uuid.h" @@ -119,6 +120,22 @@ kern_return_t CrashReportExceptionHandler::CatchMachException( return KERN_FAILURE; } + // Check for suspicious message sources. A suspicious exception message comes + // from a source other than the kernel or the process that the exception + // purportedly occurred in. + // + // TODO(mark): Consider exceptions outside of the range (0, 32) from the + // kernel to be suspicious, and exceptions other than kMachExceptionSimulated + // from the process itself to be suspicious. + pid_t audit_pid = AuditPIDFromMachMessageTrailer(trailer); + if (audit_pid != -1 && audit_pid != 0) { + pid_t exception_pid = process_snapshot.ProcessID(); + if (exception_pid != audit_pid) { + LOG(WARNING) << "exception for pid " << exception_pid << " sent by pid " + << audit_pid; + } + } + CrashpadInfoClientOptions client_options; process_snapshot.GetCrashpadOptions(&client_options); diff --git a/handler/mac/exception_handler_server.cc b/handler/mac/exception_handler_server.cc index 803ebfdc..faf8b6e0 100644 --- a/handler/mac/exception_handler_server.cc +++ b/handler/mac/exception_handler_server.cc @@ -26,9 +26,8 @@ namespace crashpad { namespace { -class ExceptionHandlerServerRun - : public UniversalMachExcServer::Interface, - public NotifyServer::Interface { +class ExceptionHandlerServerRun : public UniversalMachExcServer::Interface, + public NotifyServer::Interface { public: ExceptionHandlerServerRun( mach_port_t exception_port, @@ -100,7 +99,7 @@ class ExceptionHandlerServerRun mach_msg_return_t mr = MachMessageServer::Run(&composite_mach_message_server_, server_port_set, - MACH_MSG_OPTION_NONE, + kMachMessageReceiveAuditTrailer, MachMessageServer::kOneShot, MachMessageServer::kReceiveLargeIgnore, kMachMessageTimeoutWaitIndefinitely); @@ -148,9 +147,9 @@ class ExceptionHandlerServerRun // NotifyServer::Interface: kern_return_t DoMachNotifyPortDeleted( - notify_port_t notify, - mach_port_name_t name, - const mach_msg_trailer_t* trailer) override { + notify_port_t notify, + mach_port_name_t name, + const mach_msg_trailer_t* trailer) override { return UnimplementedNotifyRoutine(notify); } diff --git a/util/mach/exception_ports_test.cc b/util/mach/exception_ports_test.cc index 8948eae7..155e9187 100644 --- a/util/mach/exception_ports_test.cc +++ b/util/mach/exception_ports_test.cc @@ -197,6 +197,8 @@ class TestExceptionPorts : public MachMultiprocess, SetExpectedChildTermination(kTerminationSignal, signal); } + EXPECT_EQ(0, AuditPIDFromMachMessageTrailer(trailer)); + return ExcServerSuccessfulReturnValue(behavior, false); } @@ -443,7 +445,7 @@ class TestExceptionPorts : public MachMultiprocess, kern_return_t kr = MachMessageServer::Run(&universal_mach_exc_server, local_port, - MACH_MSG_OPTION_NONE, + kMachMessageReceiveAuditTrailer, MachMessageServer::kOneShot, MachMessageServer::kReceiveLargeError, kTimeoutMs); diff --git a/util/mach/mach_message.cc b/util/mach/mach_message.cc index 4d23eb25..8092f26c 100644 --- a/util/mach/mach_message.cc +++ b/util/mach/mach_message.cc @@ -14,9 +14,13 @@ #include "util/mach/mach_message.h" +#include +#include + #include #include "base/basictypes.h" +#include "base/logging.h" #include "util/misc/clock.h" namespace crashpad { @@ -213,4 +217,36 @@ const mach_msg_trailer_t* MachMessageTrailerFromHeader( return reinterpret_cast(trailer_address); } +pid_t AuditPIDFromMachMessageTrailer(const mach_msg_trailer_t* trailer) { + if (trailer->msgh_trailer_type != MACH_MSG_TRAILER_FORMAT_0) { + LOG(ERROR) << "unexpected msgh_trailer_type " << trailer->msgh_trailer_type; + return -1; + } + if (trailer->msgh_trailer_size < + REQUESTED_TRAILER_SIZE(kMachMessageReceiveAuditTrailer)) { + LOG(ERROR) << "small msgh_trailer_size " << trailer->msgh_trailer_size; + return -1; + } + + const mach_msg_audit_trailer_t* audit_trailer = + reinterpret_cast(trailer); + +#if MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_8 + pid_t audit_pid; + audit_token_to_au32(audit_trailer->msgh_audit, + nullptr, + nullptr, + nullptr, + nullptr, + nullptr, + &audit_pid, + nullptr, + nullptr); +#else + pid_t audit_pid = audit_token_to_pid(audit_trailer->msgh_audit); +#endif + + return audit_pid; +} + } // namespace crashpad diff --git a/util/mach/mach_message.h b/util/mach/mach_message.h index 05695404..be882c17 100644 --- a/util/mach/mach_message.h +++ b/util/mach/mach_message.h @@ -17,9 +17,19 @@ #include #include +#include namespace crashpad { +//! \brief A Mach message option specifying that an audit trailer should be +//! delivered during a receive operation. +//! +//! This constant is provided because the macros normally used to request this +//! behavior are cumbersome. +const mach_msg_option_t kMachMessageReceiveAuditTrailer = + MACH_RCV_TRAILER_TYPE(MACH_MSG_TRAILER_FORMAT_0) | + MACH_RCV_TRAILER_ELEMENTS(MACH_RCV_TRAILER_AUDIT); + //! \brief Special constants used as `mach_msg_timeout_t` values. enum : mach_msg_timeout_t { //! \brief When passed to MachMessageDeadlineFromTimeout(), that function will @@ -148,6 +158,22 @@ void SetMIGReplyError(mach_msg_header_t* out_header, kern_return_t error); const mach_msg_trailer_t* MachMessageTrailerFromHeader( const mach_msg_header_t* header); +//! \brief Returns the process ID of a Mach message’s sender from its audit +//! trailer. +//! +//! For the audit trailer to be present, the message must have been received +//! with #kMachMessageReceiveAuditTrailer or its macro equivalent specified in +//! the receive options. +//! +//! If the kernel is the message’s sender, a process ID of `0` will be returned. +//! +//! \param[in] trailer The trailer received with a Mach message. +//! +//! \return The process ID of the message’s sender, or `-1` on failure with a +//! message logged. It is considered a failure for \a trailer to not contain +//! audit information. +pid_t AuditPIDFromMachMessageTrailer(const mach_msg_trailer_t* trailer); + } // namespace crashpad #endif // CRASHPAD_UTIL_MACH_MACH_MESSAGE_H_ diff --git a/util/mach/mach_message_test.cc b/util/mach/mach_message_test.cc index 9877e2d1..578195a3 100644 --- a/util/mach/mach_message_test.cc +++ b/util/mach/mach_message_test.cc @@ -14,9 +14,13 @@ #include "util/mach/mach_message.h" +#include + #include "base/basictypes.h" +#include "base/mac/scoped_mach_port.h" #include "gtest/gtest.h" #include "util/mach/mach_extensions.h" +#include "util/test/mac/mach_errors.h" namespace crashpad { namespace test { @@ -104,6 +108,46 @@ TEST(MachMessage, MachMessageTrailerFromHeader) { EXPECT_EQ(&test.receive.trailer, MachMessageTrailerFromHeader(&test.receive)); } +TEST(MachMessage, AuditPIDFromMachMessageTrailer) { + base::mac::ScopedMachReceiveRight port(NewMachPort(MACH_PORT_RIGHT_RECEIVE)); + ASSERT_NE(kMachPortNull, port); + + mach_msg_empty_send_t send = {}; + send.header.msgh_bits = MACH_MSGH_BITS(MACH_MSG_TYPE_MAKE_SEND_ONCE, 0); + send.header.msgh_size = sizeof(send); + send.header.msgh_remote_port = port; + mach_msg_return_t mr = + MachMessageWithDeadline(&send.header, + MACH_SEND_MSG, + 0, + MACH_PORT_NULL, + kMachMessageDeadlineNonblocking, + MACH_PORT_NULL, + false); + ASSERT_EQ(MACH_MSG_SUCCESS, mr) + << MachErrorMessage(mr, "MachMessageWithDeadline send"); + + struct EmptyReceiveMessageWithAuditTrailer : public mach_msg_empty_send_t { + union { + mach_msg_trailer_t trailer; + mach_msg_audit_trailer_t audit_trailer; + }; + }; + + EmptyReceiveMessageWithAuditTrailer receive; + mr = MachMessageWithDeadline(&receive.header, + MACH_RCV_MSG | kMachMessageReceiveAuditTrailer, + sizeof(receive), + port, + kMachMessageDeadlineNonblocking, + MACH_PORT_NULL, + false); + ASSERT_EQ(MACH_MSG_SUCCESS, mr) + << MachErrorMessage(mr, "MachMessageWithDeadline receive"); + + EXPECT_EQ(getpid(), AuditPIDFromMachMessageTrailer(&receive.trailer)); +} + } // namespace } // namespace test } // namespace crashpad diff --git a/util/mach/notify_server_test.cc b/util/mach/notify_server_test.cc index 4799f143..f835cedf 100644 --- a/util/mach/notify_server_test.cc +++ b/util/mach/notify_server_test.cc @@ -30,7 +30,6 @@ namespace { using testing::AllOf; using testing::Eq; using testing::Invoke; -using testing::Ne; using testing::Pointee; using testing::ResultOf; using testing::Return; @@ -239,7 +238,7 @@ class NotifyServerTestBase : public testing::Test, mach_msg_return_t mr = MachMessageServer::Run(¬ify_server, ServerPort(), - MACH_MSG_OPTION_NONE, + kMachMessageReceiveAuditTrailer, MachMessageServer::kPersistent, MachMessageServer::kReceiveLargeError, kMachMessageTimeoutNonblocking); @@ -315,12 +314,14 @@ TEST_F(NotifyServerTest, MachNotifyPortDeleted) { SendOnceRightFromReceiveRight(receive_right)); ASSERT_NE(kMachPortNull, send_once_right); - ASSERT_TRUE(RequestMachPortNotification( - send_once_right, MACH_NOTIFY_DEAD_NAME, 0)); + ASSERT_TRUE( + RequestMachPortNotification(send_once_right, MACH_NOTIFY_DEAD_NAME, 0)); - EXPECT_CALL(*this, DoMachNotifyPortDeleted(ServerPort(), - send_once_right.get(), - Ne(nullptr))) + EXPECT_CALL( + *this, + DoMachNotifyPortDeleted(ServerPort(), + send_once_right.get(), + ResultOf(AuditPIDFromMachMessageTrailer, 0))) .WillOnce(Return(MIG_NO_REPLY)) .RetiresOnSaturation(); @@ -339,10 +340,12 @@ TEST_F(NotifyServerTest, MachNotifyPortDestroyed) { ASSERT_TRUE(RequestMachPortNotification( receive_right, MACH_NOTIFY_PORT_DESTROYED, 0)); - EXPECT_CALL(*this, DoMachNotifyPortDestroyed(ServerPort(), - ResultOf(IsReceiveRight, true), - Ne(nullptr), - Pointee(Eq(false)))) + EXPECT_CALL( + *this, + DoMachNotifyPortDestroyed(ServerPort(), + ResultOf(IsReceiveRight, true), + ResultOf(AuditPIDFromMachMessageTrailer, 0), + Pointee(Eq(false)))) .WillOnce(DoAll(SetArgPointee<3>(true), Return(MIG_NO_REPLY))) .RetiresOnSaturation(); @@ -371,10 +374,12 @@ TEST_F(NotifyServerTest, MachNotifyNoSenders_NoSendRight) { NewMachPort(MACH_PORT_RIGHT_RECEIVE)); ASSERT_NE(kMachPortNull, receive_right); - ASSERT_TRUE(RequestMachPortNotification( - receive_right, MACH_NOTIFY_NO_SENDERS, 0)); + ASSERT_TRUE( + RequestMachPortNotification(receive_right, MACH_NOTIFY_NO_SENDERS, 0)); - EXPECT_CALL(*this, DoMachNotifyNoSenders(ServerPort(), 0, Ne(nullptr))) + EXPECT_CALL(*this, + DoMachNotifyNoSenders( + ServerPort(), 0, ResultOf(AuditPIDFromMachMessageTrailer, 0))) .WillOnce(Return(MIG_NO_REPLY)) .RetiresOnSaturation(); @@ -393,10 +398,12 @@ TEST_F(NotifyServerTest, MachNotifyNoSenders_SendRightDeallocated) { SendRightFromReceiveRight(receive_right)); ASSERT_NE(kMachPortNull, send_right); - ASSERT_TRUE(RequestMachPortNotification( - receive_right, MACH_NOTIFY_NO_SENDERS, 1)); + ASSERT_TRUE( + RequestMachPortNotification(receive_right, MACH_NOTIFY_NO_SENDERS, 1)); - EXPECT_CALL(*this, DoMachNotifyNoSenders(ServerPort(), 1, Ne(nullptr))) + EXPECT_CALL(*this, + DoMachNotifyNoSenders( + ServerPort(), 1, ResultOf(AuditPIDFromMachMessageTrailer, 0))) .WillOnce(Return(MIG_NO_REPLY)) .RetiresOnSaturation(); @@ -420,8 +427,8 @@ TEST_F(NotifyServerTest, MachNotifyNoSenders_NoNotification) { SendRightFromReceiveRight(receive_right)); ASSERT_NE(kMachPortNull, send_right_1); - ASSERT_TRUE(RequestMachPortNotification( - receive_right, MACH_NOTIFY_NO_SENDERS, 1)); + ASSERT_TRUE( + RequestMachPortNotification(receive_right, MACH_NOTIFY_NO_SENDERS, 1)); send_right_1.reset(); @@ -438,7 +445,9 @@ TEST_F(NotifyServerTest, MachNotifySendOnce_ExplicitDeallocation) { SendOnceRightFromReceiveRight(ServerPort())); ASSERT_NE(kMachPortNull, send_once_right); - EXPECT_CALL(*this, DoMachNotifySendOnce(ServerPort(), Ne(nullptr))) + EXPECT_CALL(*this, + DoMachNotifySendOnce(ServerPort(), + ResultOf(AuditPIDFromMachMessageTrailer, 0))) .WillOnce(Return(MIG_NO_REPLY)) .RetiresOnSaturation(); @@ -470,7 +479,9 @@ TEST_F(NotifyServerTest, MachNotifySendOnce_ImplicitDeallocation) { MACH_PORT_NULL); ASSERT_EQ(MACH_MSG_SUCCESS, mr) << MachErrorMessage(mr, "mach_msg"); - EXPECT_CALL(*this, DoMachNotifySendOnce(ServerPort(), Ne(nullptr))) + EXPECT_CALL(*this, + DoMachNotifySendOnce(ServerPort(), + ResultOf(AuditPIDFromMachMessageTrailer, 0))) .WillOnce(Return(MIG_NO_REPLY)) .RetiresOnSaturation(); @@ -491,8 +502,8 @@ TEST_F(NotifyServerTest, MachNotifyDeadName) { SendOnceRightFromReceiveRight(receive_right)); ASSERT_NE(kMachPortNull, send_once_right); - ASSERT_TRUE(RequestMachPortNotification( - send_once_right, MACH_NOTIFY_DEAD_NAME, 0)); + ASSERT_TRUE( + RequestMachPortNotification(send_once_right, MACH_NOTIFY_DEAD_NAME, 0)); // send_once_right becomes a dead name with the send-once right’s original // user reference count of 1, but the dead-name notification increments the @@ -502,9 +513,9 @@ TEST_F(NotifyServerTest, MachNotifyDeadName) { DoMachNotifyDeadName(ServerPort(), AllOf(send_once_right.get(), ResultOf(DeadNameRightRefCount, 2)), - Ne(nullptr))) - .WillOnce(DoAll(WithArg<1>(Invoke(MachPortDeallocate)), - Return(MIG_NO_REPLY))) + ResultOf(AuditPIDFromMachMessageTrailer, 0))) + .WillOnce( + DoAll(WithArg<1>(Invoke(MachPortDeallocate)), Return(MIG_NO_REPLY))) .RetiresOnSaturation(); receive_right.reset(); @@ -529,8 +540,8 @@ TEST_F(NotifyServerTest, MachNotifyDeadName_NoNotification) { SendOnceRightFromReceiveRight(receive_right)); ASSERT_NE(kMachPortNull, send_once_right); - ASSERT_TRUE(RequestMachPortNotification( - send_once_right, MACH_NOTIFY_DEAD_NAME, 0)); + ASSERT_TRUE( + RequestMachPortNotification(send_once_right, MACH_NOTIFY_DEAD_NAME, 0)); RunServer(); diff --git a/util/test/mac/mach_multiprocess.cc b/util/test/mac/mach_multiprocess.cc index 9660ee0e..c558eb05 100644 --- a/util/test/mac/mach_multiprocess.cc +++ b/util/test/mac/mach_multiprocess.cc @@ -28,6 +28,7 @@ #include "gtest/gtest.h" #include "util/file/file_io.h" #include "util/mach/mach_extensions.h" +#include "util/mach/mach_message.h" #include "util/misc/scoped_forbid_return.h" #include "util/test/errors.h" #include "util/test/mac/mach_errors.h" @@ -40,7 +41,10 @@ struct SendHelloMessage : public mach_msg_base_t { }; struct ReceiveHelloMessage : public SendHelloMessage { - mach_msg_audit_trailer_t audit_trailer; + union { + mach_msg_trailer_t trailer; + mach_msg_audit_trailer_t audit_trailer; + }; }; } // namespace @@ -120,15 +124,13 @@ task_t MachMultiprocess::ChildTask() const { void MachMultiprocess::MultiprocessParent() { ReceiveHelloMessage message = {}; - kern_return_t kr = - mach_msg(&message.header, - MACH_RCV_MSG | MACH_RCV_TRAILER_TYPE(MACH_MSG_TRAILER_FORMAT_0) | - MACH_RCV_TRAILER_ELEMENTS(MACH_RCV_TRAILER_AUDIT), - 0, - sizeof(message), - info_->local_port, - MACH_MSG_TIMEOUT_NONE, - MACH_PORT_NULL); + kern_return_t kr = mach_msg(&message.header, + MACH_RCV_MSG | kMachMessageReceiveAuditTrailer, + 0, + sizeof(message), + info_->local_port, + MACH_MSG_TIMEOUT_NONE, + MACH_PORT_NULL); ASSERT_EQ(MACH_MSG_SUCCESS, kr) << MachErrorMessage(kr, "mach_msg"); // Comb through the entire message, checking every field against its expected @@ -186,6 +188,8 @@ void MachMultiprocess::MultiprocessParent() { EXPECT_EQ(getgid(), audit_rgid); ASSERT_EQ(ChildPID(), audit_pid); + ASSERT_EQ(ChildPID(), AuditPIDFromMachMessageTrailer(&message.trailer)); + auditinfo_addr_t audit_info; int rv = getaudit_addr(&audit_info, sizeof(audit_info)); ASSERT_EQ(0, rv) << ErrnoMessage("getaudit_addr"); diff --git a/util/util.gyp b/util/util.gyp index 9101cc70..96f5b15c 100644 --- a/util/util.gyp +++ b/util/util.gyp @@ -199,6 +199,7 @@ '$(SDKROOT)/System/Library/Frameworks/CoreFoundation.framework', '$(SDKROOT)/System/Library/Frameworks/Foundation.framework', '$(SDKROOT)/System/Library/Frameworks/IOKit.framework', + '$(SDKROOT)/usr/lib/libbsm.dylib', ], }, }],