Merge pull request #250 from ikedas/issue-243 by ikedas

cookie parameter protection #243
sympa-community · Apr 21, 2018 · 9c61bb2 · 9c61bb2
2 parents 08c0427 + c7d32c7
commit 9c61bb2
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 13 deletions.
diff --git a/src/lib/Conf.pm b/src/lib/Conf.pm
@@ -406,7 +406,7 @@ sub delete_binaries {
         push @files, "$Conf{'etc'}/$robot/robot.conf";
     }
     foreach my $c_file (@files) {
-        my $binary_file = $c_file . ".bin";
+        my $binary_file = $c_file . $binary_file_extension;
         if (-f $binary_file) {
             if (-w $binary_file) {
                 unlink $binary_file;
@@ -2331,11 +2331,16 @@ sub _replace_file_value_by_db_value {
 # Returns 1 or undef if something went wrong.
 sub _save_binary_cache {
     my $param = shift;
+
+    # Prevent world-readability to protect secure parameters like "cookie".
+    my $umask = umask(umask | 007);
     my $lock_fh = Sympa::LockedFile->new($param->{'target_file'}, 2, '>');
     unless ($lock_fh) {
         $log->syslog('err', 'Could not create new lock');
+        umask $umask;
         return undef;
     }
+    umask $umask;
 
     eval { Storable::store_fd($param->{'conf_to_save'}, $lock_fh); };
     if ($EVAL_ERROR) {
@@ -2344,9 +2349,7 @@ sub _save_binary_cache {
             'Failed to save the binary config %s. error: %s',
             $param->{'target_file'}, $EVAL_ERROR
         );
-        unless ($lock_fh->close()) {
-            return undef;
-        }
+        $lock_fh->close;
         return undef;
     }
     eval {
@@ -2362,14 +2365,11 @@ sub _save_binary_cache {
             'Failed to change owner of the binary file %s. error: %s',
             $param->{'target_file'}, $EVAL_ERROR
         );
-        unless ($lock_fh->close()) {
-            return undef;
-        }
-        return undef;
-    }
-    unless ($lock_fh->close()) {
+        $lock_fh->close;
         return undef;
     }
+
+    $lock_fh->close;
     return 1;
 }
 

diff --git a/src/lib/Sympa/ConfDef.pm b/src/lib/Sympa/ConfDef.pm
@@ -1238,6 +1238,7 @@ our @params = (
         'sample'     => '123456789',
         'gettext_id' => 'Secret string for generating unique keys',
         'file'       => 'sympa.conf',
+        'obfuscated' => '1',
         'gettext_comment' =>
             "This allows generated authentication keys to differ from a site to another. It is also used for encryption of user passwords stored in the database. The presence of this string is one reason why access to \"sympa.conf\" needs to be restricted to the \"sympa\" user.\nNote that changing this parameter will break all HTTP cookies stored in users' browsers, as well as all user passwords and lists X509 private keys. To prevent a catastrophe, Sympa refuses to start if this \"cookie\" parameter was changed.",
         'optional' => '1',
@@ -1760,9 +1761,10 @@ our @params = (
         'gettext_id' => 'Password used to crypt lists private keys',
         'gettext_comment' =>
             'If not defined, Sympa assumes that list private keys are not encrypted.',
-        'file'     => 'sympa.conf',
-        'edit'     => '1',
-        'optional' => '1',
+        'file'       => 'sympa.conf',
+        'edit'       => '1',
+        'obfuscated' => '1',
+        'optional'   => '1',
     },
     # Not yet implemented
     #{   'name'    => 'crl_dir',