Re: [PATCH] Implement hashed passwords for SQLite3 storage

[Sergio Durigan Junior]

On Saturday, November 02 2013, Tomasz Sterna wrote:

> The easiest would be to make a GitHub fork to ba able to create 
> GitHub pull requests.

Sorry, GitHub uses proprietary software as its backend, and its terms of
service are not very respectful IMO.  I have an account there, but I
don't use it anymore.  I hope this is not much inconvenient for you.

-- 
Sergio

Re: [PATCH] Implement hashed passwords for SQLite3 storage

[Tomasz Sterna]

Dnia 2013-10-31, czw o godzinie 16:44 -0200, Sergio Durigan Junior
pisze:
> I'll set up a git repo and publish here, so if anyone is interested
> it'll be possible to follow the development.

The easiest would be to make a GitHub fork to ba able to create 
GitHub pull requests.


-- 
Tomasz Sterna:(){ :|:&};:
Instant Messaging ConsultantOpen Source Developer 
http://abadcafe.pl/   http://www.xiaoka.com/portfolio

Re: [PATCH] Implement hashed passwords for SQLite3 storage

[Sergio Durigan Junior]

On Thursday, October 31 2013, Tomasz Sterna wrote:

>> Eventually, I intend to start working on the libdbi integration (see
>> ).  Is anyone else
>> working on this?  Maybe we could join efforts :-).
>
> Noone that I know of.
> It should be fairly easy to do basing on one of the existing backends.

Nice.  If you wouldn't mind, I actually plan to do lots of cleanups on
the code.  A quick inspection made me think that it needs an overhaul
:-).

I'll set up a git repo and publish here, so if anyone is interested
it'll be possible to follow the development.

Thanks!

-- 
Sergio

Re: [PATCH] Implement hashed passwords for SQLite3 storage

[Tomasz Sterna]

Dnia 2013-10-30, śro o godzinie 16:36 -0200, Sergio Durigan Junior
pisze:
> Wow, that was fast!  Thanks a lot :-).

Thank _you_. :-)


> Eventually, I intend to start working on the libdbi integration (see
> ).  Is anyone else
> working on this?  Maybe we could join efforts :-).

Noone that I know of.
It should be fairly easy to do basing on one of the existing backends.


-- 
Tomasz Sterna @ http://abadcafe.pl/ @ http://www.xiaoka.com/

Re: [PATCH] Implement hashed passwords for SQLite3 storage

[Sergio Durigan Junior]

On Wednesday, October 30 2013, Tomasz Sterna wrote:

> Dnia 2013-10-30, śro o godzinie 03:37 -0200, Sergio Durigan Junior
> pisze:
>> The patch is indeed a copy-and-paste of the code which adds this
>> security in other backends.  I also made a little cleanup in the
>> "check_password" function of the SQLite3 backend (obvious).  I tested
>> the patch by creating several users and logging into the server with
>> them afterwards.  Everything succeeded.
>
> Thanks. :-)
>
> Merged: 
> https://github.com/jabberd2/jabberd2/commit/3e207cfc08efdafe9a9e75dc580dd9c5bfe59554

Wow, that was fast!  Thanks a lot :-).

Eventually, I intend to start working on the libdbi integration (see
).  Is anyone else
working on this?  Maybe we could join efforts :-).

Thanks,

-- 
Sergio

Re: [PATCH] Implement hashed passwords for SQLite3 storage

[Tomasz Sterna]

Dnia 2013-10-30, śro o godzinie 03:37 -0200, Sergio Durigan Junior
pisze:
> The patch is indeed a copy-and-paste of the code which adds this
> security in other backends.  I also made a little cleanup in the
> "check_password" function of the SQLite3 backend (obvious).  I tested
> the patch by creating several users and logging into the server with
> them afterwards.  Everything succeeded.

Thanks. :-)

Merged: 
https://github.com/jabberd2/jabberd2/commit/3e207cfc08efdafe9a9e75dc580dd9c5bfe59554



-- 
Tomasz Sterna @ http://abadcafe.pl/ @ http://www.xiaoka.com/

[PATCH] Implement hashed passwords for SQLite3 storage

[Sergio Durigan Junior]

Hi there,

I'm a(n) (almost) happy Jabberd2 user :-).  I say "almost" because
unfortunately it lacks any kind of security for storing passwords when
using the SQLite3 storage backend.

While taking a look at your website and resources, I found this:



While I agree with "smokku" there, I also think this kind of issue
should not be overlooked, and the fix should not be postponed.  So I
went ahead and fixed it.

The patch is indeed a copy-and-paste of the code which adds this
security in other backends.  I also made a little cleanup in the
"check_password" function of the SQLite3 backend (obvious).  I tested
the patch by creating several users and logging into the server with
them afterwards.  Everything succeeded.

The ChangeLog seems abandoned, so I didn't bother updating it (I can do
if you want).  I also wasn't sure if I should update the NEWS file or
not, so I left it untouched too.  And please let me know if there is
something missing in my approach.

