couchdb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benoit Chesneau <bchesn...@gmail.com>
Subject Re: git commit: updated refs/heads/master to c98ba56
Date Fri, 31 May 2013 21:38:59 GMT
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 <bchesneau@gmail.com> wrote:
> Why isn't it in a branch ? :/
>
> On Fri, May 31, 2013 at 8:06 PM,  <jhs@apache.org> 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) <jhs@nodejitsu.com>
>> Authored: Fri May 31 18:06:25 2013 +0000
>> Committer: Jason Smith (work) <jhs@nodejitsu.com>
>> 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});
>>

Mime
View raw message