svarshavchik / courier

Courier Mail Server
http://www.courier-mta.org
72 stars 12 forks source link

custom mysql auth query #52

Closed jkroonza closed 9 months ago

jkroonza commented 9 months ago

Hi,

Currently we can use a select query to retrieve the username and password.

Here for password I would like a custom mechanism to indicate to execute another query to validate the username and password, eg, if both cryptpw as well as clearpw is NULL, then track a setting like:

MYSQL_CUSTOM_AUTH  SELECT some_stored_func('$(user)', '$(pass)', '$(proto)')

WHERE the query must return exactly one row, and one column, and returned column must evaluate to some definition of true for a successful login. Where in my definition of true, the returned value is any of:

  1. int > 0 (must translate fully, ie, not "1something")
  2. string literal true (case insensitive)
  3. string literal yes (case insensitive)

Anything else is treated as login failed.

user or username gets populated from the initial MYSQL_SELECT_CLAUSE, and pass from the user-supplied password. Additional information I would like available somehow is the authenticating protocol (POP3 vs IMAP), as well as security/crypt including whether or not it's explicit or starttls style.

Our motivations:

  1. We're moving to a PBKDF2 based authentication mechanism and crypt passwords will no longer be available.
  2. Our stored func logs successful authentications against protocol, in this case pop3 vs pop3s vs pop3+tls (ditto for imap).

Any mechanism to differentiate betwee the three mechanisms really is good enough so even if we end up with the six strings {POP3,IMAP}+{NONE,STLS,TLS} for example, it's fine, we just want to know who's using what without having to parse access logs ideally.

svarshavchik commented 9 months ago

This is hard to follow. MySQL's SQL can get quite complex, and it's capable of executing many different kinds of joins. In any case there's no such setting in courier-authlib.

jkroonza commented 9 months ago

Sorry for not being clear, I tried to be as clear as I could. I filed the issue as much as a reminder to myself that this needs doing as to get your input on such a feature. So it simply getting closed is a bit frustrating.

Yes, this would need to be added to courier-authlib, if I didn't absolutely need assistance on this from courier-authlib side this would have been done already without having opened an issue here.

Happy to write the code for this. Essentially it looks like a ammendment to the verify() function in authmysql.cpp, not a completely trivial one, but also not super difficult. Some information may not be readily available, but then we (not you) will need to make a call as to whether we live with what we have or try and create a patch that's acceptable to you.

svarshavchik commented 9 months ago

You're welcome to give it a shot, and I'll take a look at it.

oliv3r commented 6 months ago

While I'm also looking into something similar, as per docs:

Observe that a valid SQL expression may be used in place of any field setting, since all that happens is that the contents of the settings are inserted into an SQL SELECT statement.

Looking (pgsql for my case), https://github.com/svarshavchik/courier/blob/1baadc67e825122c0304d74adaf5d4bc292ac67e/courier-authlib/authpgsqllib.cpp#L622 we see exactly this. So there's some commas to keep in mind, e.g. it's not possible to just do things willy nilly, but a) you could even go as far as LOGIN_FIELD="some_stored_func('$(user)', '$(pass)', '$(proto)'); echo "

As for writing the code for it (again, only looking at pgsql as that's relevant for me), you could introduce a new variable (but then do it for at least both mysql and pgsql :p), CUSTOM_SQL_QUERY, check in the code if it exists, and if it does, o << "config_file.custom_sql_query" else, whatever is there now.

Are there other things I'm missing, or a I simplifying things too much? Edit: I've missed quote a bit :) Looking at more detail, I see that the query is not used to match a user, but to get everybody/all users, and then let the C++ code do the parsing of the user. Not sure if that's the most efficient way of doing things, I'm sure there was a good reason to do so @svarshavchik?

What if *sql was made responsibel for doing the work? In my setup, postfix does the same, it just sends the query to SQL. From a security pov, I think getting only a single user would be much wiser then getting all users. E.g. we'd end up with select passwd from mailbox where (user=%u and doman = %d or username=%u@%d or username=%u) (or some such). This could equalliy work with the aforementioned custom query of course, but this would simplify both the code and performance quite a bit.

(Yes, I do realize one can do a lot of fun stuff to get similar results by modifying the WHERE part of the variable and reach a similar goal ;p)

Just sharing some thoughts.

jkroonza commented 6 months ago

What I'm thinking of doing:

  1. Export a few extra variables for use in the SQL statement.
  2. Use an IF() on the password, something like:
MYSQL_SELECT_CLAUSE SELECT username, NULL, IF(stored_proc_for_auth(username, '$(password)', CONCAT('$(service)', IF($(starttls), '+tls', '')), '$(password)', 'incorrect$(password)forsure'), ...

Kind of thing (hoping I got the brackets right. That way the plaintext password will be handed back to courier-authlib for authenticating the user only if the password was already verified on SQL side, else we will give back what I believe will ALWAYS be a mismatching password.

So what I need extra in addition in to what is already there I think is basically password as an available variable in the expansion, and possibly starttls which expands to 0 or 1.

svarshavchik commented 6 months ago

You might be forgetting that you also have a WHERE clause in there. So, your SQL could be

SELECT WHERE stored_proc_for_auth( ... parameters ...)