Otherwise, I'd like to hear opinions (or acceptance, if you will!).

Thanks,

-- 
Sergio

diff --git a/etc/c2s.xml.dist.in b/etc/c2s.xml.dist.in
index 6626b43..374ba26 100644
--- a/etc/c2s.xml.dist.in
+++ b/etc/c2s.xml.dist.in
@@ -430,6 +430,23 @@
 
   
   2000
+
+  
+  
+  
+
+
+
+
+  
 
 
 
diff --git a/storage/authreg_sqlite.c b/storage/authreg_sqlite.c
index 0bbdd76..2bea9be 100644
--- a/storage/authreg_sqlite.c
+++ b/storage/authreg_sqlite.c
@@ -29,9 +29,44 @@
  * to the Jabberd project.
  */
 
+#define _XOPEN_SOURCE 500
 #include "c2s.h"
 #include 
 
+/* Windows does not have the crypt() function, let's take DES_crypt from 
OpenSSL instead */
+#if defined(HAVE_OPENSSL_CRYPTO_H) && defined(_WIN32)
+#include 
+#define crypt DES_crypt
+#define HAVE_CRYPT 1
+#else
+#ifdef HAVE_CRYPT
+#include 
+#endif
+#endif
+
+#ifdef HAVE_SSL
+/* We use OpenSSL's MD5 routines for the a1hash password type */
+#include 
+#endif
+
+#define SQLITE_LU  1024   /* maximum length of username - should correspond to 
field length */
+#define SQLITE_LR   256   /* maximum length of realm - should correspond to 
field length */
+#define SQLITE_LP   256   /* maximum length of password - should correspond to 
field length */
+
+enum sqlite3_pws_crypt {
+MPC_PLAIN,
+#ifdef HAVE_CRYPT
+MPC_CRYPT,
+#endif
+#ifdef HAVE_SSL
+MPC_A1HASH,
+#endif
+};
+
+#ifdef HAVE_CRYPT
+static char salter[] = 
"0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ./";
+#endif
+
 typedef struct moddata_st {
 sqlite3 *db;
 int txn;
@@ -41,8 +76,27 @@ typedef struct moddata_st {
 sqlite3_stmt *set_password_stmt;
 sqlite3_stmt *create_user_stmt;
 sqlite3_stmt *delete_user_stmt;
+enum sqlite3_pws_crypt password_type;
 } *moddata_t;
 
+#ifdef HAVE_SSL
+static void calc_a1hash(const char *username, const char *realm, const char 
*password, char *a1hash)
+{
+#define A1PPASS_LEN SQLITE_LU + 1 + SQLITE_LR + 1 + SQLITE_LP + 1 /* 
user:realm:password\0 */
+char buf[A1PPASS_LEN];
+unsigned char md5digest[MD5_DIGEST_LENGTH];
+int i;
+
+snprintf(buf, A1PPASS_LEN, "%.*s:%.*s:%.*s", SQLITE_LU, username, 
SQLITE_LR, realm, SQLITE_LP, password);
+
+MD5((unsigned char*)buf, strlen(buf), md5digest);
+
+for(i=0; i<16; i++) {
+sprintf(a1hash+i*2, "%02hhx", md5digest[i]);
+}
+}
+#endif
+
 static sqlite3_stmt*
 _get_stmt(authreg_t ar, sqlite3 *db, sqlite3_stmt **stmt, const char *sql)
 {
@@ -132,28 +186,58 @@ _ar_sqlite_check_password(authreg_t ar, const char 
*username, const char *realm,
  char password[257])
 {
 
-sqlite3_stmt *stmt;
+char db_pw_value[257];
+#ifdef HAVE_CRYPT
+char *crypted_pw;
+#endif
+#ifdef HAVE_SSL
+char a1hash_pw[33];
+#endif
 moddata_t data = (moddata_t) ar->private;
-int res, ret=1;
-char *sql =
-   "SELECT username FROM authreg WHERE username = ? AND password = ? AND 
realm = ?";
+int ret=1;
 
 log_debug(ZONE, "sqlite (authreg): check password");
 
-stmt = _get_stmt(ar, data->db, &data->check_password_stmt, sql);
-if (stmt == NULL) {
-   return 1;
+ret = _ar_sqlite_get_password (ar, username, realm, db_pw_value);
+if (ret)
+return ret;
+
+switch (data->password_type) {
+case MPC_PLAIN:
+ret = (strcmp (password, db_pw_value) != 0);
+break;
+
+#ifdef HAVE_CRYPT
+case MPC_CRYPT:
+crypted_pw = crypt(password,db_pw_value);
+ret = (strcmp(crypted_pw, db_pw_value) != 0);
+break;
+#endif
+
+#ifdef HAVE_SSL
+case MPC_A1HASH:
+if (strchr(username, ':')) {
+ret = 1;
+log_write(ar->c2s->log, LOG_ERR, "Username cannot contain 
: with a1hash encryption type.");
+break;
+}
+if (strchr(realm, ':')) {
+ret