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

[ON HOLD] - adds firewall whitelisting feature #211

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

yunus
Copy link
Member

@yunus yunus commented Nov 2, 2019

This constraint enables users to provide a list of allowed/whitelisted firewall rules so that they are able to detect deviations. Detailed usage information can be found inside the samples/gcp_network_firewall_whitelisting.yaml file. This rule complements the other restricted_firewalls rule. I have copied the port range functionality from there as well.

The decision sequence

Raise an alert if a firewall rule is not listed by any of the whitelist rules defined in this constraint file:

  1. Does the direction (ingress/egress) match?
  2. Do both firewall rule and whitelist rule have the same fields defined? No more no less.
  3. Do the IP and their ports match? IPs are checked by equality while ports are checked via ranges. See below as an example.
  4. Check that whitelist sourceRange/destinationRange CIDR overlaps the whole firewall rule's source range if a source range exist.
  5. Check regex match for sourceServiceAccounts, sourceTags, targetTags, and targetServiceAccounts.
    All the values in a firewall rule should be whitelisted. PARTIAL matches are NOT enough.

Matching features

It is possible to use regex, port ranges and IP CIDR ranges to define whitelists.
For instance:

  • port: "1-100" covers "80" but not "443"
  • sourceRange/destinationRange: "10.128.0.0/16" covers "10.128.1.0/24" but not "10.0.0.0/24". 0.0.0.0/0 covers all the ranges
  • sourceTags, targetTags, sourceServiceAccounts, targetServiceAccounts can be defined via regular expression statements
  • IPProtocol can be a list of protocols.

@AdrienWalkowiak
Copy link
Contributor

This looks like a duplicate of the PR I created: #158

@yunus
Copy link
Member Author

yunus commented Nov 3, 2019

#158 is a blacklist implementation while this one is a whitelist.
For instance, in #158 among 3 targetTags, even if only one of them matches it is a violation. In this whitelist case, all 3 targetTags have to match, otherwise it is a violation.

I copied port range detection logic from #158, but I had to reimplement the rest. I tried to make the matching as explicit as possible and avoided flags like any, all, *.

@MartenvanWezel
Copy link

For what it's worth, the bulk of the cases would be of the format "Do not allow a connection unless the following things match: (ip:box1, port 443,from:dmz..)"

Blacklisting 'specific' bad actors from entering your everybody-is-welcome setup is helpful too, but much less common since it requires a lot of manual work. Of course the ideal case is "blacklist a,b,c, but everybody else can only connect to vm x, nowhere else"

@AdrienWalkowiak
Copy link
Contributor

Actually the other rule will support white listing too soon (see @morgante 's comment).

From the code's perspective that should be a minimal change to add the exclude option.

@yunus
Copy link
Member Author

yunus commented Nov 5, 2019

+@morgante

I disagree with the exclude option due to the ambiguity that it brings. At the beginning I was also in favor of the exclude option since it sounds intuitive and I spent almost a day to enable it. But in the end, I find it ambiguous and vulnerable to misconfigurations.

Check following examples:

A firewall rule:
ingress-tcp-[22,80,443]-sourceRanges:[192.168.1.0/24,10.0.0.0/8]-targetTags:[webserver,bastion]

Constraints:

  1. ingress-tcp- ports:[ALL, except:22] - sourceRanges: [ALL, except:192.168.1.0/24] - targetTags [ALL, except: bastion]
  2. ingress-tcp-80- sourceRanges: [10.0.0.0/8] - targetTags: [ALL, except:webserver]

So what should be the outcome of both rules? Both of them agrees and disagrees with the firewall rule at the same time.
The first constraint allows SSH traffic from 192.168.1.0/24 to "bastion" VMs. But the firewall defines extra ports, source ranges and targetTags.
The second one is even confusing. Port 80 and cidr 10.0.0.0/8 are not allowed since they are not exceptions but there is an exception at the end for targetTag: webserver. What is the meaning of such a constraint?

Due to these ambiguities, I believe we should be strict about whitelist and blacklist. If you think in set theory, In blacklist any intersection should be a violation, while in whitelist case, only firewall rules that are subsets of a given constraint should be allowed. Combinations of both is open to mistakes.

@yunus
Copy link
Member Author

yunus commented Dec 3, 2019

Hey team, do you have more questions/suggestions?

@morgante
Copy link
Contributor

morgante commented Dec 5, 2019

@yunus I do think exclude support is required. How else could you do something like never allow SSH except to the bastion?

To be clear, "exclude" would be applied for an entire rule. You couldn't specify ports:[ALL, except:22].

@yunus
Copy link
Member Author

yunus commented Dec 6, 2019

What I do is to create a superset of allowed firewall rules. If an actual firewall rule is a subset of the whitelist that policy library defines, then it is allowed.

I see that it is not easy to understand, so I am open to suggestions. If you wish, we can have a call to discuss in depth.

Never Allow SSH except Bastion == Allow SSH only for Bastion. And the real firewall for that is:

"data": {
        "allowed": [
          {
            "IPProtocol": "tcp",
            "ports": [
              "22"
            ]
          }
        ],
        "sourceRanges": [
          "0.0.0.0/0"
        ],
        "targetTags": [
          "bastion"
        ]
      }
    }

Below are the rules that allow only the bastion VM,
one is only for tags and the other is only for SAs. See that they are superset of the above actual rule.

# Allow SSH (22) to the bastion VMs only
      # the bastion VM is defined by a service account
      - direction: ingress
        allowed: 
          -  IPProtocol: "tcp"
             ports: 
               - "22"
        targetServiceAccounts:
          - "bastion-sa@PROJECT.iam.gserviceaccount.com"
        sourceRanges:
          - "0.0.0.0/0"

      # Allow SSH (22) to the bastion VMs only
      # the bastion VM is defined by a target tag
      - direction: ingress
        allowed: 
          -  IPProtocol: "tcp"
             ports: 
               - "22"
        targetTags:
          - "^bastion$"
          - "^entry$"
        sourceRanges:
          - "0.0.0.0/0"

If a firewall rule allows both targetTag and targetServiceAccount at the same time:

      # Allow SSH to VMs defined by a target tag or service account
      - direction: ingress
        allowed: 
          -  IPProtocol: "tcp"
             ports: 
               - "22"
        targetTags:
          - "^bastion$"
        targetServiceAccounts:
          - "bastion-sa@.*.iam.gserviceaccount.com"
        sourceRanges:
          - "0.0.0.0/0"

adds more comments and examples
@morgante
Copy link
Contributor

morgante commented Dec 6, 2019

@yunus Would that be a whitelist? The issue I see is then how do we know that another rule, say http is allowed, in that mode? If the user intent is "blacklist 22 except to my bastion" they should be able to convey that.

In our discussions with customers (and current Forseti usage), I think simple "exclusion" semantics are required.

@yunus
Copy link
Member Author

yunus commented Dec 8, 2019

Thanks for the comments. You are right that in whitelist case all the rules have to be specified. But I don't claim that every customer should choose this whitelist one. Both methods has its own benefits like all other rules with blacklist & whitelist.

Let's wait for exclusion rules to make a decision.

@morgante
Copy link
Contributor

morgante commented Dec 9, 2019

I think the key is that we need to support common customer scenarios like "blacklist access to port 22 unless it is targeting my bastion host" — IMO the easiest way to do that is with exclusion rules.

Let's wait for exclusion rules to make a decision.

Will you add those to the PR?

@yunus yunus changed the title adds firewall whitelisting feature [ON HOLD] - adds firewall whitelisting feature Dec 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants