KAFKA-4759 Add support for IPv4 and IPv6 ranges in AclAuthorizer #9937
Conversation
build.gradle
Outdated
| @@ -752,6 +752,7 @@ project(':core') { | |||
| dependencies { | |||
| compile project(':clients') | |||
| compile project(':raft') | |||
| compile group: 'net.ripe.ipresource', name: 'ipresource', version: '1.47' | |||
There was a problem hiding this comment.
Would be possible to validate if it's better to import just the bits of code we need for this validation to work instead of adding a whole dependency?
There was a problem hiding this comment.
@jeqo Not sure, I'll check all the magic done by contains method this weekend.
BTW I used that library because it comes from RIPE-NCC, an authority in this field, and it's being maintained. (https://github.com/RIPE-NCC/ipresource)
There was a problem hiding this comment.
@jeqo sorry for the delay.
I've been reviewing the code flow and I don't see it easily replaceable:
- ACL IP ranges are parsed and validated as slash notation(e.g. "10.0.0.0/16"), comma prefix notation (e.g. "10.0.0.0/16,/24,/25,/31") and ranges with dash (e.g. "10.0.0.0-10.1.2.3").
- Host IPs are parsed to be valid IPv4 and IPv6 adresses.
- For IPv6 cases, it's able to handle compressed IP adresses.
- Ranges and IPs validation check not only format but values (e.g. not out bounded values are allowed as 256.0.0.0)
- Compare probably is the easier part, ensure that ACL and host IP are the same type and check if host IP is inside the range.
I'd like to add that ipresource has a good test suite, it's covering a lot of cases https://github.com/RIPE-NCC/ipresource/tree/master/src/test/java/net/ripe/ipresource
Notations supported: - CIDR blocks(192.168.1.0/24) - Ranges (192.168.1.1-192.168.1.254)
7335792 to
beea392
Compare
|
hi, I wonder is this feature implemented somewhere in another PR? |
Past month I received a PR in my branch to fix current conflicts. I will merge it as soon as possible. But as far I can said it got blocked because of the request of removing the external library. To be frank, it was rather discouraging the response time, making impossible any discussion (i.e., if the library should be used or not). Maybe things have changed, let's see. |
|
I agree with @rgo - implementing ipv4 and ipv6 string parsing, range checking might be error prone (lots of code and tests) and its better to realy on existing implementation that comes from network authority |
IP Range support
|
Seems that failures are not connected to the PR - similar ones are no trunk as well. |
|
@flyingcougar yes, I also had (unrelated) issues with the test suite at the beginning. |
|
This PR is being marked as stale since it has not had any activity in 90 days. If you If you are having difficulty finding a reviewer, please reach out on the [mailing list](https://kafka.apache.org/contact). If this PR is no longer valid or desired, please feel free to close it. If no activity occurs in the next 30 days, it will be automatically closed. |
|
/reopen |
Adds IPv4 and IPv6 subnet support to ACL authorizer.
Notation supported:
For the test strategy, I defined ranges and set ACLs with IPs included in that range and other that it is not included inside the range.
BTW suggestions are welcome (I don't know Scala and I haven't been coding in a long time) :)
JIRA ticket: https://issues.apache.org/jira/browse/KAFKA-4759
Committer Checklist (excluded from commit message)