[ENHANCEMENT] More protocols support

[npinaeva]

Is your enhancement request related to a problem? Please describe.
Currently only TCP, UDP, and SCTP protocols are supported, but there are more protocols (ICMP, ICMPv6 are the most popular requests) that may be useful. An example use case is "I want to only allow ICMP connections to implement health monitoring and deny everything else."

Describe the solution you'd like
To potentially implement it in the future, we may need to re-consider AdminNetworkPolicyPort https://github.com/kubernetes-sigs/network-policy-api/blob/main/apis/v1alpha1/shared_types.go#L52 design, which puts protocol inside the port definition, while some protocols don't have ports, it may be difficult to expand.

Describe alternatives you've considered
We could add an extra protocols field at the same level as ports https://github.com/kubernetes-sigs/network-policy-api/blob/main/apis/v1alpha1/adminnetworkpolicy_types.go#L151, but that will be confusing.

Example solution:

type AdminNetworkPolicyProtocol struct {
	NamedPort *string `json:"namedPort,omitempty"`

	TCP *PortProtocol `json:"TCP,omitempty"`
	UDP *PortProtocol `json:"UDP,omitempty"`
	SCTP *PortProtocol `json:"SCTP,omitempty"`
        // may be added in the future as
	ICMP *SimpleProtocol `json:"ICMP,omitempty"`
}

type SimpleProtocol struct {}

type PortProtocol struct {
	Ports *[]int32 `json:"ports,omitempty"`
	PortRanges *[]PortRange `json:"portRanges,omitempty"`
}

type PortRange struct {
	Start int32 `json:"start"`
	End int32 `json:"end"`
}

then the current ports spec

ports:
  - namedPort: containerPort
  - portNumber:
      protocol: TCP
      port: 1111
  - portNumber:
      protocol: TCP
      port: 2222
  - portRange:
      protocol: UDP
      start: 1
      end: 9999
  - portRange:
      protocol: SCTP
      start: 1
      end: 65535

may look like

protocols:
  - namedPort: containerPort
  - TCP:
      ports: [1111, 2222]
  - UDP:
      portRanges:
        - start: 1
          end: 9999
  - SCTP: {}

[sftim]

As a user, someone may want this extra support to be available for people who only have namespace-level write access.

[danwinship]

Making "protocols" top-level instead of "ports" seems a little less convenient/understandable for the common case.

But while we're thinking about how to rearrange things, I've always found ports really weird in NP because its behavior is asymmetric; in an ingress rule, ports effectively becomes part of the subject, while in an egress rule, it becomes part of the peer. Especially, when named ports are used, the ports section can affect which pods are part of the subject/peer set (because pods without any matching named port are considered non-matching for the rule).

I don't think there's any easy way to fix that as long as a single policy can have all of (a) a subject, (b) ingress peers, (c) egress peers. If we said that policies had to be either ingress-only or egress-only, then we could move the ports to be part of the subject section in ingress policies, and leave them with the peers in egress policies. But that might be annoying.

OTOH, the whole idea of having separate "ingress" and "egress" rules (rather than "transgress" ("transit"?) rules) doesn't make as much sense in ANP as in plain NP. With plain NP, you can only write rules with reference to your own pods, so the owner of namespace A can say "pods in namespace A don't accept most traffic, but they accept traffic from pods in namespace B", and the owner of namespace B can say "pods in namespace B can't send most traffic, but they can send traffic to pods in namespace A", but neither can specify the other. With ANP OTOH, the admin ought to be able to just say "traffic is allowed from pods in namespace B to pods in namespace A" without having to think about ingress vs egress...

So... instead of having "subject", "ingress", and "egress", an ANP could just have a list of rules, where each rule has a Source (which can be a Pods or Namespaces), and a Destination (which can be Pods, Namespaces, Nodes, or Networks). And then the Ports/Protocols obviously are associated with the Destination section only.

[danwinship]

there are more protocols (ICMP, ICMPv6 are the most popular requests) that may be useful. An example use case is "I want to only allow ICMP connections to implement health monitoring and deny everything else."

We need to be careful here: the Kubernetes Network Model does not specify anything about ICMP, and thus network plugins are allowed to implement the pod network in ways that don't give them full control over ICMP traffic. In particular, if pod-to-pod traffic goes unencapsulated over an underlying cloud network, there may not be any API that allows the network plugin to Deny ICMP traffic, and there may be no way to prevent the cloud network from denying ICMP traffic that the admin wants to Allow.

I guess that means we really do need "Extended" or whatever...

[sftim]

The network plugin usually has some lever to block packets, eg nftables locally to the node.

If the challenge is allowing traffic, that's different. But… not much different to using a NetworkPolicy to allow a Pod to make an egress connection on tcp/25, and then finding that a separate policy blocks that traffic. “Allow” doesn't mean “guaranteed to reach the destination” after all.

[danwinship]

The network plugin usually has some lever to block packets, eg nftables locally to the node.

When using something like amazon-vpc-cni-k8s, packets go directly from pods to the cloud network without passing through the host network namespace of the node. In that case, NP is implemented using AWS APIs rather than Linux kernel APIs. (Of course, AWS does let you specify ICMP rules, so this isn't a perfect example, but you get the idea.)

Anyway what I was saying about "Extended" is that there's this idea that there might be some parts of the ANP API that we say not all plugins will support, and I had been arguing before that we should get rid of that idea (which was copied from Gateway where it's more necessary), but if we're going to add ICMP rules, we might need to define it in a way that implementations aren't required to support it (though then we also need a way for users to figure out if their implementation supports it).

[sftim]

• How do we want to handle the case where the network plugin doesn't even work with plain NetworkPolicy rules? These network setups exist!
• If someone uses AdminNetworkPolicy or similar, can they as a cluster administrator use admission time checks to either block or warn about a proposed AdminNetworkPolicy that their particular cluster can't enforce?

---

I'm thinking admission-time because that's when people would actually want to get feedback (ie, putting it in .status a few milliseconds later doesn't help nearly as much; nor does firing an Event). Also, you can get the warning or rejection from a dry-run, whereas the other two mechanisms wouldn't show up.

[npinaeva]

For now we don't have any fields in ANP that are "extended", but I think the protocols use case is a good example of why we may need this mechanism (together with a mechanism to report unsupported configs).

So... instead of having "subject", "ingress", and "egress", an ANP could just have a list of rules, where each rule has a Source (which can be a Pods or Namespaces), and a Destination (which can be Pods, Namespaces, Nodes, or Networks).

I like this idea, but that will be a very big change to the existing API (that we should do before going beta). Changing only the way ports are expressed is a smaller change, but the deadline is the same. So @danwinship do you think we should fully reconsider our ingress/egress design? I will bring it up on the next meeting.

[danwinship]

Making "protocols" top-level instead of "ports" seems a little less convenient/understandable for the common case.

Suggested in the meeting this week: we could keep ports as it is, but also add protocols for other stuff, which would currently be just ICMP.

We'd need to figure out the semantics though... the simplest thing would be to say that the ports and protocols sections are mutually exclusive.

[npinaeva]

Why do you think they should be mutually exclusive? (btw do we say somewhere in ANP docs to which protocols it applies when no protocols are specificed?)

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle stale
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[astoycos]

/remove-lifecycle stale

[danwinship]

Why do you think they should be mutually exclusive?

Hm... I guess it doesn't have to be

(btw do we say somewhere in ANP docs to which protocols it applies when no protocols are specificed?)

no, I don't think so