Skip to content

Add test to use sid in user mode#332

Open
LakshK98 wants to merge 3 commits intomicrosoft:mainfrom
LakshK98:use_sid_test
Open

Add test to use sid in user mode#332
LakshK98 wants to merge 3 commits intomicrosoft:mainfrom
LakshK98:use_sid_test

Conversation

@LakshK98
Copy link
Copy Markdown
Contributor

@LakshK98 LakshK98 commented Apr 17, 2026

Added a new test case to verify that Security Identifiers (SIDs) written by a BPF program in kernel mode can be correctly received and validated in user mode using Windows APIs. The test uses bpf_prog_test_run to ensure that a SID sent through a ring buffer can be validated entirely in user mode.

Testing

NA

Documentation

NA

Installation

NA

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new ntosebpfext unit test intended to validate that a SID emitted by the process monitor eBPF program via ring buffer can be consumed in user mode and validated using Windows SID APIs.

Changes:

  • Added a ring-buffer callback that captures token_sid data for a test process ID.
  • Added a new unit test that runs the ProcessMonitor eBPF program via bpf_prog_test_run_opts, waits for a ring-buffer event, and validates the received SID using user-mode SID APIs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/ntosebpfext/ntosebpfext_unit/ntos_ebpfext_unit.cpp
Comment thread tests/ntosebpfext/ntosebpfext_unit/ntos_ebpfext_unit.cpp Outdated
Copilot AI review requested due to automatic review settings April 17, 2026 19:16
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

// The Local System SID (S-1-5-18) has 1 sub-authority.
PUCHAR sub_authority_count = GetSidSubAuthorityCount(received_sid);
REQUIRE(sub_authority_count != nullptr);
REQUIRE(*sub_authority_count == 1);
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test currently only asserts that the received SID is valid and has SubAuthorityCount == 1. This would still pass if the program/user-mode plumbing returned a different valid SID that happens to have one sub-authority (e.g., S-1-5-19), so it doesn’t fully verify that the intended SID (Local System: S-1-5-18) was received. Consider additionally validating the identifier authority and the first sub-authority value via Windows APIs (e.g., GetSidIdentifierAuthority / GetSidSubAuthority or ConvertSidToStringSidW), without comparing against the original input bytes.

Suggested change
REQUIRE(*sub_authority_count == 1);
REQUIRE(*sub_authority_count == 1);
// Fully validate that the SID received in user mode is Local System (S-1-5-18),
// not just any valid SID with one sub-authority.
PSID_IDENTIFIER_AUTHORITY identifier_authority = GetSidIdentifierAuthority(received_sid);
REQUIRE(identifier_authority != nullptr);
REQUIRE(memcmp(identifier_authority->Value, SECURITY_NT_AUTHORITY.Value, sizeof(identifier_authority->Value)) == 0);
PDWORD first_sub_authority = GetSidSubAuthority(received_sid, 0);
REQUIRE(first_sub_authority != nullptr);
REQUIRE(*first_sub_authority == SECURITY_LOCAL_SYSTEM_RID);

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

2 participants