Merge pull request #8131 from inverse-inc/fix/ou-missing-8112

Fixing missing OU in DN when creating machine account
inverse-inc · May 3, 2024 · 9a62a37 · 9a62a37
2 parents 8f525da + ee81fe3
commit 9a62a37
Show file tree

Hide file tree

Showing 4 changed files with 103 additions and 43 deletions.
diff --git a/html/pfappserver/root/src/composables/useInputValidator.js b/html/pfappserver/root/src/composables/useInputValidator.js
@@ -40,7 +40,10 @@ export const useInputValidator = (props, value, recursive = false) => {
   let localState = ref(unref(state))
   let localInvalidFeedback = ref(unref(invalidFeedback))
   let localValidFeedback = ref(unref(validFeedback))
-  let localApiFeedback = ref(unref(apiFeedback))
+  let localApiFeedback = ref(undefined)
+  watch(apiFeedback, () => {
+    localApiFeedback.value = apiFeedback.value
+  }, { immediate: true })
 
   // yup | https://github.com/jquense/yup
   let localValidator = validator

diff --git a/html/pfappserver/root/src/views/Configuration/domains/_components/TheForm.vue b/html/pfappserver/root/src/views/Configuration/domains/_components/TheForm.vue
@@ -52,6 +52,7 @@
         <form-group-ou namespace="ou"
                        :column-label="$i18n.t('OU')"
                        :text="$i18n.t(`Use a specific OU for the PacketFence account. The OU string read from top to bottom without RDNs and delimited by a '/'. (ex: Computers/Servers/Unix).`)"
+                       :api-feedback="ouFeedback"
         />
 
 
@@ -289,12 +290,27 @@ export const setup = (props, context) => {
     )
   })
 
+  const isDefaultOU = computed(() => {
+    const { ou } = form.value
+    return !!ou && /^computers$/i.test(ou) || !ou
+  })
+
+  const ouFeedback = computed(() => {
+    if (isDefaultOU.value) {
+      return undefined
+    }
+    return i18n.t(`Non-default OU is defined. LDAPS service on port 636 is required in Domain Controller.`)
+  })
+
+
   return {
     schema,
     formGroupComputedMachineAccountPassword,
     isMachineAccountHash,
     machineAccountFeedback,
-    machineAccountBind
+    machineAccountBind,
+    isDefaultOU,
+    ouFeedback
   }
 }
 

diff --git a/lib/pf/UnifiedApi/Controller/Config/Domains.pm b/lib/pf/UnifiedApi/Controller/Config/Domains.pm
@@ -35,9 +35,9 @@ use Encode qw(encode);
 use Net::DNS;
 
 use JSON;
-=head2 test_join
+=head2 create
 
-Test if a domain is properly joined
+create machine account and domain config file.
 
 =cut
 
