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.

Sign up for GitHub

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

Jump to bottom

Avoid reusing disposable VM names for some predefined time #603

Merged
merged 2 commits into from
Jul 19, 2024
Merged

Avoid reusing disposable VM names for some predefined time #603

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
376f458
Avoid reusing disposable VM names for some predefined time
alimirjamali Jun 30, 2024
dc4b63c
Unit test for avoiding reuse of dispIDs
alimirjamali Jul 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions qubes/app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,10 @@
def __init__(self, app):
self.app = app
self._dict = {}
# Recently used disposable IDs: dispid -> destroy seconds since epoch
self._recent_dispids = {}
# Avoid reuse of disposable IDs for one week
self._no_dispid_reuse_period = 7 * 24 * 60 * 60

def close(self):
del self.app
Expand Down Expand Up @@ -538,6 +542,8 @@
pass
del self._dict[vm.qid]
self.app.fire_event('domain-delete', vm=vm)
if getattr(vm, 'dispid', None):
self._recent_dispids[getattr(vm, 'dispid')] = int(time.monotonic())

Check warning on line 546 in qubes/app.py

View check run for this annotation

Codecov / codecov/patch

qubes/app.py#L546 

Added line #L546 was not covered by tests

def __contains__(self, key):
return any((key in (vm, vm.qid, vm.name))
Expand Down Expand Up @@ -583,6 +589,15 @@
for _ in range(int(qubes.config.max_dispid ** 0.5)):
dispid = random.SystemRandom().randrange(qubes.config.max_dispid)
if not any(getattr(vm, 'dispid', None) == dispid for vm in self):
if dispid in self._recent_dispids:
delta_secs = int(time.monotonic()) - \
self._recent_dispids[dispid]
if delta_secs < self._no_dispid_reuse_period:
continue
del self._recent_dispids[dispid]
self.app.log.debug(
'Reused dispID {} after {} hours'.format(
dispid, delta_secs / 60 / 60))
return dispid
raise LookupError((
'https://xkcd.com/221/',
Expand Down
18 changes: 18 additions & 0 deletions qubes/tests/app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
import qubes.tests.init
import qubes.tests.storage_reflink

import logging
import time

class TestApp(qubes.tests.TestEmitter):
pass
Expand Down Expand Up @@ -211,6 +213,7 @@ def setUp(self):
super().setUp()
self.app = TestApp()
self.vms = qubes.app.VMCollection(self.app)
self.app.log = logging.getLogger()

self.testvm1 = qubes.tests.init.TestVM(
None, None, qid=1, name='testvm1')
Expand Down Expand Up @@ -315,6 +318,21 @@ def test_100_get_new_unused_qid(self):

self.vms.get_new_unused_qid()

def test_999999_get_new_unused_dispid(self):
with mock.patch('random.SystemRandom') as random:
random.return_value.randrange.side_effect = [11, 22, 33, 44, 55, 66]
# Testing overal functionality
self.assertEqual(self.vms.get_new_unused_dispid(), 11)
self.assertEqual(self.vms.get_new_unused_dispid(), 22)
self.assertEqual(self.vms.get_new_unused_dispid(), 33)
# Testing no reuse
self.vms._recent_dispids[44] = time.monotonic()
self.assertNotEqual(self.vms.get_new_unused_dispid(), 44)
# Testing reuse after safe period
self.vms._recent_dispids[66] = time.monotonic() \
- self.vms._no_dispid_reuse_period - 1
self.assertEqual(self.vms.get_new_unused_dispid(), 66)
self.assertFalse(66 in self.vms._recent_dispids)

# def test_200_get_vms_based_on(self):
# pass
Expand Down