From af26c92399cbf574d4bd35460aa9f1dffcff1c5b7cf87ca1a65079dd0ad53f38 Mon Sep 17 00:00:00 2001 From: Larry Finger Date: Thu, 8 Jun 2017 22:26:55 +0000 Subject: [PATCH] - Revise warning screen concerning USB passthru - fixes bnc#1041137. USB passthru opens a security hole, yet it is so valuable that many users want the feature, thus it is our default. Previously, a user needed to edit a udev rule to disable passthru. The bad part was that an update of VB changed the rule back to allow passthru without any notification. These changes modify the popup to allow the user to accept or decline passthru. If the user declines, then the root password is requested and the udev rule is modified. As these modifications will be lost with the next VB update, the inode of the udev rule is kept. If the user has previously declined and the inode has changed, the popup will show the next time VB is started. File "fix_usb_rules.sh" is added. OBS-URL: https://build.opensuse.org/package/show/Virtualization/virtualbox?expand=0&rev=336 --- fix_usb_rules.sh | 7 ++++++ vbox-usb-warning.diff | 45 ++++++++++++++++++++++------------- virtualbox-wrapper.sh | 55 ++++++++++++++++++++++++++++++++++++++++--- virtualbox.changes | 11 +++++++++ virtualbox.spec | 7 +++--- 5 files changed, 102 insertions(+), 23 deletions(-) create mode 100644 fix_usb_rules.sh diff --git a/fix_usb_rules.sh b/fix_usb_rules.sh new file mode 100644 index 0000000..b33950c --- /dev/null +++ b/fix_usb_rules.sh @@ -0,0 +1,7 @@ +#!/bin/bash +# script to disable USB passthru in /etc/udev/rules.d/60-vboxdrv.rules +# if already disabled, clear the comment character +sed -i 's/#SUBSYSTEM==\"usb/SUBSYSTEM==\"usb/' /etc/udev/rules.d/60-vboxdrv.rules +# now comment the usb lines +sed -i 's/SUBSYSTEM==\"usb/#SUBSYSTEM==\"usb/' /etc/udev/rules.d/60-vboxdrv.rules + diff --git a/vbox-usb-warning.diff b/vbox-usb-warning.diff index 571987b..6142f1c 100644 --- a/vbox-usb-warning.diff +++ b/vbox-usb-warning.diff @@ -1,10 +1,8 @@ -Index: a/src/apps/Makefile.kmk +Index: VirtualBox-5.1.22/src/apps/Makefile.kmk =================================================================== ---- a/src/apps/Makefile.kmk (revision 58576) -+++ b/src/apps/Makefile.kmk (working copy) -@@ -34,7 +34,9 @@ - endif - endif +--- VirtualBox-5.1.22.orig/src/apps/Makefile.kmk ++++ VirtualBox-5.1.22/src/apps/Makefile.kmk +@@ -31,5 +31,7 @@ endif include $(PATH_SUB_CURRENT)/VBoxPermissionMessage/Makefile.kmk @@ -12,10 +10,10 @@ Index: a/src/apps/Makefile.kmk + include $(FILE_KBUILD_SUB_FOOTER) -Index: a/src/apps/VBoxUSB_DevRules/Makefile.kmk +Index: VirtualBox-5.1.22/src/apps/VBoxUSB_DevRules/Makefile.kmk =================================================================== ---- a/src/apps/VBoxUSB_DevRules/Makefile.kmk (revision 0) -+++ b/src/apps/VBoxUSB_DevRules/Makefile.kmk (working copy) +--- /dev/null ++++ VirtualBox-5.1.22/src/apps/VBoxUSB_DevRules/Makefile.kmk @@ -0,0 +1,33 @@ +# $Id: Makefile.kmk 28800 2010-04-27 08:22:32Z vboxsync $ +## @file @@ -50,21 +48,34 @@ Index: a/src/apps/VBoxUSB_DevRules/Makefile.kmk +include $(KBUILD_PATH)/subfooter.kmk + + -Index: a/src/apps/VBoxUSB_DevRules/VBoxUSB_DevRules.cpp +Index: VirtualBox-5.1.22/src/apps/VBoxUSB_DevRules/VBoxUSB_DevRules.cpp =================================================================== ---- a/src/apps/VBoxUSB_DevRules/VBoxUSB_DevRules.cpp (revision 0) -+++ b/src/apps/VBoxUSB_DevRules/VBoxUSB_DevRules.cpp (working copy) -@@ -0,0 +1,13 @@ +--- /dev/null ++++ VirtualBox-5.1.22/src/apps/VBoxUSB_DevRules/VBoxUSB_DevRules.cpp +@@ -0,0 +1,26 @@ +#include +#include ++#include ++ +int main(int argc, char *argv[]) +{ + QApplication app(argc, argv); + QMessageBox msgBox; -+ msgBox.setWindowTitle(QObject::tr("USB Rules and Permissions !")); -+ msgBox.setText(QObject::tr("USB passthru opens a security hole. Please read \n\nhttps://bugzilla.novell.com/show_bug.cgi?id=664520\n\nto understand the problem. If you really want/need to use USB passthru and are willing to accept the security risk, then do nothing. To plug the security hole, remove all 'usb' lines from /etc/udev/rules.d/60-vboxdrv.rules.\n\nThis message will not be seen again!")); -+ int ret = msgBox.exec(); ++ QPushButton *myYesButton = msgBox.addButton("Enable", QMessageBox::YesRole); ++ QPushButton *myNoButton = msgBox.addButton("Disable", QMessageBox::NoRole); ++ msgBox.setWindowTitle(QObject::tr("USB Rules and Permissions !")); ++ msgBox.setText(QObject::tr("USB passthru opens a security hole. " ++ "Please read \nhttps://bugzilla.novell.com/show_bug.cgi?id=664520\n" ++ "to understand the problem.\n\nWe regard USB passthru to be extremely useful and worth the security risk. " ++ "thus the code defaults to enabling this feature. If you agree that the risk is acceptible, then click 'Enable'.\n" ++ "You will not be asked this question again when VB is updated. If you later change your mind, run 'rm ~/.vbox/*'\n\n" ++ "If you wish to disable USB passthru to plug the security hole, then click 'Disable'. " ++ "You will be asked for the system password, and /etc/udev/rules.d/60-vboxdrv.rules will be changed.\n" ++ "These changes cannot be preserved through VB updates, thus this screen will be displayed again at that time.")); ++ msgBox.exec(); + app.quit(); -+ return 0; ++ if (msgBox.clickedButton() == myYesButton) ++ return 0; ++ return 1; +} + diff --git a/virtualbox-wrapper.sh b/virtualbox-wrapper.sh index 7c48013..d3e0b90 100644 --- a/virtualbox-wrapper.sh +++ b/virtualbox-wrapper.sh @@ -1,9 +1,58 @@ #!/bin/bash export QT_NO_KDE_INTEGRATION=1 +# make certain that the user/group combination is valid /usr/bin/id -nG | grep -v -e "root" -e "vboxusers" >/dev/null && /usr/lib/virtualbox/VBoxPermissionMessage && exit -if [ ! -f ~/.vbox/message_out ] ; then +# +# Handle the issue regarding USB passthru +# The following conditions apply: +# 1. If ~/.vbox/enable exists, the user accepts the security risk. +# 2. If ~/.vbox/disable exists, the user does not accept the risk. That file will contain the inode of /etc/udev/rules.d/60-vboxdrv.rules. +# When that changes, the VBoxUSB_DevRules will again be displayed as that means that VB has been reloaded. +# +devrules() +{ /usr/lib/virtualbox/VBoxUSB_DevRules - mkdir -p ~/.vbox/ - touch ~/.vbox/message_out + if [ $? -eq 0 ] ; then + # User accepts the risk + touch ~/.vbox/enable + rm -f ~/.vbox/disable + else + # User declines the risk - save the inode + echo "" > ~/.vbox/disable + rm -f ~/.vbox/enable + fi +} +# Start of main routine +# +# Ensure that ~/.vbox exists +mkdir -p ~/.vbox/ +# Get the inode for /etc/udev/rules.d/60-vboxdrv.rules +INODE=$(stat /etc/udev/rules.d/60-vboxdrv.rules | grep Inode | cut -d' ' -f3) +if [ ! -f ~/.vbox/enable ] && [ ! -f ~/.vbox/disable ] ; then + # Neither file exists - find what the user wants + devrules fi +# Get the original Inode if it exists +if [ -f ~/.vbox/disable ] ; then + read LINE < ~/.vbox/disable +else + LINE=" " +fi +# If user originally declined, make certain that /etc/udev/rules.d/60-vboxdrv.rules has not been changed +if [ -f ~/.vbox/disable ] && [ "$LINE" != "$INODE" ] && [ "$LINE" != "" ] ; then + # disable is selected and the Inode has changed - ask again + devrules +fi +if [ -f ~/.vbox/disable ] ; then + echo $INODE > ~/.vbox/disable + if [ "$LINE" != "$INODE" ] ; then + if [ -f /usr/bin/kdesu ] ; then + kdesu /sbin/vbox-fix-usb-rules.sh + fi + if [ -f /usr/bin/gnomesu ] ; then + gnomesu /sbin/vbox-fix-usb-rules.sh + fi + fi +fi +# Now run the VB GUI LD_LIBRARY_PATH="/usr/lib/virtualbox${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}" /usr/lib/virtualbox/VirtualBox $@ diff --git a/virtualbox.changes b/virtualbox.changes index 0a16a39..b47fbab 100644 --- a/virtualbox.changes +++ b/virtualbox.changes @@ -1,3 +1,14 @@ +------------------------------------------------------------------- +Thu Jun 8 17:37:35 UTC 2017 - Larry.Finger@lwfinger.net + +- Revise warning screen concerning USB passthru - fixes bnc#1041137. + USB passthru opens a security hole, yet it is so valuable that many users want the feature, thus it is our default. + Previously, a user needed to edit a udev rule to disable passthru. The bad part was that an update of VB changed the + rule back to allow passthru without any notification. These changes modify the popup to allow the user to accept or decline + passthru. If the user declines, then the root password is requested and the udev rule is modified. As these modifications will be + lost with the next VB update, the inode of the udev rule is kept. If the user has previously declined and the inode has changed, + the popup will show the next time VB is started. File "fix_usb_rules.sh" is added. + ------------------------------------------------------------------- Sun May 21 13:28:15 UTC 2017 - hpj@urpla.net diff --git a/virtualbox.spec b/virtualbox.spec index 4085cb9..c57f10d 100644 --- a/virtualbox.spec +++ b/virtualbox.spec @@ -51,6 +51,7 @@ Source14: vboxdrv.service Source15: vboxadd-service.service Source16: vboxconfig.sh Source17: vboxguestconfig.sh +Source18: fix_usb_rules.sh Source98: %{name}-rpmlintrc Source99: %{name}-patch-source.sh #rework init scripts to fit suse needs @@ -128,8 +129,6 @@ BuildRequires: kernel-syms BuildRequires: libcap-devel BuildRequires: libcurl-devel BuildRequires: libelf-devel -BuildRequires: libicu58_2 -BuildRequires: libicu58_2-ledata BuildRequires: libidl-devel BuildRequires: libopenssl-devel BuildRequires: libqt5-linguist @@ -622,7 +621,6 @@ install -m 644 out/linux.*/release/bin/VBox.png %{buildroot}%{_datadir}/pixmaps/ install -m 644 %{SOURCE4} %{buildroot}%{_sysconfdir}/default/virtualbox #install wrapper script install -m 644 %{SOURCE9} %{buildroot}%{_bindir}/VirtualBox - # modify and install the vboxdrv init script sed -i "s|%{NOLSB}%|yes|g;s|%{DEBIAN}%||g;s|%{PACKAGE}%|virtualbox|g" \ src/VBox/Installer/linux/vboxdrv.sh @@ -635,6 +633,7 @@ install -m 0644 %{SOURCE15} %{buildroot}%{_unitdir}/vboxadd-service.service install -m 0644 %{SOURCE15} %{buildroot}%{_unitdir}/multi-user.target.wants/vboxadd-service.service install -m 0644 %{SOURCE16} %{buildroot}/sbin/vboxconfig install -m 0644 %{SOURCE17} %{buildroot}/sbin/vboxguestconfig +install -m 0755 %{SOURCE18} %{buildroot}/sbin/vbox-fix-usb-rules.sh # Init script to start virtual boxes during boot install -m 755 %{SOURCE12} %{buildroot}%{_sysconfdir}/init.d/vboxes ln -s %{_sysconfdir}/init.d/vboxes %{buildroot}%{_sbindir}/rcvboxes @@ -883,6 +882,8 @@ export DISABLE_RESTART_ON_UPDATE=yes %verify(not mode) %attr(4750,root,vboxusers) %{_vbox_instdir}/VirtualBox #wrapper script is in bindir %attr(0755,root,root) %{_bindir}/VirtualBox +#rules fixing script is in /sbin +%attr(0755,root,root) /sbin/vbox-fix-usb-rules.sh #ldd shows libQt* dependency %{_vbox_instdir}/VBoxTestOGL #qm's translations