design doc for network policies

[jubrad]

Motivation

Design doc for network policies.

This PR adds a known-desirable feature.
https://github.com/MaterializeInc/database-issues/issues/4637

Tips for reviewer

Checklist

• This PR has adequate test coverage / QA involvement has been duly considered. (trigger-ci for additional test/nightly runs)
• This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
• If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
• If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).
• If this PR includes major user-facing behavior changes, I have pinged the relevant PM to schedule a changelog post.

[pH14]

Thanks for writing this up @jubrad! I'll defer to others for the SQL syntax proposals, but structurally this all seems sounds to me. Feels like it tackles just the right amount of scope for v1, while leaving stepping stones to future additions as needed

[ParkMyCar]

Woohoo! Thanks for writing this @jubrad, looking forward to getting it built!

[ParkMyCar]

FWIW Role specific defaults, e.g. ALTER ROLE parker SET search_path do not get inherited either

[ParkMyCar]

It might not be relevant but something that comes to mind here when thinking about shifting policies left is how we store SECRETs in an external system. Point being, there is already precedent for environmentd to store and fetch data from something other than the Catalog or Persist. I could imagine doing something similar for communicating network policies with balancers or something even further left

[ParkMyCar]

Do you know how AWS or other cloud providers handle network policies for their various services? Seems like a similar design space maybe? How to handle Webhook sources might be a point in the "per-resource type" strategy instead of a single default

[ParkMyCar]

IMO explicitly pointing users to our formal support ticket process gets us pretty far

[ggnall]

Never seen this! Agree. We do need to be cautious about how we validate users are who they say we are, but this is a great mechanism for getting our attention.

formal support ticket process gets us pretty far

[ParkMyCar]

Overall this looks good to me, I do wonder if we want to separate out COMMENT though and instead point folks towards something like:

COMMENT ON NETWORK POLICY office_01 is 'OFFICE IP - 2024-9-28'

Just so it's the same as other objects. Regardless we should probably pass this through the SQL council!

[jubrad]

Yes we'd want the comment to be on each rule which is not a resource that's stored outside of the policy.

... maybe description is a better name for this field.

FWIW my inspiration was, in part, aws security groups

[benesch]

I think comment is good—PostgreSQL (and therefore Materialize) uses "comment" to mean what AWS calls "description" pretty routinely.

Here's my recommendation: force every rule to have a name. Something like this would align with how you declare replicas for unmanaged clusters:

CREATE NETWORK POLICY OFFICE_01 (
  RULES(
    nikhils_desktop (ACTION=ALLOW, SOURCE="10.0.0.0/32"),
    justins_laptop (ACTION=ALLOW, SOURCE="10.0.0.2/32"),
  )
 );

Then you could name a specific rule in the COMMENT statement:

COMMENT ON NETWORK RULE office_01.justins_laptop is 'Laptop assigned to Justin Bradfield on 2024-01-03'

It also tees us up to support altering network policies one rule at a time, like:

ALTER NETWORK POLICY DROP RULE justins_laptop

We don't need to support that first thing, but it's nice to have the option to introduce the granular editing syntax.

[jubrad]

I assume the alter policy would actually be:
ALTER NETWORK POLICY office_01 DROP RULE justins_laptop

If we go down this path we would probably want to treat rules more like cluster replicas, I was thinking about them more like brokers in a kafka connection. They'll probably want unique names and IDs?

[benesch]

I assume the alter policy would actually be:
ALTER NETWORK POLICY office_01 DROP RULE justins_laptop

Ah whoops, yes!

If we go down this path we would probably want to treat rules more like cluster replicas, I was thinking about them more like brokers in a kafka connection. They'll probably want unique names and IDs?

Yes, exactly! I think you could punt on IDs for now, but yeah you'd want to ensure unique names.

[benesch]

Looking really good. Thanks for writing this up, @jubrad!

[benesch]

What does NetworkPolicyId look like? I'm assuming something that looks like enum RoleId or enum ClusterId, so roughly:

enum NetworkPolicyId {
    System(u64),
    User(u64),
}

[jubrad]

I think this will just be a GlobalId

[benesch]

No, it'll need to be its own type! Today GlobalIds are only for things that go in the items collection in the catalog—i.e., things in schemas, rather than things that exist in the global namespace (clusters, roles, databases). @ParkMyCar is also refactoring things to introduce a CatalogItemId, but again that will still be just for objects that exist in schemas.

[jubrad]

In that case I think it'll probably be similar to ClusterId

enum NetworkPolicyId {
    System(u64),
    User(u64),
}

I'll update the doc.

[benesch]

Will there ever be a case in Materialize where a single policy will be used in a context where there need to be both ingress and egress rules? With sources, we have push sources (webhooks) and pull sources (Kafka), but never both; it seems intuitive to me that a network policy on a webhook source would restrict inbound connections while a network policy on a Kafka source would restrict outbound connections.

