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

DHCP sandboxing failing on noexec mounted /var/tmp #521

Merged
merged 4 commits into from
Aug 24, 2020

Conversation

otubo
Copy link
Contributor

@otubo otubo commented Aug 6, 2020

If /var/tmp is mounted with noexec option the DHCP sandboxing will fail
with Permission Denied. This patch simply avoids this error by checking
the exec permission updating the dhcp path in negative case.

rhbz: https://bugzilla.redhat.com/show_bug.cgi?id=1857309

Signed-off-by: Eduardo Otubo otubo@redhat.com

otubo added 2 commits August 6, 2020 09:21
If /var/tmp is mounted with noexec option the DHCP sandboxing will fail
with Permission Denied. This patch simply avoids this error by checking
the exec permission updating the dhcp path in negative case.

rhbz: https://bugzilla.redhat.com/show_bug.cgi?id=1857309

Signed-off-by: Eduardo Otubo <otubo@redhat.com>
@github-actions
Copy link

Hello! Thank you for this proposed change to cloud-init. This pull request is now marked as stale as it has not seen any activity in 14 days. If no activity occurs within the next 7 days, this pull request will automatically close.

If you are waiting for code review and you are seeing this message, apologies! Please reply, tagging mitechie, and he will ensure that someone takes a look soon.

(If the pull request is closed, please do feel free to reopen it if you wish to continue working on it.)

@github-actions github-actions bot added the stale-pr Pull request is stale; will be auto-closed soon label Aug 21, 2020
Copy link
Member

@TheRealFalcon TheRealFalcon left a comment

Choose a reason for hiding this comment

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

Hey @otubo , thanks for contribution. This looks good. One request is that you add a test for the changes. I've attached a patch you can use. If you incorporate this we should be able to get it merged right away.

add_test.patch.txt

@TheRealFalcon TheRealFalcon removed the stale-pr Pull request is stale; will be auto-closed soon label Aug 21, 2020
Copy link
Collaborator

@smoser smoser left a comment

Choose a reason for hiding this comment

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

less than ideal in that we copy the entire binary (~500k) here just to then throw it away in the case that we couldn't execute it.

But its not significantly more hacky than the thing we're already working around (apparmor).

we really need native python dhcp client.

cloudinit/net/dhcp.py Outdated Show resolved Hide resolved
cloudinit/net/dhcp.py Outdated Show resolved Hide resolved
@mitechie
Copy link
Contributor

we really need native python dhcp client.

Yes, this is the thing we came to in trying to find the "right" fix and that's too big of a scope for the moment.

@otubo otubo requested review from smoser and TheRealFalcon August 24, 2020 09:26
@mitechie mitechie merged commit db86753 into canonical:master Aug 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants