Resolves: bug 468474

Bug Description: migration results in incomplete admin server sie
Reviewed by: nkinder (Thanks!)
Fix Description: This is a redesign of one of the core pieces of the setup/migration code - the code that adds the LDAP entries in various places.  For starters, I removed the code that would implicitly delete existing trees.  This is the root cause of this bug, and other similar problems with setup/instance creation that have been reported.  We should never implicitly delete entries.  Instead, we should explicitly delete entries by using the changetype: delete in an LDIF template file.
Another source of problems was that to update an entry, we would delete it and add it back.  This caused some configuration settings to be wiped out (e.g. encryption settings).  We cannot do this any more.  The LDIF template entries have been modified to have two sets of information for each entry that requires update - the entry to add if no entry exists (the full entry) or the changes to make to the entry if it does exist.  The code in Util.pm has been changed to ignore duplicate entries and to ignore changes made to entries that do not exist.
Another source of problems with migration is that the error checking was not adequate, especially with FileConn and dse.ldif reading.  The fix is to add better error checking and reporting in these areas of code, including error messages.
Yet another problem is the run_dir handling.  On many platforms the run_dir is shared among all DS instances and the admin server.  Older versions of the software allowed you to run the servers as root.  We have to make sure run_dir is usable by the least privileged user of all of the servers.
Platforms tested: RHEL4
Flag Day: no
Doc impact: no

ldap/admin/src/scripts/DSCreate.pm.in

@@ -215,6 +215,28 @@ sub makeDSDirs {
             return @errs;
         }
     }