If you buy this argument, I think concretely the difference would be using a generic term like ADDRESSES rather than SOURCE or TARGET or DESTINATION in network rules, and then whether the policy applied in the inbound or outbound direction would be a function of whether it was attached to a role, pull source, or push source.

    RULE (ACTION = ALLOW, ADDRESSES = 'cidr')

[jubrad]

I don't think you can intuit whether a policy/rule should be ingress or egress... two reasons:

1. If we want to introduce ingress sinks, then sinks would support both ingress and egress making it impossible to define a single policy default for sinks.
2. We want to be able to share policies between resource types without having to ensure some color match.

Thought for bullet 2
Let's say a user bob has a specific network policy bob_policy, that allows ingress access from IP 1.2.3.4/32.
It should follow that bob cannot create a source or sink that would expand their policy scope; i.e, any source or sink they create should inherit the policy applied to the user. This requires that the policy for bob be applicable to all resource types bob can create.

Alternatives for enum

• have a different ingress and egress rule set for policies,
• add direction to NetworkPolicyRule and change source to address

I like alternative 2.

[benesch]

1. If we want to introduce ingress sinks, then sinks would support both ingress and egress making it impossible to define a single policy default for sinks.

Ah this is a very good point. Let's keep the distinction between egress and ingress rules then, and prepare to allow mixing and matching ingress and egress rules in a single policy. I also like the alternative you've proposed (adding a direction to each rule).

Let's say a user bob has a specific network policy bob_policy, that allows ingress access from IP 1.2.3.4/32.
It should follow that bob cannot create a source or sink that would expand their policy scope; i.e, any source or sink they create should inherit the policy applied to the user. This requires that the policy for bob be applicable to all resource types bob can create.

Hm, I'm not sure this follows for me! I can imagine connecting as a user that's limited by a network policy to ingress from office IPs only, but wanting to create a webhook source that allows ingress from some EC2 instance somewhere that I've set up.

[jubrad]

yeah... I agree with you the network policies a user is able to associate with a given source/sink should be controlled by the policies on which the user has usage policies for, not the users existing policies. If one wants to lock that down to only the policy of the user, they could grant the user only usage privileges to that policy.

[benesch]

I think comment is good—PostgreSQL (and therefore Materialize) uses "comment" to mean what AWS calls "description" pretty routinely.

Here's my recommendation: force every rule to have a name. Something like this would align with how you declare replicas for unmanaged clusters:

CREATE NETWORK POLICY OFFICE_01 (
  RULES(
    nikhils_desktop (ACTION=ALLOW, SOURCE="10.0.0.0/32"),
    justins_laptop (ACTION=ALLOW, SOURCE="10.0.0.2/32"),
  )
 );

Then you could name a specific rule in the COMMENT statement:

COMMENT ON NETWORK RULE office_01.justins_laptop is 'Laptop assigned to Justin Bradfield on 2024-01-03'

It also tees us up to support altering network policies one rule at a time, like:

ALTER NETWORK POLICY DROP RULE justins_laptop

We don't need to support that first thing, but it's nice to have the option to introduce the granular editing syntax.

[benesch]

The way our variable inheritance works I think you just want to call this network_policy; variables that aren't overridden at the role level default to the system parameter of the same name. This is how e.g. cluster works.

[jubrad]

ok, I think I'm not fully understanding this.

My understanding for clusters is that there is a system var for cluster that selects which cluster will be selected by default on login. Users can update this var to select a different cluster. I think we have a different story for policies. Customers shouldn't be able to set their policy through a session var. In order to apply a different policy to a role you'd have to alter the role and set a new policy, or alter the default policy for everyone.

I'm happy to change the name to network_policy just want to make sure we're agreeing on the flow.

[benesch]

Customers shouldn't be able to set their policy through a session var. In order to apply a different policy to a role you'd have to alter the role and set a new policy, or alter the default policy for everyone.

💯 agree with this.

The reason I think it's still ideal to use network_policy for both the system and role variable is because you're going to need to add special cases to deny inheritance as a session variable. By default, anything settable as a role variable (e.g. cluster) is also settable as a session variable. If you add a system variable named default_network_policy and a role variable named network_policy, you're going to need to add special cases in a few places to handle the inheritance properly. Whereas if you just use network_policy, the existing inheritance should work correctly for the system/role level, and you'll only need to add one special case to ensure that users can't override the network_policy at the session level.

[benesch]

Seems like a downside worth accepting! Makes the implementation much more straightforward, and we can always add on an eventually consistent L3/L4 firewall that reads these policies and can more efficiently enforce them on incoming connections.

[benesch]

Love how you've scoped down each incremental steps here!

[arusahni]

Will there be a corresponding privilege for viewing network policies?

[jubrad]

nope

[arusahni]

Does that mean they will be globally readable, or will CREATENETWORKPOLICY also be required for reading policy info?

[jubrad]

I don't think we have any such restrictions on resources like clusters, so probably no restrictions here either.