Hi Maros,

MM3: I think setting didMetadataChange to true would make things simpler.
We don't really have a way to perform downgrade checks at the moment,
so you'd need to introduce a new mechanism.

MM4: When you parse "::ffff:192.168.1.1" with InetAddress.parse(), it
returns an Inet4Address instance.
So you should be able to check it against an IPv4 CIDR.

MM5: Following from MM4, what happens in the opposite scenario, an ACL
with "::ffff:192.168.1.1/24" and a client with an ipv6 address?

Thanks,
Mickael

On Thu, May 7, 2026 at 8:38 PM Maroš Orsák <[email protected]> wrote:
>
> Hi,
>
> Thanks for the review and feedback.
>
> > MM1: Typically in a KIP we are not really interested in the actual
> code changes but in the APIs and behavior changes.
> re MM1: Got it. Thanks.
> re MM2: Correct.
>
> re MM3:
>
> In my code changes the didMetadataChange is set to false where I checked
> that, I did not update this KIP sorry (updated). So the downgrade is
> possible but iff CIDR ACLs are no longer present (previously checked)
> otherwise it would fail on the message error, which we discussed above with
> Fede.
>
> re MM4:
>
> In my current version there is no explicit rejection. There are currently
> two forms of it (i.) current form i.e., ::ffff:<IPv4> so for
> instance ::ffff:192.168.1.1 and (ii.) the deprecated one <IPv4>
> e.g., ::192.168.1.1. Inet6Address has a method for detecting these mapped
> addresses but unfortunately it is package-private. I can add such a
> check/validation but that would complicate the overall implementation. But
> if you think we need to check such a race scenario I can update KIP with
> that (and then eventually my implementation).
>
> Cheers,
> Maros
>
> Do you want
>
> Hi,
> >
> > Thanks for the KIP. Just a few comments:
> >
> > MM1: Typically in a KIP we are not really interested in the actual
> > code changes but in the APIs and behavior changes.
> >
> > MM2: If I understand correctly an ACL with a CIDR block should behave
> > exactly the same way as if you create an ACL for each address that
> > matches the CIDR block
> > For example if I create an ACL for "1.2.3.4/30", it's like if I create
> > 4 ACLs for 1.2.3.4, 1.2.3.5, 1.2.3.6 and 1.2.3.7.
> > So using CIDR blocks allows to reduce the potential number of ACLs
> > users have to create and manage.
> >
> > MM3: In the Downgrades safety section you mention some introducing
> > some new validation logic.
> > You define the new metadata version with didMetadataChange set to
> > true, so I believe downgrade is currently not possible>
> >
> > MM4: In the IPv4-mapped IPv6 address section it's stated that
> > IPv4-mapped IPv6 addresses are not going to be supported. Are they
> > just ignored?
> > Could we catch these addresses when they are parsed and reject the
> > request if we detect one?
> >
> > Thanks,
> > Mickael
> >
> > On Thu, Feb 19, 2026 at 5:10 PM Federico Valeri <[email protected]>
> > wrote:
> > >
> > > Hi Maros, thanks for the KIP.
> > >
> > > It would be very convenient to have the possibility to use CIDR
> > > notation when adding ACL rules, but I think we miss quite a few
> > > details.
> > >
> > > FV1:  The proposal does not specify what happens when multiple ACLs
> > > with overlapping CIDR ranges apply to the same request. CIDR
> > > introduces a new dimension: subnet specificity. The KIP should clarify
> > > whether longest-prefix match apply, or whether CIDR ranges are treated
> > > like wildcards. Without this, users will create overlapping rules with
> > > unpredictable results.
> > >
> > > For example:
> > > - ALLOW 10.0.0.0/8 on topic foo
> > > - DENY 10.0.1.0/24 on topic foo
> > >
> > > Does a client at 10.0.1.5 get allowed or denied?
> > >
> > > FV2: How does the proposed matching logic handle IPv4-mapped IPv6
> > > addresses? For example, if a DENY ACL is set for 192.168.0.0/24, does
> > > it match a client whose address is reported as ::ffff:192.168.0.5?
> > >
> > > FV3: Kafka's existing ACL evaluation has precedence rules (DENY wins
> > > over ALLOW, explicit match over wildcard). How does CIDR matching fit
> > > into the existing precedence rules?
> > >
> > > FV4: I agree that we need a new metadata version because the CIDR host
> > > is stored there (AccessControlEntryRecord). What happens if a
> > > controller with CIDR support writes these records, then a downgrade
> > > occurs before all CIDR ACLs are removed?
> > >
> > > FV5: You imply that commons-net library needs to be added to the
> > > existing set of dependencies, but I think it is better to mention
> > > explicitly. Also, is it really worth adding this full networking
> > > protocol library for just one unreleased utility class? Have you
> > > looked at KIP-7 approach?
> > >
> > > FV6: It looks like we miss a test plan and maybe some performance
> > analysis.
> > >
> > > Cheers
> > > Fede
> > >
> > >
> > > On Thu, Jan 29, 2026 at 11:18 AM Maroš Orsák <[email protected]>
> > wrote:
> > > >
> > > > Hi everyone,
> > > >
> > > >
> > > > I would like to start a discussion about CIDR-based host patterns for
> > ACLs.
> > > > Details can be found in the cwiki [1].
> > > >
> > > >
> > > > [1] -
> > > >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-1276%3A+CIDR-based+Host+Patterns+for+ACLs
> > > >
> > > >
> > > > Best regards,
> > > >
> > > >
> > > > Maros Orsak
> >

Reply via email to