Token should expire in a short time period.

[gregwebs]

Bug Report

The current README states that tokens can last for 24 hours. A look at the code indicates that tokens are of the JWT type. JWT tokens should have a short time frame for renewal to limit the damage when they are stolen. Typical recommended time frames are closer to 5 minutes.

The good thing about the dashboard is that it is a non-critical component with few clients. This at least means that if it known that a token is stolen one could take the dashboard offline and wait for tokens to expire.

[breezewish]

Thanks for the report! Actually the token is used as a web authentication session token (in JWT format). Considering that it is a session token, do you think it's fine to last for 24 hours?

[gregwebs]

No, that goes against all recommendations. Longer lived tokens however can be given as refresh tokens that should be stored securely and exchanged for new tokens. This requires increased backend implementation to detect a stolen refresh token. There are other approaches we can consider.

[gregwebs]

Given that there are already server side sessions it would be more secure to use a more traditional session-based approach to auth. Alternatively, the session can be used to increase the security of the JWT implementation as mentioned.

An additional major problem stems from the fact that the token is just to lookup the user's password in the session. So the user's password is maintained in memory for the duration of the session.

But still we need to deal with the user password. The point of tokens is really that one can exchange a login action (password) for them. The login credential should not persist anywhere once the token is generated.

The reason for the persistence in the dashboard is that the dashboard relies on SQL queries. There is no change to how TiDB authenticates SQL queries, and this is via a user and password (or cert) in the MySQL protocol. TiDB itself needs to recognize tokens when SQL queries are issued. It can only securely do this if it has control over the token issuance.

[gregwebs]

It seems that the dashboard also persists sessions (and therefore passwords) to disk since they can be recovered on restart (although I assume they are encrypted).

I have outlined how we can securely implement auth. There is a section on the dashboard.

For the purposes of securing the dashboard + TiDB interaction we would not need to implement anything in TiKV yet.

[breezewish]

@gregwebs Thanks! The implementation in TiDB Dashboard is that TiDB username and password is encrypted and stored in the JWT token itself, which is then stored in browser's local storage. As long as JWT session secret keeps secret, there is no way to recover the credential. Everytime a logged in user involves with a functionality that needs TiDB connection, the TiDB connection is established on demand by decrypting the credential from the token on-the-fly. The TiDB connection is closed when the response is generated. Each TiDB connection only lives for the lifetime of a request processing. TiDB Dashboard never persist the TiDB credential. The TiDB credential only exists in the memory of TiDB Dashboard when the credential is decrypted but not GCed by Golang (this part can be improved by erasing the memory of the decrypted buffer).

[gregwebs]

Sorry, I did not fully understand the code quite right. Along those lines, I might not be understanding the implementation, but I believe the token is not encrypted. Thus stealing a token steals the user's password.

Encryption could be added (if I am right that it is missing), but it would be much better to have an approach that doesn't require persisting the password in any form.

[breezewish]

@gregwebs The data is already encrypted before signing as JWT.

[gregwebs]

oh, great, glad I missed that encryption. So the biggest issue is still the title of this issue that tokens are being used for 24 hours but the normal recommendation is for a very short time period.

A recommended approach to solve this is with a refresh token. The server needs to be able to track the usage of refresh tokens to detect when they are stolen.

Note that this essentially ends up requiring server-side sessions. So this use case for JWTs doesn't yield any benefits yet because there is a single server playing both the role of identity provider and token consumer. I think there is a good case for using JWTs in the future if they are used with different components.

[breezewish]

@gregwebs Yes. Considering that refresh token itself needs a server-side session, how about just simply introduce a traditional server-side session for all? Then we can achieve the real log-out and no need to worry about the stolen token issue (in which case we can simply log out the stolen token from server side).

[gregwebs]

That would be preferable for a normal web session use case. Sessions allow logging out the user, including automatically when they are inactive.

Would you store the session in etcd in encrypted form then?

Another issue to consider is whether dasbhoard-ui would be used as an API.

It is still worrisome that the user's password is being persisted (in encrypted form). If the server and it's encryption were breached then the attacker would get all the user's passwords that have an active session with dashboard-ui. Whereas with tokens the attacker would still need to get the tokens as well. One way to handle this is with encrypted session cookies that you can add data such as the password to. Which is similar to what you are doing with the JWT now, but there are sessions performing logout.

[breezewish]

@gregwebs Considering that dashboard is only running in one PD instance, storing the session in PD locally (where we use a sqlite for this purpose) should be enough.

For the credential issue, maybe we can:

1. Store an encryption secret KEY1 in the session store per session, which is stored in sqlite as described above. Each session have a different encryption secret.
2. Encrypt the tidb credential using the secret KEY1 when log in, stored its cipertext C1 in browser (cookie). The storing location is the same as session token.
3. Browser send C1 to dashboard server each time performing a request by using cookie. Dashbaord server decrypt it using the KEY1 in session to get the credential and log into TiDB. After login, the plaintext memory should be erased.

In this way:

1. When session is destroyed, there is no way to decrypt the tidb credential.
2. The encrypted credential is stored at browser side. Attacker cannot recover the credential of active session unless a browser request is really made and the attacker is able to monitor the cookie. Database exposure or secret exposure is not enough to expose tidb credentials.

[gregwebs]

That makes sense for dashboard-ui. Like a normal session, you would want a maximum session time and automatic logout of inactive sessions.

In the long-term I would like to have PD implement refresh tokens, so it would also make sense to try to add refresh tokens here with the goal of moving this implementation into PD. I saw this example code. I think if the refresh time is moved down to a smaller amount such that the client should be frequently refreshing then we get close to the properties we want.

[breezewish]

Hmm. Why do we need to move implementations about the refresh token to PD?

[gregwebs]

In the long-term we need a coherent auth system in TiDB that will allow for auth to PD/TiKV and work with TiDB users. I think that in any proposal, such as the one I created, PD will need to be in charge of auth.

[stale[bot]]

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[gregwebs]

I would suggest that security issues should not be automatically closed