Bug 251152
| Summary: | sysutils/bsdstats violates POLA and data protection rules | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | Ports & Packages | Reporter: | Martin Birgmeier <d8zNeCFG> | ||||||||
| Component: | Individual Port(s) | Assignee: | Lars Engels <lme> | ||||||||
| Status: | Closed FIXED | ||||||||||
| Severity: | Affects Many People | CC: | 000.fbsd, andrew, dewayne, emaste, grahamperrin, jrm, lme, portmaster, sblachmann, scrappy, swills | ||||||||
| Priority: | --- | Flags: | bugzilla:
maintainer-feedback?
(scrappy) |
||||||||
| Version: | Latest | ||||||||||
| Hardware: | Any | ||||||||||
| OS: | Any | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Martin Birgmeier
2020-11-15 10:25:37 UTC
I concur. All services *must* include an rc var of
_ENABLE||_DISABLE
meaning... that any script installed to ${LOCALBASE}/etc
*require* an xyzservice_enable=(yes|no) in /etc/rc.conf
any script not following this standard should be marked
BROKEN
Created attachment 220337 [details]
patch to port
I think this might help.
(In reply to Chris Hutchinson from comment #1) You lost me here ... it does have that, and it defaults to NO in /usr/local/etc/rc.d/bsdstats ( I just confirmed ): -- # default to disable bsdstats_enable=${bsdstats_enable:-"NO"} -- (In reply to Steve Wills from comment #2) Again, a bit lost here, but also just checking the port, and the change you made appears to already exist too: -- if [ -n "$PACKAGE_BUILDING" ]; then BSDSTATS_MONTHLY_NOW=${BSDSTATS_MONTHLY_NOW:=no} BSDSTATS_REBOOT_REPORTING=${BSDSTATS_REBOOT_REPORTING:=no} else BSDSTATS_MONTHLY_NOW=${BSDSTATS_MONTHLY_NOW:=yes} BSDSTATS_REBOOT_REPORTING=${BSDSTATS_REBOOT_REPORTING:=yes} fi -- And since at least April of this year: -- # ls -lt pkg-install -rw-r--r-- 1 root wheel 2299 Apr 30 2020 pkg-install -- Not clear what is being fixed here ... (In reply to Martin Birgmeier from comment #0) to your notes about: -- Result: - the port gets installed - during installation, without warning and without further user interaction: . it collects information and transmits it to a central site . it installs a tracking cookie/unique id in /var/db/bsdstats . it installs a setting in /etc/rc.conf making it run on every system startup . it installs periodic scripts making it run periodically . it never reveals which information is collected and sent to the central site -- that is not how it *ever* worked, or, at least, that isn't how it was designed ... in pkg-install, there is a specific section during install that not only asks you if you want to enable it, but also which features of it you would like to enable ... specifically -- if [ "$2" = "POST-INSTALL" ]; then log_file="${INSTALL_PREFIX}/var/log/bsdstats" need_to_ask=0 if [ ! -e ${log_file} ]; then need_to_ask=1 elif [ $(($(date +"%s")-$(stat -f %Sm -t %s ${log_file}))) -gt $((60*60*24*40)) ]; then need_to_ask=1 fi if [ $need_to_ask = 1 ]; then if yesno "Would you like to run BSDstats now" $BSDSTATS_MONTHLY_NOW; then ${INSTALL_PREFIX}${PKG_PREFIX}/etc/periodic/monthly/300.statistics -nodelay fi fi if [ ! -f "/etc/rc.conf" ] || [ -z $(grep bsdstats_enable /etc/rc.conf) ]; then echo echo "If you're installing BSDstats on a system that won't always be on, such as a" echo "desktop or a laptop, it is recommended that you enable it in /etc/rc.conf so" echo "that it will run on bootup. This will ensure that, even if your computer is" echo "off when \"monthly\" runs, your computer will be counted properly." echo if yesno "Would you like to enable reporting on bootup in /etc/rc.conf" $BSDSTATS_REBOOT_REPORTING; then echo "bsdstats_enable=\"YES\"" >> /etc/rc.conf fi fi fi -- I'm looking at it right now ... this was *not* how it was supposed to work ... I just cleared bsdstats from my system and reinstalled, and it is either not hitting pkg-install -or- not hitting that section of pkg-install ... (In reply to scrappy from comment #4) If PACKAGE_BUILDING is not set, the default is "yes", which it shouldn't be. (In reply to Steve Wills from comment #6) If the pkg-install is setup right ( which it isn't ), each of those variables that are set to yes are supposed to be prompted for ... which is what this code: -- -post-install: -.if ! defined(BATCH) && ! defined(PACKAGE_BUILDING) - @PKG_PREFIX="${PREFIX}" ${SH} ${PKGINSTALL} ${PKGNAME} POST-INSTALL -.endif -- that was removed on last commit *did* do ... Basically, there is a pkg-install file that the last commit to the port somehow broke, but I'm not sure of the proper fix ... So, the original code, we explicitedly called pkg-install to prompt the installer ( non-batch mode ) as to whether they want to enable bsdstats, as well as which modules within bsdstats they want to run ... That code was removed, with, I think, the intended result being that something in: -- -.include <bsd.port.post.mk> +.include <bsd.port.mk> -- would trigger it ( and it isn't ) ... found it: "If the port needs to execute commands when the binary package is installed with pkg add or pkg install, use pkg-install" - https://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/pkg-install.html So the old code was right ... if not a binary package, then explicitedly run pkg-install ... if using portmaster to install, we need that explicit ... Reverting those changes now ... Created attachment 220686 [details]
Patch to fix prompting that was removed in last commit
This patch reverts some of the changes that were made in the last commit, specifically the port is supposed to prompt, during interactive install, whether the installer wishes to actually enable both the script, as well as the individual sub-reports.
Default: install but do not enable
Comment on attachment 220337 [details]
patch to port
I've attached the proper fix, which is to revert some of the blanket changes that were made in previous commit, which removed critical installer input
(In reply to scrappy from comment #9) Testing this, I made sure to delete /var/log/bsdstats and remove entries related to bsdstat from /etc/rc.conf and /etc/periodic.conf then ran make install and it seems to default to yes still: ===> bsdstats-7.1 depends on file: /usr/local/sbin/pkg - found ===> Fetching all distfiles required by bsdstats-7.1 for building ===> Extracting for bsdstats-7.1 ===> Patching for bsdstats-7.1 ===> Configuring for bsdstats-7.1 ===> Staging for bsdstats-7.1 ===> Generating temporary packing list /bin/mkdir -p /usr/local/poudriere/ports/default/sysutils/bsdstats/work/stage/usr/local/etc/periodic/monthly install -m 555 /usr/local/poudriere/ports/default/sysutils/bsdstats/work/300.statistics /usr/local/poudriere/ports/default/sysutils/bsdstats/work/stage/usr/local/etc/periodic/monthly install -m 555 /usr/local/poudriere/ports/default/sysutils/bsdstats/work/bsdstats-send /usr/local/poudriere/ports/default/sysutils/bsdstats/work/stage/usr/local/bin/bsdstats-send Would you like to enable monthly reporting in /etc/periodic.conf [yes]? Would you like to send a list of installed hardware as well [yes]? Would you like to send a list of installed ports as well [yes]? Would you like to run it now [yes]? Posting OS statistics to rpt.bsdstats.org ... SUCCESS Posting device statistics to rpt.bsdstats.org ... SUCCESS Posting CPU information to rpt.bsdstats.org ... SUCCESS Posting port statistics to rpt.bsdstats.org ... SUCCESS If you're installing BSDstats on a system that won't always be on, such as a desktop or a laptop, it is recommended that you enable it in /etc/rc.conf so that it will run on bootup. This will ensure that, even if your computer is off when "monthly" runs, your computer will be counted properly. Would you like to enable reporting on bootup in /etc/rc.conf [yes]? ====> Compressing man pages (compress-man) ===> Staging rc.d startup script(s) ====> Running Q/A tests (stage-qa) Warning: You have disabled the licenses framework with DISABLE_LICENSES, unable to run checks ===> Installing for bsdstats-7.1 ===> Checking if bsdstats is already installed ===> Registering installation for bsdstats-7.1 Installing bsdstats-7.1... [: monthly_statistics_enable="YES": unexpected operator You installed BSDstats: script reporting statistics about your machine. To disable monthly reporting, add this line to /etc/periodic.conf: monthly_statistics_enable="NO" To disable parts of reporting, add these lines to /etc/periodic.conf: monthly_statistics_report_devices="NO" monthly_statistics_report_ports="NO" To limit which ports are being submitted: create /usr/local/etc/bsdstats.conf and list ports to report in the form ${category}/${portname}, one entry per line. It is also possible to use the special self explanatory keyword "all-ports". Example: sysutils/bsdstats ports-mgmt/pkg To enable reporting on bootup, add this line to /etc/rc.conf: bsdstats_enable="YES" To run it manually any time: bsdstats-send To view current statistics, go to: http://www.bsdstats.org/ You can build bsdstats with TOR port option to submit anonymously Checking the configs after install, it defaulted on: $ grep bsdstats /etc/rc.conf bsdstats_enable="YES" $ grep statistics /etc/periodic.conf monthly_statistics_enable="YES" monthly_statistics_report_devices="YES" monthly_statistics_report_ports="YES" $ and it sent statistics (as reported above) and created /var/log/bsdstats. Also, there seems to be a syntax error in the script (see above). Hi Marc, First, thank you for your efforts to fix this. I believe this port is suffering from too much over-engineering, by putting too much intelligence in the install script, making the solution brittle and prone to the kind of failure described in this defect report and finally analyzed by Marc. I would suggest to add a script to this port which handles all the interactive parts and then replace all the install scripting by a simple pkg-message pointing to this script. In this way, the port's complexity is significantly reduced while at the same time even better logic can be put into this script. -- Martin The updated patch doesn't really seem to solve the problem to me. Is there something else we can do here? (In reply to Steve Wills from comment #10) yes, it defaults to yes if you are doing an interactive install .. it defaults to no if you are doing a pkg install ... why would you manually install the port if you didn't want to enable it? (In reply to scrappy from comment #13) Hi Scrappy. I did this a few years ago; installed bsdstats to contribute to the in-use advocacy of FreeBSD. After reviewing the details of its operation, we decided not to use the tool because, at that time and I don't know if its changed, it revealed too many details of our systems. [Aside we use pkg -d add --no-scripts] (In reply to scrappy from comment #13) This logic could be applied to almost any package that includes a daemon, or even ones that don't like setting the users default shell to something because they installed it, but we have a policy that installing doesn't mean enabled; bsdstats isn't really an exception to this as far as I can tell. (In reply to dewayne from comment #14) Hi Dewayne, There was never a time where we forced reporting anything except for OS being run ... CPU, Ports and Devices were always "option reporting" and can be turned off individually. If you disable CPU / Ports and Devices, this is literally everything that gets recorded to the database: -- -[ RECORD 1 ]-+--------------------------------------------- id | 21456 unique_key | 7799c14fb18d9a94807a6cb2572f0339 token | 9ftlEUz62dSYtlzfsX8GcawJxmrmPq7omnddul6xsTo= country_code | PA first_connect | 2006-09-29 02:52:13.587079 enable | f last_connect | 2017-08-01 08:52:36.547784 using_ipv6 | f system_id | 21456 release | 6.1-PRERELEASE architecture | amd64 report_date | 2006-09-01 00:00:00 os_id | 3 -- Your Tokens, your country of origin, operating system ( 3 == FreeBSD ) and release ... And all you send is: unique_key, token, operating system, release and architecture And nothing is logged in terms of log files: -- <VirtualHost *:80> ServerName rpt.bsdstats.org Documentroot "/usr/local/www/bsdstats/rpt.bsdstats.org/www" DirectoryIndex index.php ErrorLog /dev/null CustomLog /dev/null combined <FilesMatch \.php$> SetHandler "proxy:unix:/tmp/bsdstats.sock|fcgi://localhost" </FilesMatch> </VirtualHost> -- I do change that if I have to debug, for the duration of debugging, but there is no logging done anywhere that records the IP that you scame in from, other then the country code being set ... and that IP disappears as soon as the script is finished running. (In reply to Steve Wills from comment #15) Honestly Steve, I'm not against ( or really care ) if the default is no vs yes ... the defaults really don't matter as long as the png-install is doing what it is supposed to do which is prompting folks for whether they want to enable it or not. So if someone can take the patch I posted and apply it, and just get rid of: -- if [ -n "$PACKAGE_BUILDING" ]; then BSDSTATS_MONTHLY_ENABLE=${BSDSTATS_MONTHLY_ENABLE:=no} BSDSTATS_MONTHLY_DEVICES=${BSDSTATS_MONTHLY_DEVICES:=no} BSDSTATS_MONTHLY_PORTS=${BSDSTATS_MONTHLY_PORTS:=no} BSDSTATS_MONTHLY_NOW=${BSDSTATS_MONTHLY_NOW:=no} BSDSTATS_REBOOT_REPORTING=${BSDSTATS_REBOOT_REPORTING:=no} else BSDSTATS_MONTHLY_ENABLE=${BSDSTATS_MONTHLY_ENABLE:=yes} BSDSTATS_MONTHLY_DEVICES=${BSDSTATS_MONTHLY_DEVICES:=yes} BSDSTATS_MONTHLY_PORTS=${BSDSTATS_MONTHLY_PORTS:=yes} BSDSTATS_MONTHLY_NOW=${BSDSTATS_MONTHLY_NOW:=yes} BSDSTATS_REBOOT_REPORTING=${BSDSTATS_REBOOT_REPORTING:=yes} fi -- out of png-install and replace it with: -- BSDSTATS_MONTHLY_ENABLE=${BSDSTATS_MONTHLY_ENABLE:=no} BSDSTATS_MONTHLY_DEVICES=${BSDSTATS_MONTHLY_DEVICES:=no} BSDSTATS_MONTHLY_PORTS=${BSDSTATS_MONTHLY_PORTS:=no} BSDSTATS_MONTHLY_NOW=${BSDSTATS_MONTHLY_NOW:=no} BSDSTATS_REBOOT_REPORTING=${BSDSTATS_REBOOT_REPORTING:=no} -- That's cool too ... it is definitely not a sticking point for me, sorry if I made it sound like it was ... (In reply to scrappy from comment #16) Thank-you very much. You've placated our concerns, I've read through /usr/local/etc/periodic/monthly/300.statistics, added to /etc/periodic.conf and we'll have it running on our 5 dev servers over the coming fortnight. Cheers Hi all, I would like to second all that Martin said in comment #11. Without combing through the ports tree, I would say the de facto approach is to install the package, but not enable daemons, scripts, etc. The pkg_message then just lets uses know that they have to add foo_enable=yes to /etc/rc.conf (or equivalent). Perhaps a good example here is security/dehydrated. Joe This issue is still open. Yesterday I installed the package with the intention to look at its scripts what they collect and send. Then it ran, without any forewarning or asking for consent, and sent my data unencrypted over the internet to an entity which I do not trust. # pkg ins bsdstats Updating FreeBSD repository catalogue... FreeBSD repository is up to date. All repositories are up to date. The following 1 package(s) will be affected (of 0 checked): New packages to be INSTALLED: bsdstats: 7.0 Number of packages to be installed: 1 8 KiB to be downloaded. Proceed with this action? [y/N]: y [1/1] Fetching bsdstats-7.0.txz: 100% 8 KiB 7.9kB/s 00:01 Checking integrity... done (0 conflicting) [1/1] Installing bsdstats-7.0... [1/1] Extracting bsdstats-7.0: 100% BSDstats runs on this system for the first time, generating registration ID Posting OS statistics to rpt.bsdstats.org ... SUCCESS Posting device statistics to rpt.bsdstats.org ... SUCCESS Posting CPU information to rpt.bsdstats.org ... SUCCESS grep: /usr/local/etc/bsdstats.conf: No such file or directory Posting port statistics to rpt.bsdstats.org ... SUCCESS [...] # See also this forum thread with more details: https://forums.freebsd.org/threads/toplist-freebsd-usage-per-1m-inhabitants.79669/post-504430 This is a serious POLA violation and breach of trust. Get this fixed asap! If this does not get fixed, it is time to contact the press, so they spread the word! After fiasco with (no)reviews for WireGuard kernel code this again shed bad light to FreeBSD reviews / committing process raising the question how could this code be committed? It is more than 10 years ago when FreeBSD started requiring all ports to use explicit enabling in rc.conf so how this port can exist for so long violating this rule? How pkg-install can be written so bad that it enables all functions by default without user's confirmation and sending data remote as a part of the install process? And people who are in charge of it (committers / ports security team) do nothing to fix it. This is really bad. Created attachment 224147 [details] Make reporting opt-in The previous submitted patches only address the post install issue. The periodic script also needs fixed to be opt-in. Currently it will run if monthly_statistics_enable is undefined or not explicitly set to NO, normal logic for periodic scripts is they will only run if their control variable (monthly_statistics_enable) is explicitly set to YES. I've attached a patch which better addresses the issues listed above: - disables reporting by default - does not use the pkg-install script to "modify the currently running system" as per the FreeBSD Porters Handbook Chapter 9.2. - does not output noise if monthly_statistics_* variables aren't enabled - add a post-upgrade message informing the user it's now opt-in and may need enabled [1] https://github.com/freebsd/freebsd-ports/blob/2f973a2955686494aff30a15f18be3fa0492d9de/sysutils/bsdstats/files/300.statistics.in#L567 Comment on attachment 220686 [details]
Patch to fix prompting that was removed in last commit
Removing this patch in favour of other one submitted that disables by default and better follows current port standadards
Comment on attachment 224147 [details]
Make reporting opt-in
If someone can commit this, looks good to me.
I'll take this. I updated the port with Andrew's patch that was approved by scrappy. |
