glpi-project / glpi-agent

GLPI Agent
GNU General Public License v2.0
243 stars 61 forks source link

Bug #374 not fully solved, if Domain Account is deleted #384

Closed AndiMb closed 1 year ago

AndiMb commented 1 year ago

Bug reporting acknowledgment

Yes, I read it

Professional support

Yes, I know

Describe the bug

374 solved a bug with wmi query timeouts, by getting the username to an SID with a powershell command avoiding the wmi querys. If there is a local profile path of an account which was deleted in the domain, the GLPI agent will still perform the wmi query, due to an "IdentityNotMappedException" of the powershell command. So the powershell command worked correctly but wasn't able to map the SID to the username, since there is no username.

To reproduce

see above

Expected behavior

do not use the wmi query, since the powershell command worked correctly

Operating system

Windows

GLPI Agent version

Nightly build (git version in additional context below)

GLPI version

Other (See additional context below)

GLPIInventory plugin or FusionInventory for GLPI plugin version

Other (See additional context below)

Additional context

GLPI 10.0.7, GLPI-Agent 1.5-git3c8816ae, GLPIInventory plugin 1.2.1

g-bougard commented 1 year ago

Hi @AndiMb

the fix for #374 was to use the following PS script with __USER_SID__ string replaced:

# Setup encoding to UTF-8
$PreviousEncoding = [console]::OutputEncoding
$OutputEncoding   = [console]::InputEncoding = [console]::OutputEncoding = New-Object System.Text.UTF8Encoding

((New-Object System.Security.Principal.SecurityIdentifier(__USER_SID__)).Translate([System.Security.Principal.NTAccount])).Value

# Restore encoding
$OutputEncoding = [console]::InputEncoding = [console]::OutputEncoding = $PreviousEncoding

What's the "standard" output of that script for the case you're reporting ("stderr" output is not caught by the agent) ? Or how can we decompose the middle line into a code producing an error on "standard" output we can correctly handle in the agent ? Can we still have a username from that domain deleted account or should we report something like domain deleted account as username ?

g-bougard commented 1 year ago

Maybe something like this if I follow this doc: https://learn.microsoft.com/en-us/powershell/scripting/learn/deep-dives/everything-about-exceptions

try {
    ((New-Object System.Security.Principal.SecurityIdentifier(__USER_SID__)).Translate([System.Security.Principal.NTAccount])).Value
}
catch {
    Write-Output "Exception: $PSItem"
}

And so what's the output in your case ?

AndiMb commented 1 year ago

The output in my case is:

Exception calling "Translate" with "1" argument(s): "Some or all identity references could not be translated."
At line:1 char:1
+   ((New-Object System.Security.Principal.SecurityIdentifier(__USER_SID__)) ...
+   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [], MethodInvocationException
    + FullyQualifiedErrorId : IdentityNotMappedException

But in my case, the output is in German. So perhaps it is a good idea to use the FullyQualifiedErrorId:

try {
    ((New-Object System.Security.Principal.SecurityIdentifier(__USER_SID__)).Translate([System.Security.Principal.NTAccount])).Value
}
catch {
    Write-Output "Exception: $($PSItem.FullyQualifiedErrorId)"
}

Another way could be to check if S-1-5-domain-500 of the account's domain can be translated. If this works and a domain account name is given, the powershell works in general. Any error of the SID to account name translation process should be a problem with the specific account and not a general problem of the script or the command. In that case domain deleted account should be reported.

g-bougard commented 1 year ago

Hi @AndiMb

then can you try to apply the following patch ?

diff --git a/lib/GLPI/Agent/Tools/Win32/Users.pm b/lib/GLPI/Agent/Tools/Win32/Users.pm
index 99eeab722..222162244 100644
--- a/lib/GLPI/Agent/Tools/Win32/Users.pm
+++ b/lib/GLPI/Agent/Tools/Win32/Users.pm
@@ -6,6 +6,7 @@ use parent 'Exporter';

 use Encode qw(decode encode);

+use GLPI::Agent::Logger;
 use GLPI::Agent::Tools::Win32;

 our @EXPORT = qw(
@@ -22,7 +23,7 @@ sub getSystemUserProfiles {
         query      => "SELECT * FROM Win32_UserProfile WHERE LocalPath IS NOT NULL AND Special=FALSE",
         properties => [ qw/Sid Loaded LocalPath/ ],
     )) {
-        next unless $userprofile->{Sid} && $userprofile->{Sid} =~ /^S-\d+-5-21-/;
+        next unless $userprofile->{Sid} && $userprofile->{Sid} =~ /^S-\d+-5-21(-\d+)+$/;

         next unless defined($userprofile->{Loaded}) && defined($userprofile->{LocalPath});

@@ -58,13 +59,23 @@ sub getProfileUsername {
             $PreviousEncoding = [console]::OutputEncoding
             $OutputEncoding   = [console]::InputEncoding = [console]::OutputEncoding = New-Object System.Text.UTF8Encoding

-            ((New-Object System.Security.Principal.SecurityIdentifier("'.$user->{SID}.'")).Translate([System.Security.Principal.NTAccount])).Value
+            try {
+                ((New-Object System.Security.Principal.SecurityIdentifier("'.$user->{SID}.'")).Translate([System.Security.Principal.NTAccount])).Value
+            }
+            catch {
+                Write-Output "Exception: $($PSItem.FullyQualifiedErrorId)"
+            }

             # Restore encoding
             $OutputEncoding = [console]::InputEncoding = [console]::OutputEncoding = $PreviousEncoding
         '
     );
     if ($ntaccount) {
+        if ($ntaccount =~ /^Exception: (.*)/) {
+            my $logger = GLPI::Agent::Logger->new();
+            $logger->info("Got PowerShell Exception when looking for $user->{SID} profile username: $1");
+            return "Domain deleted account" if $1 && $1 eq "IdentityNotMappedException";
+        }
         my ($username) = $ntaccount =~ /^[^\\]*\\(.*)$/;
         return $username if defined($username) && length($username);
     }

Here is an archive containing the patched file, you can directly replace the corresponding file and restart the service: Glpi-Agent-Tools-Win32-Users.zip

AndiMb commented 1 year ago

Thanks, this is working in my case. I have the info in the log, that there was a powershell error and everything runs correctly.

g-bougard commented 1 year ago

Thank you again @AndiMb for your help on that WMI timeout issues. Next nightly build will include the fix.