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

feat: fail jobs configured to run as worker agent user on Windows #230

Merged
merged 1 commit into from
Mar 21, 2024

Conversation

sakshie95
Copy link
Contributor

What was the problem/requirement? (What/Why)

We have determined that the Worker Agent needs Administrator privileges in Windows.
This poses risk that any queues configured to run jobs as the agent user would then have administrator privileges.

What was the solution? (How)

If the agent is running as an administrator and receives a job and the BatchGetJobEntity API request for the jobDetails returns a jobRunAsUserrunAs is WORKER_AGENT_USER, then we should fail the job with a message explaining that the queue and fleet are setup in an insecure configuration and the job would run with administrator privileges.

What is the impact of this change?

Better security

How was this change tested?

Added unit tests

Was this change documented?

N/A

Is this a breaking change?

N/A

@sakshie95 sakshie95 requested a review from a team as a code owner March 21, 2024 01:37
Copy link
Contributor

@ddneilson ddneilson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for getting this done so quickly!

@@ -708,6 +708,22 @@ def _create_new_sessions(
# Requires some updates to the code below
try:
job_details = job_entities.job_details()

# For Windows the WA runs as Administrator so fail jobs that were configured to runAs - WORKER_AGENT_USER as that would provide Admin privileges to the job
if (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I expect that we may end up relaxing this to Administrator + Running-as-a-service in future, but this is the agreed upon behavior right now.

@jusiskin jusiskin added enhancement New feature or request security Pull requests that could impact security labels Mar 21, 2024
@jusiskin jusiskin self-requested a review March 21, 2024 16:47
Copy link
Contributor

@jusiskin jusiskin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's merge this since it addresses a security concern. We can relax the logic later for edge-cases where the worker doesn't need to run as an admin.

@jusiskin jusiskin changed the title fix: Fail jobs configured to run as worker agent when Admin feat: fail jobs configured to run as worker agent user on Windows Mar 21, 2024
@ddneilson ddneilson enabled auto-merge (squash) March 21, 2024 17:25
@ddneilson ddneilson force-pushed the sakshie/fail_jobs_run_as_admin branch from da63815 to 95cf7b5 Compare March 21, 2024 17:26
@ddneilson ddneilson disabled auto-merge March 21, 2024 17:26
@ddneilson ddneilson enabled auto-merge (squash) March 21, 2024 17:26
@ddneilson ddneilson force-pushed the sakshie/fail_jobs_run_as_admin branch from 95cf7b5 to d9a9cbf Compare March 21, 2024 17:28
@ddneilson ddneilson merged commit 7ce01a8 into mainline Mar 21, 2024
12 checks passed
@ddneilson ddneilson deleted the sakshie/fail_jobs_run_as_admin branch March 21, 2024 17:34
sakshie95 pushed a commit that referenced this pull request Mar 21, 2024
baxeaz pushed a commit that referenced this pull request Mar 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request security Pull requests that could impact security
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants