Hi, Benoit. I did not put this in a branch since it was a single commit. I think I missed that IRC meeting and did not realize the policy.
My original implementation assumed a "raw" update if the password had a "-pbkdf2-" prefix. Yes, that means people can no longer have a password of literally "-pbkdf2-<etc.>" but I figured CouchDB should DTRT in that one special case. The new way (?raw=true), the API is explicit rather than implicit. That is "simple" in its own way. On Sat, Jun 1, 2013 at 4:38 AM, Benoit Chesneau <[email protected]> wrote: > So to be clear I don't understand why this patch landed in master > after the discussion we had on irc about it. Discussion was supposed > to continue on the ml... > > Anyway I will repeat my argument against this ugly "raw" parameter. We > don't need it. > > Actually we are checking if the password match an hash signature to > know if we have to hash it or not . Ie we are checking if it starts > with 'hashtype-' I don't see any real reason to not do the same on > PUT. > > 1. Matching an hash signature on PUT is consistant with the current > API we have to transform a password in the ini > > 2. It's likely that people will have a password "hashtype-blah" . So > the argument that there can be a namespace pollution doesn't really > work as well. > > 3. It used to work that way before. > > So rather than using this parameter we should rather just match > against the hash signature . It would also keep the api simple. > > > > On Fri, May 31, 2013 at 8:47 PM, Benoit Chesneau <[email protected]> > wrote: > > Why isn't it in a branch ? :/ > > > > On Fri, May 31, 2013 at 8:06 PM, <[email protected]> wrote: > >> Updated Branches: > >> refs/heads/master f15f54d56 -> c98ba5612 > >> > >> > >> Allow storing a pre-hashed admin password > >> > >> When duplicating a couch, it is difficult to copy the _config/admins/* > >> values. Storing the encoded value does not work because that value is > >> re-hashed when stored. (Your password is the literal string > >> "-pbkdf2-abcdef...".) > >> > >> This change will store any config setting unmodified if ?raw=true is > >> in the query string. > >> > >> Updating _config/admins/* already requires admin privileges, so there is > >> no change to the security. > >> > >> > >> Project: http://git-wip-us.apache.org/repos/asf/couchdb/repo > >> Commit: http://git-wip-us.apache.org/repos/asf/couchdb/commit/c98ba561 > >> Tree: http://git-wip-us.apache.org/repos/asf/couchdb/tree/c98ba561 > >> Diff: http://git-wip-us.apache.org/repos/asf/couchdb/diff/c98ba561 > >> > >> Branch: refs/heads/master > >> Commit: c98ba5612e313b252e5b7ac91b3772c226b82217 > >> Parents: f15f54d > >> Author: Jason Smith (work) <[email protected]> > >> Authored: Fri May 31 18:06:25 2013 +0000 > >> Committer: Jason Smith (work) <[email protected]> > >> Committed: Fri May 31 18:06:25 2013 +0000 > >> > >> ---------------------------------------------------------------------- > >> share/doc/src/configuring.rst | 26 +++++++++++++ > >> share/www/script/test/config.js | 48 > ++++++++++++++++++++++++ > >> src/couchdb/couch_httpd_misc_handlers.erl | 42 +++++++++++++++++--- > >> 3 files changed, 109 insertions(+), 7 deletions(-) > >> ---------------------------------------------------------------------- > >> > >> > >> > http://git-wip-us.apache.org/repos/asf/couchdb/blob/c98ba561/share/doc/src/configuring.rst > >> ---------------------------------------------------------------------- > >> diff --git a/share/doc/src/configuring.rst > b/share/doc/src/configuring.rst > >> index 4b8bb11..8d3e704 100644 > >> --- a/share/doc/src/configuring.rst > >> +++ b/share/doc/src/configuring.rst > >> @@ -240,6 +240,32 @@ supports querying, deleting or creating new admin > accounts: > >> "architect": > "-pbkdf2-43ecbd256a70a3a2f7de40d2374b6c3002918834,921a12f74df0c1052b3e562a23cd227f,10000" > >> } > >> > >> +If you already have a salted, encrypted password string (for example, > >> +from an old ``local.ini`` file, or from a different CouchDB server), > then > >> +you can store the "raw" encrypted string, without having CouchDB doubly > >> +encrypt it. > >> + > >> +.. code-block:: bash > >> + > >> + shell> PUT /_config/admins/architect?raw=true HTTP/1.1 > >> + Accept: application/json > >> + Content-Type: application/json > >> + Content-Length: 89 > >> + Host: localhost:5984 > >> + > >> + > > "-pbkdf2-43ecbd256a70a3a2f7de40d2374b6c3002918834,921a12f74df0c1052b3e562a23cd227f,10000" > >> + > >> + HTTP/1.1 200 OK > >> + Cache-Control: must-revalidate > >> + Content-Length: 89 > >> + Content-Type: application/json > >> + Date: Fri, 30 Nov 2012 11:39:18 GMT > >> + Server: CouchDB/1.3.0 (Erlang OTP/R15B02) > >> + > >> +.. code-block:: json > >> + > >> + > > "-pbkdf2-43ecbd256a70a3a2f7de40d2374b6c3002918834,921a12f74df0c1052b3e562a23cd227f,10000" > >> + > >> Further details are available in ``security_``, including configuring > the > >> work factor for ``PBKDF2``, and the algorithm itself at > >> `PBKDF2 (RFC-2898) <http://tools.ietf.org/html/rfc2898>`_. > >> > >> > http://git-wip-us.apache.org/repos/asf/couchdb/blob/c98ba561/share/www/script/test/config.js > >> ---------------------------------------------------------------------- > >> diff --git a/share/www/script/test/config.js > b/share/www/script/test/config.js > >> index 5382778..193aa89 100644 > >> --- a/share/www/script/test/config.js > >> +++ b/share/www/script/test/config.js > >> @@ -72,6 +72,54 @@ couchTests.config = function(debug) { > >> config = JSON.parse(xhr.responseText); > >> T(config == "bar"); > >> > >> + // Server-side password hashing, and raw updates disabling that. > >> + var password_plain = 's3cret'; > >> + var password_hashed = null; > >> + > >> + xhr = CouchDB.request("PUT", "/_config/admins/administrator",{ > >> + body : JSON.stringify(password_plain), > >> + headers: {"X-Couch-Persist": "false"} > >> + }); > >> + TEquals(200, xhr.status, "Create an admin in the config"); > >> + > >> + T(CouchDB.login("administrator", password_plain).ok); > >> + > >> + xhr = CouchDB.request("GET", "/_config/admins/administrator"); > >> + password_hashed = JSON.parse(xhr.responseText); > >> + T(password_hashed.match(/^-pbkdf2-/) || > password_hashed.match(/^-hashed-/), > >> + "Admin password is hashed"); > >> + > >> + xhr = CouchDB.request("PUT", > "/_config/admins/administrator?raw=nothanks",{ > >> + body : JSON.stringify(password_hashed), > >> + headers: {"X-Couch-Persist": "false"} > >> + }); > >> + TEquals(400, xhr.status, "CouchDB rejects an invalid 'raw' option"); > >> + > >> + xhr = CouchDB.request("PUT", > "/_config/admins/administrator?raw=true",{ > >> + body : JSON.stringify(password_hashed), > >> + headers: {"X-Couch-Persist": "false"} > >> + }); > >> + TEquals(200, xhr.status, "Set an raw, pre-hashed admin password"); > >> + > >> + xhr = CouchDB.request("PUT", > "/_config/admins/administrator?raw=false",{ > >> + body : JSON.stringify(password_hashed), > >> + headers: {"X-Couch-Persist": "false"} > >> + }); > >> + TEquals(200, xhr.status, "Set an admin password with raw=false"); > >> + > >> + // The password is literally the string "-pbkdf2-abcd...". > >> + T(CouchDB.login("administrator", password_hashed).ok); > >> + > >> + xhr = CouchDB.request("GET", "/_config/admins/administrator"); > >> + T(password_hashed != JSON.parse(xhr.responseText), > >> + "Hashed password was not stored as a raw string"); > >> + > >> + xhr = CouchDB.request("DELETE", "/_config/admins/administrator",{ > >> + headers: {"X-Couch-Persist": "false"} > >> + }); > >> + TEquals(200, xhr.status, "Delete an admin from the config"); > >> + T(CouchDB.logout().ok); > >> + > >> // Non-term whitelist values allow further modification of the > whitelist. > >> xhr = CouchDB.request("PUT", "/_config/httpd/config_whitelist",{ > >> body : JSON.stringify("!This is an invalid Erlang term!"), > >> > >> > http://git-wip-us.apache.org/repos/asf/couchdb/blob/c98ba561/src/couchdb/couch_httpd_misc_handlers.erl > >> ---------------------------------------------------------------------- > >> diff --git a/src/couchdb/couch_httpd_misc_handlers.erl > b/src/couchdb/couch_httpd_misc_handlers.erl > >> index d1f947d..96a05c6 100644 > >> --- a/src/couchdb/couch_httpd_misc_handlers.erl > >> +++ b/src/couchdb/couch_httpd_misc_handlers.erl > >> @@ -219,13 +219,35 @@ handle_config_req(Req) -> > >> > >> % PUT /_config/Section/Key > >> % "value" > >> -handle_approved_config_req(#httpd{method='PUT', path_parts=[_, > Section, Key]}=Req, Persist) -> > >> - Value = case Section of > >> - <<"admins">> -> > >> - > couch_passwords:hash_admin_password(couch_httpd:json_body(Req)); > >> - _ -> > >> - couch_httpd:json_body(Req) > >> +handle_approved_config_req(Req, Persist) -> > >> + Query = couch_httpd:qs(Req), > >> + UseRawValue = case lists:keyfind("raw", 1, Query) of > >> + false -> false; % Not specified > >> + {"raw", ""} -> false; % Specified with no value, i.e. "?raw" > and "?raw=" > >> + {"raw", "false"} -> false; > >> + {"raw", "true"} -> true; > >> + {"raw", InvalidValue} -> InvalidValue > >> + end, > >> + handle_approved_config_req(Req, Persist, UseRawValue). > >> + > >> +handle_approved_config_req(#httpd{method='PUT', path_parts=[_, > Section, Key]}=Req, > >> + Persist, UseRawValue) > >> + when UseRawValue =:= false orelse UseRawValue =:= true -> > >> + RawValue = couch_httpd:json_body(Req), > >> + Value = case UseRawValue of > >> + true -> > >> + % Client requests no change to the provided value. > >> + RawValue; > >> + false -> > >> + % Pre-process the value as necessary. > >> + case Section of > >> + <<"admins">> -> > >> + couch_passwords:hash_admin_password(RawValue); > >> + _ -> > >> + RawValue > >> + end > >> end, > >> + > >> OldValue = couch_config:get(Section, Key, ""), > >> case couch_config:set(Section, Key, ?b2l(Value), Persist) of > >> ok -> > >> @@ -233,8 +255,14 @@ handle_approved_config_req(#httpd{method='PUT', > path_parts=[_, Section, Key]}=Re > >> Error -> > >> throw(Error) > >> end; > >> + > >> +handle_approved_config_req(#httpd{method='PUT'}=Req, _Persist, > UseRawValue) -> > >> + Err = io_lib:format("Bad value for 'raw' option: ~s", > [UseRawValue]), > >> + send_json(Req, 400, {[{error, ?l2b(Err)}]}); > >> + > >> % DELETE /_config/Section/Key > >> > -handle_approved_config_req(#httpd{method='DELETE',path_parts=[_,Section,Key]}=Req, > Persist) -> > >> > +handle_approved_config_req(#httpd{method='DELETE',path_parts=[_,Section,Key]}=Req, > >> + Persist, _UseRawValue) -> > >> case couch_config:get(Section, Key, null) of > >> null -> > >> throw({not_found, unknown_config_value}); > >> >
