You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by rnewson <gi...@git.apache.org> on 2015/06/23 12:23:20 UTC

[GitHub] couchdb-couch pull request: Configurable password scheme

GitHub user rnewson opened a pull request:

    https://github.com/apache/couchdb-couch/pull/60

    Configurable password scheme

    This gives the administrator control over which algorithm is used to
    hash passwords and a separate control over whether this happens on
    successful authentication or only at password change time.
    
    closes COUCHDB-2725

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/rnewson/couchdb-couch 2725-configurable-password-scheme

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/couchdb-couch/pull/60.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #60
    
----
commit 5270be18b293623eb7530e409941eab4c76a02f6
Author: Robert Newson <rn...@apache.org>
Date:   2015-06-16T16:17:44Z

    Configurable password scheme
    
    This gives the administrator control over which algorithm is used to
    hash passwords and a separate control over whether this happens on
    successful authentication or only at password change time.
    
    closes COUCHDB-2725

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch pull request: Configurable password scheme

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch/pull/60#discussion_r33027960
  
    --- Diff: src/couch_httpd_auth.erl ---
    @@ -372,9 +372,11 @@ maybe_value(Key, Else, Fun) ->
     
     maybe_upgrade_password_hash(Req, UserName, Password, UserProps,
             AuthModule, AuthCtx) ->
    +    Upgrade = config:get("couch_httpd_auth", "upgrade_password_on_auth", "true"),
    --- End diff --
    
    [config:get_boolean/3](https://github.com/apache/couchdb-config/blob/master/src/config.erl#L102-L108) is better here.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch pull request: Configurable password scheme

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch/pull/60#discussion_r33028682
  
    --- Diff: src/couch_passwords.erl ---
    @@ -30,6 +30,14 @@ simple(Password, Salt) when is_binary(Password), is_binary(Salt) ->
     hash_admin_password(ClearPassword) when is_list(ClearPassword) ->
         hash_admin_password(?l2b(ClearPassword));
     hash_admin_password(ClearPassword) when is_binary(ClearPassword) ->
    +    Scheme = config:get("couch_httpd_auth", "password_scheme", "pbkdf2"),
    +    hash_admin_password(Scheme, ClearPassword).
    +
    +hash_admin_password("simple", ClearPassword) ->
    --- End diff --
    
    not by default. If you change password_scheme to simple, then the server will continue to use that algorithm. This is all about migration.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch pull request: Configurable password scheme

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch/pull/60#discussion_r33031108
  
    --- Diff: src/couch_passwords.erl ---
    @@ -30,6 +30,14 @@ simple(Password, Salt) when is_binary(Password), is_binary(Salt) ->
     hash_admin_password(ClearPassword) when is_list(ClearPassword) ->
         hash_admin_password(?l2b(ClearPassword));
     hash_admin_password(ClearPassword) when is_binary(ClearPassword) ->
    +    Scheme = config:get("couch_httpd_auth", "password_scheme", "pbkdf2"),
    +    hash_admin_password(Scheme, ClearPassword).
    +
    +hash_admin_password("simple", ClearPassword) ->
    --- End diff --
    
    we didn't, that's the enhancement. Plaintext admin passwords used to be one-pass sha'ed, then from 1.2 or so they were pbkdf2'd. Already hashed passwords are left as they are.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch pull request: Configurable password scheme

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch/pull/60#discussion_r33028633
  
    --- Diff: src/couch_users_db.erl ---
    @@ -71,7 +82,9 @@ save_doc(#doc{body={Body}} = Doc) ->
             Body2 = ?replace(Body1, ?DERIVED_KEY, DerivedKey),
             Body3 = ?replace(Body2, ?SALT, Salt),
             Body4 = proplists:delete(?PASSWORD, Body3),
    -        Doc#doc{body={Body4}}
    +        Doc#doc{body={Body4}};
    +    {_ClearPassword, _Scheme} ->
    +        throw({forbidden, "Server cannot hash passwords at this time."})
    --- End diff --
    
    I didn't want to expose the invalid value the administrator had set by mistake.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch pull request: Configurable password scheme

Posted by janl <gi...@git.apache.org>.
Github user janl commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch/pull/60#discussion_r33028464
  
    --- Diff: src/couch_passwords.erl ---
    @@ -30,6 +30,14 @@ simple(Password, Salt) when is_binary(Password), is_binary(Salt) ->
     hash_admin_password(ClearPassword) when is_list(ClearPassword) ->
         hash_admin_password(?l2b(ClearPassword));
     hash_admin_password(ClearPassword) when is_binary(ClearPassword) ->
    +    Scheme = config:get("couch_httpd_auth", "password_scheme", "pbkdf2"),
    +    hash_admin_password(Scheme, ClearPassword).
    +
    +hash_admin_password("simple", ClearPassword) ->
    --- End diff --
    
    Is this re-introducing the salt+sha1 hash from the pre-pbkdf2 days or am I reading this wrong?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch pull request: Configurable password scheme

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch/pull/60#discussion_r33029145
  
    --- Diff: src/couch_users_db.erl ---
    @@ -71,7 +82,9 @@ save_doc(#doc{body={Body}} = Doc) ->
             Body2 = ?replace(Body1, ?DERIVED_KEY, DerivedKey),
             Body3 = ?replace(Body2, ?SALT, Salt),
             Body4 = proplists:delete(?PASSWORD, Body3),
    -        Doc#doc{body={Body4}}
    +        Doc#doc{body={Body4}};
    +    {_ClearPassword, _Scheme} ->
    +        throw({forbidden, "Server cannot hash passwords at this time."})
    --- End diff --
    
    May be log then that fact? Otherwise such mistakes will remain unnoticed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch pull request: Configurable password scheme

Posted by janl <gi...@git.apache.org>.
Github user janl commented on the pull request:

    https://github.com/apache/couchdb-couch/pull/60#issuecomment-114555086
  
    LGTM +1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch pull request: Configurable password scheme

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch/pull/60#discussion_r33028596
  
    --- Diff: src/couch_passwords.erl ---
    @@ -30,6 +30,14 @@ simple(Password, Salt) when is_binary(Password), is_binary(Salt) ->
     hash_admin_password(ClearPassword) when is_list(ClearPassword) ->
         hash_admin_password(?l2b(ClearPassword));
     hash_admin_password(ClearPassword) when is_binary(ClearPassword) ->
    +    Scheme = config:get("couch_httpd_auth", "password_scheme", "pbkdf2"),
    +    hash_admin_password(Scheme, ClearPassword).
    +
    +hash_admin_password("simple", ClearPassword) ->
    +    Salt = couch_uuids:random(),
    +    Hash = crypto:sha(<<ClearPassword/binary, Salt/binary>>),
    +    ?l2b("-hashed-" ++ couch_util:to_hex(Hash) ++ "," ++ ?b2l(Salt));
    --- End diff --
    
    I was deliberately following the previous code and I doubt this is a performance critical piece.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch pull request: Configurable password scheme

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch/pull/60#discussion_r33027973
  
    --- Diff: src/couch_passwords.erl ---
    @@ -30,6 +30,14 @@ simple(Password, Salt) when is_binary(Password), is_binary(Salt) ->
     hash_admin_password(ClearPassword) when is_list(ClearPassword) ->
         hash_admin_password(?l2b(ClearPassword));
     hash_admin_password(ClearPassword) when is_binary(ClearPassword) ->
    +    Scheme = config:get("couch_httpd_auth", "password_scheme", "pbkdf2"),
    +    hash_admin_password(Scheme, ClearPassword).
    +
    +hash_admin_password("simple", ClearPassword) ->
    +    Salt = couch_uuids:random(),
    +    Hash = crypto:sha(<<ClearPassword/binary, Salt/binary>>),
    +    ?l2b("-hashed-" ++ couch_util:to_hex(Hash) ++ "," ++ ?b2l(Salt));
    --- End diff --
    
    May be `lists:flatten(["-hashed-", couch_util:to_hex(Hash), ",", ?b2l(Salt)])`? Does lesser copies.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch pull request: Configurable password scheme

Posted by janl <gi...@git.apache.org>.
Github user janl commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch/pull/60#discussion_r33029166
  
    --- Diff: src/couch_passwords.erl ---
    @@ -30,6 +30,14 @@ simple(Password, Salt) when is_binary(Password), is_binary(Salt) ->
     hash_admin_password(ClearPassword) when is_list(ClearPassword) ->
         hash_admin_password(?l2b(ClearPassword));
     hash_admin_password(ClearPassword) when is_binary(ClearPassword) ->
    +    Scheme = config:get("couch_httpd_auth", "password_scheme", "pbkdf2"),
    +    hash_admin_password(Scheme, ClearPassword).
    +
    +hash_admin_password("simple", ClearPassword) ->
    --- End diff --
    
    How did we handle this in the past?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch pull request: Configurable password scheme

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch/pull/60#discussion_r33027996
  
    --- Diff: src/couch_users_db.erl ---
    @@ -71,7 +82,9 @@ save_doc(#doc{body={Body}} = Doc) ->
             Body2 = ?replace(Body1, ?DERIVED_KEY, DerivedKey),
             Body3 = ?replace(Body2, ?SALT, Salt),
             Body4 = proplists:delete(?PASSWORD, Body3),
    -        Doc#doc{body={Body4}}
    +        Doc#doc{body={Body4}};
    +    {_ClearPassword, _Scheme} ->
    +        throw({forbidden, "Server cannot hash passwords at this time."})
    --- End diff --
    
    May be explicitly return response that specified Scheme is not supported now?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch pull request: Configurable password scheme

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/couchdb-couch/pull/60


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---