Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/changelog/3080.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
area: Machine Learning
issues: []
pr: 3080
summary: Reject non-native ABIs in ML seccomp filter (socketcall/getuid collision)
type: bug
15 changes: 9 additions & 6 deletions include/seccomp/CSystemCallFilter.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,23 @@ namespace seccomp {
//! Installs secure computing modes for Linux, macOS and Windows
//!
//! DESCRIPTION:\n
//! ML processes require a subset of system calls to function correctly.
//! These are create a named pipe, connect to a named pipe, read and write
//! no other system calls are necessary and should be resticted to prevent
//! malicious actions.
//! ML processes require a subset of system calls to function correctly:
//! creating a named pipe, connecting to a named pipe, and reading and
//! writing. No other system calls are necessary, so the rest should be
//! restricted to prevent malicious actions.
//!
//! IMPLEMENTATION DECISIONS:\n
//! Implementations are platform specific more details can be found in the
//! particular .cc files.
//!
//! Linux:
//! Seccomp BPF is used to restrict system calls on kernels since 3.5.
//! The filter first requires seccomp_data.arch to match the native ABI
//! (rejecting compat ABIs such as i386 int 0x80 on x86_64, which would
//! otherwise collide with allowlisted syscall numbers).
//!
//! macOs:
//! The sandbox facility is used to restict access to system resources.
//! macOS:
//! The sandbox facility is used to restrict access to system resources.
//!
//! Windows:
//! Job Objects prevent the process spawning another.
Expand Down
20 changes: 16 additions & 4 deletions lib/seccomp/CSystemCallFilter_Linux.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include <core/CLogger.h>

#include <cerrno>
#include <cstddef>
#include <cstdint>
#include <cstring>

Expand All @@ -30,12 +31,23 @@ namespace {
// The x64 ABI should fail these calls
const std::uint32_t UPPER_NR_LIMIT = 0x3FFFFFFF;

// Offset to the nr field in struct seccomp_data
const std::uint32_t SECCOMP_DATA_NR_OFFSET = 0x00;

const struct sock_filter FILTER[] = {
// Reject non-native ABIs before matching syscall numbers. Without this,
// an x86_64 process can issue int 0x80 (i386) and hit number collisions —
// e.g. i386 socketcall (102) matches the allowlisted x86_64 getuid (102).
// See elastic/security#12621 / HackerOne report on ML seccomp bypass.
// This prefix is self-contained (immediate RET on mismatch) so the relative
// jump offsets in the nr allowlist below are unchanged.
BPF_STMT(BPF_LD | BPF_W | BPF_ABS, offsetof(struct seccomp_data, arch)),
#ifdef __x86_64__
BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, AUDIT_ARCH_X86_64, 1, 0),
#elif defined(__aarch64__)
BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, AUDIT_ARCH_AARCH64, 1, 0),
#endif
BPF_STMT(BPF_RET | BPF_K, SECCOMP_RET_ERRNO | (EACCES & SECCOMP_RET_DATA)),

// Load the system call number into accumulator
BPF_STMT(BPF_LD | BPF_W | BPF_ABS, SECCOMP_DATA_NR_OFFSET),
BPF_STMT(BPF_LD | BPF_W | BPF_ABS, offsetof(struct seccomp_data, nr)),

#ifdef __x86_64__
// The statx, rseq and clone3 syscalls won't be defined on a RHEL/CentOS 7 build
Expand Down
97 changes: 97 additions & 0 deletions lib/seccomp/unittest/CSystemCallFilterTest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,15 @@
#include <boost/test/unit_test.hpp>

#include <atomic>
#include <cerrno>
#include <cstdlib>
#include <string>

#if defined(Linux) && defined(__x86_64__)
#include <csetjmp>
#include <csignal>
#endif

BOOST_AUTO_TEST_SUITE(CSystemCallFilterTest)

namespace {
Expand Down Expand Up @@ -162,6 +168,71 @@ bool versionIsBefore3_5(int major, int minor) {
return false;
}
#endif

#if defined(Linux) && defined(__x86_64__)
// i386 ABI numbers from asm/unistd_32.h. Not available as macros when
// compiling a pure x86_64 translation unit, so hard-code the ones we need.
constexpr long I386_NR_GETUID{24};
// Collides with allowlisted x86_64 getuid (102) — the HackerOne #12621 case.
constexpr long I386_NR_SOCKETCALL{102};

sigjmp_buf g_i386ProbeJmpBuf;

void i386ProbeSignalHandler(int /*sig*/) {
siglongjmp(g_i386ProbeJmpBuf, 1);
}

// Issue a 32-bit ABI syscall via int $0x80. Returns false if the kernel
// lacks IA32 emulation (fault), otherwise writes the syscall result to result.
bool tryI386Syscall(long nr, long& result) {
struct sigaction faultHandler {};
faultHandler.sa_handler = &i386ProbeSignalHandler;
sigemptyset(&faultHandler.sa_mask);
faultHandler.sa_flags = 0;

struct sigaction previousSegv {};
struct sigaction previousIll {};
BOOST_TEST_REQUIRE(sigaction(SIGSEGV, &faultHandler, &previousSegv) == 0);
BOOST_TEST_REQUIRE(sigaction(SIGILL, &faultHandler, &previousIll) == 0);

bool available{false};
if (sigsetjmp(g_i386ProbeJmpBuf, 1) == 0) {
long ret;
// The i386 ABI passes arguments in ebx/ecx/edx/esi/edi; zero them so the
// probe is deterministic and side-effect free if the syscall is ever
// (wrongly) permitted rather than denied. "cc" is clobbered because
// int $0x80 (and the kernel entry path) may modify the condition-code
// flags. (ebp is the rarely-used 6th arg; none of the probed calls use
// it, and it has no simple GCC constraint.)
// NOLINTNEXTLINE(hicpp-no-assembler)
asm volatile("int $0x80"
: "=a"(ret)
: "a"(nr), "b"(0), "c"(0), "d"(0), "S"(0), "D"(0)
: "memory", "cc");
result = ret;
Comment thread
edsavage marked this conversation as resolved.
available = true;
}

BOOST_TEST_REQUIRE(sigaction(SIGSEGV, &previousSegv, nullptr) == 0);
BOOST_TEST_REQUIRE(sigaction(SIGILL, &previousIll, nullptr) == 0);
return available;
}

long i386Syscall(long nr) {
long ret;
// Zero the i386 argument registers (ebx/ecx/edx/esi/edi) so the call is
// deterministic and side-effect free in the failure mode this test guards
// against (the syscall being permitted instead of denied). "cc" is
// clobbered because int $0x80 (and the kernel entry path) may modify the
// condition-code flags.
// NOLINTNEXTLINE(hicpp-no-assembler)
asm volatile("int $0x80"
: "=a"(ret)
: "a"(nr), "b"(0), "c"(0), "d"(0), "S"(0), "D"(0)
: "memory", "cc");
return ret;
Comment thread
edsavage marked this conversation as resolved.
}
#endif // Linux && __x86_64__
}

BOOST_AUTO_TEST_CASE(testSystemCallFilter) {
Expand All @@ -185,9 +256,35 @@ BOOST_AUTO_TEST_CASE(testSystemCallFilter) {
// system call filters are applied
BOOST_TEST_REQUIRE(systemCall());

#if defined(Linux) && defined(__x86_64__)
// Soft-detect a *usable* IA32 compat entry. Kernels without
// CONFIG_IA32_EMULATION (or with it disabled at runtime) fault on int $0x80
// from a 64-bit process; some configurations instead stub it to return
// -ENOSYS. In either case there is no compat ABI to reject, so skip the
// assertion rather than fail CI.
long i386GetuidResult{0};
const bool i386EntryAvailable{tryI386Syscall(I386_NR_GETUID, i386GetuidResult)};
// i386 getuid cannot fail, so a non-negative result confirms the call was
// actually serviced by the 32-bit compat table (rather than stubbed out).
const bool i386CompatUsable{i386EntryAvailable && i386GetuidResult >= 0};
if (i386CompatUsable) {
LOG_INFO(<< "i386 syscall entry available; will assert arch denial after filter install");
} else {
LOG_INFO(<< "i386 syscall entry unavailable; skipping compat-ABI seccomp assertion");
}
#endif

// Install the filter
ml::seccomp::CSystemCallFilter::installSystemCallFilter();

#if defined(Linux) && defined(__x86_64__)
if (i386CompatUsable) {
// Without the seccomp_data.arch gate, i386 socketcall (102) matches
// allowlisted x86_64 getuid (102) and is wrongly permitted.
BOOST_REQUIRE_EQUAL(-EACCES, i386Syscall(I386_NR_SOCKETCALL));
}
#endif

BOOST_REQUIRE_MESSAGE(systemCall() == false, "Calling std::system should fail");

// Operations that must function after seccomp is initialised
Expand Down