search

StarRocks / starrocks

The world's fastest open query engine for sub-second analytics both on and off the data lakehouse. With the flexibility to support nearly any scenario, StarRocks provides best-in-class performance for multi-dimensional analytics, real-time analytics, and ad-hoc queries. A Linux Foundation project.
https://starrocks.io
Apache License 2.0
9.19k stars 1.82k forks source link

FE print token in FE log #52535

Open xyllq999 opened 3 weeks ago

xyllq999 commented 3 weeks ago

FE will print token in fe log。 image image

I have two options to solve this problem, either by including a function at each location where the uri is printed and masking the token field. Or change GET to POST to solve the problem completely, but this UT is not very easy to rewrite. Would you like to ask what you think of this? @kevincai

StarRocks version (Required)

3.2.0+ and more

xyllq999 commented 3 weeks ago

I've done the modification to POST in the internal code, and it works perfectly. But I don't seem to be able to fulfill the community's requirements for UT. Which way do you think is better? If you adopt POST, please help solve the UT problem, thank you very much. @kevincai

kevincai commented 3 weeks ago

please connect the PR this issue

xyllq999 commented 3 weeks ago

please connect the PR this issue

So is it the way to mention POST or which way? Personally, I prefer the modification of the POST mode.

kevincai commented 3 weeks ago

it is better to eliminate the token, turn to signature validation, no matter it is a get or post.

That is:

client: sign the url with the token, generate a signature and append the signature as a query parameter. server: use the token to generate the signature again, if the signature matches the one in query url, the validation passed, otherwise reject the request.

This way the token will be never transfterred in wire in plaintext.

xyllq999 commented 3 weeks ago

it is better to eliminate the token, turn to signature validation, no matter it is a get or post.

That is:

client: sign the url with the token, generate a signature and append the signature as a query parameter. server: use the token to generate the signature again, if the signature matches the one in query url, the validation passed, otherwise reject the request.

This way the token will be never transfterred in wire in plaintext.

You have a good point. However, I think that such a change is too big for a PR, and I may not be able to complete the change perfectly and ensure the compatibility of upgrades and downgrades. Therefore, I wonder if I can change the mode to POST to prevent tokens from being printed in plaintext in logs. As for the plaintext transmission problem, we have transformed HTTP into HTTPS internally. However, this code cannot be contributed to the community. We use our own encryption and decryption.

kevincai commented 3 weeks ago

It will also introduce compatibility between upgrade and downgrade by changing from GET to POST. The compatibility issue can't be avoided either way.

xyllq999 commented 3 weeks ago

It will also introduce compatibility between upgrade and downgrade by changing from GET to POST. The compatibility issue can't be avoided either way.

Internal heartbeat interfaces are not sensed by external invoking. Why is there a compatibility problem?

xyllq999 commented 3 weeks ago

It will also introduce compatibility between upgrade and downgrade by changing from GET to POST. The compatibility issue can't be avoided either way.

So do you prefer to call a function that shields tokens where the URL is printed?

kevincai commented 3 weeks ago

It will also introduce compatibility between upgrade and downgrade by changing from GET to POST. The compatibility issue can't be avoided either way.

Internal heartbeat interfaces are not sensed by external invoking. Why is there a compatibility problem?

during the upgrade/downgrade in a HA cluster, the new leader can't probe the follower in older version, which leads to wrong state/aliveness judgement.

kevincai commented 3 weeks ago

It will also introduce compatibility between upgrade and downgrade by changing from GET to POST. The compatibility issue can't be avoided either way.

So do you prefer to call a function that shields tokens where the URL is printed?

If it can't be eliminated at all for now, just mask it as a lazy workaround.

xyllq999 commented 3 weeks ago

It will also introduce compatibility between upgrade and downgrade by changing from GET to POST. The compatibility issue can't be avoided either way.

So do you prefer to call a function that shields tokens where the URL is printed?

If it can't be eliminated at all for now, just mask it as a lazy workaround.

Know what you mean, thanks for answering my questions, and I'll submit my code as soon as possible.