lets-order-some-fries opened a new pull request, #67007:
URL: https://github.com/apache/doris/pull/67007

   ### What problem does this PR solve?
   
   Related PR: #66836
   
   Problem Summary:
   
   #66836 masked `tls_private_key_password` at the BE's shared configuration 
export point, because the value was returned in plaintext. The FE declares the 
same configuration key verbatim in `Config.java`, and it was not masked there.
   
   The FE already has the facility this needs: `ConfField.sensitive()` drives 
`ConfigBase.maskIfSensitive()`, which is applied by both `ConfigBase.dump()` 
and `ConfigBase.getConfigInfo()` — the two accessors behind `SHOW FRONTEND 
CONFIG`, `/rest/v1/config/fe`, and `/rest/v2/manager/node/configuration_info`. 
Only `auth_token` and `fe_meta_auth_token` set the flag, so 
`tls_private_key_password` was returned verbatim by all three.
   
   `key_store_password` is masked in the same commit. It is the password for 
the FE HTTPS service key store, it is read by `DorisFE.setKeyStorePassword` and 
`InternalHttpsUtils`, and it leaks through exactly the same three dump paths. 
Grouping the two avoids leaving a second plaintext secret one line away from 
the one being fixed.
   
   Deliberately **not** masked, to keep this change narrow and reviewable:
   
   - `mysql_ssl_default_ca_certificate_password` and 
`mysql_ssl_default_server_certificate_password` default to the documented 
literal `doris`. Masking them would permanently hide a published default rather 
than a secret.
   - `initial_root_password` is documented as a 2-staged SHA-1 hash, not a 
plaintext password.
   
   Neither value is round-tripped by any caller: `ConfigBase.dump()` and 
`getConfigInfo()` feed rendering paths only (`ConfigController`, `NodeAction`, 
`ShowConfigCommand`), and runtime mutation goes through 
`ConfigBase.setMutableConfig`, so masking cannot affect configuration behaviour.
   
   ### Release note
   
   `SHOW FRONTEND CONFIG` and the FE configuration REST endpoints now return 
`********` instead of the configured value for `tls_private_key_password` and 
`key_store_password`, matching the existing behaviour for `auth_token` and 
`fe_meta_auth_token`.
   
   ### Check List (For Author)
   
   - Test <!-- At least one of them must be included. -->
       - [ ] Regression test
       - [x] Unit Test
       - [ ] Manual test (add detailed scripts or steps below)
       - [ ] No need to test or manual test. Explain why:
           - [ ] This is a refactor/code format and no logic has been changed.
           - [ ] Previous test can cover this change.
           - [ ] No code files have been changed.
           - [ ] Other reason <!-- Add your reason?  -->
   
   Added `ConfigTest.testTlsAndKeyStorePasswordsAreMaskedWhenSet`, next to the 
existing `testAuthTokenIsMaskedWhenSet`, asserting both keys are masked in 
`ConfigBase.dump()` and `ConfigBase.getConfigInfo()`.
   
   ```
   mvn -f fe/pom.xml -pl :fe-common -am test -Dtest=ConfigTest
     -> Tests run: 7, Failures: 0, Errors: 0, Skipped: 0
   
   cd fe && mvn clean checkstyle:check
     -> BUILD SUCCESS
   ```
   
   I also confirmed the test fails without the `Config.java` change, so it 
genuinely covers the behaviour:
   
   ```
   testTlsAndKeyStorePasswordsAreMaskedWhenSet
     expected:<[********]> but was:<[super-secret-tls-password]>
   ```
   
   No regression test was added: config masking needs no running cluster, and 
the existing unit test file already covers this behaviour for the other 
sensitive configs.
   
   - Behavior changed:
       - [ ] No.
       - [x] Yes. <!-- Explain the behavior change -->
   
   The two configuration values are now reported as `********` by the config 
dump APIs when set. An empty value is still shown as empty, so "unset" remains 
visible.
   
   - Does this need documentation?
       - [x] No.
       - [ ] Yes. <!-- Add document PR link here. eg: 
https://github.com/apache/doris-website/pull/1214 -->
   
   ### Check List (For Reviewer who merge this PR)
   
   - [ ] Confirm the release note
   - [ ] Confirm test cases
   - [ ] Confirm document
   - [ ] Add branch pick label <!-- Add branch pick label that this PR should 
merge into -->
   


-- 
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