Re: [Maria-developers] 14cc679c95e: MDEV-27831 Let the SQL SERVICE user set the current user name.
Hi, Alexey, On Mar 04, Alexey Botchkov wrote:
revision-id: 14cc679c95e (mariadb-10.7.2-8-g14cc679c95e) parent(s): 33fd136c61b author: Alexey Botchkov committer: Alexey Botchkov timestamp: 2022-02-15 11:35:18 +0400 message:
MDEV-27831 Let the SQL SERVICE user set the current user name.
The 'user' argument added to the mysql_real_connect_local.
I think this is wrong on many levels. A plugin name is already known inside the plugin, you shouldn't force the plugin to pass its own name as an argument in multiple places, the server should determine it automatically. You should not set current_user to an arbitrary string for audit plugin to see it. current_user is the name of the user account and it's used in many places as such. Try, for example, to create a view or a stored procedure. Who will be a definer? Setting only user() might be ok. Setting @@proxy_user or @@external_user is even better, if your audit plugin can show them. @@external_user would be the best, I think it's purely informational. Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
I still like my approach.
A plugin name is already known inside the plugin, the server should determine it automatically.
current_user is the name of the user account and it's used in many places as such. Try, for example, to create a view or a stored procedure. Who will be a definer? If not specified, the definer is going to be username@''. And as a result the view or the procedure will be not functional. But i think it's rather correct. The user of the SQL service has to specify
Firstly I don't see any good way for the service to know the name of the plugin that called the mysql_real_connect_local. Technically this call doesn't even have to be hard linked to a plugin. Can be just done by a part of the server. Then why limit the plugin like this? That doesn't add much to the security as the plugin can replace that string anyway. Also one plugin can have more than one connection and I can imagine that different usernames for these connections ake sence. the definer explicitly.
Setting only user() might be ok. I can agree with that. Setting the ctx->user only. In this case the DEFINER of the view/procedure is going to be empty if not explicitly specified. Though don't see any advantage to what is now.
Best regards. HF On Fri, Mar 4, 2022 at 1:08 PM Sergei Golubchik <serg@mariadb.org> wrote:
Hi, Alexey,
On Mar 04, Alexey Botchkov wrote:
revision-id: 14cc679c95e (mariadb-10.7.2-8-g14cc679c95e) parent(s): 33fd136c61b author: Alexey Botchkov committer: Alexey Botchkov timestamp: 2022-02-15 11:35:18 +0400 message:
MDEV-27831 Let the SQL SERVICE user set the current user name.
The 'user' argument added to the mysql_real_connect_local.
I think this is wrong on many levels.
A plugin name is already known inside the plugin, you shouldn't force the plugin to pass its own name as an argument in multiple places, the server should determine it automatically.
You should not set current_user to an arbitrary string for audit plugin to see it. current_user is the name of the user account and it's used in many places as such. Try, for example, to create a view or a stored procedure. Who will be a definer?
Setting only user() might be ok. Setting @@proxy_user or @@external_user is even better, if your audit plugin can show them. @@external_user would be the best, I think it's purely informational.
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
_______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
Hi, Alexey, On Apr 04, Alexey Botchkov wrote:
I still like my approach.
A plugin name is already known inside the plugin, the server should determine it automatically.
Firstly I don't see any good way for the service to know the name of the plugin that called the mysql_real_connect_local.
I don't see either. I can only think of something like #define plugin_name "spider" (for example), or const char * const plugin_name = "spider"; and #define mysql_real_connect_local(M) sql_service->mysql_real_connect_local_func(M, plugin_name) but it's not per plugin it's per *.so. If one .so would have many plugins, they'll all will have the same "plugin_name" and I have no solution for that. So it's a rather lousy solution, and I hoped you could come up with something better :)
Technically this call doesn't even have to be hard linked to a plugin. Can be just done by a part of the server.
The server doesn't have to use *plugin services*, services are an API for plugins to use.
Then why limit the plugin like this? That doesn't add much to the security as the plugin can replace that string anyway.
Not for security. It's to avoid boilerplate, to not force plugins to tell the server what the server already knows.
Also one plugin can have more than one connection and I can imagine that different usernames for these connections make sence.
well, the point was to identify what plugin makes the call. and almost always it will be a plugin name.
current_user is the name of the user account and it's used in many places as such. Try, for example, to create a view or a stored procedure. Who will be a definer? If not specified, the definer is going to be username@''. And as a result the view or the procedure will be not functional. But i think it's rather correct. The user of the SQL service has to specify the definer explicitly.
No, using pluginname@'' can hardly be correct. ''@'' is more reasonable. And it's not only definer, it's what CURRENT_USER shows, it's what privileges are used. Surely, not privileges of the pluginname@'' account, there is no such account. So CURRENT_USER should not be pluginname@''.
Setting only user() might be ok. I can agree with that. Setting the ctx->user only. In this case the DEFINER of the view/procedure is going to be empty if not explicitly specified. Though don't see any advantage to what is now.
USER() is purely informational. CURRENT_USER() is not, it has a clearly defined meaning, it's the name of the account that the privilege system uses. Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
So it's a rather lousy solution, and I hoped you could come up with something better :)
The server doesn't have to use *plugin services*, services are an API for plugins to use. No need to use the 'services' infrastructure indeed. But the functions
Well i can think of something composed from __FILE__ __LINE__. It'll provide the information about the connection's origin, which will be enought in 99% of cases. Unfortunately that Still it won't be clear what plugin of possible many utilizing that code is actually doing the call. There's no information about it anywhere on the stack or arguments. It's possible to add that argument to THD - like current_plugin and set it befor the call to the plugin function. Though it seems way too much work to just set the name for the connection's user. So i think just the 'username' argument to the mysql_real_connect_local() is the nicest way out. BTW the mysql_real_connect() has this argument. that are exported with the services are often called in the server.
It's to avoid boilerplate, to not force plugins to tell the server what the server already knows.
...
well, the point was to identify what plugin makes the call. and almost always it will be a plugin name.
As we discussed it's not trivial for the server to get that data. And just one extra argument to the function call doesn't seem to be a complication. Oherwise we can use just 'sql_service' username for everybody. Since there is just few clients working at the moment, it's usually possible to figure out from the log what plugin it was.
No, using pluginname@'' can hardly be correct. ''@'' is more reasonable. And it's not only definer, it's what CURRENT_USER shows, it's what privileges are used. Surely, not privileges of the pluginname@'' account, there is no such account. So CURRENT_USER should not be pluginname@''.
Ok, that'll work. The CURRENT_USER is empty, so the default DEFINER for the VIEW/PROCEDURE. Regards. HF On Mon, Apr 4, 2022 at 8:20 PM Sergei Golubchik <serg@mariadb.org> wrote:
Hi, Alexey,
On Apr 04, Alexey Botchkov wrote:
I still like my approach.
A plugin name is already known inside the plugin, the server should determine it automatically.
Firstly I don't see any good way for the service to know the name of the plugin that called the mysql_real_connect_local.
I don't see either. I can only think of something like
#define plugin_name "spider"
(for example), or
const char * const plugin_name = "spider";
and
#define mysql_real_connect_local(M) sql_service->mysql_real_connect_local_func(M, plugin_name)
but it's not per plugin it's per *.so. If one .so would have many plugins, they'll all will have the same "plugin_name" and I have no solution for that.
So it's a rather lousy solution, and I hoped you could come up with something better :)
Technically this call doesn't even have to be hard linked to a plugin. Can be just done by a part of the server.
The server doesn't have to use *plugin services*, services are an API for plugins to use.
Then why limit the plugin like this? That doesn't add much to the security as the plugin can replace that string anyway.
Not for security. It's to avoid boilerplate, to not force plugins to tell the server what the server already knows.
Also one plugin can have more than one connection and I can imagine that different usernames for these connections make sence.
well, the point was to identify what plugin makes the call. and almost always it will be a plugin name.
current_user is the name of the user account and it's used in many places as such. Try, for example, to create a view or a stored procedure. Who will be a definer? If not specified, the definer is going to be username@''. And as a result the view or the procedure will be not functional. But i think it's rather correct. The user of the SQL service has to specify the definer explicitly.
No, using pluginname@'' can hardly be correct. ''@'' is more reasonable. And it's not only definer, it's what CURRENT_USER shows, it's what privileges are used. Surely, not privileges of the pluginname@'' account, there is no such account. So CURRENT_USER should not be pluginname@''.
Setting only user() might be ok. I can agree with that. Setting the ctx->user only. In this case the DEFINER of the view/procedure is going to be empty if not explicitly specified. Though don't see any advantage to what is now.
USER() is purely informational. CURRENT_USER() is not, it has a clearly defined meaning, it's the name of the account that the privilege system uses.
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
participants (2)
-
Alexey Botchkov -
Sergei Golubchik