@@ -87,6 +87,7 @@ sub create {
     my $dns_name = $item->{dns_name};
     my $workgroup = $item->{workgroup};
     my $real_computer_name = $item->{server_name};
+    my $ou = $item->{ou};
 
     if ($computer_name eq "%h") {
         $real_computer_name = hostname();
@@ -122,16 +123,18 @@ sub create {
         return $self->render_error(422, "Unable to determine AD server's IP address.\n")
     }
 
-    my $baseDN = $dns_name;
-    $baseDN = generate_baseDN($dns_name);
-
     if (!is_nt_hash_pattern($computer_password)) {
-        my ($add_status, $add_result) = pf::domain::add_computer(" ", $real_computer_name, $computer_password, $ad_server_ip, $ad_server_host, $baseDN, $workgroup, $workgroup, $bind_dn, $bind_pass);
+        my ($add_status, $add_result) = pf::domain::add_computer(" ", $real_computer_name, $computer_password, $ad_server_ip, $ad_server_host, $dns_name, $workgroup, $ou, $bind_dn, $bind_pass);
         if ($add_status == $FALSE) {
             if ($add_result =~ /already exists(.+)use \-no\-add/) {
-                ($add_status, $add_result) = pf::domain::add_computer("-no-add", $real_computer_name, $computer_password, $ad_server_ip, $ad_server_host, $baseDN, $workgroup, $workgroup, $bind_dn, $bind_pass);
+                ($add_status, $add_result) = pf::domain::add_computer("-delete", $real_computer_name, $computer_password, $ad_server_ip, $ad_server_host, $dns_name, $workgroup, $ou, $bind_dn, $bind_pass);
+                if ($add_status == $FALSE) {
+                    $self->render_error(422, "Unable to add machine account: removing existing machine account failed with following error: $add_result");
+                    return 0;
+                }
+                ($add_status, $add_result) = pf::domain::add_computer(" ", $real_computer_name, $computer_password, $ad_server_ip, $ad_server_host, $dns_name, $workgroup, $ou, $bind_dn, $bind_pass);
                 if ($add_status == $FALSE) {
-                    $self->render_error(422, "Unable to add machine account with following error: $add_result");
+                    $self->render_error(422, "Unable to add machine account: recreating machine account with following error: $add_result");
                     return 0;
                 }
             }
@@ -188,6 +191,7 @@ sub update {
     my $dns_name = $new_item->{dns_name};
     my $workgroup = $old_item->{workgroup};
     my $real_computer_name = $old_item->{server_name};
+    my $ou = $new_item->{ou};
 
     if ($computer_name eq "%h") {
         $real_computer_name = hostname();
@@ -224,24 +228,25 @@ sub update {
     }
 
     if (!is_nt_hash_pattern($new_data->{machine_account_password}) && ($new_data->{machine_account_password} ne $old_item->{machine_account_password})) {
-        my $baseDN = $dns_name;
-        $baseDN = generate_baseDN($dns_name);
-
-        my ($add_status, $add_result) = pf::domain::add_computer("-no-add", $real_computer_name, $computer_password, $ad_server_ip, $ad_server_host, $baseDN, $workgroup, $workgroup, $bind_dn, $bind_pass);
+        my ($add_status, $add_result) = pf::domain::add_computer("-delete", $real_computer_name, $computer_password, $ad_server_ip, $ad_server_host, $dns_name, $workgroup, $ou, $bind_dn, $bind_pass);
         if ($add_status == $FALSE) {
-            if ($add_result =~ /Account.+not found in/) {
-                ($add_status, $add_result) = pf::domain::add_computer(" ", $real_computer_name, $computer_password, $ad_server_ip, $ad_server_host, $baseDN, $workgroup, $workgroup, $bind_dn, $bind_pass);
-                if ($add_status == $FALSE) {
-                    $self->render_error(422, "Unable to add machine account with following error: $add_result");
-                    return 0;
-                }
-            }
-            else {
-                $self->render_error(422, "Unable to add machine account with following error: $add_result");
+            unless ($add_result =~ /Account (.+) not found in/) {
+                $self->render_error(422, "Unable to update - remove existing machine account with following error: $add_result");
                 return 0;
             }
         }
+
+        ($add_status, $add_result) = pf::domain::add_computer(" ", $real_computer_name, $computer_password, $ad_server_ip, $ad_server_host, $dns_name, $workgroup, $ou, $bind_dn, $bind_pass);
+        if ($add_status == $FALSE) {
+            $self->render_error(422, "Unable to add machine account with following error: $add_result");
+            return 0;
+        }
+
         $new_data->{machine_account_password} = md4_hex(encode("utf-16le", $new_data->{machine_account_password}));
+        $new_data->{ou} = $new_item->{ou}
+    }
+    else {
+        $new_data->{ou} = $old_item->{ou}
     }
 
     $new_data->{server_name} = $computer_name;
@@ -255,19 +260,6 @@ sub update {
     $self->render(status => 200, json => $self->update_response($form));
 }
 
-sub generate_baseDN {
-    my $ret = "";
-
-    my ($dns_name) = @_;
-    my @array = split(/\./, $dns_name);
-
-    foreach my $element (@array) {
-        $ret .= "DC=$element,";
-    }
-    $ret =~ s/,$//;
-    return $ret;
-}
-
 sub is_nt_hash_pattern {
     my ($password) = @_;
     $password =~ s/^\s+|\s+$//g;

diff --git a/lib/pf/domain.pm b/lib/pf/domain.pm
@@ -26,7 +26,7 @@ use Encode qw(encode);
 use File::Slurp;
 
 # This is to create the templates for the domain info
-our $TT_OPTIONS = {ABSOLUTE => 1};
+our $TT_OPTIONS = { ABSOLUTE => 1 };
 our $template = Template->new($TT_OPTIONS);
 
 our $ADD_COMPUTERS_BIN = '/usr/local/pf/bin/impacket-addcomputer';
@@ -43,7 +43,7 @@ sub run {
     my $result = `$cmd`;
     my $code = $? >> 8;
 
-    return ($code , $result);
+    return ($code, $result);
 }
 
 =head2 test_join
@@ -54,16 +54,32 @@ Executes the command in the OS to test the domain join
 
 sub add_computer {
     my $option = shift;
-    my ($computer_name, $computer_password, $domain_controller_ip, $domain_controller_host, $baseDN, $computer_group, $workgroup, $bind_dn, $bind_pass) = @_;
+    my ($computer_name, $computer_password, $domain_controller_ip, $domain_controller_host, $dns_name, $workgroup, $ou, $bind_dn, $bind_pass) = @_;
+
+    if (!defined($ou)) {
+        $ou = ""
+    }
+
+    $ou =~ s/^\s+|\s+$//g;
+    $ou =~ s/^['"]|['"]$//g;
+
+    my $method = "LDAPS";
+    if (uc($ou) eq "COMPUTERS" || $ou eq "") {
+        $method = "SAMR"
+    }
 
     $computer_name = escape_bind_user_string($computer_name) . "\$";
     $computer_password = escape_bind_user_string($computer_password);
-    my $domain_auth = escape_bind_user_string("$workgroup/$bind_dn");
-    my $nt_hash = md4_hex(encode("utf-16le", $bind_pass));
+    my $domain_auth = escape_bind_user_string("$dns_name/$bind_dn:$bind_pass");
+    my $baseDN = generate_base_dn($dns_name);
+    my $computer_group = generate_computer_group($dns_name, $ou);
+
+    $baseDN = escape_bind_user_string($baseDN);
+    $computer_group = escape_bind_user_string($computer_group);
 
     my $result;
     eval {
-        my $command = "$ADD_COMPUTERS_BIN -computer-name $computer_name -computer-pass '$computer_password' -dc-ip $domain_controller_ip -dc-host '$domain_controller_host' -baseDN '$baseDN' -computer-group $computer_group '$domain_auth' -hashes ':$nt_hash' $option";
+        my $command = "$ADD_COMPUTERS_BIN -computer-name '$computer_name' -computer-pass '$computer_password' -dc-ip $domain_controller_ip -dc-host '$domain_controller_host' -baseDN '$baseDN' -computer-group '$computer_group' '$domain_auth' $option -method=$method";
         $result = pf_run($command, accepted_exit_status => [ 0 ]);
     };
     if ($@) {
@@ -92,7 +108,7 @@ sub add_computer {
 =head2 escape_bind_user_string
 
 Escapes the bind user string for any simple quote
-
+' -> '\''
 =cut
 
 sub escape_bind_user_string {
@@ -101,7 +117,40 @@ sub escape_bind_user_string {
     return $s;
 }
 
+sub generate_base_dn {
+    my $ret = "";
+
+    my ($dns_name) = @_;
+    my @array = split(/\./, $dns_name);
+
+    foreach my $element (@array) {
+        $ret .= "DC=$element,";
+    }
+    $ret =~ s/,$//;
+    return $ret;
+}
+
+sub generate_computer_group {
+    my $ret = "";
+    my ($dns_name, $ou) = @_;
 
+    my $base_dn = generate_base_dn($dns_name);
+
+    # for OU=Computer or OU="", we put the machine account to CN=Computers.
+    if (!defined($ou) || uc($ou) eq "COMPUTERS" || $ou eq "") {
+        return "CN=Computers," . $base_dn;
+    }
+
+    # Handle real OU strings
+    my @array = split(/\//, $ou);
+    my $dn_ou = "";
+
+    foreach my $element (@array) {
+        $dn_ou = "OU=$element," . $dn_ou;
+    }
+    $dn_ou =~ s/,$//;
+    return $dn_ou . ",$base_dn";
+}