Skip to content

Upgrade password hashes on authentication #152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
KlausTrainer wants to merge 2 commits into from
Closed

Upgrade password hashes on authentication #152

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c10361e
Remove client-side password crypto from JS tests
KlausTrainer Feb 19, 2014
266725e
Upgrade password hashes on authentication
KlausTrainer Feb 19, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions share/www/script/couch_test_runner.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -460,9 +460,7 @@ CouchDB.user_prefix = "org.couchdb.user:";
CouchDB.prepareUserDoc = function(user_doc, new_password) {
user_doc._id = user_doc._id || CouchDB.user_prefix + user_doc.name;
if (new_password) {
// handle the password crypto
user_doc.salt = CouchDB.newUuids(1)[0];
user_doc.password_sha = hex_sha1(new_password + user_doc.salt);
user_doc.password = new_password;
}
user_doc.type = "user";
if (!user_doc.roles) {
Expand Down
12 changes: 2 additions & 10 deletions share/www/script/test/auth_cache.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,11 +184,7 @@ couchTests.auth_cache = function(debug) {
hits_before = hits_after;
misses_before = misses_after;

var new_salt = CouchDB.newUuids(1)[0];
var new_passwd = hex_sha1("foobar" + new_salt);
fdmanana.salt = new_salt;
fdmanana.password_sha = new_passwd;

fdmanana.password = "foobar";
T(authDb.save(fdmanana).ok);

// cache was refreshed
Expand All @@ -206,11 +202,7 @@ couchTests.auth_cache = function(debug) {
misses_before = misses_after;

// and yet another update
new_salt = CouchDB.newUuids(1)[0];
new_passwd = hex_sha1("javascript" + new_salt);
fdmanana.salt = new_salt;
fdmanana.password_sha = new_passwd;

fdmanana.password = "javascript";
T(authDb.save(fdmanana).ok);

// cache was refreshed
Expand Down
11 changes: 5 additions & 6 deletions share/www/script/test/cookie_auth.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ couchTests.cookie_auth = function(debug) {

// we can't create docs with malformed ids
var badIdDoc = CouchDB.prepareUserDoc({
name: "foo"
name: "w00x"
}, "bar");

badIdDoc._id = "org.apache.couchdb:w00x";
Expand Down Expand Up @@ -153,8 +153,8 @@ couchTests.cookie_auth = function(debug) {
usersDb.deleteDoc(jchrisUserDoc);
T(false && "Can't delete other users docs. Should have thrown an error.");
} catch (e) {
TEquals("forbidden", e.error);
TEquals(403, usersDb.last_req.status);
TEquals("not_found", e.error);
TEquals(404, usersDb.last_req.status);
}

// TODO should login() throw an exception here?
Expand Down Expand Up @@ -197,8 +197,8 @@ couchTests.cookie_auth = function(debug) {
usersDb.save(jasonUserDoc);
T(false && "Can't update someone else's user doc. Should have thrown an error.");
} catch (e) {
T(e.error == "forbidden");
T(usersDb.last_req.status == 403);
T(e.error == "not_found");
T(usersDb.last_req.status == 404);
}

// test that you can't edit roles unless you are admin
Expand Down Expand Up @@ -272,7 +272,6 @@ couchTests.cookie_auth = function(debug) {

var usersDb = new CouchDB("test_suite_users", {"X-Couch-Full-Commit":"false"});
usersDb.deleteDb();
usersDb.createDb();

run_on_modified_server(
[
Expand Down
61 changes: 59 additions & 2 deletions share/www/script/test/users_db_security.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,63 @@ couchTests.users_db_security = function(debug) {
TEquals(true, userDoc.derived_key != jchrisDoc.derived_key,
"should have new derived_key");

// SHA-1 password hashes are upgraded to PBKDF2 on successful
// authentication
var rnewsonDoc = {
_id: "org.couchdb.user:rnewson",
type: "user",
name: "rnewson",
// password: "plaintext_password",
password_sha: "e29dc3aeed5abf43185c33e479f8998558c59474",
salt: "24f1e0a87c2e374212bda1073107e8ae",
roles: []
};

var password_sha = rnewsonDoc.password_sha,
salt = rnewsonDoc.salt,
derived_key,
iterations;

usersDb.save(rnewsonDoc);
rnewsonDoc = open_as(usersDb, rnewsonDoc._id, "jan");
T(!rnewsonDoc.password_scheme);
T(!rnewsonDoc.derived_key);
T(!rnewsonDoc.iterations);

// check that we don't upgrade when the password is wrong
TEquals("unauthorized", CouchDB.login("rnewson", "wrong_password").error);
rnewsonDoc = open_as(usersDb, rnewsonDoc._id, "jan");
TEquals(salt, rnewsonDoc.salt);
TEquals(password_sha, rnewsonDoc.password_sha);
T(!rnewsonDoc.password_scheme);
T(!rnewsonDoc.derived_key);
T(!rnewsonDoc.iterations);

TEquals(true, CouchDB.login("rnewson", "plaintext_password").ok);
rnewsonDoc = usersDb.open(rnewsonDoc._id);
TEquals("pbkdf2", rnewsonDoc.password_scheme);
T(rnewsonDoc.salt != salt);
T(!rnewsonDoc.password_sha);
T(rnewsonDoc.derived_key);
T(rnewsonDoc.iterations);

salt = rnewsonDoc.salt,
derived_key = rnewsonDoc.derived_key,
iterations = rnewsonDoc.iterations;

// check that authentication is still working
// and everything is staying the same now
CouchDB.logout();
TEquals(true, CouchDB.login("rnewson", "plaintext_password").ok);
rnewsonDoc = usersDb.open(rnewsonDoc._id);
TEquals("pbkdf2", rnewsonDoc.password_scheme);
TEquals(salt, rnewsonDoc.salt);
T(!rnewsonDoc.password_sha);
TEquals(derived_key, rnewsonDoc.derived_key);
TEquals(iterations, rnewsonDoc.iterations);

CouchDB.logout();

// user should not be able to read another user's user document
var fdmananaDoc = {
_id: "org.couchdb.user:fdmanana",
Expand Down Expand Up @@ -209,11 +266,11 @@ couchTests.users_db_security = function(debug) {

// admin should be able to read from any view
var result = view_as(usersDb, "user_db_auth/test", "jan");
TEquals(3, result.total_rows, "should allow access and list two users to admin");
TEquals(4, result.total_rows, "should allow access and list four users to admin");

// db admin should be able to read from any view
var result = view_as(usersDb, "user_db_auth/test", "benoitc");
TEquals(3, result.total_rows, "should allow access and list two users to db admin");
TEquals(4, result.total_rows, "should allow access and list four users to db admin");


// non-admins can't read design docs
Expand Down
35 changes: 27 additions & 8 deletions src/couchdb/couch_httpd_auth.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,14 @@ default_authentication_handler(Req) ->
nil ->
throw({unauthorized, <<"Name or password is incorrect.">>});
UserProps ->
case authenticate(?l2b(Pass), UserProps) of
UserName = ?l2b(User),
Password = ?l2b(Pass),
case authenticate(Password, UserProps) of
true ->
UserProps2 = maybe_upgrade_password_hash(UserName, Password, UserProps),
Req#httpd{user_ctx=#user_ctx{
Copy link
Copy Markdown
Member

@rnewson rnewson Feb 19, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the maybe_upgrade should happen here, in the true clause.

Copy link
Copy Markdown
Contributor Author

@KlausTrainer KlausTrainer Feb 19, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, thanks. I got that :)

name=?l2b(User),
roles=couch_util:get_value(<<"roles">>, UserProps, [])
name=UserName,
roles=couch_util:get_value(<<"roles">>, UserProps2, [])
}};
_Else ->
throw({unauthorized, <<"Name or password is incorrect.">>})
Expand Down Expand Up @@ -263,15 +266,16 @@ handle_session_req(#httpd{method='POST', mochi_req=MochiReq}=Req) ->
UserName = ?l2b(couch_util:get_value("name", Form, "")),
Password = ?l2b(couch_util:get_value("password", Form, "")),
?LOG_DEBUG("Attempt Login: ~s",[UserName]),
User = case couch_auth_cache:get_user_creds(UserName) of
UserProps = case couch_auth_cache:get_user_creds(UserName) of
nil -> [];
Result -> Result
end,
UserSalt = couch_util:get_value(<<"salt">>, User, <<>>),
case authenticate(Password, User) of
case authenticate(Password, UserProps) of
true ->
UserProps2 = maybe_upgrade_password_hash(UserName, Password, UserProps),
% setup the session cookie
Secret = ?l2b(ensure_cookie_auth_secret()),
UserSalt = couch_util:get_value(<<"salt">>, UserProps2),
CurrentTime = make_cookie_time(),
Cookie = cookie_auth_cookie(Req, ?b2l(UserName), <<Secret/binary, UserSalt/binary>>, CurrentTime),
% TODO document the "next" feature in Futon
Expand All @@ -284,8 +288,8 @@ handle_session_req(#httpd{method='POST', mochi_req=MochiReq}=Req) ->
send_json(Req#httpd{req_body=ReqBody}, Code, Headers,
{[
{ok, true},
{name, couch_util:get_value(<<"name">>, User, null)},
{roles, couch_util:get_value(<<"roles">>, User, [])}
{name, couch_util:get_value(<<"name">>, UserProps2, null)},
{roles, couch_util:get_value(<<"roles">>, UserProps2, [])}
]});
_Else ->
% clear the session
Expand Down Expand Up @@ -340,6 +344,21 @@ maybe_value(_Key, undefined, _Fun) -> [];
maybe_value(Key, Else, Fun) ->
[{Key, Fun(Else)}].

maybe_upgrade_password_hash(UserName, Password, UserProps) ->
case couch_util:get_value(<<"password_scheme">>, UserProps, <<"simple">>) of
<<"simple">> ->
DbName = ?l2b(couch_config:get("couch_httpd_auth", "authentication_db", "_users")),
couch_util:with_db(DbName, fun(UserDb) ->
UserProps2 = proplists:delete(<<"password_sha">>, UserProps),
UserProps3 = [{<<"password">>, Password} | UserProps2],
NewUserDoc = couch_doc:from_json_obj({UserProps3}),
{ok, _NewRev} = couch_db:update_doc(UserDb, NewUserDoc, []),
couch_auth_cache:get_user_creds(UserName)
end);
_ ->
UserProps
end.

authenticate(Pass, UserProps) ->
UserSalt = couch_util:get_value(<<"salt">>, UserProps, <<>>),
{PasswordHash, ExpectedHash} =
Expand Down