Bug 202203 - acct(5): accounting, the default rc.conf doesn't match periodic.conf
Summary: acct(5): accounting, the default rc.conf doesn't match periodic.conf
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: conf (show other bugs)
Version: 10.2-STABLE
Hardware: Any Any
: --- Affects Many People
Assignee: Ian Lepore
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-08-09 17:54 UTC by Yanhui Shen
Modified: 2019-09-16 01:55 UTC (History)
2 users (show)

See Also:
koobs: mfc-stable11?
koobs: mfc-stable12?


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Yanhui Shen 2015-08-09 17:54:37 UTC
The first issue:

According to /etc/defaults/rc.donf, accounting service is disabled by default.
While in /etc/defaults/periodic.conf, 310.accounting is enabled and will be executed everyday.
Therefore the following error message shows up in periodic's daily log(which is /var/log/daily.log by default):

Rotating accounting logs and gathering statistics:
Cannot 'rotate_log' accounting. Set accounting_enable to YES in /etc/rc.conf or use 'onerotate_log' instead of 'rotate_log'.
cp: acct.0: No such file or directory
sa: open /var/account/acct.merge for read/write: No such file or directory
rm: acct.merge: No such file or directory

----------------------

The second issue:

According to FreeBSD Handbook(https://www.freebsd.org/doc/handbook/security-accounting.html) the file permission of /var/account/acct should be 600, while in script /etc/rc.d/accounting, the permission of that file will be always changed to 644. I think either Handbook or rc script must be wrong.
Comment 1 commit-hook freebsd_committer 2019-07-07 17:16:26 UTC
A commit references this bug:

Author: ian
Date: Sun Jul  7 17:15:45 UTC 2019
New revision: 349807
URL: https://svnweb.freebsd.org/changeset/base/349807

Log:
  Eliminate spurious periodic.daily error message for rotating accounting log.

  In 2011, r218961 removed local code for rotating logs in favor of using the
  rotate_log command in etc/rc.d/accounting.  If the accounting service is
  activated then subsequently de-activated in rc.conf but still remains active
  in periodic.conf, then you get an error message every day in the periodic
  jobs about being unable to rotate the logs.

  With this change to use "onerotate_log", the log rotation will happen the
  first time periodic daily runs after accounting was disabled but periodic
  accounting was left enabled.  After that happens once, the /var/account/acct
  will no longer exist, which results in a different path through the periodic
  code and no more error messages will appear (unless daily_show_badconfig is
  set, in which case the admin will be told that periodic security processing
  is enabled but the accounting file is not present).

  This is only a partial fix for the problems reported in PR 202203.

  PR:		202203

Changes:
  head/usr.sbin/periodic/etc/daily/310.accounting
Comment 2 Ian Lepore freebsd_committer 2019-07-07 18:18:16 UTC
r349807 should eliminate the spurious daily error messages.  I believe that leaves two things to fix:

 1. the rc.d/accounting script recreates the acct file every day with insecure file mode bits (likewise when it creates the /var/account dir).

 2. The advice in the handbook has become outdated.


For #1, I've posted a phab review, https://reviews.freebsd.org/D20876


For #2, I propose updating the handbook.  I'm not a docs person, so I don't have a diff for that, but I propose that the new sequence for enabling it be changed from touch/chmod/accton/sysrc to:

   service accounting enable
   service accounting start

Then a paragraph should be added about file security, something like:

The accounting information is stored in files located in /var/account, which is automatically created, if necessary, the first time the accounting service starts.  These files contain sensitive information, including all the commands issued by all users.  Write access to the files is limited to root, and read access is limited to root and members of the wheel group.  To also prevent members of wheel from reading the files, change the mode of the /var/account directory to allow access only by root.
Comment 3 commit-hook freebsd_committer 2019-07-13 16:08:26 UTC
A commit references this bug:

Author: ian
Date: Sat Jul 13 16:07:38 UTC 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

Changes:
  head/libexec/rc/rc.d/accounting
Comment 4 commit-hook freebsd_committer 2019-08-11 23:01:46 UTC
A commit references this bug:

Author: ian
Date: Sun Aug 11 23:01:32 UTC 2019
New revision: 350878
URL: https://svnweb.freebsd.org/changeset/base/350878

Log:
  MFC r349807, r349974, r349976, r350324, r350361, r350445

  r349807:
  Eliminate spurious periodic.daily error message for rotating accounting log.

  In 2011, r218961 removed local code for rotating logs in favor of using the
  rotate_log command in etc/rc.d/accounting.  If the accounting service is
  activated then subsequently de-activated in rc.conf but still remains active
  in periodic.conf, then you get an error message every day in the periodic
  jobs about being unable to rotate the logs.

  With this change to use "onerotate_log", the log rotation will happen the
  first time periodic daily runs after accounting was disabled but periodic
  accounting was left enabled.  After that happens once, the /var/account/acct
  will no longer exist, which results in a different path through the periodic
  code and no more error messages will appear (unless daily_show_badconfig is
  set, in which case the admin will be told that periodic security processing
  is enabled but the accounting file is not present).

  This is only a partial fix for the problems reported in PR 202203.

  PR:		202203

  r349974:
  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

  r349976:
  Add an entry mentioning the permission/mode change to daily accounting files.

  r350324:
  Fix indentation (spaces->tab).

  r350361:
  Re-wrap the text at 80 columns after fixing the indent in the prior commit.

  r350445:
  Create the /var/account dir with mode 0750; this is a followup to r349974.

  The rc.d/account script contains code to create the /var/account dir, so
  it hadn't occurred to me that it is normally created via mtree; thanks to
  jilles@ for pointing it out.

Changes:
_U  stable/12/
  stable/12/UPDATING
  stable/12/etc/mtree/BSD.var.dist
  stable/12/libexec/rc/rc.d/accounting
  stable/12/usr.sbin/periodic/etc/daily/310.accounting
Comment 5 commit-hook freebsd_committer 2019-08-11 23:01:48 UTC
A commit references this bug:

Author: ian
Date: Sun Aug 11 23:01:32 UTC 2019
New revision: 350878
URL: https://svnweb.freebsd.org/changeset/base/350878

Log:
  MFC r349807, r349974, r349976, r350324, r350361, r350445

  r349807:
  Eliminate spurious periodic.daily error message for rotating accounting log.

  In 2011, r218961 removed local code for rotating logs in favor of using the
  rotate_log command in etc/rc.d/accounting.  If the accounting service is
  activated then subsequently de-activated in rc.conf but still remains active
  in periodic.conf, then you get an error message every day in the periodic
  jobs about being unable to rotate the logs.

  With this change to use "onerotate_log", the log rotation will happen the
  first time periodic daily runs after accounting was disabled but periodic
  accounting was left enabled.  After that happens once, the /var/account/acct
  will no longer exist, which results in a different path through the periodic
  code and no more error messages will appear (unless daily_show_badconfig is
  set, in which case the admin will be told that periodic security processing
  is enabled but the accounting file is not present).

  This is only a partial fix for the problems reported in PR 202203.

  PR:		202203

  r349974:
  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

  r349976:
  Add an entry mentioning the permission/mode change to daily accounting files.

  r350324:
  Fix indentation (spaces->tab).

  r350361:
  Re-wrap the text at 80 columns after fixing the indent in the prior commit.

  r350445:
  Create the /var/account dir with mode 0750; this is a followup to r349974.

  The rc.d/account script contains code to create the /var/account dir, so
  it hadn't occurred to me that it is normally created via mtree; thanks to
  jilles@ for pointing it out.

Changes:
_U  stable/12/
  stable/12/UPDATING
  stable/12/etc/mtree/BSD.var.dist
  stable/12/libexec/rc/rc.d/accounting
  stable/12/usr.sbin/periodic/etc/daily/310.accounting
Comment 6 commit-hook freebsd_committer 2019-09-16 01:55:51 UTC
A commit references this bug:

Author: loader
Date: Mon Sep 16 01:55:48 UTC 2019
New revision: 53406
URL: https://svnweb.freebsd.org/changeset/doc/53406

Log:
  Update the Process Accounting section.

  PR:		202203
  Reviewed by:	ian
  Submitted by:	ian
  Differential Revision:	https://reviews.freebsd.org/D20878

Changes:
  head/en_US.ISO8859-1/books/handbook/security/chapter.xml