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: <https://github.com/jabberd2/jabberd2/issues/27> 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 @@ <!-- SQLite busy-timeout in milliseconds. --> <busy-timeout>2000</busy-timeout> + + <!-- Passwords in DB may be stored in plain or hashed format --> + <!-- NOTE: If you are using hashed passwords, the only auth + method that will work is PLAIN. + Make sure that you disabled others in 'mechanisms' + sections of the config file. --> + <password_type> + <!-- only one may be enabled here --> + <plaintext/> + <!-- use crypt(3)ed passwords + <crypt/> + --> + <!-- use A1HASH passwords + This stores the MD5 digest of user:realm:password in the database + <a1hash/> + --> + </password_type> </sqlite> <!-- MySQL module configuration --> 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 <sqlite3.h> +/* Windows does not have the crypt() function, let's take DES_crypt from OpenSSL instead */ +#if defined(HAVE_OPENSSL_CRYPTO_H) && defined(_WIN32) +#include <openssl/des.h> +#define crypt DES_crypt +#define HAVE_CRYPT 1 +#else +#ifdef HAVE_CRYPT +#include <unistd.h> +#endif +#endif + +#ifdef HAVE_SSL +/* We use OpenSSL's MD5 routines for the a1hash password type */ +#include <openssl/md5.h> +#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 = 1; + log_write(ar->c2s->log, LOG_ERR, "Realm cannot contain : with a1hash encryption type."); + break; + } + calc_a1hash(username, realm, password, a1hash_pw); + ret = (strncmp(a1hash_pw, db_pw_value, 32) != 0); + break; +#endif + + default: + /* should never happen */ + ret = 1; + log_write(ar->c2s->log, LOG_ERR, "Unknown encryption type which passed through config check."); + break; } - sqlite3_bind_text(stmt, 1, username, -1, SQLITE_STATIC); - sqlite3_bind_text(stmt, 2, password, -1, SQLITE_STATIC); - sqlite3_bind_text(stmt, 3, realm, -1, SQLITE_STATIC); - - res = sqlite3_step(stmt); - if (res == SQLITE_ROW) { - ret = 0; - } - sqlite3_reset(stmt); return ret; } @@ -173,7 +257,25 @@ _ar_sqlite_set_password(authreg_t ar, const char *username, const char *realm, "UPDATE authreg SET password = ? WHERE username = ? AND realm = ?"; log_debug(ZONE, "sqlite (authreg): set password"); - + +#ifdef HAVE_CRYPT + if (data->password_type == MPC_CRYPT) { + char salt[39] = "$6$rounds=50000$"; + int i; + + srand(time(0)); + for(i=0; i<22; i++) + salt[16+i] = salter[rand()%64]; + + strcpy(password, crypt(password, salt)); + } +#endif +#ifdef HAVE_SSL + else if (data->password_type == MPC_A1HASH) { + calc_a1hash(username, realm, password, password); + } +#endif + stmt = _get_stmt(ar, data->db, &data->set_password_stmt, sql); if (stmt == NULL) { return 1; @@ -328,6 +430,22 @@ ar_init(authreg_t ar) sqlite3_busy_timeout(db, atoi(busy_timeout)); } + + /* get encryption type used in DB */ + if (config_get_one(ar->c2s->config, "authreg.sqlite.password_type.plaintext", 0)) { + data->password_type = MPC_PLAIN; +#ifdef HAVE_CRYPT + } else if (config_get_one(ar->c2s->config, "authreg.sqlite.password_type.crypt", 0)) { + data->password_type = MPC_CRYPT; +#endif +#ifdef HAVE_SSL + } else if (config_get_one(ar->c2s->config, "authreg.sqlite.password_type.a1hash", 0)) { + data->password_type = MPC_A1HASH; +#endif + } else { + data->password_type = MPC_PLAIN; + } + ar->private = data; ar->user_exists = _ar_sqlite_user_exists;
