Hi Fede and thanks for the detailed review and let me answer your questions.
FV1 and FV3: So currently Kafka uses the DENY-wins model (i.e., so if any
DENY ACL matches by exact IP, wildcat or CIDR) the request will be denied
no matter of any ALLOW ACLs. So in your provided example the client at
10.0.1.5 will be denied. Also when we configure
ALLOW * on topic foo
DENY 10.0.1.0/24 on topic foo
client with 10.0.1.100 will be denied. On the other hand, clients from
outside of the Deny CIDR range will be ALLOWED (e.g., 10.0.2.100).
FV2:
Yeah, this is a good question. In practice it's non-issue because of how
Java handles it. I mean when we resolve ::ffff:192.168.0.5 an IPv4-mapped
IPv6 address it returns IPv4. In this most cases a client connecting from
::ffff:192.168.0.5 -> host string would be 192.168.0.5 and then in ACL
pattern goes 192.168.0.0/24 will goes through the IPv4 SubnetUtils branch
(i.e., no : in pattern) and SubnetUtils.isInRange("192.168.0.5) will match
correctly. I can think of one scenario that wouldn't match ... is if
someone explicitly createse an ACL with the IPv6 CIDR ::ffff:192.168.0.0/120
and so the ACL pattern contains : will go through SubnetUtils6 branch and
client 192.168.0.5 -> SubnetUtils6.isInRange("192.168.0.5") would likely
throw/fail.
Anyway I think this edge case is a bit non-stardard way. I think the
practical guidance is to use 192.168.0.0/24 for IPv4 ranges and use native
IPv6 CIDR for IPv6 ranges (probably I could document this in the KIP to
make it more explicit?)
FV4:
If a downgrade occurs where a cluster still has these CIDR ACLs, then there
are still those CIDR ACLs but cannot be managed (which is a problem). So I
think we should add a pre-downgrade validation (i.e., no CIDR ACLs are
present when we downgrade) then we simply downgrade and no in-consistent
state would happen?
FV5:
Hmmm, interesting. I didn't want to do/make implementation around IPs and
rather to stick with some well known library, which is battle tested over
the years. At least *commons-net *is battle tested and widely used AFAIK
since 2005 (for *IPv4 part*) and IPv6 follows the same pattern. Not sure
how others think about it.
FV6:
Yeah, I can update KIP to mention specific tests but I am not sure
performance analysis is needed as CIDR check should be trivial in terms of
time complexity and same for cost of existing exact string match.
Best regards,
Maros
