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
Fix Committed
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  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.