Config option to allow only secured client connections
(1) Introduced a new config option onlySecureClientsAllowed in the Bookie Configuration. Default value is considered false. (2) If onlySecureClientsAllowed is set to True, the Bookies only allow the Clients to communicate over TLS. Any requests from the Clients without TLS enabled will be rejected and Client gets the Security Exception.
Regarding EUA: it is better to use it in order not to change the wire protocol and keep 100% compatibility with all BK clients in the wild. I do not expect this to fix the v2 protocol problem.
The v2 protocol follows a different code path. I did not check the code but maybe your solution is ok. If you see tests passing then we are on our way
Regarding EUA: it is better to use it in order not to change the wire protocol and keep 100% compatibility with all BK clients in the wild
Makes sense. Will rework the change to use EUA and my 'fix' for handling v2 as well.
@Ghatage @eolivelli Any updates on this issue? What is the current blocker?
@Ghatage @eolivelli Any updates on this issue? What is the current blocker?
Supporting v2 connections is the current blocker.
Here, we check for the type of the message, but that is false when msg is v2 and hence that check fails and allows the v2 unsecured connections through.
Not sure how to check for v2 in ServerSideHandler.
In ClientSideHandler we pass isUsingV2Protocol, which would be useful in ServerSideHandler but isn't present.
Any suggestions?
@Ghatage do you have time to continue this patch ?
Yes, just stuck on the V2 case. Will give it some more thought and post an update soon!
On Thu, Jul 23, 2020, 12:38 AM Enrico Olivelli [email protected] wrote:
@Ghatage https://github.com/Ghatage do you have time to continue this patch ?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/apache/bookkeeper/pull/2368#issuecomment-662863147, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAX6QQN2H3AL3MG6UPUUQATR47SIRANCNFSM4OLY7IFA .
@Ghatage @eolivelli any updates on this?
@Ghatage @eolivelli - Any updates on this?
@Ghatage do you need any other hints ?
@Ghatage do you have time to resolve the conflicts and complete this work ? it would be a super great feature for 4.12.0 users
@sijie @Ghatage I am not sure it is worth to wait for this patch to land before cutting 4.12, it is a security feature. We can deliver it with 4.12.1.
I am removing the 4.12 label
@Ghatage probably this is the best time to resume this work. We are far away from the new major release and we will have time to let this implementation settle
@Ghatage do you have cycles to move forward this useful work ?
@Ghatage if you do not have much time, we could ask for someone to pick up the patch ?
cc @nicoloboschi @dianacle
@Ghatage wow, welcome back :)
@eolivelli @shoothzj Please merge whenever convenient!
ping @eolivelli