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

NAS-130747 / 25.04 / Do not allow unprivileged users to see other users queued jobs that u… #14319

Merged
merged 1 commit into from
Aug 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 12 additions & 1 deletion src/middlewared/middlewared/job.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from collections import OrderedDict
import copy
import enum
import errno
from functools import partial
import logging
import os
Expand Down Expand Up @@ -125,7 +126,17 @@ def add(self, job):
if job.options["lock_queue_size"] is not None:
queued_jobs = [another_job for another_job in self.queue if another_job.lock is job.lock]
if len(queued_jobs) >= job.options["lock_queue_size"]:
return queued_jobs[-1]
for queued_job in reversed(queued_jobs):
if not credential_is_limited_to_own_jobs(job.credentials):
return queued_job
if (
job.credentials.is_user_session and
queued_job.credentials.is_user_session and
job.credentials.user['username'] == queued_job.credentials.user['username']
):
return queued_job

raise CallError('This job is already being performed by another user', errno.EBUSY)

self.deque.add(job)
self.queue.append(job)
Expand Down
2 changes: 1 addition & 1 deletion src/middlewared/middlewared/service/decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ def item_method(fn):


def job(
lock=None, lock_queue_size=None, logs=False, process=False, pipes=None, check_pipes=True, transient=False,
lock=None, lock_queue_size=5, logs=False, process=False, pipes=None, check_pipes=True, transient=False,
description=None, abortable=False
):
"""
Expand Down
46 changes: 46 additions & 0 deletions tests/api2/test_job_lock.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import contextlib
import errno
import os
import time

import pytest

from middlewared.service_exception import CallError
from middlewared.test.integration.assets.account import unprivileged_user_client
from middlewared.test.integration.utils import call, mock, ssh


Expand Down Expand Up @@ -124,3 +127,46 @@ def mock(self, job, *args):
return 42
"""):
assert call("test.test1") == 42


@pytest.mark.flaky(reruns=5, reruns_delay=5)
def test_lock_queue_unprivileged_user_can_access_own_jobs():
with unprivileged_user_client(allowlist=[{"method": "CALL", "resource": "test.test1"}]) as c:
with mock("test.test1", """
from middlewared.service import job

@job(lock="test", lock_queue_size=1)
def mock(self, job, *args):
import time
time.sleep(5)
"""):
j1 = c.call("test.test1")
j2 = c.call("test.test1")
j3 = c.call("test.test1")
assert j3 == j2

call("core.job_wait", j1, job=True)
call("core.job_wait", j2, job=True)


@pytest.mark.flaky(reruns=5, reruns_delay=5)
def test_lock_queue_unprivileged_user_cant_access_others_jobs():
with unprivileged_user_client(allowlist=[{"method": "CALL", "resource": "test.test1"}]) as c:
with mock("test.test1", """
from middlewared.service import job

@job(lock="test", lock_queue_size=1)
def mock(self, job, *args):
import time
time.sleep(5)
"""):
j1 = call("test.test1")
j2 = call("test.test1")
try:
with pytest.raises(CallError) as ve:
c.call("test.test1")

assert ve.value.errno == errno.EBUSY
finally:
call("core.job_wait", j1, job=True)
call("core.job_wait", j2, job=True)
Loading