-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Implement dns_lookup_events table on Windows #8553
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| } | ||
| } | ||
|
|
||
| void EtwPublisherProcesses::initializeHardVolumeConversions() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactored to parent class.
| 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}); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactored to parent class
zwass
left a comment
There was a problem hiding this 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) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use unicode version
There was a problem hiding this comment.
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, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use unicode version
directionless
left a comment
There was a problem hiding this 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 I fixed up the remaining test issues. Please re-review, thanks! |
sharvilshah
left a comment
There was a problem hiding this 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
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.