Re: svn commit: r349974 - head/libexec/rc/rc.d

2019-07-28 Thread Jilles Tjoelker
On Sat, Jul 13, 2019 at 04:07:38PM +, Ian Lepore wrote:
> Author: ian
> Date: Sat Jul 13 16:07:38 2019
> New Revision: 349974
> URL: https://svnweb.freebsd.org/changeset/base/349974

> Log:
>   Limit access to system accounting files.

>   In 2013 the security chapter of the Handbook was updated in r42501 to
>   suggest limiting access to the system accounting file [*1] by creating the
>   initial file with a mode of 0600. This was in part based on a discussion in
>   the forums [*2]. Unfortunately, this advice is overridden by the fact that a
>   new file is created as part of periodic daily processing, and the file mode
>   is set by the rc.d/accounting script.

>   These changes update the accounting script to create the directory with mode
>   0750 if it doesn't already exist, and to create the daily file with mode
>   0640. This limits write access to root only, read access to root and members
>   of wheel, and eliminates world access completely. For admins who want to
>   prevent even members of wheel from accessing the files, the mode of the
>   /var/account directory can be manually changed to 0700, because the script
>   never creates or changes that directory if it already exists.

I like it. However, the /var/account directory is normally created by
mtree: etc/mtree/BSD.var.dist. Perhaps the permissions should be
adjusted there as well.

-- 
Jilles Tjoelker
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r349974 - head/libexec/rc/rc.d

2019-07-14 Thread Enji Cooper (yaneurabeya)

> On Jul 13, 2019, at 09:07, Ian Lepore  wrote:
> 
> Author: ian
> Date: Sat Jul 13 16:07:38 2019
> New Revision: 349974
> URL: https://svnweb.freebsd.org/changeset/base/349974
> 
> Log:
>  Limit access to system accounting files.
> 
>  In 2013 the security chapter of the Handbook was updated in r42501 to
>  suggest limiting access to the system accounting file [*1] by creating the
>  initial file with a mode of 0600. This was in part based on a discussion in
>  the forums [*2]. Unfortunately, this advice is overridden by the fact that a
>  new file is created as part of periodic daily processing, and the file mode
>  is set by the rc.d/accounting script.
> 
>  These changes update the accounting script to create the directory with mode
>  0750 if it doesn't already exist, and to create the daily file with mode
>  0640. This limits write access to root only, read access to root and members
>  of wheel, and eliminates world access completely. For admins who want to
>  prevent even members of wheel from accessing the files, the mode of the
>  /var/account directory can be manually changed to 0700, because the script
>  never creates or changes that directory if it already exists.
> 
>  The accounting_rotate_log() function now also handles the error cases of no
>  existing log file to rotate, and attempting to rotate the file multiple
>  times (.0 file already exists).
> 
>  Another small change here eliminates the complexity of the mktemp/chmod/mv
>  sequence for creating a new acct file by using install(1) with the flags
>  needed to directly create the file with the desired ownership and
>  modes. That allows coalescing two separate if checkyesno accounting_enable
>  blocks into one.
> 
>  These changes were inspired by my investigation of PR 202203.
> 
>  [1] https://www.freebsd.org/doc/handbook/security-accounting.html
>  [2] http://forums.freebsd.org/showthread.php?t=41059
> 
>  PR:  202203
>  Differential Revision:   https://reviews.freebsd.org/D20876

Does this deserve a “Relnotes: yes”…?
Thanks!
-Enji
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


svn commit: r349974 - head/libexec/rc/rc.d

2019-07-13 Thread Ian Lepore
Author: ian
Date: Sat Jul 13 16:07:38 2019
New Revision: 349974
URL: https://svnweb.freebsd.org/changeset/base/349974

Log:
  Limit access to system accounting files.
  
  In 2013 the security chapter of the Handbook was updated in r42501 to
  suggest limiting access to the system accounting file [*1] by creating the
  initial file with a mode of 0600. This was in part based on a discussion in
  the forums [*2]. Unfortunately, this advice is overridden by the fact that a
  new file is created as part of periodic daily processing, and the file mode
  is set by the rc.d/accounting script.
  
  These changes update the accounting script to create the directory with mode
  0750 if it doesn't already exist, and to create the daily file with mode
  0640. This limits write access to root only, read access to root and members
  of wheel, and eliminates world access completely. For admins who want to
  prevent even members of wheel from accessing the files, the mode of the
  /var/account directory can be manually changed to 0700, because the script
  never creates or changes that directory if it already exists.
  
  The accounting_rotate_log() function now also handles the error cases of no
  existing log file to rotate, and attempting to rotate the file multiple
  times (.0 file already exists).
  
  Another small change here eliminates the complexity of the mktemp/chmod/mv
  sequence for creating a new acct file by using install(1) with the flags
  needed to directly create the file with the desired ownership and
  modes. That allows coalescing two separate if checkyesno accounting_enable
  blocks into one.
  
  These changes were inspired by my investigation of PR 202203.
  
  [1] https://www.freebsd.org/doc/handbook/security-accounting.html
  [2] http://forums.freebsd.org/showthread.php?t=41059
  
  PR:   202203
  Differential Revision:https://reviews.freebsd.org/D20876

Modified:
  head/libexec/rc/rc.d/accounting

Modified: head/libexec/rc/rc.d/accounting
==
--- head/libexec/rc/rc.d/accounting Sat Jul 13 15:53:28 2019
(r349973)
+++ head/libexec/rc/rc.d/accounting Sat Jul 13 16:07:38 2019
(r349974)
@@ -21,23 +21,27 @@ start_cmd="accounting_start"
 stop_cmd="accounting_stop"
 rotate_log_cmd="accounting_rotate_log"
 
+create_accounting_file()
+{
+   install -o root -g wheel -m 0640 /dev/null "${accounting_file}"
+}
+
 accounting_start()
 {
local _dir
 
_dir="${accounting_file%/*}"
if [ ! -d "$_dir" ]; then
-   if ! mkdir -p "$_dir"; then
+   if ! mkdir -p -m 0750 "$_dir"; then
err 1 "Could not create $_dir."
fi
fi
 
if [ ! -e "$accounting_file" ]; then
echo -n "Creating accounting file ${accounting_file}"
-   touch "$accounting_file"
+   create_accounting_file
echo '.'
fi
-   chmod 644 "$accounting_file"
 
echo "Turning on accounting."
${accounting_command} ${accounting_file}
@@ -51,21 +55,24 @@ accounting_stop()
 
 accounting_rotate_log()
 {
-   local _dir _file
+   # Note that this function must handle being called as "onerotate_log"
+   # (by the periodic scripts) when accounting is disabled, and handle
+   # being called multiple times (by an admin making mistakes) without
+   # anything having actually rotated the old .0 file out of the way.
 
-   _dir="${accounting_file%/*}"
-   cd $_dir
+   if [ -e "${accounting_file}.0" ]; then
+   err 1 "Cannot rotate accounting log, ${accounting_file}.0 
already exists."
+   fi
 
-   if checkyesno accounting_enable; then
-   _file=`mktemp newacct-X`
-   chmod 644 $_file
-   ${accounting_command} ${_dir}/${_file}
+   if [ ! -e "${accounting_file}" ]; then
+   err 1 "Cannot rotate accounting log, ${accounting_file} does 
not exist."
fi
 
mv ${accounting_file} ${accounting_file}.0
 
if checkyesno accounting_enable; then
-   mv $_file ${accounting_file}
+   create_accounting_file
+   ${accounting_command} "${accounting_file}"
fi
 }
 
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"