Support for encrypted passwords seems to be missing when using postgresql

Bug #615157 reported by Keith Gillis
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Jabberd
Low
Tomasz Sterna

Bug Description

There is a password_type option in the mysql section of the default c2s.xml file but no password_type option in the pgsql section. From what I can tell by looking at the documentation and code encrypted passwords not not seem to be supported when using postgresql.

I'm running jabberd2 2.2.8-2ubuntu4 on ubuntu 9.10

Tomasz Sterna (smoku)
Changed in jabberd2:
importance: Undecided → Wishlist
status: New → Opinion
Revision history for this message
D'Arcy Cain (darcy-6) wrote :

I have a patch for this. The attached patch mostly copies the code from the MySQL driver with changes for PG. There are also a few other minor changes.

 - Removed some extraneous, trailing whitespace.
 - Added some missing "static" keywords.
 - Removed quotes from queries.

The last is probably a separate issue. Let me know if you need the patch split. To explain, the existing code wraps the PG entities in quotes. Sounds like a good idea but it breaks some code. In my case, for example, I need this query:

  SELECT * from vex.jabberauth WHERE username =...

The "vex" part is a PostgreSQL schema. Putting quotes around the table turns it into a table called "vex.jabberauth" in the public (default) schema instead of table "jabberauth" in the "vex" schema. I think that if the caller needs quotes there they should add them in c2s.xml.

Revision history for this message
Tomasz Sterna (smoku) wrote :

Already fixed in 152dcafc

Changed in jabberd2:
status: Opinion → Fix Committed
Revision history for this message
Tomasz Sterna (smoku) wrote :

I added the missing 'static' keywords though.
Thanks. :-)

Revision history for this message
Tomasz Sterna (smoku) wrote :

Fixed in 2.2.15

Changed in jabberd2:
status: Fix Committed → Fix Released
Revision history for this message
D'Arcy Cain (darcy-6) wrote :

Can you please clarify what exactly is fixed in 2.2.15? I just installed 2.2.16 and neither the pgsql crypt feature or the namespace quoting fix are in it.

Revision history for this message
Tomasz Sterna (smoku) wrote :

Your patch was merged in https://github.com/Jabberd2/jabberd2/commit/152dcafc on 2012-02-12 resulting in status 'Fix Committed'
On 2012-04-30 jabberd 2.2.15 was released moving this bug Fix Committed → Fix Released

Revision history for this message
Tomasz Sterna (smoku) wrote :

Oh. Now I see it wasn't your patch but similar functionality.
This bug need reopening then.

Changed in jabberd2:
assignee: nobody → Tomasz Sterna (smoku)
importance: Wishlist → Low
status: Fix Released → Confirmed
Revision history for this message
D'Arcy Cain (darcy-6) wrote :

My patch no longer applies cleanly. I have created a new one against 2.17. Hopefully this can be applied before the code base changes again.

Revision history for this message
Tomasz Sterna (smoku) wrote :

I think there is a problem with your patch.
It removes "authreg.pgsql.sql.checkpassword" configuration option support (setting the query for ctx->sql_check_password).
It always uses _ar_pgsql_get_password() method.

This breaks the use case, when one defines the hashing function in query and uses DB to hash the provided password for comparison. (like "SELECT login FROM user WHERE login = '%s' AND password = MD5(%s) AND domain = '%s'")
There are people using this option (though it is undocumented in sm.xml) and we cannot remove its support.

Revision history for this message
D'Arcy Cain (darcy-6) wrote :

Yes, I was trying to make it as close to the MySQL module behaviour as possible. I guess MySQL doesn't have that option.

I will try to figure this out and generate a new patch. I guess need to look at authreg.pgsql.sql.checkpassword and branch to the old code if it is defined. I think I should create a new function _ar_pgsql_dbcheck_password with the old code and point to it in the init function if needed.

I also wonder if this crypt code shouldn't be in authreg.c but that's a larger rototill.

Revision history for this message
D'Arcy Cain (darcy-6) wrote :

OK, here is the new patch.

Revision history for this message
Tomasz Sterna (smoku) wrote :

It doesn't apply cleanly on the current codebase either. ;-)
But it was trivial to merge it.

Merged at: https://github.com/Jabberd2/jabberd2/commit/dbfba4645f1c055d8e4fd675221085584e1a0187

Changed in jabberd2:
status: Confirmed → Fix Committed
Revision history for this message
D'Arcy Cain (darcy-6) wrote : Re: [Bug 615157] Re: Support for encrypted passwords seems to be missing when using postgresql

On Thu, 11 Oct 2012 12:24:45 -0000
Tomasz Sterna <email address hidden> wrote:

> It doesn't apply cleanly on the current codebase either. ;-)
> But it was trivial to merge it.

Cool. Thanks.

--
D'Arcy J.M. Cain
System Administrator, Vex.Net
http://www.Vex.Net/ IM:<email address hidden>

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers