Skip to content

Conversation

@zwass
Copy link
Member

@zwass zwass commented Feb 20, 2025

Uses ETW events to get DNS lookups.

Includes some refactors to move shared functionality into the parent class for ETW publishers.

Tested manually and includes integration test.

@zwass zwass added virtual tables events Related to osquery's evented tables or eventing subsystem Windows labels Feb 20, 2025
@zwass zwass requested review from a team as code owners February 20, 2025 19:57
}
}

void EtwPublisherProcesses::initializeHardVolumeConversions() {
Copy link
Member Author

Choose a reason for hiding this comment

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

Refactored to parent class.

Comment on lines -412 to -472
void EtwPublisherProcesses::updateHardVolumeWithLogicalDrive(
std::string& path) {
// Updating the hardvolume entries with logical volume data
for (const auto& [hardVolume, logicalDrive] : hardVolumeDrives_) {
size_t pos = 0;
if ((pos = path.find(hardVolume, pos)) != std::string::npos) {
path.replace(pos, hardVolume.length(), logicalDrive);
break;
}
}
}

void EtwPublisherProcesses::updateUserInfo(const std::string& userSid,
std::string& username) {
// Updating user information using gathered user SIDs as input
auto usernameIt = usernamesBySIDs_.find(userSid);
if (usernameIt != usernamesBySIDs_.end()) {
auto cachedUsername = usernameIt->second;
username.assign(cachedUsername);
} else {
PSID pSid = nullptr;

if (!ConvertStringSidToSidA(userSid.c_str(), &pSid) || pSid == nullptr) {
// Inserting empty username to avoid the lookup logic to be called again
usernamesBySIDs_.insert({userSid, ""});
return;
}

std::vector<char> domainNameStr(MAX_PATH - 1, 0x0);
std::vector<char> userNameStr(MAX_PATH - 1, 0x0);
DWORD domainNameSize = MAX_PATH;
DWORD userNameSize = MAX_PATH;
SID_NAME_USE sidType = SID_NAME_USE::SidTypeInvalid;

if (!LookupAccountSidA(NULL,
pSid,
userNameStr.data(),
&userNameSize,
domainNameStr.data(),
&domainNameSize,
&sidType) ||
strlen(domainNameStr.data()) == 0 ||
strlen(domainNameStr.data()) >= MAX_PATH ||
strlen(userNameStr.data()) == 0 ||
strlen(userNameStr.data()) >= MAX_PATH ||
sidType == SID_NAME_USE::SidTypeInvalid) {
// Inserting empty username to avoid the lookup logic to be called again
usernamesBySIDs_.insert({userSid, ""});
LocalFree(pSid);
return;
}

LocalFree(pSid);

username.append(domainNameStr.data());
username.append("\\");
username.append(userNameStr.data());

usernamesBySIDs_.insert({userSid, username});
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Refactored to parent class

Copy link
Member Author

@zwass zwass left a comment

Choose a reason for hiding this comment

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

Suggestions from @Smjert.

} else {
PSID pSid = nullptr;

if (!ConvertStringSidToSidA(userSid.c_str(), &pSid) || pSid == nullptr) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Use unicode version

Copy link
Member Author

Choose a reason for hiding this comment

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

Addressing this in a separate PR as it will affect both the existing process events table and this one.

DWORD userNameSize = MAX_PATH;
SID_NAME_USE sidType = SID_NAME_USE::SidTypeInvalid;

if (!LookupAccountSidA(NULL,
Copy link
Member Author

Choose a reason for hiding this comment

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

Use unicode version

directionless
directionless previously approved these changes Mar 29, 2025
Copy link
Member

@directionless directionless left a comment

Choose a reason for hiding this comment

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

Didn't test, seems fine.

directionless
directionless previously approved these changes Apr 8, 2025
@zwass
Copy link
Member Author

zwass commented Apr 9, 2025

@directionless I fixed up the remaining test issues. Please re-review, thanks!

@zwass zwass requested a review from directionless April 9, 2025 22:13
Copy link
Member

@sharvilshah sharvilshah left a comment

Choose a reason for hiding this comment

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

Haven't tested it, but looks good

@zwass zwass merged commit c18a20d into osquery:master Apr 10, 2025
22 checks passed
@zwass zwass deleted the dns-lookup-events branch April 10, 2025 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

events Related to osquery's evented tables or eventing subsystem virtual tables Windows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants