From 86a935e77924be3875693a4502a25d7716a3c4a1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?=
 <marmarek@invisiblethingslab.com>
Date: Wed, 14 Jun 2017 03:58:55 +0200
Subject: [PATCH] qubes.NotifyTools: ignore '/qubes-tools/version' completely

It isn't used for anything, so simply ignore it for good.

https://github.com/QubesOS/qubes-core-admin/pull/109#discussion_r121421409
---
 qubes/api/misc.py          |  2 +-
 qubes/ext/core_features.py |  6 ------
 qubes/tests/api_misc.py    | 22 ----------------------
 qubes/tests/ext.py         | 12 ------------
 4 files changed, 1 insertion(+), 41 deletions(-)

diff --git a/qubes/api/misc.py b/qubes/api/misc.py
index 8839ca629..d107476da 100644
--- a/qubes/api/misc.py
+++ b/qubes/api/misc.py
@@ -75,7 +75,7 @@ def qubes_notify_tools(self):
 
         untrusted_features = {}
         safe_set = string.ascii_letters + string.digits
-        expected_features = ('version', 'qrexec', 'gui', 'default-user')
+        expected_features = ('qrexec', 'gui', 'default-user')
         for feature in expected_features:
             untrusted_value = self.src.qdb.read('/qubes-tools/' + feature)
             if untrusted_value:
diff --git a/qubes/ext/core_features.py b/qubes/ext/core_features.py
index 87d096ad1..7cd182727 100644
--- a/qubes/ext/core_features.py
+++ b/qubes/ext/core_features.py
@@ -31,12 +31,6 @@ def qubes_features_request(self, vm, event, untrusted_features):
                 'Ignoring qubes.NotifyTools for template-based VM')
             return
 
-        # for now used only to check for the tools presence
-        if 'version' in untrusted_features:
-            # any suspicious string will raise exception here,
-            # but otherwise ignored
-            int(untrusted_features['version'])
-
         requested_features = {}
         for feature in ('qrexec', 'gui'):
             untrusted_value = untrusted_features.get(feature, None)
diff --git a/qubes/tests/api_misc.py b/qubes/tests/api_misc.py
index e205c3aec..d153623be 100644
--- a/qubes/tests/api_misc.py
+++ b/qubes/tests/api_misc.py
@@ -125,13 +125,11 @@ def test_010_notify_tools(self):
             mock.call.save()
         ])
         self.assertEqual(self.src.mock_calls, [
-            mock.call.qdb.read('/qubes-tools/version'),
             mock.call.qdb.read('/qubes-tools/qrexec'),
             mock.call.qdb.read('/qubes-tools/gui'),
             mock.call.qdb.read('/qubes-tools/default-user'),
             mock.call.fire_event('features-request', untrusted_features={
                 'gui': '1',
-                'version': '1',
                 'default-user': 'user',
                 'qrexec': '1'}),
         ])
@@ -148,7 +146,6 @@ def test_013_notify_tools_no_version(self):
         response = self.call_mgmt_func(b'qubes.NotifyTools')
         self.assertIsNone(response)
         self.assertEqual(self.src.mock_calls, [
-            mock.call.qdb.read('/qubes-tools/version'),
             mock.call.qdb.read('/qubes-tools/qrexec'),
             mock.call.qdb.read('/qubes-tools/gui'),
             mock.call.qdb.read('/qubes-tools/default-user'),
@@ -159,23 +156,6 @@ def test_013_notify_tools_no_version(self):
         ])
         self.assertEqual(self.app.mock_calls, [mock.call.save()])
 
-    def test_014_notify_tools_invalid_version(self):
-        qdb_entries = {
-            '/qubes-tools/version': b'this is invalid',
-            '/qubes-tools/qrexec': b'0',
-            '/qubes-tools/gui': b'0',
-            '/qubes-tools/os': b'Linux',
-            '/qubes-tools/default-user': b'user',
-        }
-        self.configure_qdb(qdb_entries)
-        with self.assertRaises(AssertionError):
-            self.call_mgmt_func(b'qubes.NotifyTools')
-        # should be rejected later
-        self.assertEqual(self.src.mock_calls, [
-            mock.call.qdb.read('/qubes-tools/version'),
-        ])
-        self.assertEqual(self.app.mock_calls, [])
-
     def test_015_notify_tools_invalid_value_qrexec(self):
         qdb_entries = {
             '/qubes-tools/version': b'1',
@@ -189,7 +169,6 @@ def test_015_notify_tools_invalid_value_qrexec(self):
             self.call_mgmt_func(b'qubes.NotifyTools')
         self.assertEqual(self.app.mock_calls, [])
         self.assertEqual(self.src.mock_calls, [
-            mock.call.qdb.read('/qubes-tools/version'),
             mock.call.qdb.read('/qubes-tools/qrexec'),
         ])
 
@@ -206,7 +185,6 @@ def test_016_notify_tools_invalid_value_gui(self):
             self.call_mgmt_func(b'qubes.NotifyTools')
         self.assertEqual(self.app.mock_calls, [])
         self.assertEqual(self.src.mock_calls, [
-            mock.call.qdb.read('/qubes-tools/version'),
             mock.call.qdb.read('/qubes-tools/qrexec'),
             mock.call.qdb.read('/qubes-tools/gui'),
         ])
diff --git a/qubes/tests/ext.py b/qubes/tests/ext.py
index 3dc229a2c..fbb9ed284 100644
--- a/qubes/tests/ext.py
+++ b/qubes/tests/ext.py
@@ -101,18 +101,6 @@ def test_013_notify_tools_no_version(self):
             ('fire_event', ('template-postinstall',), {})
         ])
 
-    def test_014_notify_tools_invalid_version(self):
-        del self.vm.template
-        with self.assertRaises(ValueError):
-            self.ext.qubes_features_request(self.vm, 'features-request',
-                untrusted_features={
-                    'version': 'this is invalid',
-                    'qrexec': '1',
-                    'gui': '1',
-                    'default-user': 'user',
-                })
-        self.assertEqual(self.vm.mock_calls, [])
-
     def test_015_notify_tools_invalid_value_qrexec(self):
         del self.vm.template
         self.ext.qubes_features_request(self.vm, 'features-request',