couchdb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jason Smith <...@iriscouch.com>
Subject Re: git commit: updated refs/heads/master to c98ba56
Date Fri, 31 May 2013 23:30:12 GMT
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 <bchesneau@gmail.com> 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 <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
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message