liaoxin01 opened a new pull request, #66745:
URL: https://github.com/apache/doris/pull/66745

   ### What problem does this PR solve?
   
   Problem Summary:
   
   `HttpRequest::debug_string()` masks the whole `Authorization` header, so the 
BE request log records that a request carried credentials but not whose. An 
operation such as `/api/update_config` therefore leaves no trace of who issued 
it:
   
   ```
   I20260813 17:06:05.399653 config_action.cpp:90] HttpRequest:
   uri:/api/update_config?flush_thread_num_per_store=4&persist=true
   headers:
   key=Authorization, value=***MASKED***
   ```
   
   This PR keeps the user name of HTTP Basic credentials and masks only the 
password, so the same line becomes auditable:
   
   ```
   key=Authorization, value=root:***MASKED***
   ```
   
   Every other sensitive header (`token`, `auth-token`, `proxy-authorization`), 
every non-Basic scheme, and any credential that cannot be decoded stays masked 
in full. The rendered value is a rendering rather than the header value, which 
is base64 encoded.
   
   Two code paths wrote credentials to the logs in clear text, and are fixed 
here as well:
   
   1. `HttpHandlerWithAuth::on_header()` streamed `TCheckAuthRequest` directly. 
The thrift-generated `printTo()` dumps every field, so the password reached 
`be.WARNING`:
   
      ```
      W...] permission verification failed, request: 
TCheckAuthRequest(cluster=, user=root, passwd=Secret123, user_ip=127.0.0.1, ...)
      ```
   
      The failure is now identified by user, user ip, cluster and the masked 
URL.
   
   2. `BaseController.getAuthorizationInfo()` logged the raw `Authorization` 
header at INFO level when it failed to parse it, that is 
`base64(user:password)`. This is a *parse* failure rather than an 
authentication failure, so it runs before the password is verified and the 
header often carries valid credentials. A client that omits the `Basic ` 
prefix, for example, produces:
   
      ```
      INFO [BaseController.getAuthorizationInfo():286] parse auth info failed, 
Authorization header b3BzX2JvdDpPcHNCb3QyMDI2eA==, url /api/_set_config
      ```
   
      which decodes to a working credential. Only whether the header was absent 
or malformed is logged now, which keeps the diagnostic value: the common 
failure (no header at all) never carried a credential anyway.
   
   ### Release note
   
   None
   
   ### Check List (For Author)
   
   - Test
       - [x] Manual test (add detailed scripts or steps below)
   
       Verified on a single FE + single BE cluster.
   
       User name is kept, password is masked:
   
       ```
       curl -X POST -u root:Secret123 
"http://127.0.0.1:8040/api/update_config?flush_thread_num_per_store=4";
       # be.INFO -> key=Authorization, value=root:***MASKED***
       ```
   
       Credential shapes that must not yield a user name:
   
       | `Authorization` header | logged value |
       | --- | --- |
       | `Basic <base64(alice:secret)>` | `alice:***MASKED***` |
       | `basic` / `BASIC` (RFC 7617 is case insensitive) | 
`alice:***MASKED***` |
       | `Basic  <base64>` (two spaces) | `***MASKED***` |
       | `<base64>` with no scheme | `***MASKED***` |
       | `Bearer <token>` | `***MASKED***` |
       | `token: <value>` header | `***MASKED***` |
   
       A password containing colons is still split correctly (`-u 
'admin:pa:ss:word'` logs `admin:***MASKED***`).
   
       Auth failure no longer prints the password, with `enable_all_http_auth = 
true`:
   
       ```
       curl -X POST -u root:WrongPassword123 
"http://127.0.0.1:8040/api/update_config?flush_thread_num_per_store=8";
       # be.WARNING -> permission verification failed, user: root, user_ip: 
127.0.0.1, cluster: , url: /api/update_config?flush_thread_num_per_store=8
       # grep -c WrongPassword123 be.WARNING be.INFO -> 0, 0
       ```
   
       FE no longer prints the header:
   
       ```
       curl -H "Authorization: $(echo -n 'user:pass' | base64)" 
"http://127.0.0.1:8030/api/_set_config?qe_max_connection=2048";
       # fe.log -> parse auth info failed, Authorization header is malformed, 
url /api/_set_config
       ```
   
   - Behavior changed:
       - [x] Yes. Log content only. Credentials are no longer written to 
`fe.log`, `be.INFO` or `be.WARNING`, and the `Authorization` header is rendered 
as `<user>:***MASKED***` in the BE request log instead of `***MASKED***`.
   
   - Does this need documentation?
       - [x] No.
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to