+    # run_dir is a special case because it is usually shared among
+    # all instances and the admin server
+    # all instances must be able to write to it
+    # if the SuiteSpotUserID is root or 0, we can just skip
+    # this because root will have access to it - we really
+    # shouldn't be using root anyway, primarily just for
+    # legacy migration support
+    # if there are two different user IDs that need access
+    # to this directory, then SuiteSpotGroup must be defined,
+    # and both users must be members of the SuiteSpotGroup
+    if (($inf->{General}->{SuiteSpotUserID} eq 'root') ||
+        (defined($inf->{General}->{SuiteSpotUserID}) &&
+         ($inf->{General}->{SuiteSpotUserID} =~ /^0$/))) {
+        # skip
+        debug(3, "Root user " . $inf->{General}->{SuiteSpotUserID} . " already has access to $inf->{slapd}->{run_dir} - skipping\n");
+    } else {
+        my $dir = $inf->{slapd}->{run_dir};
+        # rwx by user only, or by user & group if a group is defined
+        @errs = changeOwnerMode($inf, 7, $dir, 7);
+        debug(3, "Changed owner of $dir to " . $inf->{General}->{SuiteSpotUserID} . ": error @errs\n");
+        debug(3, "\t" . `/bin/ls -ld $dir`);
+    }
     # set the group of the parent dir of config_dir and inst_dir
     if (defined($inf->{General}->{SuiteSpotGroup})) {
         for (qw(inst_dir config_dir)) {
@@ -372,7 +394,10 @@ sub createConfigFile {
         }
     }
 
-    $conn->write($conffile);
+    if (!$conn->write($conffile)) {
+        $conn->close();
+        return ("error_writing_ldif", $conffile, $!);
+    }
     $conn->close();
 
     if (@errs = changeOwnerMode($inf, 6, $conffile)) {
@@ -506,11 +531,21 @@ sub initDatabase {
         
         my ($fh, $templdif) = tempfile("ldifXXXXXX", SUFFIX => ".ldif", OPEN => 0,
                                        DIR => File::Spec->tmpdir);
+        if (!$templdif) {
+            return ('error_creating_templdif', $!);
+        }
         my $conn = new FileConn;
         $conn->setNamingContext($inf->{slapd}->{Suffix});
         getMappedEntries($mapper, \@ldiffiles, \@errs, \&check_and_add_entry,
                          [$conn]);
-        $conn->write($templdif);
+        if (@errs) {
+            $conn->close();
+            return @errs;
+        }
+        if (!$conn->write($templdif)) {
+            $conn->close();
+            return ('error_writing_ldif', $templdif, $!);
+        }
         $conn->close();
         if (@errs) {
             return @errs;

ldap/admin/src/scripts/DSMigration.pm.in

@@ -938,11 +938,15 @@ sub migrateDS {
         # extract the information needed for ds_newinst.pl
         my $oldconfigdir = "$mig->{oldsroot}/$inst/config";
         my $inf = createInfFromConfig($oldconfigdir, $inst, \@errs);
-        debug(2, "Using inffile $inf->{filename} created from $oldconfigdir\n");
         if (@errs) {
             $mig->msg(@errs);
             return 0;
         }
+        if (!$inf) {
+            $mig->msg($FATAL, 'error_opening_dseldif', "$oldconfigdir/dse.ldif", $!);
+            return 0;
+        }
+        debug(2, "Using inffile $inf->{filename} created from $oldconfigdir\n");
 
         # create servers but do not start them until after databases
         # have been migrated
@@ -960,7 +964,16 @@ sub migrateDS {
         }
 
         my $src = new FileConn("$oldconfigdir/dse.ldif", 1); # read-only
+        if (!$src) {
+            $mig->msg($FATAL, 'error_opening_dseldif', "$oldconfigdir/dse.ldif", $!);
+            return 0;
+        }
         my $dest = new FileConn("$mig->{configdir}/$inst/dse.ldif");
+        if (!$dest) {
+            $src->close();
+            $mig->msg($FATAL, 'error_opening_dseldif', "$mig->{configdir}/$inst/dse.ldif", $!);
+            return 0;
+        }
 
         @errs = migrateDSInstance($mig, $inst, $src, $dest);
         $src->close();

ldap/admin/src/scripts/FileConn.pm

@@ -67,7 +67,9 @@ sub new {
         $self->setNamingContext($_);
     }
     $self->setNamingContext(""); # root DSE
-    $self->read($filename);
+    if (!$self->read($filename)) {
+        return;
+    }
 
     return $self;
 }
@@ -90,10 +92,14 @@ sub read {
     }
 
     if (!$self->{filename}) {
-        return;
+        return 1; # no filename given - ok
+    }
+
+    if (!open( MYLDIF, "$filename" )) {
+        confess "Can't open $filename: $!";
+        return 0;
     }
 
-    open( MYLDIF, "$filename" ) || confess "Can't open $filename: $!";
     my $in = new Mozilla::LDAP::LDIF(*MYLDIF);
     $self->{reading} = 1;
     while ($ent = readOneEntry $in) {
@@ -103,6 +109,8 @@ sub read {
     }
     delete $self->{reading};
     close( MYLDIF );
+
+    return 1;
 }
 
 sub setNamingContext {
@@ -175,16 +183,22 @@ sub write {
     }
 
     if (!$self->{filename} or $self->{readonly} or $self->{reading}) {
-        return;
+        return 1; # ok - no filename given - just ignore
+    }
+
+    if (!open( MYLDIF, ">$filename" )) {
+        confess "Can't write $filename: $!";
+        return 0;
     }
 
-    open( MYLDIF, ">$filename" ) || confess "Can't write $filename: $!";
     $self->iterate("", LDAP_SCOPE_SUBTREE, \&writecb, \*MYLDIF);
     for (keys %{$self->{namingContexts}}) {
         next if (!$_); # skip "" - we already did that
         $self->iterate($_, LDAP_SCOPE_SUBTREE, \&writecb, \*MYLDIF);
     }
     close( MYLDIF );
+
+    return 1;
 }
 
 sub setErrorCode {
@@ -372,8 +386,7 @@ sub add {
     if ($self->isNamingContext($ndn) and
         !exists($self->{$ndn}->{data})) {
         $self->{$ndn}->{data} = $entry;
-        $self->write();
-        return 1;
+        return $self->write();
     }
 
     if (exists($self->{$ndn})) {
@@ -415,9 +428,7 @@ sub update {
     # process omits the deleted attrs via the Entry FETCH, FIRSTKEY, and NEXTKEY
     # methods
     $self->{$ndn}->{data} = cloneEntry($entry);
-    $self->write();
-
-    return 1;
+    return $self->write();
 }
 
 sub delete {
@@ -464,8 +475,7 @@ sub delete {
     # delete this node
     delete $self->{$ndn};
 
-    $self->write();
-    return 1;
+    return $self->write();
 }
 
 1;

ldap/admin/src/scripts/Util.pm.in

@@ -40,7 +40,7 @@ package Util;
 
 use Mozilla::LDAP::Conn;
 use Mozilla::LDAP::Utils qw(normalizeDN);
-use Mozilla::LDAP::API; # Direct access to C API
+use Mozilla::LDAP::API qw(:constant ldap_explode_dn ldap_err2string) ; # Direct access to C API
 use Mozilla::LDAP::LDIF;
 
 require Exporter;
@@ -172,85 +172,6 @@ sub delete_all
     return 0;
 }
 
-my %ignorelist = (
-    "nsslapd-directory", "nsslapd-directory",
-    "nsslapd-require-index", "nsslapd-require-index",
-    "nsslapd-readonly", "nsslapd-readonly",
-    "modifytimestamp", "modifyTimestamp",
-    "createtimestamp", "createTimestamp",
-    "installationtimestamp", "installationTimestamp",
-    "creatorsname", "creatorsName",
-    "modifiersname", "modifiersName",
-    "numsubordinates", "numSubordinates"
-);
-
-my %speciallist = (
-    "uniquemember", 1,
-    "aci", 1
-);
-
-# compare 2 entries
-# return 0 if they match 100% (exception: %ignorelist).
-# return 1 if they match except %speciallist.
-# return -1 if they do not match.
-sub comp_entries
-{
-    my ($e0, $e1) = @_;
-    my $rc = 0;
-    foreach my $akey ( keys %{$e0} )
-    {
-        next if ( $ignorelist{lc($akey)} );
-        my $aval0 = $e0->{$akey};
-        my $aval1 = $e1->{$akey};
-        my $a0max = $#{$aval0};
-        my $a1max = $#{$aval1};
-        my $amin = $#{$aval0};
-        if ( $a0max != $a1max )
-        {
-            if ( $speciallist{lc($akey)} )
-            {
-                $rc = 1;
-                if ( $a0max < $a1max )
-                {
-                    $amin = $a0max;
-                }
-                else
-                {
-                    $amin = $a1max;
-                }
-            }
-            else
-            {
-                $rc = -1;
-                return $rc;
-            }
-        }
-        my @sval0 = sort { $a cmp $b } @{$aval0};
-        my @sval1 = sort { $a cmp $b } @{$aval1};
-        for ( my $i = 0; $i <= $amin; $i++ )
-        {
-            my $isspecial = -1;
-            if ( $sval0[$i] ne $sval1[$i] )
-            {
-                if ( 0 > $isspecial )
-                {
-                    $isspecial = $speciallist{lc($akey)};
-                }
-                if ( $isspecial )
-                {
-                    $rc = 1;
-                }
-                else
-                {
-                    $rc = -1;
-                    return $rc;
-                }
-            }
-        }
-    }
-    return $rc;
-}
-
 # if the entry does not exist on the server, add the entry.
 # otherwise, do nothing
 # you can use this as the callback to getMappedEntries, so
@@ -272,9 +193,18 @@ sub check_and_add_entry
     my $sentry = $conn->search($aentry->{dn}, "base", "(objectclass=*)", 0, ("*", "aci"));
     if ($sentry) {
         debug(3, "check_and_add_entry: Found entry " . $sentry->getDN() . "\n");
+        if (! @ctypes) { # entry exists, and this is not a modify op
+            debug(3, "check_and_add_entry: skipping entry " . $sentry->getDN() . "\n");
+            return 1; # ignore - return success
+        }
     } else {
         debug(3, "check_and_add_entry: Entry not found " . $aentry->{dn} .
               " error " . $conn->getErrorString() . "\n");
+        if (@ctypes) { # uh oh - attempt to del/mod an entry that doesn't exist
+            debug(3, "check_and_add_entry: attepting to @ctypes the entry " . $aentry->{dn} .
+                  " that does not exist\n");
+            return 1; # ignore - return success
+        }
     }
     do
     {
@@ -289,39 +219,7 @@ sub check_and_add_entry
         my $op = $OP_NONE;
         if ( 0 > $#ctypes )    # aentry: complete entry
         {
-            $op = $OP_ADD;
-
-            my $rc = -1;
-            if ( $sentry && !$fresh )
-            {
-                $rc = comp_entries( $sentry, $aentry );
-            }
-            if ( 0 == $rc && !$fresh )
-            {
-                # the identical entry exists on the configuration DS.
-                # no need to add the entry.
-                $op = $OP_NONE;
-                goto out;
-            }
-            elsif ( (1 == $rc) && !$fresh )
-            {
-                $op = $OP_MOD;
-                @addtypes = keys %{$aentry}; # add all attrs
-            }
-            elsif ( $sentry && $sentry->{dn} )
-            {
-                # $fresh || $rc == -1
-                # an entry having the same DN exists, but the attributes do not
-                # match.  remove the entry and the subtree underneath.
-                debug(1, "Deleting an entry dn: $sentry->{dn} ...\n");
-                $rc = delete_all($conn, $sentry);
-                if ( 0 != $rc )
-                {
-                    push @{$errs}, 'error_deleteall_entries', $sentry->{dn}, $conn->getErrorString();
-                    debug(1, "Error deleting $sentry->{dn}\n");
-                    return 0;
-                }
-            }
+            $op = $OP_ADD; # just add the entry
         }
         else    # aentry: modify format
         {
@@ -371,9 +269,13 @@ sub check_and_add_entry
             }
             debug(1, "Entry $aentry->{dn} is deleted\n");
         }
-        elsif ( 0 < $op )    # $sentry exists
+        elsif ( 0 < $op )    # modify op
         {
             my $attr;
+            my @errsToIgnore;
+            if (@addtypes) {
+                push @errsToIgnore, LDAP_TYPE_OR_VALUE_EXISTS;
+            }                
             foreach $attr ( @addtypes )
             {
                 foreach my $val ($aentry->getValues($attr))
@@ -388,6 +290,9 @@ sub check_and_add_entry
                 debug(3, "Replacing attr=$attr values=" . $aentry->getValues($attr) . " to entry $aentry->{dn}\n");
                 $sentry->setValues($attr, @vals);
             }
+            if (@deltypes) {
+                push @errsToIgnore, LDAP_NO_SUCH_ATTRIBUTE;
+            }
             foreach $attr ( @deltypes )
             {
                 # removeValue takes a single value only
@@ -410,11 +315,15 @@ sub check_and_add_entry
             if ( $rc != 0 )
             {
                 my $string = $conn->getErrorString();
-                push @{$errs}, 'error_updating_entry', $sentry->{dn}, $string;
                 debug(1, "ERROR: updating an entry $sentry->{dn} failed, error: $string\n");
-                $aentry->printLDIF();
-                $conn->close();
-                return 0;
+                if (grep /^$rc$/, @errsToIgnore) {
+                    debug(1, "Ignoring error $rc returned by adding @addtypes deleting @deltypes\n");
+                } else {
+                    push @{$errs}, 'error_updating_entry', $sentry->{dn}, $string;
+                    $aentry->printLDIF();
+                    $conn->close();
+                    return 0;
+                }
             }
         }
         if ( $sentry )
@@ -793,19 +702,32 @@ sub createInfFromConfig {
     my $fname = "$configdir/dse.ldif";
     my $id;
     ($id = $inst) =~ s/^slapd-//;
-    if (! -f $fname) {
+    if (! -f $fname || ! -r $fname) {
         push @{$errs}, "error_opening_dseldif", $fname, $!;
         return 0;
     }
     my $conn = new FileConn($fname, 1);
+    if (!$conn) {
+        push @{$errs}, "error_opening_dseldif", $fname, $!;
+        return 0;
+    }
 
     my $ent = $conn->search("cn=config", "base", "(objectclass=*)");
     if (!$ent) {
         push @{$errs}, "error_opening_dseldif", $fname, $!;
+        $conn->close();
         return 0;
     }
 
     my ($outfh, $inffile) = tempfile(SUFFIX => '.inf');
+    if (!$outfh || !$inffile) {
+        push @{$errs}, "error_opening_tempinf", $fname, $!;
+        if ($outfh) {
+            close $outfh;
+        }
+        $conn->close();
+        return 0;
+    }
     print $outfh "[General]\n";
     print $outfh "FullMachineName = ", $ent->getValues('nsslapd-localhost'), "\n";
     print $outfh "SuiteSpotUserID = ", $ent->getValues('nsslapd-localuser'), "\n";

ldap/admin/src/scripts/remove-ds.pl.in

@@ -162,6 +162,10 @@ if ( ! -d $configdir )
 # read the config file to find out the paths
 my $dseldif = "@instconfigdir@/$instname/dse.ldif";
 my $conn = new FileConn($dseldif);
+if (!$conn) {
+    print STDERR "Error: Could not open config file $dseldif: Error $!\n";
+    exit 1;
+}
 
 my $dn = "cn=config";
 my $entry = $conn->search($dn, "base", "(cn=*)", 0);

ldap/admin/src/scripts/setup-ds.res.in

@@ -127,3 +127,7 @@ Please remove it first if you really want to recreate it,\
 or use a different ServerIdentifier to create another instance.\n
 error_opening_init_ldif = Could not open the initial LDIF file '%s'.\
 The file was not found or could not be read.\n
+error_opening_dseldif = Could not open the DSE config file '%s'. Error: %s\n
+error_opening_tempinf = Could not create temporary .inf file for config. Error: %s\n
+error_writing_ldif = Could not write the LDIF file '%s'.  Error: %s\n
+error_creating_templdif = Could not create temporary LDIF file. Error: %s\n