ldap/admin/src/scripts/DSMigration.pm.in | 6 ++- ldap/admin/src/scripts/DSUtil.pm.in | 33 +++++-------------- ldap/admin/src/scripts/Inf.pm | 53 +++++++++++++++++++++++-------- ldap/admin/src/scripts/Migration.pm.in | 17 +-------- ldap/admin/src/scripts/Setup.pm.in | 17 +-------- ldap/admin/src/scripts/migrate-ds.pl.in | 13 +++++-- ldap/admin/src/scripts/setup-ds.pl.in | 7 +--- 7 files changed, 68 insertions(+), 78 deletions(-)
New commits: commit 84b40e3a1d249ec7d74a3d89ca1bb3294ab65e6d Author: Rich Megginson rmeggins@redhat.com Date: Tue May 25 12:15:18 2010 -0600
Bug 593392 - setup-ds-admin.pl -k creates world readable file
https://bugzilla.redhat.com/show_bug.cgi?id=593392 Resolves: bug 593392 Bug Description: setup-ds-admin.pl -k creates world readable file Reviewed by: thoger (Thanks!) Branch: Directory_Server_8_2_Branch Fix Description: Use umask to make sure we create a .inf file that is only viewable by the user. In addition, only create the temp file and filename when necessary. In some places, the code was creating a .inf file on disk when it could just create one in memory. The code should check to see if the Inf object has a file associated with it before attemtping to unlink it. Make sure we do not unlink a .inf file given with -f on the command line. If the user specified -k, always write to a temp file using __temp__ as the keyword to tell Inf->write to generate a temp file. Platforms tested: RHEL5 x86_64 Flag Day: no Doc impact: no (cherry picked from commit 8e6e74009c80a7032308657e71450cb5aed9483c)
diff --git a/ldap/admin/src/scripts/DSMigration.pm.in b/ldap/admin/src/scripts/DSMigration.pm.in index 5070972..e406942 100644 --- a/ldap/admin/src/scripts/DSMigration.pm.in +++ b/ldap/admin/src/scripts/DSMigration.pm.in @@ -1105,7 +1105,7 @@ sub migrateDS { $mig->msg($FATAL, 'error_opening_dseldif', "$oldconfigdir/dse.ldif", $!); return 0; } - debug(2, "Using inffile $inf->{filename} created from $oldconfigdir\n"); + debug(2, "Using inf created from $oldconfigdir\n");
# create servers but do not start them until after databases # have been migrated @@ -1113,7 +1113,9 @@ sub migrateDS {
# create the new instance @errs = createDSInstance($inf); - unlink($inf->{filename}); + if ($inf->{filename}) { + unlink($inf->{filename}); + } if (@errs) { $mig->msg(@errs); $mig->msg($FATAL, 'error_creating_dsinstance', $inst); diff --git a/ldap/admin/src/scripts/DSUtil.pm.in b/ldap/admin/src/scripts/DSUtil.pm.in index acb6991..3276497 100644 --- a/ldap/admin/src/scripts/DSUtil.pm.in +++ b/ldap/admin/src/scripts/DSUtil.pm.in @@ -769,30 +769,19 @@ sub createInfFromConfig { 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"; - print $outfh "[slapd]\n"; - print $outfh "RootDN = ", $ent->getValues('nsslapd-rootdn'), "\n"; - print $outfh "RootDNPwd = ", $ent->getValues('nsslapd-rootpw'), "\n"; - print $outfh "ServerPort = ", $ent->getValues('nsslapd-port'), "\n"; - print $outfh "ServerIdentifier = $id\n"; + my $inf = new Inf(); + $inf->{General}->{FullMachineName} = $ent->getValues('nsslapd-localhost'); + $inf->{General}->{SuiteSpotUserID} = $ent->getValues('nsslapd-localuser'); + $inf->{slapd}->{RootDN} = $ent->getValues('nsslapd-rootdn'); + $inf->{slapd}->{RootDNPwd} = $ent->getValues('nsslapd-rootpw'); + $inf->{slapd}->{ServerPort} = $ent->getValues('nsslapd-port'); + $inf->{slapd}->{ServerIdentifier} = $id;
my $suffix; $ent = $conn->search("cn=ldbm database,cn=plugins,cn=config", "one", "(objectclass=*)"); if (!$ent) { push @{$errs}, "error_opening_dseldif", $fname, $!; - close $outfh; $conn->close(); return 0; } @@ -807,7 +796,6 @@ sub createInfFromConfig { $ent = $conn->search("cn=config", "base", "(objectclass=*)"); if (!$ent) { push @{$errs}, "error_opening_dseldif", $fname, $!; - close $outfh; $conn->close(); return 0; } @@ -816,12 +804,9 @@ sub createInfFromConfig { $conn->close();
if ($inst_dir) { - print $outfh "inst_dir = $inst_dir\n"; + $inf->{slapd}->{inst_dir} = $inst_dir; } - print $outfh "Suffix = $suffix\n"; - close $outfh; - - my $inf = new Inf($inffile); + $inf->{slapd}->{Suffix} = $suffix;
return $inf; } diff --git a/ldap/admin/src/scripts/Inf.pm b/ldap/admin/src/scripts/Inf.pm index bb22913..a102eb6 100644 --- a/ldap/admin/src/scripts/Inf.pm +++ b/ldap/admin/src/scripts/Inf.pm @@ -41,6 +41,8 @@
package Inf;
+use File::Temp qw(tempfile tempdir); + #require Exporter; #@ISA = qw(Exporter); #@EXPORT = qw(); @@ -50,6 +52,9 @@ sub new { my $self = {};
$self->{filename} = shift; + $self->{writable} = shift; # do not overwrite user supplied file + # if you want to init an Inf with a writable file, use + # $inf = new Inf($filename, 1)
$self = bless $self, $type;
@@ -162,7 +167,7 @@ sub writeSection { my $section = $self->{$name}; if (ref($section) eq 'HASH') { print $fh "[$name]\n"; - for my $key (keys %{$section}) { + for my $key (sort keys %{$section}) { if (defined($section->{$key})) { my $val = $section->{$key}; $val =~ s/\n/\\n/g; # make continuation lines @@ -175,28 +180,50 @@ sub writeSection { sub write { my $self = shift; my $filename = shift; + my $fh;
- if ($filename) { + return if ($filename and $filename eq "-"); + + # see if user wants to force use of a temp file + if ($filename and $filename eq '__temp__') { + $self->{writable} = 1; + $filename = ''; + delete $self->{filename}; + } + + if (!$self->{writable}) { + return; # do not overwrite read only file + } + + if ($filename) { # use user supplied filename $self->{filename} = $filename; - } else { + } elsif ($self->{filename}) { # use existing filename $filename = $self->{filename}; + } else { # create temp filename + ($fh, $self->{filename}) = tempfile("setupXXXXXX", UNLINK => 0, + SUFFIX => ".inf", OPEN => 1, + DIR => File::Spec->tmpdir); }
- return if ($filename eq "-"); - - if (!open(INF, ">$filename")) { - print STDERR "Error: could not write inf file $filename: $!\n"; - return; + my $savemask = umask(0077); + if (!$fh) { + if (!open(INF, ">$filename")) { + print STDERR "Error: could not write inf file $filename: $!\n"; + umask($savemask); + return; + } + $fh = *INF; } # write General section first - $self->writeSection('General', *INF); - print INF "\n"; + $self->writeSection('General', $fh); + print $fh "\n"; for my $key (keys %{$self}) { next if ($key eq 'General'); - $self->writeSection($key, *INF); - print INF "\n"; + $self->writeSection($key, $fh); + print $fh "\n"; } - close INF; + close $fh; + umask($savemask); }
sub updateFromArgs { diff --git a/ldap/admin/src/scripts/Migration.pm.in b/ldap/admin/src/scripts/Migration.pm.in index 1942c8b..66618c8 100644 --- a/ldap/admin/src/scripts/Migration.pm.in +++ b/ldap/admin/src/scripts/Migration.pm.in @@ -54,9 +54,6 @@ use Exporter (); @EXPORT = qw(); @EXPORT_OK = qw();
-# tempfiles -use File::Temp qw(tempfile tempdir); - # hostname use Net::Domain qw(hostfqdn);
@@ -68,8 +65,6 @@ use Mozilla::LDAP::LDIF;
use Getopt::Long;
-use File::Temp qw(tempfile tempdir); - use SetupLog; use DSUtil;
@@ -210,7 +205,6 @@ sub init { $actualsroot =~ s//+$//; # trim trailing '/'s, if any $self->{actualsroot} = $actualsroot || $self->{oldsroot}; $self->{silent} = $silent; - $self->{inffile} = $inffile; $self->{keep} = $keep; $self->{preonly} = $preonly; $self->{logfile} = $logfile; @@ -218,18 +212,11 @@ sub init { $self->{log} = new SetupLog($self->{logfile}, "migrate"); $self->{start_servers} = 1; # start servers as soon as they are migrated # if user supplied inf file, use that to initialize - if (defined($self->{inffile})) { - $self->{inf} = new Inf($self->{inffile}); + if (defined($inffile)) { + $self->{inf} = new Inf($inffile); } else { $self->{inf} = new Inf; } - my $fh; - # create a temp inf file for writing for other processes - # never overwrite the user supplied inf file - ($fh, $self->{inffile}) = tempfile("migrateXXXXXX", UNLINK => !$keep, - SUFFIX => ".inf", OPEN => 0, - DIR => File::Spec->tmpdir); - $self->{inf}->{filename} = $self->{inffile};
# see if user passed in default inf values - also, command line # arguments override those passed in via an inf file - this diff --git a/ldap/admin/src/scripts/Setup.pm.in b/ldap/admin/src/scripts/Setup.pm.in index 52300db..753062d 100644 --- a/ldap/admin/src/scripts/Setup.pm.in +++ b/ldap/admin/src/scripts/Setup.pm.in @@ -52,9 +52,6 @@ use Exporter (); @EXPORT = qw($SILENT $EXPRESS $TYPICAL $CUSTOM); @EXPORT_OK = qw($SILENT $EXPRESS $TYPICAL $CUSTOM);
-# tempfiles -use File::Temp qw(tempfile tempdir); - # hostname use Net::Domain qw(hostfqdn);
@@ -66,8 +63,6 @@ use Mozilla::LDAP::LDIF;
use Getopt::Long;
-use File::Temp qw(tempfile tempdir); - use SetupLog; use DSUtil; use Inf; @@ -141,7 +136,6 @@ sub init { );
$self->{silent} = $silent; - $self->{inffile} = $inffile; $self->{keep} = $keep; $self->{preonly} = $preonly; $self->{update} = $update; @@ -149,18 +143,11 @@ sub init { $self->{logfile} = $logfile; $self->{log} = new SetupLog($self->{logfile}); # if user supplied inf file, use that to initialize - if (defined($self->{inffile})) { - $self->{inf} = new Inf($self->{inffile}); + if (defined($inffile)) { + $self->{inf} = new Inf($inffile); } else { $self->{inf} = new Inf; } - my $fh; - # create a temp inf file for writing for other processes - # never overwrite the user supplied inf file - ($fh, $self->{inffile}) = tempfile("setupXXXXXX", UNLINK => !$keep, - SUFFIX => ".inf", OPEN => 0, - DIR => File::Spec->tmpdir); - $self->{inf}->{filename} = $self->{inffile};
# see if user passed in default inf values - also, command line # arguments override those passed in via an inf file - this diff --git a/ldap/admin/src/scripts/migrate-ds.pl.in b/ldap/admin/src/scripts/migrate-ds.pl.in index df14ea0..cd42800 100644 --- a/ldap/admin/src/scripts/migrate-ds.pl.in +++ b/ldap/admin/src/scripts/migrate-ds.pl.in @@ -68,9 +68,14 @@ $mig->msg('end_ds_migration'); $mig->doExit(0);
END { - if ($mig) { - if (!$mig->{keep}) { - unlink $mig->{inffile}; - } + if ($mig and $mig->{keep}) { + $mig->{inf}->write("__temp__"); } } + +# emacs settings +# Local Variables: +# mode:perl +# indent-tabs-mode: nil +# tab-width: 4 +# End: diff --git a/ldap/admin/src/scripts/setup-ds.pl.in b/ldap/admin/src/scripts/setup-ds.pl.in index 266d396..7613ed8 100644 --- a/ldap/admin/src/scripts/setup-ds.pl.in +++ b/ldap/admin/src/scripts/setup-ds.pl.in @@ -74,7 +74,6 @@ if (!$setup->{silent}) { if ($rc) { $setup->doExit(); } - $setup->{inf}->write(); }
my @errs; @@ -105,10 +104,8 @@ if (@errs) { $setup->doExit(0);
END { - if ($setup) { - if (!$setup->{keep}) { - unlink $setup->{inffile}; - } + if ($setup and $setup->{keep}) { + $setup->{inf}->write("__temp__"); } }
389-commits@lists.fedoraproject.org
-
Richard Allen Megginson