Bug 223461 - [FIX] 200.backup-passwd does not filter passwords properly
Summary: [FIX] 200.backup-passwd does not filter passwords properly
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 11.1-STABLE
Hardware: Any Any
: --- Affects Some People
Assignee: Ed Maste
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2017-11-06 05:28 UTC by Andre Albsmeier
Modified: 2018-05-23 14:07 UTC (History)
1 user (show)

See Also:
emaste: mfc-stable11?
emaste: mfc-stable10?


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Andre Albsmeier 2017-11-06 05:28:58 UTC
If /etc/periodic/daily/200.backup-passwd finds difference in master.passwd.bak and master.passwd.bak2, it tries to filter out encrypted passwords so they don't get sent by mail. However, this does not work for lines without the +/- prefix from diff. Here, toor changed but root was left alone:

Backup passwd and group files:
... passwd diffs:
--- /var/backups/master.passwd.bak      2017-11-04 12:31:02.788214000 +0100
+++ /etc/master.passwd  2017-11-05 13:23:53.606509000 +0100
@@ -1,7 +1,7 @@
 # $FreeBSD: stable/11/etc/master.passwd 299365 2016-05-10 12:47:36Z bcr $
 #
 root:$6$4wTiD2ItHpuB....:0:0:std:0:0:Charlie &:/root:/bin/zsh
-toor:(password):0:0:std:0:0:Bourne-again Superuser:/root:/bin/sh
+toor:(password):0:0:std:0:0:Bourne-again Superuser:/root:/bin/sh
 daemon:*:1:1::0:0:Owner of many system processes:/root:/usr/sbin/nologin
 operator:*:2:5::0:0:System &:/:/usr/sbin/nologin
 bin:*:3:7::0:0:Binaries Commands and Source:/:/usr/sbin/nologin

Fix:
make the leading +/- optional by using

sed 's/^\([-+]\{0,1\}[^-+:]*\):[^:]*:/\1:(password):/'

or use a modern RE:

sed -E 's/^([-+]?[^-+:]*):[^:]*:/\1:(password):/'

Possibly, it can even be changed to:

sed -E 's/^([-+]?[^:]*):[^:]*:/\1:(password):/'

but I am not sure about that (maybe that would give bad interaction with NIS
or whatever)...
Comment 1 Ed Maste freebsd_committer freebsd_triage 2017-11-20 19:32:45 UTC
What do you think about

sed 's/^\([-+ ][^-+:]*\):[^:]*:/\1:(password):/'

i.e., just adding ' ' to the first []
Comment 2 Andre Albsmeier 2017-11-21 05:20:08 UTC
Seems to be another one of probably 1000 possibilites ;-)
Comment 3 commit-hook freebsd_committer freebsd_triage 2017-11-21 20:32:47 UTC
A commit references this bug:

Author: emaste
Date: Tue Nov 21 20:31:55 UTC 2017
New revision: 326074
URL: https://svnweb.freebsd.org/changeset/base/326074

Log:
  filter all passwords (not only changed) from periodic passwd backup

  The periodic 200.backup-passwd script outputs any differences it finds
  in master.passwd, relative to the previous backup.  It intends to elide
  the encrypted password field, but previously did so only for changed
  lines (i.e., those beginning with - or + in the diff).

  Apply the sed expression also to unchanged lines to also elide their
  passwords.

  PR:		223461
  Reported by:	Andre Albsmeier
  MFC after:	2 weeks
  Sponsored by:	The FreeBSD Foundation

Changes:
  head/etc/periodic/daily/200.backup-passwd
Comment 4 commit-hook freebsd_committer freebsd_triage 2017-12-05 01:36:08 UTC
A commit references this bug:

Author: emaste
Date: Tue Dec  5 01:35:04 UTC 2017
New revision: 326546
URL: https://svnweb.freebsd.org/changeset/base/326546

Log:
  MFC r326074: filter all passwords (not only changed) from periodic passwd backup

  The periodic 200.backup-passwd script outputs any differences it finds
  in master.passwd, relative to the previous backup.  It intends to elide
  the encrypted password field, but previously did so only for changed
  lines (i.e., those beginning with - or + in the diff).

  Apply the sed expression also to unchanged lines to also elide their
  passwords.

  PR:		223461
  Reported by:	Andre Albsmeier
  Sponsored by:	The FreeBSD Foundation

Changes:
_U  stable/11/
  stable/11/etc/periodic/daily/200.backup-passwd
Comment 5 Eitan Adler freebsd_committer freebsd_triage 2018-05-23 10:27:46 UTC
batch change of PRs untouched in 2018 marked "in progress" back to open.
Comment 6 commit-hook freebsd_committer freebsd_triage 2018-05-23 14:06:02 UTC
A commit references this bug:

Author: emaste
Date: Wed May 23 14:05:56 UTC 2018
New revision: 334097
URL: https://svnweb.freebsd.org/changeset/base/334097

Log:
  MFC r326074: filter all passwords (not only changed) from periodic passwd backup

  The periodic 200.backup-passwd script outputs any differences it finds
  in master.passwd, relative to the previous backup.  It intends to elide
  the encrypted password field, but previously did so only for changed
  lines (i.e., those beginning with - or + in the diff).

  Apply the sed expression also to unchanged lines to also elide their
  passwords.

  PR:		223461
  Reported by:	Andre Albsmeier
  Sponsored by:	The FreeBSD Foundation

Changes:
_U  stable/10/
  stable/10/etc/periodic/daily/200.backup-passwd