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

Add a binary file for setting up firewall rules on the VM #2147

Merged
merged 15 commits into from
Jan 26, 2021

Conversation

larohra
Copy link
Contributor

@larohra larohra commented Jan 25, 2021

Description

This is the 1st PR for the change to add firewall rules to the VM on system reboot (since iptables are not persisted on reboots).

This PR deals with -

  • Creating a new binary file for the sole purpose of setting up waagent-network rules
  • Separating out the bare minimum needed for setting up the firewall rules

Issue #


PR information

  • The title of the PR is clear and informative.
  • There are a small number of commits, each of which has an informative message. This means that previously merged commits do not appear in the history of the PR. For information on cleaning up the commits in your pull request, see this page.
  • Except for special cases involving multiple contributors, the PR is started from a fork of the main repository, not a branch.
  • If applicable, the PR references the bug/issue that it fixes in the description.
  • New Unit tests were added for the changes made and Travis.CI is passing.

Quality of Code and Contribution Guidelines

return command

@staticmethod
def get_iptables_accept_command(wait, command, destination, owner_uid):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These 2 commands are all we need to setup the rules on system reboot so just created a new Util just for them

@codecov
Copy link

codecov bot commented Jan 25, 2021

Codecov Report

Merging #2147 (9cb1668) into develop (f0f326b) will increase coverage by 0.06%.
The diff coverage is 78.57%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2147      +/-   ##
===========================================
+ Coverage    70.64%   70.71%   +0.06%     
===========================================
  Files           95       95              
  Lines        12939    12965      +26     
  Branches      1842     1844       +2     
===========================================
+ Hits          9141     9168      +27     
  Misses        3390     3390              
+ Partials       408      407       -1     
Impacted Files Coverage Δ
azurelinuxagent/agent.py 48.95% <ø> (ø)
azurelinuxagent/common/osutil/default.py 59.86% <57.14%> (+0.15%) ⬆️
azurelinuxagent/common/utils/networkutil.py 89.65% <82.85%> (-4.58%) ⬇️
azurelinuxagent/ga/collect_telemetry_events.py 90.34% <0.00%> (+0.68%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f0f326b...9cb1668. Read the comment docs.

This service would run before the network would be setup, so we need to maintain state of the previous Wireserver IP
to ensure we cover all attack vectors to the Wire IP.
"""
_setup_firewall_rules()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The idea is to create a new service file which would call this binary to setup the network rules on startup. Adding another parameter to our /sbin/waagent binary was being a bit painful because that way we would've needed to separate out all dependencies on the VM (like file access, network access, etc) because we plan to run this service way high up in the boot order where these bare minimum dependencies would not be available.

Setting up the new service would come in subsequent PRs

@larohra larohra merged commit 7c12eea into Azure:develop Jan 26, 2021
@larohra larohra deleted the separate-iptables branch January 26, 2021 18:02
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.

2 participants