Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lynis needs to drop privilleges to support homebrew on MacOSX #382

Closed
AlainG80 opened this issue Apr 29, 2017 · 8 comments · May be fixed by #1564
Closed

Lynis needs to drop privilleges to support homebrew on MacOSX #382

AlainG80 opened this issue Apr 29, 2017 · 8 comments · May be fixed by #1564
Assignees

Comments

@AlainG80
Copy link

AlainG80 commented Apr 29, 2017

Given I run the version of today + mac ports patch:
When scanning I need to login as root.
Then I get the following error in my report:

[+] Ports and packages

  • Searching package managers
    • Searching brew [ FOUND ]
    • Querying brew for installed packages
      Error: Running Homebrew as root is extremely dangerous and no longer supported.
      As Homebrew does not drop privileges on installation you would be giving all
      build scripts full access to your system.
  • Checking ports for updates [ FOUND ]

Logfile /var/log/lynis.log doesn't give any more details.
Expected behavior: the program should drop privileges.

@mboelen
Copy link
Member

mboelen commented Apr 29, 2017

Thanks for reporting this. Lynis doesn't drop privileges as it is a shell script and it does not have a default user to use (like nginx would use www-data on Ubuntu). If you would run Lynis as a non-privileged user, this error does not show up, right?

Not sure if there is a right fix here. If you run Lynis as root user (or indirectly via sudo), then you would also run Brew as root user. Unless there is a default user for Brew. Any suggestion from your side?

@mboelen mboelen self-assigned this Apr 29, 2017
@AlainG80
Copy link
Author

AlainG80 commented Apr 29, 2017

Macports has a user account, but my version of homebrew does not.
Beter test if this breaks FreeBSD, otherwise we need an uname -eq Darwin test:

#################################################################################
 #
     # Test        : PKGS-7301
-    # Description : Query FreeBSD pkg
+    # Description : Query FreeBSD/MacOSX pkg
     if [ -x ${ROOTDIR}usr/sbin/pkg ]; then PREQS_MET="YES"; else PREQS_MET="NO"; fi
     Register --test-no PKGS-7301 --preqs-met ${PREQS_MET} --weight L --network NO --category security --description "Query NetBSD pkg"
     if [ ${SKIPTEST} -eq 0 ]; then
@@ -95,14 +119,33 @@
         LogText "Result: Found brew"
         Report "package_manager[]=brew"
         LogText "Test: Querying brew to get package list"
-        Display --indent 4 --text "- Querying brew for installed packages"
+        Display --indent 6 --text "- Querying brew for installed packages"
         LogText "Output:"; LogText "-----"
-        GPACKAGES=$(brew list)
-        for J in ${GPACKAGES}; do
-            LogText "Found package ${J}"
-            INSTALLED_PACKAGES="${INSTALLED_PACKAGES}|${J}"
+        if [ $EUID -eq 0 ]; then
+            GPACKAGES=$(/usr/bin/sudo -u $SUDO_USER /usr/local/bin/brew list)
+        else
+            GPACKAGES=$(/usr/local/bin/brew list)
+        fi
+        for ITEM in ${GPACKAGES}; do
+            LogText "Found package ${ITEM}"
+            INSTALLED_PACKAGES="${INSTALLED_PACKAGES}|${ITEM}"
         done
-      else
+        if [ $EUID -eq 0 ]; then
+            UPACKAGES=$(/usr/bin/sudo -u $SUDO_USER /usr/local/bin/brew outdated)
+        else
+            UPACKAGES=$(/usr/local/bin/brew outdated)
+        fi
+        if [ -n UPACKAGES ]; then
+            PACKAGE_AUDIT_TOOL="brew"
+            PACKAGE_AUDIT_TOOL_FOUND=1
+            LogText "Test: Checking brew to get list of outdated packages" 
+            Display --indent 4 --text "- Checking brew for outdated packages" --result "${STATUS_FOUND}" --color YELLOW
+            for ITEM in ${UPACKAGES}; do
+                LogText "Found package ${ITEM}"
+                INSTALLED_PACKAGES="${INSTALLED_PACKAGES}|${ITEM}"
+            done
+        else
+            Display --indent 4 --text "- Querying brew for outdated packages" --result "${STATUS_NOT_FOUND}" --color GREEN
+        fi
+    else
         LogText "Result: brew can NOT be found on this system"
     fi
 #
@@ -586,6 +629,30 @@

I strongly recommend that the long term solution is that the program makes an unprivileged daemon account, for security purposes under any OS. Many binaries don't have a fixed path, or do not need to be run with EUID=0.

To get a free uid under MacOSX:
uid=$(($(${SUDOBINARY} ${_dscl} . list /Users uid | ${GREPBINARY} -o '5..' | ${SORTBINARY} -u | ${TAILBINARY} -1)+1))
${_dscl} . -create /Users/$username UniqueID $uid
${_dscl} . -create /Users/$USERNAME UserShell /usr/bin/false

https://serverfault.com/questions/182347/add-daemon-account-on-os-x

@AlainG80
Copy link
Author

With homebrew I can still run old mac versions:

--- ./include/osdetection.bak	2017-04-30 11:23:40.000000000 +0100
+++ ./include/osdetection	2017-04-30 11:25:37.000000000 +0100
@@ -46,6 +46,8 @@
                 OS_VERSION_NAME="unknown"
                 OS_FULLNAME="macOS (unknown version)"
                 case ${OS_VERSION} in
+                    10.7 | 10.7.[0-9]*) OS_FULLNAME="Mac OS X 10.7 (Lion)" ;;
+                    10.8 | 10.8.[0-9]*) OS_FULLNAME="Mac OS X 10.8 (Mountain Lion)" ;;
                     10.9 | 10.9.[0-9]*) OS_FULLNAME="Mac OS X 10.9 (Mavericks)" ;;
                     10.10 | 10.10.[0-9]*) OS_FULLNAME="Mac OS X 10.10 (Yosemite)" ;;
                     10.11 | 10.11.[0-9]*) OS_FULLNAME="Mac OS X 10.11 (El Capitan)" ;;

@mboelen
Copy link
Member

mboelen commented Apr 30, 2017

Thanks for suggestions. This is an interesting case and has to be researched more in depth from our side, to see if this is in line with the goals of Lynis.

As Lynis is not a daemon process, we rather not switch back to a normal user. That is up to the user to do, especially as there could be multiple users on a single system that have Brew installed. Lynis does not have to be executed as root. So if Brew is discovered for the root user, and Brew gives a warning, it might be something to "accept" from an auditing point of view.

Just to be sure: on your system, do you have Brew installed as a root user, or only as a normal user?

@AlainG80
Copy link
Author

Homebrew is installed under my normal user account. Only the make install step could require elevated privileges. That applies to all package auditing tools.
Using the $SUDO_USER variable should not conflict with multiple users accounts.
If a daemon is not an option for the GPL version, than at least all executables should have a full path to prevent privilege escalation.

@jpartain89
Copy link
Contributor

Well, since errors are already displayed when running as NOT sudo, saying not every test would run without it, for the time being the same could be displayed when running AS sudo, that HomeBrew tests will NOT run when using Sudo.... Seeing as that is what happens... Therefore stopping HomeBrew tests on Sudo.

@mboelen
Copy link
Member

mboelen commented Mar 10, 2018

Closing this issue at this moment, as there is not enough support for this item. We monitored the item to see if more macOS users feel the same. We rather prevent the usage of sudo or having the hard requirement of a dedicated user.

@mboelen mboelen closed this as completed Mar 10, 2018
@jpartain89
Copy link
Contributor

Well, it doesn't exactly seem that there is a large macOS user base that is active here on this page. Seeing as I just reported the issue about Lynis not identifying Mojave, of which any number of beta testers who would have been using Lynis would've reported such an issue months ago...

The better reasoning could've been "the means of circumventing this issue is much too difficult and doesn't work with the rest of our code" instead of "our already tiny macOS user base just didn't respond with the same furor as any of the larger user base...

Apologies if the above opinion came off as overly rude, but I don't care to sugarcoat things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants