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

Create scs-XXXX-vN-security-groups-decision-record.md #495

Merged
merged 13 commits into from
Mar 27, 2024

Conversation

josephineSei
Copy link
Contributor

We need a decision record for a possible standard for security groups to discuss all options and be able to look back to our decision every time.

It is part of the issue: #473

First draft of a decision record for a possible standard for security groups

Signed-off-by: josephineSei <[email protected]>
fix some spelling and linter errors

Signed-off-by: josephineSei <[email protected]>
fix linter errors

Signed-off-by: josephineSei <[email protected]>
@fkr
Copy link
Member

fkr commented Mar 6, 2024

Added @bitkeks and myself to the list of reviewers.

@josephineSei
Copy link
Contributor Author

@fkr @bitkeks @markus-hentsch and anyone who reviews this PR: Please state what your preferred solution would be, like Anja already did.

fix some wording

Co-authored-by: anjastrunk <[email protected]>
Signed-off-by: josephineSei <[email protected]>
Copy link
Contributor

@anjastrunk anjastrunk left a comment

Choose a reason for hiding this comment

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

Add decision and consequences.

Co-authored-by: Markus Hentsch <[email protected]>
Signed-off-by: josephineSei <[email protected]>
Copy link
Contributor

@artificial-intelligence artificial-intelligence left a comment

Choose a reason for hiding this comment

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

Sorry for my very long and hopefully not sounding very negative review.
I really found no other avenue to make my point, thus this wall of text. I didn't think it was appropriate to block any of the preceding calls with my arguments.

If you have any questions please don't hesitate to reach out here or via matrix/mail/irc/jitsi.

I think it is important that we don't only view this problem space through the lens of a CSP for a public cloud, because there are many different usage scenarios for an open source IAAS cloud, e.g. private clouds where your customers are project teams with different degrees of security knowledge.

At least this is my impression, which very well might be wrong.

Thanks for your consideration and time reading all of this (if you made it this far!)

Standards/scs-XXXX-vN-security-groups-decision-record.md Outdated Show resolved Hide resolved
Standards/scs-XXXX-vN-security-groups-decision-record.md Outdated Show resolved Hide resolved
Standards/scs-XXXX-vN-security-groups-decision-record.md Outdated Show resolved Hide resolved
Standards/scs-XXXX-vN-security-groups-decision-record.md Outdated Show resolved Hide resolved
Co-authored-by: Sven <[email protected]>
Signed-off-by: josephineSei <[email protected]>
Copy link
Contributor Author

@josephineSei josephineSei left a comment

Choose a reason for hiding this comment

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

Thank you for your input, I will rewrite the context part and add your suggestions for pros and cons

Standards/scs-XXXX-vN-security-groups-decision-record.md Outdated Show resolved Hide resolved
@josephineSei
Copy link
Contributor Author

I found a way to edit the default rules for both: default and custom groups:

stack@devstack:~/devstack$ openstack default security group rule create --ingress --protocol tcp --dst-port 22 --for-default-sg --for-custom-sg
+-------------------------+--------------------------------------+
| Field                   | Value                                |
+-------------------------+--------------------------------------+
| description             |                                      |
| direction               | ingress                              |
| ether_type              | IPv4                                 |
| id                      | b0bfe643-7d17-4f85-96f3-fc6aab75748d |
| port_range_max          | 22                                   |
| port_range_min          | 22                                   |
| protocol                | tcp                                  |
| remote_address_group_id | None                                 |
| remote_group_id         | None                                 |
| remote_ip_prefix        | 0.0.0.0/0                            |
| used_in_default_sg      | True                                 |
| used_in_non_default_sg  | True                                 |
+-------------------------+--------------------------------------+
stack@devstack:~/devstack$ openstack default security group rule create --ingress --protocol tcp --dst-port 80 --description http
+-------------------------+--------------------------------------+
| Field                   | Value                                |
+-------------------------+--------------------------------------+
| description             | http                                 |
| direction               | ingress                              |
| ether_type              | IPv4                                 |
| id                      | 25391f6c-2521-4878-a046-e71c42040f2e |
| port_range_max          | 80                                   |
| port_range_min          | 80                                   |
| protocol                | tcp                                  |
| remote_address_group_id | None                                 |
| remote_group_id         | None                                 |
| remote_ip_prefix        | 0.0.0.0/0                            |
| used_in_default_sg      | False                                |
| used_in_non_default_sg  | True                                 |
+-------------------------+--------------------------------------+

This is pretty new (only implemented in the 2023.2 release): https://bugs.launchpad.net/neutron/+bug/1983053

This brings me to the conclusion, that we may need a standard for at least those default rules. So not a standard for security groups per se, but a standard for the default rules, that are used when creating new default or custom security groups. (default-security-group-rules standard)

I would like to have a clear separation between a standard for default rules for security groups and a standard or guideline for default security groups. So I created an extra issue for this: #521

Adding a pro and con discussion in the Context section as suggested.
Also added two options how to proceed - one of them a pre-requirement.

Signed-off-by: josephineSei <[email protected]>
Copy link
Contributor

@artificial-intelligence artificial-intelligence left a comment

Choose a reason for hiding this comment

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

I like Option 2 the most (it arguably already includes option 3 to a degree).

Thanks for incorporating all the feedback! LGTM

1. Having a set of correctly configured security groups could reduce misconfiguration from users
2. Re-using correctly configured security groups saves time for users
3. Auditing security groups would be way easier for operators when helping customers
4. The configuration for the SGs can be done by networking experts, which may result in a higher security level as when users without expertise configure them
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
4. The configuration for the SGs can be done by networking experts, which may result in a higher security level as when users without expertise configure them
4. The configuration for the security groups can be done by networking experts, which may result in a higher security level as when users without expertise configure them

Copy link
Contributor

@anjastrunk anjastrunk left a comment

Choose a reason for hiding this comment

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

LGTM.

I found some typos...

As anyone until now was for Option 2 (maybe including Option 3). I wrote that decision into the DR.

Signed-off-by: josephineSei <[email protected]>
Copy link
Member

@bitkeks bitkeks left a comment

Choose a reason for hiding this comment

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

Sharing mutable security groups is not safe. Option 1 gets a no from me.

Creating a standardized default security group lgtm, but what ports? SSH and ICMP should suffice.

In the end, option 3 is my favorite, together with 0. If users decide to delete the default groups in their project (downside of option 2), they are responsible.

@markus-hentsch
Copy link
Contributor

@bitkeks

Sharing mutable security groups is not safe. Option 2 gets a no from me.

I think you are referring to option 1 here, no? Option 2 does not aim to share any groups but instead (re)create them for each project individually after project creation in a project-scoped fashion.

Otherwise I agree with your sentiments. 0 and 3 in combination sounds good.

@josephineSei josephineSei merged commit 5e836db into main Mar 27, 2024
6 checks passed
@josephineSei josephineSei deleted the security-groups-decision-record branch March 27, 2024 10:26
@bitkeks
Copy link
Member

bitkeks commented Mar 27, 2024

I think you are referring to option 1 here, no?

Yes, sorry for the confusion, my response wasn't zero-based 😉

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.

7 participants