From 19390dd8e2396a86538b24c2419ac39615c02b9e Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Tue, 12 Jan 2016 00:52:42 +0100 Subject: [PATCH 01/19] log a warning when passwords would fail for being case-sensitive. add a test to enforce the status quo, for now. --- src/kernel/faction.c | 8 +++++++- src/kernel/faction.test.c | 11 +++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/kernel/faction.c b/src/kernel/faction.c index d13c1ea90..4b41e58cb 100755 --- a/src/kernel/faction.c +++ b/src/kernel/faction.c @@ -313,7 +313,13 @@ unit *addplayer(region * r, faction * f) bool checkpasswd(const faction * f, const char *passwd) { - return (passwd && unicode_utf8_strcasecmp(f->passw, passwd) == 0); + if (!passwd) return false; + if (strcmp(f->passw, passwd)==0) return true; + if (unicode_utf8_strcasecmp(f->passw, passwd) == 0) { + log_warning("case-sensitive password check failed: %s", factionname(f)); + return true; + } + return false; } variant read_faction_reference(struct storage * store) diff --git a/src/kernel/faction.test.c b/src/kernel/faction.test.c index 9c85884b7..578c31c86 100644 --- a/src/kernel/faction.test.c +++ b/src/kernel/faction.test.c @@ -119,6 +119,16 @@ static void test_addfaction(CuTest *tc) { test_cleanup(); } +static void test_check_passwd(CuTest *tc) { + faction *f; + + f = test_create_faction(0); + faction_setpassword(f, "password"); + CuAssertIntEquals(tc, true, checkpasswd(f, "password")); + CuAssertIntEquals(tc, true, checkpasswd(f, "PASSWORD")); + CuAssertIntEquals(tc, false, checkpasswd(f, "assword")); +} + static void test_get_monsters(CuTest *tc) { faction *f; @@ -185,5 +195,6 @@ CuSuite *get_faction_suite(void) SUITE_ADD_TEST(suite, test_get_monsters); SUITE_ADD_TEST(suite, test_set_origin); SUITE_ADD_TEST(suite, test_set_origin_bug); + SUITE_ADD_TEST(suite, test_check_passwd); return suite; } From dc6cc41d2d2a35906c46f5e6bf1423ac7dd6c03f Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Tue, 12 Jan 2016 02:27:22 +0100 Subject: [PATCH 02/19] additional warnings --- src/kernel/faction.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/kernel/faction.c b/src/kernel/faction.c index 4b41e58cb..3ed6ee095 100755 --- a/src/kernel/faction.c +++ b/src/kernel/faction.c @@ -319,6 +319,7 @@ bool checkpasswd(const faction * f, const char *passwd) log_warning("case-sensitive password check failed: %s", factionname(f)); return true; } + log_warning("password check failed: %s", factionname(f)); return false; } From b6d44410b7e0becaa272b340371699414432e0e5 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Tue, 12 Jan 2016 06:46:51 +0100 Subject: [PATCH 03/19] make the password pseudo-private to faction.c --- src/bind_faction.c | 9 +----- src/kernel/faction.c | 46 ++++++++++++++++++++-------- src/kernel/faction.h | 5 ++-- src/kernel/faction.test.c | 2 +- src/kernel/save.c | 8 ++--- src/laws.c | 63 ++------------------------------------- src/laws.h | 1 - src/laws.test.c | 4 ++- src/report.c | 5 +--- src/study.test.c | 1 + src/tests.c | 1 + 11 files changed, 51 insertions(+), 94 deletions(-) diff --git a/src/bind_faction.c b/src/bind_faction.c index 19f4c8989..1e6547fb1 100644 --- a/src/bind_faction.c +++ b/src/bind_faction.c @@ -380,13 +380,6 @@ static int tolua_faction_create(lua_State * L) return 1; } -static int tolua_faction_get_password(lua_State * L) -{ - faction *self = (faction *)tolua_tousertype(L, 1, 0); - tolua_pushstring(L, faction_getpassword(self)); - return 1; -} - static int tolua_faction_set_password(lua_State * L) { faction *self = (faction *)tolua_tousertype(L, 1, 0); @@ -561,7 +554,7 @@ void tolua_faction_open(lua_State * L) tolua_variable(L, TOLUA_CAST "heroes", tolua_faction_get_heroes, NULL); tolua_variable(L, TOLUA_CAST "maxheroes", tolua_faction_get_maxheroes, NULL); - tolua_variable(L, TOLUA_CAST "password", tolua_faction_get_password, + tolua_variable(L, TOLUA_CAST "password", NULL, tolua_faction_set_password); tolua_variable(L, TOLUA_CAST "email", tolua_faction_get_email, tolua_faction_set_email); diff --git a/src/kernel/faction.c b/src/kernel/faction.c index 3ed6ee095..56996d606 100755 --- a/src/kernel/faction.c +++ b/src/kernel/faction.c @@ -104,7 +104,7 @@ static void free_faction(faction * f) free(f->email); free(f->banner); - free(f->passw); + free(f->_password); free(f->name); if (f->seen_factions) { ql_free(f->seen_factions); @@ -251,7 +251,9 @@ faction *addfaction(const char *email, const char *password, log_warning("Invalid email address for faction %s: %s\n", itoa36(f->no), email); } + if (!password) password = itoa36(rng_int()); faction_setpassword(f, password); + ADDMSG(&f->msgs, msg_message("changepasswd", "value", password)); f->alliance_joindate = turn; f->lastorders = turn; @@ -314,8 +316,8 @@ unit *addplayer(region * r, faction * f) bool checkpasswd(const faction * f, const char *passwd) { if (!passwd) return false; - if (strcmp(f->passw, passwd)==0) return true; - if (unicode_utf8_strcasecmp(f->passw, passwd) == 0) { + if (strcmp(f->_password, passwd)==0) return true; + if (unicode_utf8_strcasecmp(f->_password, passwd) == 0) { log_warning("case-sensitive password check failed: %s", factionname(f)); return true; } @@ -565,11 +567,9 @@ void faction_setbanner(faction * self, const char *banner) void faction_setpassword(faction * f, const char *passw) { - free(f->passw); - if (passw) - f->passw = _strdup(passw); - else - f->passw = _strdup(itoa36(rng_int())); + assert(passw); + free(f->_password); + f->_password = _strdup(passw); } bool valid_race(const struct faction *f, const struct race *rc) @@ -584,11 +584,6 @@ bool valid_race(const struct faction *f, const struct race *rc) } } -const char *faction_getpassword(const faction * f) -{ - return f->passw; -} - struct alliance *f_get_alliance(const struct faction *f) { if (f->alliance && !(f->alliance->flags & ALF_NON_ALLIED)) { @@ -853,3 +848,28 @@ faction *dfindhash(int no) } return 0; } + +int writepasswd(void) +{ + FILE *F; + char zText[128]; + + sprintf(zText, "%s/passwd", basepath()); + F = fopen(zText, "w"); + if (!F) { + perror(zText); + } + else { + faction *f; + log_info("writing passwords..."); + + for (f = factions; f; f = f->next) { + fprintf(F, "%s:%s:%s:%u\n", + factionid(f), f->email, f->_password, f->subscription); + } + fclose(F); + return 0; + } + return 1; +} + diff --git a/src/kernel/faction.h b/src/kernel/faction.h index 933421da5..153a948b1 100644 --- a/src/kernel/faction.h +++ b/src/kernel/faction.h @@ -68,7 +68,7 @@ extern "C" { char *name; char *banner; char *email; - char *passw; + char *_password; int max_spelllevel; struct spellbook *spellbook; const struct locale *locale; @@ -121,6 +121,7 @@ extern "C" { struct faction *addfaction(const char *email, const char *password, const struct race *frace, const struct locale *loc, int subscription); bool checkpasswd(const faction * f, const char *passwd); + int writepasswd(void); void destroyfaction(faction ** f); bool faction_alive(const struct faction *f); @@ -152,7 +153,7 @@ extern "C" { const char *faction_getemail(const struct faction *self); void faction_setemail(struct faction *self, const char *email); - const char *faction_getpassword(const struct faction *self); + // const char *faction_getpassword(const struct faction *self); void faction_setpassword(struct faction *self, const char *password); bool valid_race(const struct faction *f, const struct race *rc); diff --git a/src/kernel/faction.test.c b/src/kernel/faction.test.c index 578c31c86..905704e83 100644 --- a/src/kernel/faction.test.c +++ b/src/kernel/faction.test.c @@ -107,7 +107,7 @@ static void test_addfaction(CuTest *tc) { CuAssertPtrEquals(tc, NULL, (void *)f->ursprung); CuAssertPtrEquals(tc, (void *)factions, (void *)f); CuAssertStrEquals(tc, "test@eressea.de", f->email); - CuAssertStrEquals(tc, "hurrdurr", f->passw); + CuAssertIntEquals(tc, true, checkpasswd(f, "hurrdurr")); CuAssertPtrEquals(tc, (void *)lang, (void *)f->locale); CuAssertIntEquals(tc, 1234, f->subscription); CuAssertIntEquals(tc, 0, f->flags); diff --git a/src/kernel/save.c b/src/kernel/save.c index de8f84490..fa8da2a7e 100644 --- a/src/kernel/save.c +++ b/src/kernel/save.c @@ -1216,7 +1216,7 @@ faction *readfaction(struct gamedata * data) } READ_STR(data->store, name, sizeof(name)); - f->passw = _strdup(name); + faction_setpassword(f, name); if (data->version < NOOVERRIDE_VERSION) { READ_STR(data->store, 0, 0); } @@ -1320,10 +1320,10 @@ void writefaction(struct gamedata *data, const faction * f) } WRITE_INT(data->store, f->alliance_joindate); - WRITE_STR(data->store, (const char *)f->name); - WRITE_STR(data->store, (const char *)f->banner); + WRITE_STR(data->store, f->name); + WRITE_STR(data->store, f->banner); WRITE_STR(data->store, f->email); - WRITE_TOK(data->store, (const char *)f->passw); + WRITE_TOK(data->store, f->_password); WRITE_TOK(data->store, locale_name(f->locale)); WRITE_INT(data->store, f->lastorders); WRITE_INT(data->store, f->age); diff --git a/src/laws.c b/src/laws.c index 904563a79..a78a3371f 100755 --- a/src/laws.c +++ b/src/laws.c @@ -733,9 +733,6 @@ void nmr_warnings(void) faction *f, *fa; #define FRIEND (HELP_GUARD|HELP_MONEY) for (f = factions; f; f = f->next) { - if (f->age <= 1) { - ADDMSG(&f->msgs, msg_message("changepasswd", "value", f->passw)); - } if (!fval(f, FFL_NOIDLEOUT) && turn > f->lastorders) { ADDMSG(&f->msgs, msg_message("nmr_warning", "")); if (turn - f->lastorders == NMRTimeout() - 1) { @@ -2168,16 +2165,13 @@ int password_cmd(unit * u, struct order *ord) } } } - free(u->faction->passw); if (!pwok) { cmistake(u, ord, 283, MSG_EVENT); - u->faction->passw = _strdup(itoa36(rng_int())); - } - else { - u->faction->passw = _strdup(pwbuf); + strlcpy(pwbuf, itoa36(rng_int()), sizeof(pwbuf)); } + faction_setpassword(u->faction, pwbuf); ADDMSG(&u->faction->msgs, msg_message("changepasswd", - "value", u->faction->passw)); + "value", pwbuf)); return 0; } @@ -4256,32 +4250,6 @@ static void maintain_buildings_1(region * r) maintain_buildings(r, false); } -/** warn about passwords that are not US ASCII. - * even though passwords are technically UTF8 strings, the server receives - * them as part of the Subject of an email when reports are requested. - * This means that we need to limit them to ASCII characters until that - * mechanism has been changed. - */ -static int warn_password(void) -{ - faction *f; - for (f = factions; f; f = f->next) { - bool pwok = true; - const char *c = f->passw; - while (*c && pwok) { - if (!isalnum((unsigned char)*c)) - pwok = false; - c++; - } - if (!pwok) { - free(f->passw); - f->passw = _strdup(itoa36(rng_int())); - ADDMSG(&f->msgs, msg_message("illegal_password", "newpass", f->passw)); - } - } - return 0; -} - void init_processor(void) { int p; @@ -4463,31 +4431,6 @@ void processorders(void) /* immer ausführen, wenn neue Sprüche dazugekommen sind, oder sich * Beschreibungen geändert haben */ update_spells(); - warn_password(); -} - -int writepasswd(void) -{ - FILE *F; - char zText[128]; - - sprintf(zText, "%s/passwd", basepath()); - F = fopen(zText, "w"); - if (!F) { - perror(zText); - } - else { - faction *f; - log_info("writing passwords..."); - - for (f = factions; f; f = f->next) { - fprintf(F, "%s:%s:%s:%u\n", - factionid(f), f->email, f->passw, f->subscription); - } - fclose(F); - return 0; - } - return 1; } void update_subscriptions(void) diff --git a/src/laws.h b/src/laws.h index e245f3c01..6c8633536 100755 --- a/src/laws.h +++ b/src/laws.h @@ -38,7 +38,6 @@ extern "C" { extern int dropouts[2]; extern int *age; - int writepasswd(void); void demographics(void); void immigration(void); void update_guards(void); diff --git a/src/laws.test.c b/src/laws.test.c index 91b04b257..5d21250cd 100644 --- a/src/laws.test.c +++ b/src/laws.test.c @@ -1061,6 +1061,7 @@ static void test_ally_cmd(CuTest *tc) { test_cleanup(); u = test_create_unit(test_create_faction(0), test_create_region(0, 0, 0)); f = test_create_faction(0); + u->faction->locale = lang = get_or_create_locale("de"); locale_setstring(lang, parameters[P_NOT], "NICHT"); locale_setstring(lang, parameters[P_GUARD], "BEWACHE"); @@ -1105,8 +1106,9 @@ static void test_nmr_warnings(CuTest *tc) { CuAssertIntEquals(tc, 0, f1->age); nmr_warnings(); CuAssertPtrNotNull(tc, f1->msgs); - CuAssertPtrNotNull(tc, test_find_messagetype(f1->msgs, "changepasswd")); + CuAssertPtrNotNull(tc, test_find_messagetype(f1->msgs, "nmr_warning")); CuAssertPtrNotNull(tc, f2->msgs); + CuAssertPtrNotNull(tc, f2->msgs->begin); CuAssertPtrNotNull(tc, test_find_messagetype(f2->msgs, "nmr_warning")); CuAssertPtrNotNull(tc, test_find_messagetype(f2->msgs, "nmr_warning_final")); test_cleanup(); diff --git a/src/report.c b/src/report.c index 7b04e020f..f22cb6049 100644 --- a/src/report.c +++ b/src/report.c @@ -1427,7 +1427,7 @@ report_template(const char *filename, report_context * ctx, const char *charset) newline(out); newline(out); - sprintf(buf, "%s %s \"%s\"", LOC(f->locale, "ERESSEA"), factionid(f), f->passw); + sprintf(buf, "%s %s \"password\"", LOC(f->locale, "ERESSEA"), factionid(f)); rps_nowrap(out, buf); newline(out); newline(out); @@ -2112,9 +2112,6 @@ const char *charset) if (f->age <= 2) { const char *s; - RENDER(f, buf, sizeof(buf), ("newbie_password", "password", f->passw)); - newline(out); - centre(out, buf, true); s = locale_getstring(f->locale, "newbie_info_1"); if (s) { newline(out); diff --git a/src/study.test.c b/src/study.test.c index 4b30ae842..6b9d97787 100644 --- a/src/study.test.c +++ b/src/study.test.c @@ -45,6 +45,7 @@ static void setup_study(study_fixture *fix, skill_t sk) { fix->teachers[1] = test_create_unit(f, r); assert(fix->teachers[1]); fix->teachers[1]->thisorder = create_order(K_TEACH, f->locale, "%s", itoa36(fix->u->no)); + test_clear_messages(f); } static void test_study_no_teacher(CuTest *tc) { diff --git a/src/tests.c b/src/tests.c index c5f98502e..6fef425d0 100644 --- a/src/tests.c +++ b/src/tests.c @@ -64,6 +64,7 @@ struct region *test_create_region(int x, int y, const terrain_type *terrain) struct faction *test_create_faction(const struct race *rc) { faction *f = addfaction("nobody@eressea.de", NULL, rc ? rc : test_create_race("human"), default_locale, 0); + test_clear_messages(f); return f; } From ae0206652e12d99a1a0f42a74894cdb4fef582b4 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Tue, 12 Jan 2016 07:25:57 +0100 Subject: [PATCH 04/19] passwords are write-only, cannot read them in tests --- scripts/tests/common.lua | 3 ++- scripts/tests/orders.lua | 5 ++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/tests/common.lua b/scripts/tests/common.lua index 43c77f93f..49c0416a8 100644 --- a/scripts/tests/common.lua +++ b/scripts/tests/common.lua @@ -1083,7 +1083,8 @@ function test_parser() local file = io.open(filename, "w") assert_not_nil(file) - file:write('ERESSEA ' .. itoa36(f.id) .. ' "' .. f.password .. '"\n') + f.password = 'Hodor' + file:write('ERESSEA ' .. itoa36(f.id) .. ' "Hodor"\n') file:write('EINHEIT ' .. itoa36(u.id) .. "\n") file:write("BENENNEN EINHEIT 'Goldene Herde'\n") file:close() diff --git a/scripts/tests/orders.lua b/scripts/tests/orders.lua index e0d42258f..a0b03f825 100644 --- a/scripts/tests/orders.lua +++ b/scripts/tests/orders.lua @@ -73,12 +73,10 @@ function test_process_settings() f.options = 0 u:add_order("EMAIL herp@derp.com") u:add_order("BANNER 'Herpderp'") - u:add_order("PASSWORT 'HerpDerp'") u:add_order("OPTION AUSWERTUNG") eressea.process.settings() assert_equal("herp@derp.com", f.email) assert_equal("Herpderp", f.info) - assert_equal("HerpDerp", f.password) assert_equal(1, f.options) end @@ -98,7 +96,8 @@ end function test_process_quit() fno = f.id - u:add_order("STIRB '" .. u.faction.password .. "'") + u.faction.password = 'Hodor' + u:add_order("STIRB 'Hodor'") assert_not_equal(nil, _G.get_faction(fno)) eressea.process.quit() eressea.write_game('test.dat') From 54d25e91dd32009b7f8d788e27311bd48fa85af8 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Tue, 12 Jan 2016 23:52:30 +0100 Subject: [PATCH 05/19] add a new password module start adding password hashing logic (WIP) does not yet pass all tests --- src/bind_faction.c | 5 ++++- src/kernel/faction.c | 34 +++++++++++++++++++++++----------- src/kernel/faction.h | 3 +-- src/kernel/faction.test.c | 3 ++- src/kernel/save.c | 5 +++-- src/kernel/version.h | 4 ++-- src/laws.c | 3 ++- src/util/CMakeLists.txt | 2 ++ src/util/password.c | 35 +++++++++++++++++++++++++++++++++++ src/util/password.h | 7 +++++++ src/util/password.test.c | 19 +++++++++++++++++++ 11 files changed, 100 insertions(+), 20 deletions(-) create mode 100644 src/util/password.c create mode 100644 src/util/password.h create mode 100644 src/util/password.test.c diff --git a/src/bind_faction.c b/src/bind_faction.c index 1e6547fb1..264e869ed 100644 --- a/src/bind_faction.c +++ b/src/bind_faction.c @@ -31,6 +31,8 @@ without prior permission by the authors of Eressea. #include #include +#include + #include #include @@ -383,7 +385,8 @@ static int tolua_faction_create(lua_State * L) static int tolua_faction_set_password(lua_State * L) { faction *self = (faction *)tolua_tousertype(L, 1, 0); - faction_setpassword(self, tolua_tostring(L, 2, 0)); + const char * passw = tolua_tostring(L, 2, 0); + faction_setpassword(self, password_hash(passw)); return 0; } diff --git a/src/kernel/faction.c b/src/kernel/faction.c index 56996d606..1d70bc867 100755 --- a/src/kernel/faction.c +++ b/src/kernel/faction.c @@ -19,7 +19,6 @@ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. #include #include #include "faction.h" - #include "alliance.h" #include "ally.h" #include "curse.h" @@ -46,6 +45,7 @@ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. #include #include #include +#include #include #include #include @@ -63,6 +63,7 @@ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. #include #include #include +#include faction *factions; @@ -252,7 +253,7 @@ faction *addfaction(const char *email, const char *password, } if (!password) password = itoa36(rng_int()); - faction_setpassword(f, password); + faction_setpassword(f, password_hash(password)); ADDMSG(&f->msgs, msg_message("changepasswd", "value", password)); f->alliance_joindate = turn; @@ -313,16 +314,27 @@ unit *addplayer(region * r, faction * f) return u; } +extern char *sha256_crypt(const char *key, const char *salt); + +const char * mksalt(char *salt, size_t len) { + char *dst = salt; + int ent = (int)time(0); + // FIXME: worst ever salt generation + while (dst < salt + len) { + *dst++ = itoa36(ent & rng_int())[0]; + } + return salt; +} + bool checkpasswd(const faction * f, const char *passwd) { if (!passwd) return false; - if (strcmp(f->_password, passwd)==0) return true; - if (unicode_utf8_strcasecmp(f->_password, passwd) == 0) { - log_warning("case-sensitive password check failed: %s", factionname(f)); - return true; + + if (password_verify(f->_password, passwd) == VERIFY_FAIL) { + log_warning("password check failed: %s", factionname(f)); + return false; } - log_warning("password check failed: %s", factionname(f)); - return false; + return true; } variant read_faction_reference(struct storage * store) @@ -565,11 +577,11 @@ void faction_setbanner(faction * self, const char *banner) self->banner = _strdup(banner); } -void faction_setpassword(faction * f, const char *passw) +void faction_setpassword(faction * f, const char *pwhash) { - assert(passw); + assert(pwhash && pwhash[0] == '$'); free(f->_password); - f->_password = _strdup(passw); + f->_password = _strdup(pwhash); } bool valid_race(const struct faction *f, const struct race *rc) diff --git a/src/kernel/faction.h b/src/kernel/faction.h index 153a948b1..6369444f5 100644 --- a/src/kernel/faction.h +++ b/src/kernel/faction.h @@ -153,8 +153,7 @@ extern "C" { const char *faction_getemail(const struct faction *self); void faction_setemail(struct faction *self, const char *email); - // const char *faction_getpassword(const struct faction *self); - void faction_setpassword(struct faction *self, const char *password); + void faction_setpassword(struct faction *self, const char *pwhash); bool valid_race(const struct faction *f, const struct race *rc); void faction_getorigin(const struct faction * f, int id, int *x, int *y); diff --git a/src/kernel/faction.test.c b/src/kernel/faction.test.c index 905704e83..14e3e5ac0 100644 --- a/src/kernel/faction.test.c +++ b/src/kernel/faction.test.c @@ -8,6 +8,7 @@ #include #include #include +#include #include "monster.h" #include @@ -123,7 +124,7 @@ static void test_check_passwd(CuTest *tc) { faction *f; f = test_create_faction(0); - faction_setpassword(f, "password"); + faction_setpassword(f, password_hash("password")); CuAssertIntEquals(tc, true, checkpasswd(f, "password")); CuAssertIntEquals(tc, true, checkpasswd(f, "PASSWORD")); CuAssertIntEquals(tc, false, checkpasswd(f, "assword")); diff --git a/src/kernel/save.c b/src/kernel/save.c index fa8da2a7e..609f9e5b8 100644 --- a/src/kernel/save.c +++ b/src/kernel/save.c @@ -64,13 +64,14 @@ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. #include #include #include -#include +#include #include #include #include #include #include +#include #include #include #include @@ -1216,7 +1217,7 @@ faction *readfaction(struct gamedata * data) } READ_STR(data->store, name, sizeof(name)); - faction_setpassword(f, name); + faction_setpassword(f, (data->version >= CRYPT_VERSION) ? name : password_hash(name)); if (data->version < NOOVERRIDE_VERSION) { READ_STR(data->store, 0, 0); } diff --git a/src/kernel/version.h b/src/kernel/version.h index 19c564c1e..c18957530 100644 --- a/src/kernel/version.h +++ b/src/kernel/version.h @@ -33,8 +33,8 @@ #define SPELL_LEVEL_VERSION 348 /* f->max_spelllevel gets stored, not calculated */ #define OWNER_3_VERSION 349 /* regions store last owner, not last alliance */ #define ATTRIBOWNER_VERSION 350 /* all attrib_type functions know who owns the attribute */ - -#define RELEASE_VERSION ATTRIBOWNER_VERSION /* current datafile */ +#define CRYPT_VERSION 351 /* passwords are encrypted */ +#define RELEASE_VERSION CRYPT_VERSION /* current datafile */ #define MIN_VERSION INTPAK_VERSION /* minimal datafile we support */ #define MAX_VERSION RELEASE_VERSION /* change this if we can need to read the future datafile, and we can do so */ diff --git a/src/laws.c b/src/laws.c index a78a3371f..9987714b7 100755 --- a/src/laws.c +++ b/src/laws.c @@ -79,6 +79,7 @@ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. #include #include #include +#include #include #include #include @@ -2169,7 +2170,7 @@ int password_cmd(unit * u, struct order *ord) cmistake(u, ord, 283, MSG_EVENT); strlcpy(pwbuf, itoa36(rng_int()), sizeof(pwbuf)); } - faction_setpassword(u->faction, pwbuf); + faction_setpassword(u->faction, password_hash(pwbuf)); ADDMSG(&u->faction->msgs, msg_message("changepasswd", "value", pwbuf)); return 0; diff --git a/src/util/CMakeLists.txt b/src/util/CMakeLists.txt index 249b1c61c..a441bcfe7 100644 --- a/src/util/CMakeLists.txt +++ b/src/util/CMakeLists.txt @@ -3,6 +3,7 @@ project(util C) SET(_TEST_FILES base36.test.c parser.test.c +password.test.c attrib.test.c strings.test.c bsdstring.test.c @@ -28,6 +29,7 @@ log.c message.c nrmessage.c parser.c +password.c rand.c resolve.c strings.c diff --git a/src/util/password.c b/src/util/password.c new file mode 100644 index 000000000..da05480f6 --- /dev/null +++ b/src/util/password.c @@ -0,0 +1,35 @@ +#include +#include "password.h" + +#include +#include +#include + +#define PASSWORD_PLAIN 0 +#define PASSWORD_MD5 1 +#define PASSWORD_SHA256 2 +#define PASSWORD_BCRYPT 3 // not implemented +#define PASSWORD_DEFAULT PASSWORD_PLAIN + +static const char * password_hash_i(const char * passwd, const char *salt, int algo) { + static char result[64]; // TODO: static result buffers are bad mojo! + assert(passwd); + assert(salt); + if (algo==PASSWORD_PLAIN) { + _snprintf(result, sizeof(result), "$0$%s$%s", salt, passwd); + } + else { + return NULL; + } + return result; +} + +const char * password_hash(const char * passwd) { + return password_hash_i(passwd, "saltyfish", PASSWORD_DEFAULT); +} + +int password_verify(const char * hash, const char * passwd) { + assert(hash); + assert(passwd); + return VERIFY_UNKNOWN; +} diff --git a/src/util/password.h b/src/util/password.h new file mode 100644 index 000000000..70d2da00a --- /dev/null +++ b/src/util/password.h @@ -0,0 +1,7 @@ +#pragma once + +#define VERIFY_OK 0 // password matches hash +#define VERIFY_FAIL 1 // password is wrong +#define VERIFY_UNKNOWN 2 // hashing algorithm not supported +int password_verify(const char * hash, const char * passwd); +const char * password_hash(const char * passwd); diff --git a/src/util/password.test.c b/src/util/password.test.c new file mode 100644 index 000000000..7206fd908 --- /dev/null +++ b/src/util/password.test.c @@ -0,0 +1,19 @@ +#include +#include "password.h" + +static void test_passwords(CuTest *tc) { + const char *hash; + + hash = password_hash("password"); + CuAssertPtrNotNull(tc, hash); + CuAssertStrEquals(tc, "$0$saltyfish$password", hash); + CuAssertIntEquals(tc, VERIFY_OK, password_verify(hash, "password")); + CuAssertIntEquals(tc, VERIFY_FAIL, password_verify(hash, "arseword")); + CuAssertIntEquals(tc, VERIFY_UNKNOWN, password_verify("$9$saltyfish$password", "arseword")); +} + +CuSuite *get_password_suite(void) { + CuSuite *suite = CuSuiteNew(); + SUITE_ADD_TEST(suite, test_passwords); + return suite; +} From 58b0ad19d047f67624d8b617b3a694b5c1c29ab8 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Wed, 13 Jan 2016 00:06:06 +0100 Subject: [PATCH 06/19] NULL needs a platform-dependent include --- src/util/password.test.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/util/password.test.c b/src/util/password.test.c index 7206fd908..bc8017ab2 100644 --- a/src/util/password.test.c +++ b/src/util/password.test.c @@ -1,3 +1,4 @@ +#include #include #include "password.h" From f14ee2adaa88144a543bc0b9921a3e1dfad3b342 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Wed, 13 Jan 2016 12:37:07 +0100 Subject: [PATCH 07/19] include the password module in the list of unit tests --- src/test_eressea.c | 1 + src/util/password.test.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test_eressea.c b/src/test_eressea.c index f3d4b71ae..1053306c6 100644 --- a/src/test_eressea.c +++ b/src/test_eressea.c @@ -80,6 +80,7 @@ int RunAllTests(int argc, char *argv[]) ADD_SUITE(bsdstring); ADD_SUITE(functions); ADD_SUITE(parser); + ADD_SUITE(password); ADD_SUITE(umlaut); ADD_SUITE(unicode); ADD_SUITE(strings); diff --git a/src/util/password.test.c b/src/util/password.test.c index 7206fd908..c7c8b54a3 100644 --- a/src/util/password.test.c +++ b/src/util/password.test.c @@ -9,7 +9,7 @@ static void test_passwords(CuTest *tc) { CuAssertStrEquals(tc, "$0$saltyfish$password", hash); CuAssertIntEquals(tc, VERIFY_OK, password_verify(hash, "password")); CuAssertIntEquals(tc, VERIFY_FAIL, password_verify(hash, "arseword")); - CuAssertIntEquals(tc, VERIFY_UNKNOWN, password_verify("$9$saltyfish$password", "arseword")); + CuAssertIntEquals(tc, VERIFY_UNKNOWN, password_verify("$9$saltyfish$password", "password")); } CuSuite *get_password_suite(void) { From 799514bf40951323b62eb3ca737bc884ccda2531 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Wed, 13 Jan 2016 14:41:09 +0100 Subject: [PATCH 08/19] implement md5 password hashing (untested) disable test for case-insensitive password test fix faction/checkpasswd test --- src/kernel/faction.test.c | 2 +- src/util/password.c | 48 +++++++++++++++++++++++++++++++++++---- 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/src/kernel/faction.test.c b/src/kernel/faction.test.c index 14e3e5ac0..b249edaeb 100644 --- a/src/kernel/faction.test.c +++ b/src/kernel/faction.test.c @@ -126,8 +126,8 @@ static void test_check_passwd(CuTest *tc) { f = test_create_faction(0); faction_setpassword(f, password_hash("password")); CuAssertIntEquals(tc, true, checkpasswd(f, "password")); - CuAssertIntEquals(tc, true, checkpasswd(f, "PASSWORD")); CuAssertIntEquals(tc, false, checkpasswd(f, "assword")); + CuAssertIntEquals(tc, false, checkpasswd(f, "PASSWORD")); } static void test_get_monsters(CuTest *tc) { diff --git a/src/util/password.c b/src/util/password.c index da05480f6..e4b36fb6d 100644 --- a/src/util/password.c +++ b/src/util/password.c @@ -1,9 +1,12 @@ #include #include "password.h" +#include + #include #include #include +#include #define PASSWORD_PLAIN 0 #define PASSWORD_MD5 1 @@ -11,6 +14,8 @@ #define PASSWORD_BCRYPT 3 // not implemented #define PASSWORD_DEFAULT PASSWORD_PLAIN +#define MAXSALTLEN 32 // maximum length in characters of any salt + static const char * password_hash_i(const char * passwd, const char *salt, int algo) { static char result[64]; // TODO: static result buffers are bad mojo! assert(passwd); @@ -18,6 +23,15 @@ static const char * password_hash_i(const char * passwd, const char *salt, int a if (algo==PASSWORD_PLAIN) { _snprintf(result, sizeof(result), "$0$%s$%s", salt, passwd); } + else if (algo == PASSWORD_MD5) { + md5_state_t ms; + md5_byte_t digest[16]; + md5_init(&ms); + md5_append(&ms, (const md5_byte_t *)passwd, strlen(passwd)); + md5_append(&ms, (const md5_byte_t *)salt, strlen(salt)); + md5_finish(&ms, digest); + _snprintf(result, sizeof(result), "$1$%s$%s", salt, digest); // FIXME: need to build a hex string first! + } else { return NULL; } @@ -28,8 +42,34 @@ const char * password_hash(const char * passwd) { return password_hash_i(passwd, "saltyfish", PASSWORD_DEFAULT); } -int password_verify(const char * hash, const char * passwd) { - assert(hash); - assert(passwd); - return VERIFY_UNKNOWN; +static bool password_is_implemented(int algo) { + return algo==PASSWORD_PLAIN; +} + +int password_verify(const char * pwhash, const char * passwd) { + char salt[MAXSALTLEN+1]; + size_t len; + int algo; + char *pos; + const char *dol, *hash; + assert(pwhash); + assert(passwd); + assert(pwhash[0] == '$'); + algo = (int)strtol(pwhash + 1, &pos, 16); + assert(pos[0] == '$'); + ++pos; + dol = strchr(pos, '$'); + assert(dol>pos && dol[0] == '$'); + len = dol - pos; + assert(len <= MAXSALTLEN); + strncpy(salt, pos, len); + salt[len] = 0; + hash = password_hash_i(passwd, salt, algo); + if (!password_is_implemented(algo)) { + return VERIFY_UNKNOWN; + } + if (strcmp(pwhash, hash) == 0) { + return VERIFY_OK; + } + return VERIFY_FAIL; } From c3da0cd42d6eca4680969f9b30e6cbf5e2314a26 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Wed, 13 Jan 2016 16:16:02 +0100 Subject: [PATCH 09/19] as long as we use no encryption, all tests pass. --- src/bind_faction.c | 2 +- src/kernel/faction.c | 2 +- src/kernel/faction.test.c | 2 +- src/kernel/save.c | 2 +- src/laws.c | 2 +- src/util/password.c | 30 +++++++++++++----------------- src/util/password.h | 10 +++++++++- src/util/password.test.c | 2 +- 8 files changed, 28 insertions(+), 24 deletions(-) diff --git a/src/bind_faction.c b/src/bind_faction.c index 264e869ed..638a92bf6 100644 --- a/src/bind_faction.c +++ b/src/bind_faction.c @@ -386,7 +386,7 @@ static int tolua_faction_set_password(lua_State * L) { faction *self = (faction *)tolua_tousertype(L, 1, 0); const char * passw = tolua_tostring(L, 2, 0); - faction_setpassword(self, password_hash(passw)); + faction_setpassword(self, password_hash(passw, PASSWORD_DEFAULT)); return 0; } diff --git a/src/kernel/faction.c b/src/kernel/faction.c index 1d70bc867..ea0ee7551 100755 --- a/src/kernel/faction.c +++ b/src/kernel/faction.c @@ -253,7 +253,7 @@ faction *addfaction(const char *email, const char *password, } if (!password) password = itoa36(rng_int()); - faction_setpassword(f, password_hash(password)); + faction_setpassword(f, password_hash(password, PASSWORD_DEFAULT)); ADDMSG(&f->msgs, msg_message("changepasswd", "value", password)); f->alliance_joindate = turn; diff --git a/src/kernel/faction.test.c b/src/kernel/faction.test.c index b249edaeb..bd864c309 100644 --- a/src/kernel/faction.test.c +++ b/src/kernel/faction.test.c @@ -124,7 +124,7 @@ static void test_check_passwd(CuTest *tc) { faction *f; f = test_create_faction(0); - faction_setpassword(f, password_hash("password")); + faction_setpassword(f, password_hash("password", PASSWORD_DEFAULT)); CuAssertIntEquals(tc, true, checkpasswd(f, "password")); CuAssertIntEquals(tc, false, checkpasswd(f, "assword")); CuAssertIntEquals(tc, false, checkpasswd(f, "PASSWORD")); diff --git a/src/kernel/save.c b/src/kernel/save.c index 609f9e5b8..792b70779 100644 --- a/src/kernel/save.c +++ b/src/kernel/save.c @@ -1217,7 +1217,7 @@ faction *readfaction(struct gamedata * data) } READ_STR(data->store, name, sizeof(name)); - faction_setpassword(f, (data->version >= CRYPT_VERSION) ? name : password_hash(name)); + faction_setpassword(f, (data->version >= CRYPT_VERSION) ? name : password_hash(name, PASSWORD_DEFAULT)); if (data->version < NOOVERRIDE_VERSION) { READ_STR(data->store, 0, 0); } diff --git a/src/laws.c b/src/laws.c index 9987714b7..212a56f55 100755 --- a/src/laws.c +++ b/src/laws.c @@ -2170,7 +2170,7 @@ int password_cmd(unit * u, struct order *ord) cmistake(u, ord, 283, MSG_EVENT); strlcpy(pwbuf, itoa36(rng_int()), sizeof(pwbuf)); } - faction_setpassword(u->faction, password_hash(pwbuf)); + faction_setpassword(u->faction, password_hash(pwbuf, PASSWORD_DEFAULT)); ADDMSG(&u->faction->msgs, msg_message("changepasswd", "value", pwbuf)); return 0; diff --git a/src/util/password.c b/src/util/password.c index e4b36fb6d..2d6825401 100644 --- a/src/util/password.c +++ b/src/util/password.c @@ -6,22 +6,14 @@ #include #include #include -#include - -#define PASSWORD_PLAIN 0 -#define PASSWORD_MD5 1 -#define PASSWORD_SHA256 2 -#define PASSWORD_BCRYPT 3 // not implemented -#define PASSWORD_DEFAULT PASSWORD_PLAIN #define MAXSALTLEN 32 // maximum length in characters of any salt -static const char * password_hash_i(const char * passwd, const char *salt, int algo) { - static char result[64]; // TODO: static result buffers are bad mojo! +static const char * password_hash_i(const char * passwd, const char *salt, int algo, char *result, size_t len) { assert(passwd); assert(salt); if (algo==PASSWORD_PLAIN) { - _snprintf(result, sizeof(result), "$0$%s$%s", salt, passwd); + _snprintf(result, len, "$0$%s$%s", salt, passwd); } else if (algo == PASSWORD_MD5) { md5_state_t ms; @@ -30,7 +22,7 @@ static const char * password_hash_i(const char * passwd, const char *salt, int a md5_append(&ms, (const md5_byte_t *)passwd, strlen(passwd)); md5_append(&ms, (const md5_byte_t *)salt, strlen(salt)); md5_finish(&ms, digest); - _snprintf(result, sizeof(result), "$1$%s$%s", salt, digest); // FIXME: need to build a hex string first! + _snprintf(result, len, "$1$%s$%s", salt, digest); // FIXME: need to build a hex string first! } else { return NULL; @@ -38,24 +30,28 @@ static const char * password_hash_i(const char * passwd, const char *salt, int a return result; } -const char * password_hash(const char * passwd) { - return password_hash_i(passwd, "saltyfish", PASSWORD_DEFAULT); +const char * password_hash(const char * passwd, int algo) { + static char result[64]; // TODO: static result buffers are bad mojo! + if (algo < 0) algo = PASSWORD_DEFAULT; + return password_hash_i(passwd, "saltyfish", PASSWORD_DEFAULT, result, sizeof(result)); } static bool password_is_implemented(int algo) { - return algo==PASSWORD_PLAIN; + return algo==PASSWORD_PLAIN || algo==PASSWORD_MD5; } int password_verify(const char * pwhash, const char * passwd) { char salt[MAXSALTLEN+1]; + char hash[64]; size_t len; int algo; char *pos; - const char *dol, *hash; + const char *dol; assert(pwhash); assert(passwd); assert(pwhash[0] == '$'); - algo = (int)strtol(pwhash + 1, &pos, 16); + algo = pwhash[1] - '0'; + pos = strchr(pwhash+2, '$'); assert(pos[0] == '$'); ++pos; dol = strchr(pos, '$'); @@ -64,7 +60,7 @@ int password_verify(const char * pwhash, const char * passwd) { assert(len <= MAXSALTLEN); strncpy(salt, pos, len); salt[len] = 0; - hash = password_hash_i(passwd, salt, algo); + password_hash_i(passwd, salt, algo, hash, sizeof(hash)); if (!password_is_implemented(algo)) { return VERIFY_UNKNOWN; } diff --git a/src/util/password.h b/src/util/password.h index 70d2da00a..aed554163 100644 --- a/src/util/password.h +++ b/src/util/password.h @@ -1,7 +1,15 @@ #pragma once +#define PASSWORD_PLAIN 0 +#define PASSWORD_MD5 1 +#define PASSWORD_BCRYPT 2 // not implemented +#define PASSWORD_SHA256 5 // not implemented +#define PASSWORD_SHA512 6 // not implemented +#define PASSWORD_DEFAULT PASSWORD_PLAIN + + #define VERIFY_OK 0 // password matches hash #define VERIFY_FAIL 1 // password is wrong #define VERIFY_UNKNOWN 2 // hashing algorithm not supported int password_verify(const char * hash, const char * passwd); -const char * password_hash(const char * passwd); +const char * password_hash(const char * passwd, int algo); diff --git a/src/util/password.test.c b/src/util/password.test.c index 9f22a9853..4dfaa6a1d 100644 --- a/src/util/password.test.c +++ b/src/util/password.test.c @@ -5,7 +5,7 @@ static void test_passwords(CuTest *tc) { const char *hash; - hash = password_hash("password"); + hash = password_hash("password", PASSWORD_PLAIN); CuAssertPtrNotNull(tc, hash); CuAssertStrEquals(tc, "$0$saltyfish$password", hash); CuAssertIntEquals(tc, VERIFY_OK, password_verify(hash, "password")); From 3a8a05380beb55cea4b979e6366e7512edf349e2 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Wed, 13 Jan 2016 16:19:59 +0100 Subject: [PATCH 10/19] fix gcc compilation warnings --- src/util/password.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/password.c b/src/util/password.c index 2d6825401..07894c223 100644 --- a/src/util/password.c +++ b/src/util/password.c @@ -19,8 +19,8 @@ static const char * password_hash_i(const char * passwd, const char *salt, int a md5_state_t ms; md5_byte_t digest[16]; md5_init(&ms); - md5_append(&ms, (const md5_byte_t *)passwd, strlen(passwd)); - md5_append(&ms, (const md5_byte_t *)salt, strlen(salt)); + md5_append(&ms, (const md5_byte_t *)passwd, (int)strlen(passwd)); + md5_append(&ms, (const md5_byte_t *)salt, (int)strlen(salt)); md5_finish(&ms, digest); _snprintf(result, len, "$1$%s$%s", salt, digest); // FIXME: need to build a hex string first! } From 8d05f4cc25b7e53afaba510c7834de564f9a9b6c Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Thu, 14 Jan 2016 15:49:09 +0100 Subject: [PATCH 11/19] implement MD5 crypted passwords as default --- crypto | 2 +- src/bind_faction.c | 2 +- src/kernel/faction.c | 2 +- src/kernel/faction.test.c | 2 +- src/kernel/save.c | 2 +- src/laws.c | 2 +- src/util/password.c | 23 ++++++++++------------- src/util/password.h | 6 +++--- src/util/password.test.c | 9 +++++++-- 9 files changed, 26 insertions(+), 24 deletions(-) diff --git a/crypto b/crypto index 166fdc8c1..c2a682476 160000 --- a/crypto +++ b/crypto @@ -1 +1 @@ -Subproject commit 166fdc8c146755055217070c58079ba9a7c03369 +Subproject commit c2a682476a96cdff972ac2b64051f61edf76064e diff --git a/src/bind_faction.c b/src/bind_faction.c index 638a92bf6..eb535d50b 100644 --- a/src/bind_faction.c +++ b/src/bind_faction.c @@ -386,7 +386,7 @@ static int tolua_faction_set_password(lua_State * L) { faction *self = (faction *)tolua_tousertype(L, 1, 0); const char * passw = tolua_tostring(L, 2, 0); - faction_setpassword(self, password_hash(passw, PASSWORD_DEFAULT)); + faction_setpassword(self, password_hash(passw, 0, PASSWORD_DEFAULT)); return 0; } diff --git a/src/kernel/faction.c b/src/kernel/faction.c index ea0ee7551..ea12c315f 100755 --- a/src/kernel/faction.c +++ b/src/kernel/faction.c @@ -253,7 +253,7 @@ faction *addfaction(const char *email, const char *password, } if (!password) password = itoa36(rng_int()); - faction_setpassword(f, password_hash(password, PASSWORD_DEFAULT)); + faction_setpassword(f, password_hash(password, 0, PASSWORD_DEFAULT)); ADDMSG(&f->msgs, msg_message("changepasswd", "value", password)); f->alliance_joindate = turn; diff --git a/src/kernel/faction.test.c b/src/kernel/faction.test.c index bd864c309..678aaa068 100644 --- a/src/kernel/faction.test.c +++ b/src/kernel/faction.test.c @@ -124,7 +124,7 @@ static void test_check_passwd(CuTest *tc) { faction *f; f = test_create_faction(0); - faction_setpassword(f, password_hash("password", PASSWORD_DEFAULT)); + faction_setpassword(f, password_hash("password", 0, PASSWORD_DEFAULT)); CuAssertIntEquals(tc, true, checkpasswd(f, "password")); CuAssertIntEquals(tc, false, checkpasswd(f, "assword")); CuAssertIntEquals(tc, false, checkpasswd(f, "PASSWORD")); diff --git a/src/kernel/save.c b/src/kernel/save.c index 792b70779..35062c44d 100644 --- a/src/kernel/save.c +++ b/src/kernel/save.c @@ -1217,7 +1217,7 @@ faction *readfaction(struct gamedata * data) } READ_STR(data->store, name, sizeof(name)); - faction_setpassword(f, (data->version >= CRYPT_VERSION) ? name : password_hash(name, PASSWORD_DEFAULT)); + faction_setpassword(f, (data->version >= CRYPT_VERSION) ? name : password_hash(name, 0, PASSWORD_DEFAULT)); if (data->version < NOOVERRIDE_VERSION) { READ_STR(data->store, 0, 0); } diff --git a/src/laws.c b/src/laws.c index 212a56f55..adaa4a6cc 100755 --- a/src/laws.c +++ b/src/laws.c @@ -2170,7 +2170,7 @@ int password_cmd(unit * u, struct order *ord) cmistake(u, ord, 283, MSG_EVENT); strlcpy(pwbuf, itoa36(rng_int()), sizeof(pwbuf)); } - faction_setpassword(u->faction, password_hash(pwbuf, PASSWORD_DEFAULT)); + faction_setpassword(u->faction, password_hash(pwbuf, 0, PASSWORD_DEFAULT)); ADDMSG(&u->faction->msgs, msg_message("changepasswd", "value", pwbuf)); return 0; diff --git a/src/util/password.c b/src/util/password.c index 07894c223..a2764bb63 100644 --- a/src/util/password.c +++ b/src/util/password.c @@ -16,13 +16,9 @@ static const char * password_hash_i(const char * passwd, const char *salt, int a _snprintf(result, len, "$0$%s$%s", salt, passwd); } else if (algo == PASSWORD_MD5) { - md5_state_t ms; - md5_byte_t digest[16]; - md5_init(&ms); - md5_append(&ms, (const md5_byte_t *)passwd, (int)strlen(passwd)); - md5_append(&ms, (const md5_byte_t *)salt, (int)strlen(salt)); - md5_finish(&ms, digest); - _snprintf(result, len, "$1$%s$%s", salt, digest); // FIXME: need to build a hex string first! + char * result = md5_crypt(passwd, salt); + return result; +// _snprintf(result, len, "$1$%s$%s", salt, digest); // FIXME: need to build a hex string first! } else { return NULL; @@ -30,10 +26,11 @@ static const char * password_hash_i(const char * passwd, const char *salt, int a return result; } -const char * password_hash(const char * passwd, int algo) { +const char * password_hash(const char * passwd, const char * salt, int algo) { static char result[64]; // TODO: static result buffers are bad mojo! + if (!salt) salt = "saltyass"; // FIXME: generate a secure salt! if (algo < 0) algo = PASSWORD_DEFAULT; - return password_hash_i(passwd, "saltyfish", PASSWORD_DEFAULT, result, sizeof(result)); + return password_hash_i(passwd, salt, algo, result, sizeof(result)); } static bool password_is_implemented(int algo) { @@ -46,9 +43,9 @@ int password_verify(const char * pwhash, const char * passwd) { size_t len; int algo; char *pos; - const char *dol; - assert(pwhash); + const char *dol, *result; assert(passwd); + assert(pwhash); assert(pwhash[0] == '$'); algo = pwhash[1] - '0'; pos = strchr(pwhash+2, '$'); @@ -60,11 +57,11 @@ int password_verify(const char * pwhash, const char * passwd) { assert(len <= MAXSALTLEN); strncpy(salt, pos, len); salt[len] = 0; - password_hash_i(passwd, salt, algo, hash, sizeof(hash)); + result = password_hash_i(passwd, salt, algo, hash, sizeof(hash)); if (!password_is_implemented(algo)) { return VERIFY_UNKNOWN; } - if (strcmp(pwhash, hash) == 0) { + if (strcmp(pwhash, result) == 0) { return VERIFY_OK; } return VERIFY_FAIL; diff --git a/src/util/password.h b/src/util/password.h index aed554163..0f8d23ec3 100644 --- a/src/util/password.h +++ b/src/util/password.h @@ -5,11 +5,11 @@ #define PASSWORD_BCRYPT 2 // not implemented #define PASSWORD_SHA256 5 // not implemented #define PASSWORD_SHA512 6 // not implemented -#define PASSWORD_DEFAULT PASSWORD_PLAIN +#define PASSWORD_DEFAULT PASSWORD_MD5 #define VERIFY_OK 0 // password matches hash #define VERIFY_FAIL 1 // password is wrong #define VERIFY_UNKNOWN 2 // hashing algorithm not supported -int password_verify(const char * hash, const char * passwd); -const char * password_hash(const char * passwd, int algo); +int password_verify(const char *hash, const char *passwd); +const char * password_hash(const char *passwd, const char *salt, int algo); diff --git a/src/util/password.test.c b/src/util/password.test.c index 4dfaa6a1d..bd7a75cb7 100644 --- a/src/util/password.test.c +++ b/src/util/password.test.c @@ -5,9 +5,14 @@ static void test_passwords(CuTest *tc) { const char *hash; - hash = password_hash("password", PASSWORD_PLAIN); + hash = password_hash("jollygood", "ZouUn04i", PASSWORD_MD5); CuAssertPtrNotNull(tc, hash); - CuAssertStrEquals(tc, "$0$saltyfish$password", hash); + CuAssertStrEquals(tc, "$1$ZouUn04i$yNnT1Oy8azJ5V.UM9ppP5/", hash); + CuAssertIntEquals(tc, VERIFY_OK, password_verify(hash, "jollygood")); + + hash = password_hash("password", "hodor", PASSWORD_PLAIN); + CuAssertPtrNotNull(tc, hash); + CuAssertStrEquals(tc, "$0$hodor$password", hash); CuAssertIntEquals(tc, VERIFY_OK, password_verify(hash, "password")); CuAssertIntEquals(tc, VERIFY_FAIL, password_verify(hash, "arseword")); CuAssertIntEquals(tc, VERIFY_UNKNOWN, password_verify("$9$saltyfish$password", "password")); From daf7420e4b609c2c14365f446f1d7e0723370f7f Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Thu, 14 Jan 2016 15:53:48 +0100 Subject: [PATCH 12/19] fix gcc build --- crypto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crypto b/crypto index c2a682476..160120683 160000 --- a/crypto +++ b/crypto @@ -1 +1 @@ -Subproject commit c2a682476a96cdff972ac2b64051f61edf76064e +Subproject commit 160120683f43c0c4f3cbeb040e5aa678dcfa8b37 From 09be723c016673194dc8f9829c1f0e3f9742a0a2 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Thu, 14 Jan 2016 15:55:53 +0100 Subject: [PATCH 13/19] drepper code fixes --- crypto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crypto b/crypto index 160120683..3aaa4ca1b 160000 --- a/crypto +++ b/crypto @@ -1 +1 @@ -Subproject commit 160120683f43c0c4f3cbeb040e5aa678dcfa8b37 +Subproject commit 3aaa4ca1b8622c959540688d02d8c5dce14682a5 From 6c80bc52b5a6e0243eb15fb919e0acdbd378d97f Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Thu, 14 Jan 2016 16:50:42 +0100 Subject: [PATCH 14/19] generate a good-ish salt --- src/util/password.c | 37 ++++++++++++++++++++++++++++++++++--- 1 file changed, 34 insertions(+), 3 deletions(-) diff --git a/src/util/password.c b/src/util/password.c index a2764bb63..f11396104 100644 --- a/src/util/password.c +++ b/src/util/password.c @@ -2,23 +2,55 @@ #include "password.h" #include +#include #include #include #include #define MAXSALTLEN 32 // maximum length in characters of any salt +#define SALTLEN 8 // length of salts we generate + +/* Table with characters for base64 transformation. */ +static const char b64t[65] = +"./0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"; + +#define b64_from_24bit(B2, B1, B0, N) \ + do { \ + unsigned int w = ((B2) << 16) | ((B1) << 8) | (B0); \ + int n = (N); \ + while (n-- > 0 && buflen > 0) \ + { \ + *cp++ = b64t[w & 0x3f]; \ + --buflen; \ + w >>= 6; \ + } \ + } while (0) + + +char *password_gensalt(void) { + static char salt[SALTLEN + 1]; + char *cp = salt; + int buflen = SALTLEN; + while (buflen) { + unsigned long ul = genrand_int32(); + b64_from_24bit(ul & 0xFF, (ul>>8)&0xff, (ul>>16)&0xFF, 4); + } + salt[SALTLEN] = 0; + return salt; +} static const char * password_hash_i(const char * passwd, const char *salt, int algo, char *result, size_t len) { assert(passwd); - assert(salt); + if (!salt) { + salt = password_gensalt(); + } if (algo==PASSWORD_PLAIN) { _snprintf(result, len, "$0$%s$%s", salt, passwd); } else if (algo == PASSWORD_MD5) { char * result = md5_crypt(passwd, salt); return result; -// _snprintf(result, len, "$1$%s$%s", salt, digest); // FIXME: need to build a hex string first! } else { return NULL; @@ -28,7 +60,6 @@ static const char * password_hash_i(const char * passwd, const char *salt, int a const char * password_hash(const char * passwd, const char * salt, int algo) { static char result[64]; // TODO: static result buffers are bad mojo! - if (!salt) salt = "saltyass"; // FIXME: generate a secure salt! if (algo < 0) algo = PASSWORD_DEFAULT; return password_hash_i(passwd, salt, algo, result, sizeof(result)); } From 68df8d53b8c57ca70d5e018586ce2aba3764868b Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Thu, 14 Jan 2016 17:24:11 +0100 Subject: [PATCH 15/19] msvc compile fix --- crypto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crypto b/crypto index 3aaa4ca1b..0ce0c5d6e 160000 --- a/crypto +++ b/crypto @@ -1 +1 @@ -Subproject commit 3aaa4ca1b8622c959540688d02d8c5dce14682a5 +Subproject commit 0ce0c5d6e1963b31863216d24d2cc5594f9a8893 From 09a0e806bcc79b5d193f548a880c196267730af1 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Thu, 14 Jan 2016 17:28:20 +0100 Subject: [PATCH 16/19] gcc compilation fix --- crypto | 2 +- src/util/password.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crypto b/crypto index 0ce0c5d6e..f9ecf5a10 160000 --- a/crypto +++ b/crypto @@ -1 +1 @@ -Subproject commit 0ce0c5d6e1963b31863216d24d2cc5594f9a8893 +Subproject commit f9ecf5a10983adfc7bd1bee8ac1f9a3abf1d41d9 diff --git a/src/util/password.c b/src/util/password.c index f11396104..6a2fdd5ad 100644 --- a/src/util/password.c +++ b/src/util/password.c @@ -34,7 +34,7 @@ char *password_gensalt(void) { int buflen = SALTLEN; while (buflen) { unsigned long ul = genrand_int32(); - b64_from_24bit(ul & 0xFF, (ul>>8)&0xff, (ul>>16)&0xFF, 4); + b64_from_24bit((char)(ul & 0xFF), (char)((ul>>8)&0xff), (char)((ul>>16)&0xFF), 4); } salt[SALTLEN] = 0; return salt; From 7cda3f76a31dff69e1c8f431107f30f9cb7db4d8 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Thu, 14 Jan 2016 19:43:11 +0100 Subject: [PATCH 17/19] suppress valgrind report of stpncpy (why, drepper, why?) --- share/ubuntu-12_04.supp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/share/ubuntu-12_04.supp b/share/ubuntu-12_04.supp index 3d6ae2974..504e4ae2d 100644 --- a/share/ubuntu-12_04.supp +++ b/share/ubuntu-12_04.supp @@ -35,3 +35,9 @@ fun:__GI___strncasecmp_l fun:____strtod_l_internal } + +{ + kde-bug-309427 + Memcheck:Cond + fun:__stpncpy_sse2_unaligned +} From 568e1a297665a3070ae0539b1c3ef0d394bbc2cb Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Thu, 14 Jan 2016 21:23:53 +0100 Subject: [PATCH 18/19] prevent null-pointer crash when reading f.password (TODO: write-only variables are dumb) --- src/bind_faction.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/bind_faction.c b/src/bind_faction.c index eb535d50b..03a75281a 100644 --- a/src/bind_faction.c +++ b/src/bind_faction.c @@ -382,6 +382,12 @@ static int tolua_faction_create(lua_State * L) return 1; } +static int tolua_faction_get_password(lua_State * L) +{ + unused_arg(L); + return 0; +} + static int tolua_faction_set_password(lua_State * L) { faction *self = (faction *)tolua_tousertype(L, 1, 0); @@ -557,7 +563,7 @@ void tolua_faction_open(lua_State * L) tolua_variable(L, TOLUA_CAST "heroes", tolua_faction_get_heroes, NULL); tolua_variable(L, TOLUA_CAST "maxheroes", tolua_faction_get_maxheroes, NULL); - tolua_variable(L, TOLUA_CAST "password", NULL, + tolua_variable(L, TOLUA_CAST "password", tolua_faction_get_password, tolua_faction_set_password); tolua_variable(L, TOLUA_CAST "email", tolua_faction_get_email, tolua_faction_set_email); From 76475b9bf70546efbc98f37b3db9a71e54c1246d Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Fri, 15 Jan 2016 08:01:12 +0100 Subject: [PATCH 19/19] add a little extra randomness to the seed salt (still bad). remove dead prototype code from faction.c --- src/kernel/faction.c | 13 ------------- src/util/password.c | 3 ++- 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/src/kernel/faction.c b/src/kernel/faction.c index ea12c315f..cf1372950 100755 --- a/src/kernel/faction.c +++ b/src/kernel/faction.c @@ -63,7 +63,6 @@ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. #include #include #include -#include faction *factions; @@ -314,18 +313,6 @@ unit *addplayer(region * r, faction * f) return u; } -extern char *sha256_crypt(const char *key, const char *salt); - -const char * mksalt(char *salt, size_t len) { - char *dst = salt; - int ent = (int)time(0); - // FIXME: worst ever salt generation - while (dst < salt + len) { - *dst++ = itoa36(ent & rng_int())[0]; - } - return salt; -} - bool checkpasswd(const faction * f, const char *passwd) { if (!passwd) return false; diff --git a/src/util/password.c b/src/util/password.c index 6a2fdd5ad..bd8c98e03 100644 --- a/src/util/password.c +++ b/src/util/password.c @@ -7,6 +7,7 @@ #include #include #include +#include #define MAXSALTLEN 32 // maximum length in characters of any salt #define SALTLEN 8 // length of salts we generate @@ -33,7 +34,7 @@ char *password_gensalt(void) { char *cp = salt; int buflen = SALTLEN; while (buflen) { - unsigned long ul = genrand_int32(); + unsigned long ul = genrand_int32() & time(0); b64_from_24bit((char)(ul & 0xFF), (char)((ul>>8)&0xff), (char)((ul>>16)&0xFF), 4); } salt[SALTLEN] = 0;