Infinite ports and CIDRs #1222
Merged
Infinite ports and CIDRs #1222
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Deploy Preview for tetragon ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
kevsecurity
force-pushed
the
pr/kevsecurity/infinite-ports-cidrs
branch
from
91aa4aa
to
f23cc64
Compare
kkourt
requested changes
Jul 17, 2023
There was a problem hiding this comment.
Thanks!
I went over the first patch, and overall looks good to me!
Please find some comments below.
Most of them are nits, but the one that I'd like to be addressed if possible is checking the type of arguments and reject invalid policies (assuming that applies here).
As also discussed offline, it would be great if we could add some more tests for this new functionality.
kevsecurity
force-pushed
the
pr/kevsecurity/infinite-ports-cidrs
branch
from
f23cc64
to
fd5dbf3
Compare
kevsecurity
force-pushed
the
pr/kevsecurity/infinite-ports-cidrs
branch
from
fd5dbf3
to
958d704
Compare
mtardy
reviewed
Jul 17, 2023
kevsecurity
force-pushed
the
pr/kevsecurity/infinite-ports-cidrs
branch
from
958d704
to
3e197cc
Compare
The sock and skb types permit matching on port numbers. The port numbers were stored as a list in the kernel selectors, which forced an upper limit on the number of ports that could be specified. This commit stores the specified ports in an argfilter map so that theoretically any number of ports could be specified without exceeding any obvious limits. The map is a hash of port numbers instead of a list. In addition, all value types, including SPort and DPort can now take a range, specified as min:max. Signed-off-by: Kevin Sheldrake <kevin.sheldrake@isovalent.com>
For sock and skb types it is possible to match on source and destination ports. This commit adds the ability to match on the source or destinations ports not being in a list or range. Signed-off-by: Kevin Sheldrake <kevin.sheldrake@isovalent.com>
The socket matchers for sock and skb types can match on a set or range of specified ports, for both membership and non-membership of the set, and for both source and destination ports. One obvious use case would be to match on the set of privileged ports, from 0 to 1023 inclusive. As this is a common, unchanging set, it makes sense to provide options to match on this set without having to specify it as a range. This will reduce the number of ports needed to be set in a map, will reduce the number of argfilter maps, and will reduce the complexity in the BPF code by a map lookup. This commit introduces the SPortPriv, NotSPortPriv, DPortPriv and NotDPortPriv operators that match the sock and skb ports to the privileged set. They do not take a value argument. Signed-off-by: Kevin Sheldrake <kevin.sheldrake@isovalent.com>
We currently supply a list of IPv4 CIDRs to match sock and skb types on. This is limited to a small number due to complexity reasons. There is a desire to be able to match on any number of CIDRs. This commit moves the specification of the CIDRs to a LPM TRIE map which allows longest prefix matching (which was designed for CIDR lookups). Any number of CIDRs can now be specified. Also, check that protocol is only specified for sock/skb types. Signed-off-by: Kevin Sheldrake <kevin.sheldrake@isovalent.com>
Given that we can match on IPv4 CIDRs, it makes sense that we should also be able to match on address not being in the specified ranges. These Not matchers can be combined with the existing ones to exclude individual addresses or sub-ranges from larger ranges. For example, DAddr: 192.168.1.0/24 would match everything in the given subnet. But combined with NotDAddr: 192.168.1.1 would exclude this specific IP address from the match. Alternatively, just using NotDAddr: 127.0.0.0/8 would exclude all localhost traffic from the match. Signed-off-by: Kevin Sheldrake <kevin.sheldrake@isovalent.com>
This commit adds documentation for matching socks and skbs. Signed-off-by: Kevin Sheldrake <kevin.sheldrake@isovalent.com>
kevsecurity
force-pushed
the
pr/kevsecurity/infinite-ports-cidrs
branch
from
3e197cc
to
de5a841
Compare
kkourt
approved these changes
Jul 19, 2023
This commit adds tests to confirm that the sock/skb operators are working correctly. Limited the new tests to TCP sock only, because skb works in exactly the same way and adding tests for UDP and/or skb would only duplicate tests for little benefit. Also, stop checking protocol on sock tests as this is unreliable. Signed-off-by: Kevin Sheldrake <kevin.sheldrake@isovalent.com>
kevsecurity
force-pushed
the
pr/kevsecurity/infinite-ports-cidrs
branch
from
de5a841
to
61c8eaf
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This series adds functionality to socket matching for sock and skb types. Previously, ports and CIDRs could be specified, but only a limited number would be accepted, due to complexity issues surrounding checking members of lists. This series uses an argfilter map to specify ports so that theoretically any number could be specified, and a new addr4filter map to specify IPv4 CIDRs, that are matched using a longest prefix match TRIE map.
In addition, ports can now be specified as ranges using the 'min:max' format, and new operators SPortPriv and DPortPriv were added to specify the range of privileged ports, from 0 to 1023 inclusive. On top of that, all port and address operators now can be prefixed with the Not keyword to create an inverse match, e.g. NotSAddr, and these can be combined with their positive equivalents to remove entires from a larger set.