Bug 221866
| Summary: | [patch] ls -liTd in 100.chksetuid with large inodes will cause daily security run output to misreport setuid changes | ||||||
|---|---|---|---|---|---|---|---|
| Product: | Base System | Reporter: | Derek Schrock <dereks> | ||||
| Component: | bin | Assignee: | freebsd-bugs (Nobody) <bugs> | ||||
| Status: | New --- | ||||||
| Severity: | Affects Some People | CC: | jlh, pstef | ||||
| Priority: | --- | Keywords: | patch | ||||
| Version: | 11.1-RELEASE | ||||||
| Hardware: | Any | ||||||
| OS: | Any | ||||||
| Attachments: |
|
||||||
I briefly looked at this today. I think the stat change is acceptable. Or maybe I could try to implement a flag in diff that would ignore leading blank characters and then we could use it via $security_status_diff_flags. This is similar to hacking check_diff as you proposed, but more elegant. I thought that using find's -ls was a very good idea due to the side effect of decreasing the number of spawned processes. But since I saw the output I think is too wide. |
Created attachment 185827 [details] Use stat instead of ls Due to ls padding left spaces when -i is used with multiple files a new setuid program with a large(r) (character wise) inode than the previous day will cause 100.chksetuid to show all setuid programs have changed due to the extra left spaces. ex: $ ls -liTd /bin/rcp /sbin/mksnap_ffs 7056686 -r-sr-xr-x 1 root wheel 20912 Jul 26 00:41:31 2017 /bin/rcp* 5544103 -r-sr-xr-- 1 root operator 10600 Jul 26 00:41:33 2017 /sbin/mksnap_ffs* $ ls -liTd /bin/rcp /sbin/mksnap_ffs /cold/backups/ircbsd/Saturday/repos/mfsbsd/tmp/mfs/bin/rcp 7056686 -r-sr-xr-x 1 root wheel 20912 Jul 26 00:41:31 2017 /bin/rcp* 14240023 -r-sr-xr-x 1 dereks wheel 20912 Jul 20 22:09:31 2017 /cold/backups/ircbsd/Saturday/repos/mfsbsd/tmp/mfs/bin/rcp* 5544103 -r-sr-xr-- 1 root operator 10600 Jul 26 00:41:33 2017 /sbin/mksnap_ffs* Comparing outputs with diff: $ diff -u <(ls -liTd /bin/rcp /sbin/mksnap_ffs) <(ls -liTd /bin/rcp /sbin/mksnap_ffs /cold/backups/ircbsd/Saturday/repos/mfsbsd/tmp/mfs/bin/rcp) --- /tmp//sh-np.MBFmbA 2017-08-27 15:45:41.762541000 -0400 +++ /tmp//sh-np.tQ7OPb 2017-08-27 15:45:41.762802000 -0400 @@ -1,2 +1,3 @@ -7056686 -r-sr-xr-x 1 root wheel 20912 Jul 26 00:41:31 2017 /bin/rcp* -5544103 -r-sr-xr-- 1 root operator 10600 Jul 26 00:41:33 2017 /sbin/mksnap_ffs* + 7056686 -r-sr-xr-x 1 root wheel 20912 Jul 26 00:41:31 2017 /bin/rcp* +14240023 -r-sr-xr-x 1 dereks wheel 20912 Jul 20 22:09:31 2017 /cold/backups/ircbsd/Saturday/repos/mfsbsd/tmp/mfs/bin/rcp* + 5544103 -r-sr-xr-- 1 root operator 10600 Jul 26 00:41:33 2017 /sbin/mksnap_ffs* The left side space appears as if something changed with /bin/rcp and mksnp_fss. This could (and does) result in large daily security emails if there's a lot of setuid programs. Possible solutions (ranked in order): - Use stat will the same output as ls -liTd in 100.chksetuid's find (see patch) - Strip the left side spaces from 100.chksetuid ls' output (see ideal than stat(1)) - Does use find's -exec + and use ; instead (use more processes and ; will be slower) - Have check_diff remove left side spaces (possible side effects) - Use find's -ls (not the same output as ls -liTd or stat) Using diff with stat: $ diff -u <(stat -f '%i %Sp %l %Su %Sg %t%10z %Sm %N' /bin/rcp /sbin/mksnap_ffs) <(stat -f '%i %Sp %l %Su %Sg %t%10z %Sm %N' /bin/rcp /sbin/mksnap_ffs /cold/backups/ircbsd/Saturday/repos/mfsbsd/tmp/mfs/bin/rcp) --- /tmp//sh-np.qVqZSi 2017-08-27 15:46:38.188383000 -0400 +++ /tmp//sh-np.bEjyNk 2017-08-27 15:46:38.188730000 -0400 @@ -1,2 +1,3 @@ 7056686 -r-sr-xr-x 1 root wheel 20912 Jul 26 00:41:31 2017 /bin/rcp 5544103 -r-sr-xr-- 1 root operator 10600 Jul 26 00:41:33 2017 /sbin/mksnap_ffs +14240023 -r-sr-xr-x 1 dereks wheel 20912 Jul 20 22:09:31 2017 /cold/backups/ircbsd/Saturday/repos/mfsbsd/tmp/mfs/bin/rcp One downside will be the first time 100.chksetuid runs with stat you'll see all setuid changed due to spacing. If this isn't desired stripping left side spaces from ls output before it gets to check_diff would be next best?