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

I5520 ip tables #5559

Merged
merged 12 commits into from
Dec 2, 2024
Merged

I5520 ip tables #5559

merged 12 commits into from
Dec 2, 2024

Conversation

kayiwa
Copy link
Member

@kayiwa kayiwa commented Nov 25, 2024

use a iptables template to set firewall rules.
the order of the in your configuration matter

@kayiwa kayiwa marked this pull request as ready for review November 25, 2024 17:40
@kayiwa kayiwa marked this pull request as draft November 25, 2024 20:58
kayiwa and others added 10 commits November 26, 2024 09:26
the order of rules matters in ufw. We add the new defaults for ssh
we create a template file that can allow users to place these rules and
have predictable implementation

Closes #5520
we are using abid-staging2 for our firewall rules
add the ssh subnets to the group_vars

Co-authored-by: Vickie Karasic <[email protected]>
in order to make modifications to the firewall we need to disable it
then reload the new configuration

Co-authored-by: Alicia Cozine <[email protected]>
we swap out ufw to the user.rules
the preferred way is to use the before and after rules
@kayiwa kayiwa marked this pull request as ready for review November 26, 2024 15:17
@kayiwa kayiwa force-pushed the i5520_ip_tables branch 2 times, most recently from 25e065b to 6392784 Compare November 26, 2024 15:25
@kayiwa kayiwa requested a review from acozine November 26, 2024 15:35
Copy link
Contributor

@acozine acozine left a comment

Choose a reason for hiding this comment

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

Oops, I left this in draft form. . . .

Comment on lines +5 to +6
- protocol: tcp
source: 10.249.64.0/18
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that we have the functionality we want, can we add descriptive names? Something like this:

Suggested change
- protocol: tcp
source: 10.249.64.0/18
- rule: SSH from libnet
protocol: tcp
source: 10.249.64.0/18

- service: http
action: ACCEPT
- protocol: tcp
source: 10.249.0.0/18
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this CIDR range include 10.249.64.0/18?

group_vars/abid/staging.yml Show resolved Hide resolved
@@ -15,21 +15,16 @@ the examples below allow ssh, http, and redis to those CIDR subnets. For ssh mak

```yaml
ufw_firewall_rules:
Copy link
Contributor

Choose a reason for hiding this comment

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

Whatever we do in the group vars about adding name/description to the rules, let's do it here too.

roles/ufw_firewall/defaults/main.yml Show resolved Hide resolved
state: restarted
- name: Reload UFW
ansible.builtin.command: ufw reload
changed_when: false
Copy link
Contributor

Choose a reason for hiding this comment

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

We shouldn't need changed_when: false on the handler. The handler only runs when some other task reports a change.

roles/ufw_firewall/handlers/main.yml Outdated Show resolved Hide resolved
@acozine
Copy link
Contributor

acozine commented Dec 2, 2024

My remaining review comments will be addressed in the next PR. This is a step forward.

@acozine acozine merged commit d8c7c4a into main Dec 2, 2024
72 checks passed
@acozine acozine deleted the i5520_ip_tables branch December 2, 2024 19:55
kayiwa added a commit that referenced this pull request Dec 13, 2024
we have elected to define networks in IT-Handbook
added clearer examples that use this #5559

Our previous values matched a format that is no longer true

Co-authored-by: Vickie Karasic <[email protected]>
kayiwa added a commit that referenced this pull request Dec 20, 2024
we have elected to define networks in IT-Handbook
added clearer examples that use this #5559

Our previous values matched a format that is no longer true

Co-authored-by: Vickie Karasic <[email protected]>
kayiwa added a commit that referenced this pull request Dec 20, 2024
we have elected to define networks in IT-Handbook
added clearer examples that use this #5559

Our previous values matched a format that is no longer true

Co-authored-by: Vickie Karasic <[email protected]>
kayiwa added a commit that referenced this pull request Dec 20, 2024
we have elected to define networks in IT-Handbook
added clearer examples that use this #5559

Our previous values matched a format that is no longer true

Co-authored-by: Vickie Karasic <[email protected]>
kayiwa added a commit that referenced this pull request Dec 31, 2024
we have elected to define networks in IT-Handbook
added clearer examples that use this #5559

Our previous values matched a format that is no longer true

Co-authored-by: Vickie Karasic <[email protected]>
kayiwa added a commit that referenced this pull request Jan 9, 2025
we have elected to define networks in IT-Handbook
added clearer examples that use this #5559

Our previous values matched a format that is no longer true

Co-authored-by: Vickie Karasic <[email protected]>
kayiwa added a commit that referenced this pull request Jan 9, 2025
* adding dev-friendly ufw variables for our networks

Co-authored-by: Alicia Cozine <[email protected]>
Co-authored-by: Francis Kayiwa <[email protected]>

* adding variables to ufw readme and default files

Co-authored-by: Alicia Cozine <[email protected]>
Co-authored-by: Francis Kayiwa <[email protected]>

* improve documentation

we have elected to define networks in IT-Handbook
added clearer examples that use this #5559

Our previous values matched a format that is no longer true

Co-authored-by: Vickie Karasic <[email protected]>

* add ability to loop over campus network

* add a template that will dynamically assign networks

include documentation on how to use the role

---------

Co-authored-by: Alicia Cozine <[email protected]>
Co-authored-by: Francis Kayiwa <[email protected]>
Co-authored-by: Francis Kayiwa <[email protected]>
Co-authored-by: Vickie Karasic <[email protected]>
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