User Access check before start importing

[cyliu0]

Feature Request

Is your feature request related to a problem? Please describe:

It seems that we don't check the user's permissions at the beginning of the lightning. It could be a security flaw.

Describe the feature you'd like:

Check the user's permission by lightning before start importing for every backend.

Describe alternatives you've considered:

Teachability, Documentation, Adoption, Optimization:

[kennytm]

For Lightning via SQL we should have already required the SUPER privilege.

For TiDB backend, there is no security flaw as the RBAC system should be able to reject those CREATE / INSERT / REPLACE statements if the user has no permission.

For Local/Importer backend, there is no security flaw either as the user must have already infiltrated. That user able to run Lightning Local/Importer backend can already access the PD/TiKV ports and bypass RBAC by reading the KV data. They could even start a TiDB as root (or removing the root check) with security.skip-grant-table = true to conveniently read all your sensitive data.

The user access check can act as UI improvement but it won't fix any security issue, nor the lack of it being a security flaw.

It is also expensive to check without execution if the user really has the sufficient permission since a GRANT can be applied on column level and you don't know until reading the SQL/CSV file if those revoked columns exists.

# as user root:

mysql> create user aaaa;
Query OK, 0 rows affected (0.03 sec)
mysql> grant select, insert(a) on test.a to aaaa;
Query OK, 0 rows affected (0.01 sec)

# as user aaaa:

mysql> insert into a (a,b) values (1,2);
ERROR 1143 (42000): INSERT command denied to user 'aaaa'@'localhost' for column 'b' in table 'a'
mysql> insert into a (a) values (1);
Query OK, 1 row affected (0.01 sec)

[cyliu0]

For Lightning via SQL we should have already required the SUPER privilege.

For TiDB backend, there is no security flaw as the RBAC system should be able to reject those CREATE / INSERT / REPLACE statements if the user has no permission.

Agree with the above.

For Local/Importer backend, there is no security flaw either as the user must have already infiltrated.

Partially agree. Since our TiDB should be a complete database product with security features, I can not say that the above example explains our situation perfectly. I don't think it covers the internal cyber-attacks which our products should prevent.

That user able to run Lightning Local/Importer backend can already access the PD/TiKV ports and bypass RBAC by reading the KV data. They could even start a TiDB as root (or removing the root check) with security.skip-grant-table = true to conveniently read all your sensitive data.

The user access check can act as UI improvement but it won't fix any security issue, nor the lack of it being a security flaw.

Well, this is the tricky part of our products. In this particular case, the users without insert/admin privilege could insert/checksum with lightning local backend. It makes me feel insecure, so do some customers maybe. I'm not gonna deny that security itself could be varied in different scenarios. Some compliances/customers could be really tough on this topic and they may require that every layer of their stack is secure independently. I understand that the current design of pd/kv doesn't require authorization between nodes. But I don't even think this is the right way it should be for some security-sensitive customers. Of course, you can require TLS. But it's not user level. Security should be a series of general features across our product lines. Just my two cents.

It is also expensive to check without execution if the user really has the sufficient permission since a GRANT can be applied on column level and you don't know until reading the SQL/CSV file if those revoked columns exists.

# as user root:

mysql> create user aaaa;
Query OK, 0 rows affected (0.03 sec)
mysql> grant select, insert(a) on test.a to aaaa;
Query OK, 0 rows affected (0.01 sec)

# as user aaaa:

mysql> insert into a (a,b) values (1,2);
ERROR 1143 (42000): INSERT command denied to user 'aaaa'@'localhost' for column 'b' in table 'a'
mysql> insert into a (a) values (1);
Query OK, 1 row affected (0.01 sec)

Totally agree. But can we require that the lightning user with a specific set of privileges on table level and database level? Sometimes flexibility brings complexity too.

[cyliu0]

Maybe we need to define our security boundary to be on the same page

[cyliu0]

And it might take too much time to get error for missing one of the permissions when users importing large data with tidb backend. It's not effecient.

[cyliu0]

It seems we have an issue to plan to achieve part of this on the tikv side tikv/tikv#8621