You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@couchdb.apache.org by jh...@apache.org on 2013/05/31 20:06:53 UTC

git commit: updated refs/heads/master to c98ba56

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) <jh...@nodejitsu.com>
Authored: Fri May 31 18:06:25 2013 +0000
Committer: Jason Smith (work) <jh...@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});


Re: git commit: updated refs/heads/master to c98ba56

Posted by Benoit Chesneau <bc...@gmail.com>.
On Jun 1, 2013 6:37 AM, "Jason Smith" <jh...@iriscouch.com> wrote:

>
> Both methods have their own restrictions.
>
> When editing an .ini file, you *can* trivially set a password cyphertext
> (just paste it in), but you cannot set a password to literally
> "-pbkdf2-...". (You could calculate the hash yourself and then paste
that.)

I'm not sure what you mean there. What you can do actually  is pasting the
full hash or the or setting a plain text password that will be transformed
when couchdb start. The password is detected as an hash if it match a
signature (for now -hashtype-... ).


>
> When using the API, you *can* trivially set a password to literal
> "-pbkdf2-..." but you cannot set the password to some cyphertext, as-is.
> (And reversing the hash back to the plaintext is impossible.)

Actually yes, anything passed via the HTTP API is considered as a plain
text password. Which is what you are trying to fix as far as I understand.


The question is why do you want to let the possibility to pass a password
looking like an hash? Who want to really do that ? When you could handle it
like the api for the ini file does. Ie. checking if the string passed match
an hash signature and if yes consider this is an hash, else this is a plain
text password that need to be encoded. Just like the ini API,

>
> In other words, even if you are admin on couch A and admin on couch B, you
> cannot synchronize the admin passwords using the API.
>
That's not true actually if you pass an ini file with an hash. I do this to
deploy some nodes actually.


- benoit

Re: git commit: updated refs/heads/master to c98ba56

Posted by Jason Smith <jh...@iriscouch.com>.
On Sat, Jun 1, 2013 at 9:14 AM, Randall Leeds <ra...@gmail.com>wrote:

> On Fri, May 31, 2013 at 4:30 PM, Jason Smith <jh...@iriscouch.com> wrote:
> > 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.
>
> Isn't that already a restriction in the sense that you can't put a
> plaint-text password in the .ini that starts with -pbkdf2-?
>

Both methods have their own restrictions.

When editing an .ini file, you *can* trivially set a password cyphertext
(just paste it in), but you cannot set a password to literally
"-pbkdf2-...". (You could calculate the hash yourself and then paste that.)

When using the API, you *can* trivially set a password to literal
"-pbkdf2-..." but you cannot set the password to some cyphertext, as-is.
(And reversing the hash back to the plaintext is impossible.)

In other words, even if you are admin on couch A and admin on couch B, you
cannot synchronize the admin passwords using the API.


> > The new way (?raw=true), the API is explicit rather than implicit. That
> is
> > "simple" in its own way.
>

> But less simple in that it's a new parameter and new feature (assuming
> what I said before is correct -- that it was not possible via the .ini
> to have this kind of password). I'm leaning toward Benoit's suggestion
> on this one.
>

​I agree with Benoit too. However, I brought this up on IRC and literally
every person there had their own opinion. (Dikjan had a cool idea of
sending an object instead of a string.) It was a textbook bike shed.

Somebody made a decent argument that Couch should not "DWIM" depending on
the content of your config value. That way would change the API (in a rare
situation, I admit). With the "raw" option, the API is merely extended.

Re: git commit: updated refs/heads/master to c98ba56

Posted by Randall Leeds <ra...@gmail.com>.
On Fri, May 31, 2013 at 4:30 PM, Jason Smith <jh...@iriscouch.com> wrote:
> 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.

Isn't that already a restriction in the sense that you can't put a
plaint-text password in the .ini that starts with -pbkdf2-?

>
> The new way (?raw=true), the API is explicit rather than implicit. That is
> "simple" in its own way.

But less simple in that it's a new parameter and new feature (assuming
what I said before is correct -- that it was not possible via the .ini
to have this kind of password). I'm leaning toward Benoit's suggestion
on this one.

Re: git commit: updated refs/heads/master to c98ba56

Posted by Jason Smith <jh...@iriscouch.com>.
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 <bc...@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 <bc...@gmail.com>
> wrote:
> > Why isn't it in a branch ? :/
> >
> > On Fri, May 31, 2013 at 8:06 PM,  <jh...@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) <jh...@nodejitsu.com>
> >> Authored: Fri May 31 18:06:25 2013 +0000
> >> Committer: Jason Smith (work) <jh...@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});
> >>
>

Re: git commit: updated refs/heads/master to c98ba56

Posted by Benoit Chesneau <bc...@gmail.com>.
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 <bc...@gmail.com> wrote:
> Why isn't it in a branch ? :/
>
> On Fri, May 31, 2013 at 8:06 PM,  <jh...@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) <jh...@nodejitsu.com>
>> Authored: Fri May 31 18:06:25 2013 +0000
>> Committer: Jason Smith (work) <jh...@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});
>>

Re: git commit: updated refs/heads/master to c98ba56

Posted by Benoit Chesneau <bc...@gmail.com>.
Why isn't it in a branch ? :/

On Fri, May 31, 2013 at 8:06 PM,  <jh...@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) <jh...@nodejitsu.com>
> Authored: Fri May 31 18:06:25 2013 +0000
> Committer: Jason Smith (work) <jh...@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});
>