[Dovecot] Bug in driver-mysql.c + fix
I tried to use MySQL stored procedures from dovecot:
password_query = CALL user_pass_check('%n', '%d', '%w')
user_query = CALL user_info('%n', '%d')This failed with the message: User query failed: PROCEDURE imap.user_info can't return a result set in the given context
The root of this problem is that mysql_real_connect() needs to be called with option CLIENT_MULTI_RESULTS and mysql_next_result() called to retrieve extra results (that will not exist in the way that we use it). I attach a patch that fixes this problem -- the patch is against dovecot-1.2.10. This works for me ... but could probably do with testing by other people.
BTW: I got the same problem with exim this morning, wrote a patch that has now been accepted. The dovecot patch is similar.
I am looking to use mysql procedures, there are some interesting things that can be done. Two suggestions that I have will help with this:
There be variable (say) %o - this be the obscured password, ie what password_query returns.
that dovecot look for either ''password_query'' as above, or ''password_check''. password_check would NOT return a password, but would be given %o and determine itself if the password is correct. It would return the other values (user, userdb_home, ...) and auth_result that would encode success/retry/fail (0, 1, 2 - or maybe more structured [**]) and auth_reason some human readable reason. The ''nologin'' value encodes some of this.
The motivation for this is that my stored procedure will record the number of successive login failures and lock the account after 3 of them. It would also be possible to do time based restrictions & the such.
Also: by passing %o the password is not sent in plain to the database server - which will increase security.
I will publish my stored procedures when done.
Regards
[**] eg taking ideas from the HTTP codes: 200 - OK 300 5 - try again in 5 minutes 301 2 9 - try again on tuesday at 9am 400 - Login forbidden, no reason given 401 - bad username and/or password 402 - account locked administratively 403 - too many failed login attempts 500 - authentication system error The above would allow a native language version of auth_reason to be produced
-- Alain Williams Linux/GNU Consultant - Mail systems, Web sites, Networking, Programmer, IT Lecturer. +44 (0) 787 668 0256 http://www.phcomp.co.uk/ Parliament Hill Computers Ltd. Registration Information: http://www.phcomp.co.uk/contact.php Past chairman of UKUUG: http://www.ukuug.org/ #include <std_disclaimer.h>
Sorry, been busy + also this mail somehow got marked as read.
On Fri, 2010-03-05 at 23:44 +0000, Alain Williams wrote:
I tried to use MySQL stored procedures from dovecot:
password_query = CALL user_pass_check('%n', '%d', '%w')
user_query = CALL user_info('%n', '%d')
This failed with the message: User query failed: PROCEDURE imap.user_info can't return a result set in the given context
I thought it was possible to avoid that error by implementing the MySQL procedure in a specific way?..
The root of this problem is that mysql_real_connect() needs to be called with option CLIENT_MULTI_RESULTS
The problem with doing that is that 1) it's not normally necessary and more importantly 2) doing that makes any potential SQL injection security holes a lot easier to exploit. So I'm not all that eager in adding such code, especially if it can be worked around another way..
I am looking to use mysql procedures, there are some interesting things that can be done. Two suggestions that I have will help with this:
There be variable (say) %o - this be the obscured password, ie what password_query returns.
that dovecot look for either ''password_query'' as above, or ''password_check''. password_check would NOT return a password, but would be given %o and determine itself if the password is correct. It would return the other values (user, userdb_home, ...) and auth_result that would encode success/retry/fail (0, 1, 2 - or maybe more structured [**]) and auth_reason some human readable reason. The ''nologin'' value encodes some of this.
The motivation for this is that my stored procedure will record the number of successive login failures and lock the account after 3 of them. It would also be possible to do time based restrictions & the such.
Also: by passing %o the password is not sent in plain to the database server - which will increase security.
So what kind of obscuring where you thinking about? You could already use "%Mw", which gives you MD5 of the password.
And password_query can already do basically what you were thinking about with password_check:
password_query = select NULL as password, 'Y' as nopassword FROM users WHERE username = '%u' and password = '%w'
or something. The main problem with that is that it can't differentiate between "wrong password" and "user doesn't exist", so it logs all password failures as "user doesn't exist".
On Wed, 2010-03-10 at 16:18 +0200, Timo Sirainen wrote:
password_query = CALL user_pass_check('%n', '%d', '%w')
user_query = CALL user_info('%n', '%d')
This failed with the message: User query failed: PROCEDURE imap.user_info can't return a result set in the given context
I thought it was possible to avoid that error by implementing the MySQL procedure in a specific way?..
"Statements that return a result set can be used within a stored procedcure but not within a stored function. This prohibition includes SELECT statements that do not have an INTO var_list clause and other statements such as SHOW, EXPLAIN, and CHECK TABLE."
Sounds like if you used SELECT .. INTO and returned those variables, it would work?
On Wed, Mar 10, 2010 at 04:23:23PM +0200, Timo Sirainen wrote:
On Wed, 2010-03-10 at 16:18 +0200, Timo Sirainen wrote:
password_query = CALL user_pass_check('%n', '%d', '%w')
user_query = CALL user_info('%n', '%d')
This failed with the message: User query failed: PROCEDURE imap.user_info can't return a result set in the given context
I thought it was possible to avoid that error by implementing the MySQL procedure in a specific way?..
"Statements that return a result set can be used within a stored procedcure but not within a stored function. This prohibition includes SELECT statements that do not have an INTO var_list clause and other statements such as SHOW, EXPLAIN, and CHECK TABLE."
Sounds like if you used SELECT .. INTO and returned those variables, it would work?
I tried that just now, with my patch not implemented and get the error about 'context' above Unfortunately that does not seem to work.
The SQL that I am using is below, if someone can make it work without my patch I would be grateful.
DROP PROCEDURE IF EXISTS user_pass_check_t !!
CREATE PROCEDURE user_pass_check_t(local_part TEXT, in_domain TEXT, test_pass TEXT) NOT DETERMINISTIC -- login state of the user may change COMMENT "Return user information as needed by dovecot" BEGIN DECLARE usr_addr VARCHAR(128); DECLARE usr_pass VARCHAR(64); DECLARE usr_home VARCHAR(256); DECLARE usr_uid INTEGER; DECLARE usr_gid INTEGER;
SELECT CONCAT(userid, '@', domain), CONCAT('{DIGEST-MD5}', password), home, uid, gid \
INTO usr_addr, usr_pass, usr_home, usr_uid, usr_gid
FROM imap.users WHERE userid = local_part AND domain = in_domain;
SELECT usr_addr AS user, usr_pass AS password, usr_home AS userdb_home, usr_uid AS userdb_uid, usr_gid AS userdb_gid;END !!
-- Alain Williams Linux/GNU Consultant - Mail systems, Web sites, Networking, Programmer, IT Lecturer. +44 (0) 787 668 0256 http://www.phcomp.co.uk/ Parliament Hill Computers Ltd. Registration Information: http://www.phcomp.co.uk/contact.php Past chairman of UKUUG: http://www.ukuug.org/ #include <std_disclaimer.h>
On Wed, Mar 10, 2010 at 04:18:19PM +0200, Timo Sirainen wrote:
On Fri, 2010-03-05 at 23:44 +0000, Alain Williams wrote: ...
This failed with the message: User query failed: PROCEDURE imap.user_info can't return a result set in the given context
I thought it was possible to avoid that error by implementing the MySQL procedure in a specific way?..
I have tried ... see another thread.
See: http://dev.mysql.com/doc/refman/5.1/en/mysql-real-connect.html
Just under the table near the top it says:
If your program uses CALL statements to execute stored procedures, the CLIENT_MULTI_RESULTS flag must be enabled.The root of this problem is that mysql_real_connect() needs to be called with option CLIENT_MULTI_RESULTS
The problem with doing that is that 1) it's not normally necessary and more importantly 2) doing that makes any potential SQL injection security holes a lot easier to exploit. So I'm not all that eager in adding such code, especially if it can be worked around another way..
CLIENT_MULTI_STATEMENTS allows multiple statements in one call (you separate by ','). CLIENT_MULTI_RESULTS does not imply CLIENT_MULTI_STATEMENTS. Is this what you were concerned about ?
Look at the URL above for details.
I am looking to use mysql procedures, there are some interesting things that can be done. Two suggestions that I have will help with this:
There be variable (say) %o - this be the obscured password, ie what password_query returns.
that dovecot look for either ''password_query'' as above, or ''password_check''. password_check would NOT return a password, but would be given %o and determine itself if the password is correct. It would return the other values (user, userdb_home, ...) and auth_result that would encode success/retry/fail (0, 1, 2 - or maybe more structured [**]) and auth_reason some human readable reason. The ''nologin'' value encodes some of this.
The motivation for this is that my stored procedure will record the number of successive login failures and lock the account after 3 of them. It would also be possible to do time based restrictions & the such.
Also: by passing %o the password is not sent in plain to the database server - which will increase security.
So what kind of obscuring where you thinking about? You could already use "%Mw", which gives you MD5 of the password.
That is not how I store passwords - I keep them as DIGEST-MD5, this is: md5('username:domain:password') So I want %o to be that value. Squirrelmail should be able to deduce that from the line in the dovecot-sql.conf: default_pass_scheme="DIGEST-MD5"
And password_query can already do basically what you were thinking about with password_check:
password_query = select NULL as password, 'Y' as nopassword FROM users WHERE username = '%u' and password = '%w'
or something. The main problem with that is that it can't differentiate between "wrong password" and "user doesn't exist", so it logs all password failures as "user doesn't exist".
I am trying to find a definition of the API to plugins, ... if the SQL stored procedure can return arbitrary variables that can then be used by PHP plugins then I can do things like issuing a warning about the password about to expire, number of failed login attempts since the last success, ... Ie all sorts of things that the authentication stored procedures could store and manage.
-- Alain Williams Linux/GNU Consultant - Mail systems, Web sites, Networking, Programmer, IT Lecturer. +44 (0) 787 668 0256 http://www.phcomp.co.uk/ Parliament Hill Computers Ltd. Registration Information: http://www.phcomp.co.uk/contact.php Past chairman of UKUUG: http://www.ukuug.org/ #include <std_disclaimer.h>
On Wed, 2010-03-10 at 17:03 +0000, Alain Williams wrote:
The problem with doing that is that 1) it's not normally necessary and more importantly 2) doing that makes any potential SQL injection security holes a lot easier to exploit. So I'm not all that eager in adding such code, especially if it can be worked around another way..
CLIENT_MULTI_STATEMENTS allows multiple statements in one call (you separate by ','). CLIENT_MULTI_RESULTS does not imply CLIENT_MULTI_STATEMENTS. Is this what you were concerned about ?
Yeah, I mixed up MULTI_STATEMENTS and MULTI_RESULTS. So I can enable the MULTI_RESULTS I guess..
I committed your patch, but with some changes. In error conditions it would have leaked memory. http://hg.dovecot.org/dovecot-2.0/rev/612db456c090
That is not how I store passwords - I keep them as DIGEST-MD5, this is: md5('username:domain:password') So I want %o to be that value. Squirrelmail should be able to deduce that from the line in the dovecot-sql.conf: default_pass_scheme="DIGEST-MD5"
Well, yeah.. That would be possible to implement. But not a very good idea to waste everyone's CPU by calculating that checksum for each lookup, when you're the only one using it. So it should be a var-expand modified instead of variable, so you could then use e.g. %Sw that expands to %w through default_pass_scheme (and only when it's used).
The problem is, var-expand code doesn't currently support adding more modifiers. So its API would need to be changed.
I am trying to find a definition of the API to plugins, ...
There are many kinds of plugins, but none really seem to fit what you wanted to do below.
if the SQL stored procedure can return arbitrary variables that can then be used by PHP plugins then I can do things like issuing a warning about the password about to expire, number of failed login attempts since the last success, ... Ie all sorts of things that the authentication stored procedures could store and manage.
One possibility would be to return 'reason' string from password_query for failures, which contains all of the information you want to know. And if you don't want it to be visible to non-webmail clients, you could return it only when '%r'='127.0.0.1'.
participants (2)
-
Alain Williams
-
Timo Sirainen