You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@zookeeper.apache.org by "anmolnar (via GitHub)" <gi...@apache.org> on 2026/06/23 22:26:49 UTC

[PR] ZOOKEEPER-5058 Remove special characters from ensemble name before logging in EnsembleAuthenticationProvider [zookeeper]

anmolnar opened a new pull request, #2409:
URL: https://github.com/apache/zookeeper/pull/2409

   cc @phunt @ztzg 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] ZOOKEEPER-5058: Remove special characters from ensemble name before logging in EnsembleAuthenticationProvider [zookeeper]

Posted by "anmolnar (via GitHub)" <gi...@apache.org>.
anmolnar merged PR #2409:
URL: https://github.com/apache/zookeeper/pull/2409


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] ZOOKEEPER-5058: Remove special characters from ensemble name before logging in EnsembleAuthenticationProvider [zookeeper]

Posted by "anmolnar (via GitHub)" <gi...@apache.org>.
anmolnar commented on PR #2409:
URL: https://github.com/apache/zookeeper/pull/2409#issuecomment-4802598466

   Made the changes. I think the downside of that is we don't have control over it, needs additional administrative task if user doesn't use our sample config and it introduces a new dependency. Apart from those, it works fine.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] ZOOKEEPER-5058: Remove special characters from ensemble name before logging in EnsembleAuthenticationProvider [zookeeper]

Posted by "anmolnar (via GitHub)" <gi...@apache.org>.
anmolnar commented on PR #2409:
URL: https://github.com/apache/zookeeper/pull/2409#issuecomment-4897560679

   @phunt @PDavid I reverted the patch to the original implementation.
   Please review and approve if you're happy with it.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] ZOOKEEPER-5058: Remove special characters from ensemble name before logging in EnsembleAuthenticationProvider [zookeeper]

Posted by "anmolnar (via GitHub)" <gi...@apache.org>.
anmolnar commented on PR #2409:
URL: https://github.com/apache/zookeeper/pull/2409#issuecomment-4803456052

   Looks like the change causes some problems in other unit tests too.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] ZOOKEEPER-5058: Remove special characters from ensemble name before logging in EnsembleAuthenticationProvider [zookeeper]

Posted by "anmolnar (via GitHub)" <gi...@apache.org>.
anmolnar commented on PR #2409:
URL: https://github.com/apache/zookeeper/pull/2409#issuecomment-4802319380

   I like the logback approach too.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] ZOOKEEPER-5058: Remove special characters from ensemble name before logging in EnsembleAuthenticationProvider [zookeeper]

Posted by "anmolnar (via GitHub)" <gi...@apache.org>.
anmolnar commented on PR #2409:
URL: https://github.com/apache/zookeeper/pull/2409#issuecomment-4792859699

   > Thanks for this fix. 👍
   > 
   > Just a question: Would it be a enough to not log the client input itself - e.g. the unexpected ensemble name here? What is the advantage of logging the wrong ensemble name?
   
   Thanks for the review. That's actually a very good point!
   Anybody has objection against removing the logging of the ensemble name?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] ZOOKEEPER-5058: Remove special characters from ensemble name before logging in EnsembleAuthenticationProvider [zookeeper]

Posted by "PDavid (via GitHub)" <gi...@apache.org>.
PDavid commented on PR #2409:
URL: https://github.com/apache/zookeeper/pull/2409#issuecomment-4796620905

   I see that this is a trade-off: it can be useful if we log it. If we don't log it, it is more secure.
   
   How about integrating org.owasp:security-logging-logback and define a custom conversionRule for a CRLFConverter in our logback XML to sanitize input? This would be a general solution for the whole project.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] ZOOKEEPER-5058: Remove special characters from ensemble name before logging in EnsembleAuthenticationProvider [zookeeper]

Posted by "phunt (via GitHub)" <gi...@apache.org>.
phunt commented on PR #2409:
URL: https://github.com/apache/zookeeper/pull/2409#issuecomment-4795384533

   My 0.02 - it seems like ensemble name could be useful for troubleshooting purposes. Given we have this issue for logging of other externally controlled data perhaps we could map to eg "*" (via subroutine used across the codebase) and indicate in the log string itself that the name may be masked? Perhaps a general such note in the documentation somewhere that says along the lines of "We may mask certain logged externally controled fields with '*' to protect against log corruption" (not sure the technical term to be used here... ).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org