[GitHub] [pulsar] michaeljmarshall opened a new issue #11708: [Websocket] Pulsar Request Logger Logs Token Query Param for WS

[GitBox <gi...@apache.org>]

michaeljmarshall opened a new issue #11708:
URL: https://github.com/apache/pulsar/issues/11708


   **Is your enhancement request related to a problem? Please describe.**
   When opening a websocket connection with the Pulsar Websocket Service, the request is logged. When the token is passed as a query param, the token is logged because we log the original URI, which includes the query params. Given that tokens are sensitive, the token param should not be logged. Here is a sample log line with my actual token replaced with `<token>`:
   
   `17:45:19.582 [pulsar-websocket-web-1-5] INFO  org.eclipse.jetty.server.RequestLog - 10.192.2.75 - - [18/Aug/2021:17:45:19 +0000] "GET /ws/v2/consumer/persistent/public/default/tc1-messages/tc1-sub?token=<token>&subscriptionType=Exclusive HTTP/1.1" 101 0 "-" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Firefox/91.0" 16`
   
   **Describe the solution you'd like**
   I'd like to see the token parameter removed from the log. I see several options.
   
   1. Modify the request logger to remove all query params.
   2. Use a separate request logger for endpoints that expect sensitive data (like the `/ws/*`).
   3. Extend the log writer used by jetty so that we can attempt to mask sensitive query params from log lines using regex.
   
   I'm not sure which solution makes the most sense. I think 1 or 3 seem the most reasonable to me.
   
   The current request logger is implemented (in multiple places) as follows:
   
   ```java
               RequestLogHandler requestLogHandler = new RequestLogHandler();
               Slf4jRequestLog requestLog = new Slf4jRequestLog();
               requestLog.setExtended(true);
               requestLog.setLogTimeZone(TimeZone.getDefault().getID());
               requestLog.setLogLatency(true);
               requestLogHandler.setRequestLog(requestLog);
               handlers.add(0, new ContextHandlerCollection());
               handlers.add(requestLogHandler);
   ```
   
   Note first that `Slf4jRequestLog` is already deprecated.
   
   **Describe alternatives you've considered**
   I supplied 3 options above. 
   


-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] codelipenghui commented on issue #11708: [Websocket] Pulsar Request Logger Logs Token Query Param for WS

[GitBox <gi...@apache.org>]

codelipenghui commented on issue #11708:
URL: https://github.com/apache/pulsar/issues/11708#issuecomment-1058887073


   The issue had no activity for 30 days, mark with Stale label.


-- 
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: commits-unsubscribe@pulsar.apache.org

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