-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Use cookie authentication during replication #172
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use cookie authentication during replication #172
Conversation
See https://issues.apache.org/jira/browse/COUCHDB-1953 (cherry picked from commit 824869c)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't it be read in the cookie header?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's possible, but what advantage would it get us? I had previously implemented it, but then threw it away again. One disadvantage of that is that it makes us rely on clocks being synchronized, which is an assumption we probably like to avoid here.
As of now, we unfortunately don't have any knowledge about the expiry time of a cookie here (there's only have the timestamp). Choosing one minute seems like a good compromise to me, as it's unlikely that people will configuring a session timeout below that. However, it might be good to consequently limit the session timeout to a minimum of one minute and document it appropriately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@KlausTrainer How about to use couch_httpd_auth/timeout config value instead? It uses for the same case: to check the cookie for expiration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On Mar 11, 2014 10:20 AM, "Klaus Trainer" notifications@github.com wrote:
In src/couch_replicator/src/couch_replicator_httpc.erl:
- true -> start_new_session(HttpDb, Worker)
- end.
+need_new_session(#httpdb{credentials = undefined}) ->
- false;
+need_new_session(#httpdb{credentials = Credentials}) ->
- case ets:lookup(Credentials, cookie) of
- [] ->
true;- [{cookie, _, UpdatedAt}] ->
%% As we don't know when the cookie will expire, we just decide%% that we want a new session if the current one is older than%% one minute.OneMinute = 60 \* 1000000, % microsecondsYes, that's possible, but what advantage would it get us? I had
previously implemented it, but then threw it away again. One disadvantage
of that is that it makes us rely on clocks being synchronized, which is an
assumption we probably like to avoid here.As of now, we unfortunately don't have any knowledge about the expiry
time of a cookie here (there's only have the timestamp). Choosing one
minute seems like a good compromise to me, as it's unlikely that people
will configuring a session timeout below that. However, it might be good to
consequently limit the session timeout to a minimum of one minute and
document it appropriately.
it would allows us to reduce the number of calls which is already pretty
high. what for example when the cookie never expires?
and well TS - Now = expiry time.
- benoit
Reply to this email directly or view it on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On Tue, Mar 11, 2014 at 10:56 AM, Klaus Trainer notifications@github.comwrote:
In src/couch_replicator/src/couch_replicator_httpc.erl:
- true -> start_new_session(HttpDb, Worker)
- end.
+need_new_session(#httpdb{credentials = undefined}) ->
- false;
+need_new_session(#httpdb{credentials = Credentials}) ->
- case ets:lookup(Credentials, cookie) of
- [] ->
true;- [{cookie, _, UpdatedAt}] ->
%% As we don't know when the cookie will expire, we just decide%% that we want a new session if the current one is older than%% one minute.OneMinute = 60 \* 1000000, % microseconds@benoitc https://github.com/benoitc I'm sorry, I don't see how TS - Nowcould relate to the expiry time that's configured on the remote server.
Well when you get the expires times in the cookie and compare it to the
current hour on your machine you know the delay in which the cookie is
supposed to expire. So either you check against the cookie expires times
each time, or set a timer to remove it from the ets which will trigger the
401 the next time ou try to authenticate. In other cases you will have this
401 anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well when you get the expires times in the cookie and compare it to
the current hour on your machine you know the delay in which the
cookie is supposed to expire.
Being able to do that would be nice, but the problem is that currently
there is no expiry time in the cookie. The timestamp contained in the
cookie is generated by make_cookie_time/0, which is the time in
seconds at the point in time when the cookie was created, and therefore
not really helpful. We can currently only make guesses about how long
a cookie is be valid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On Tue, Mar 11, 2014 at 6:06 PM, Klaus Trainer notifications@github.comwrote:
In src/couch_replicator/src/couch_replicator_httpc.erl:
- true -> start_new_session(HttpDb, Worker)
- end.
+need_new_session(#httpdb{credentials = undefined}) ->
- false;
+need_new_session(#httpdb{credentials = Credentials}) ->
- case ets:lookup(Credentials, cookie) of
- [] ->
true;- [{cookie, _, UpdatedAt}] ->
%% As we don't know when the cookie will expire, we just decide%% that we want a new session if the current one is older than%% one minute.OneMinute = 60 \* 1000000, % microsecondsWell when you get the expires times in the cookie and compare it to
the current hour on your machine you know the delay in which the
cookie is supposed to expire.Being able to do that would be nice, but the problem is that currently
there is no expiry time in the cookie. The timestamp contained in the
cookie is generated by make_cookie_time/0, which is the time in
seconds at the point in time when the cookie was created, and therefore
not really helpful. We can currently only make guesses about how long
a cookie is be valid.
If the expires time is not present, then why not adding it? But If I read
wel the code of mochiweb_cookies:cookie/3 [1] you get the expires time
from the cookie header:
https://github.com/mochi/mochiweb/blob/master/src/mochiweb_cookies.erl#L63
and then it should be possible to do the above. Once I have access to a
machine i will do the test.
Reply to this email directly or view it on GitHubhttps://github.com//pull/172/files#r10483984
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like an expiry time is only set if the
"allow_persistent_cookies" configuration is set to true, which is
false by default, however. See the max_age/0 function in the
couch_httpd_auth module.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On Tue, Mar 11, 2014 at 6:28 PM, Klaus Trainer notifications@github.comwrote:
In src/couch_replicator/src/couch_replicator_httpc.erl:
- true -> start_new_session(HttpDb, Worker)
- end.
+need_new_session(#httpdb{credentials = undefined}) ->
- false;
+need_new_session(#httpdb{credentials = Credentials}) ->
- case ets:lookup(Credentials, cookie) of
- [] ->
true;- [{cookie, _, UpdatedAt}] ->
%% As we don't know when the cookie will expire, we just decide%% that we want a new session if the current one is older than%% one minute.OneMinute = 60 \* 1000000, % microsecondsIt looks like an expiry time is only set if the
"allow_persistent_cookies" configuration is set to true, which is
false by default, however. See the max_age/0 function in the
couch_httpd_auth module.
yeah we are sending a wrong header imo.
- benoit
--
Reply to this email directly or view it on GitHubhttps://github.com//pull/172/files#r10485167
.
That's interesting issue: couch_passwords:hash_admin_password accepts password as binary string, but list one had been passed instead. This causes crush with function_clause reason. Ok, but this crush left hidden for R15/R16 - only R14 shows stack trace in output and alerts that's something wrong. To be honest, *sometimes* it's also possible to reproduce this test suite crush with modern Erlang releases, but it will be about Bad Plan: planned 27 test, but run only 26. Nothing specific. So, silent crush prevented other tests to be run and also counted by the plan. Now this is fixed.
The issue happened from time to time on CentOS system: one, two or few tests failed because ref count wasn't decremented till the very moment when this value was requested and the result returned back. Adding sleep timeout helps to synchronise calls and while 0.1 sec sleep is good, but not enough - with 0.2 sleep floating errors happens no more.
- I tried to not be super heavy handed, only using <%- for values that
could be set with XSS payloads or otherwise come from a user/data.
Conflicts:
src/fauxton/app/addons/auth/templates/nav_link_title.html
COUCHDB-2220 COUCHDB-1669
Admin users are stored in .ini files and are not full-fledged user documents. Internally, a fake document is made to allow insertion into the auth cache. CouchDB 1.6 introduced a feature to upgrade password hashes from the legacy simple hash scheme to the stronger PBKDF2 scheme. It inappropriately attempted to do this to the fake admin docs, which do not pass the _design/_auth validation checks. This is fortunate, however, as CouchDB would then have written the admin users into the users database causing widespread confusion and fear.
…ashing - closes COUCHDB-2298 - closes COUCHDB-2299
The documentation was incorrect insofar that it only described its functionality for client verification, although the configuration is used for server verification as well.
To COPY a doc the client must specify the target doc id in Destination header. Additionally, it may add the revision parameter if document already exists and we want to copy over it.
Fixes COUCHDB-1891
Based on Patrick Antivackis work. Thanks! COUCHDB-241
Thanks @zemirco for the report.
Patch made by @janl for COUCHDB-1275.
COUCHDB-1011 Signed-off-by: Alexander Shorin <kxepal@apache.org>
The note is related to the delete paragraph text, not to the seealso text so move the note to improve readability. Signed-off-by: Alexander Shorin <kxepal@apache.org>
Signed-off-by: Alexander Shorin <kxepal@apache.org>
This closes apache#209 Signed-off-by: Alexander Shorin <kxepal@apache.org>
In `couch_replicator_httpc:setup/1`, if the URL contains basic auth credentials, the replicator now removes those credentials from the URL and inserts them into a new ets table, whose identifier is stored in the returned `httpdb` record in a new field named `credentials`. Then, if during replication there's a HTTP status code 401, the replicator does a POST-request to the remote DB's `/_session` endpoint, in which the request body contains the user name and password from the basic auth credentials stored in the ets table. If that request is successful, the session cookie is inserted into the ets table together with the current timestamp returned by `os:timestamp/0`. From that point on, `couch_replicator_httpc:send_req/3` always retrieves the cookie from the ets table and sets it as value in the "Cookie" request header. When a new session cookie is set, the cookie value, together with a new timestamp, will be updated in the ets table accordingly. Now, after having started a new session, the request that generated the response status code 401 is repeated. In order to prevent the replicator from wrongly starting a new session after getting a 401 status code and then repeating the request again (and thereby causing an infinite loop), it is checked whether the difference between the current timestamp returned by `os:timestamp/0` and the timestamp stored together with the session cookie is greater than one minute. A new session is started and the request is repeated only if the difference is greater than one minute. Otherwise it is proceeded as ever, i.e., the response with the 401 status code is returned.
We need that in order to make sure that the cookie authentication that we do during replication by now works as expected.
97b7b4e to
85eb4b5
Compare
|
Closing in favour of a new pull request targeting the 1.x.x branch. |
Add missing dep examples and fix existing ones
Minor corrections to Explain documentation
Minor corrections to Explain documentation
In
couch_replicator_httpc:setup/1, if the URL contains basic authcredentials, the replicator now removes those credentials from the URL
and inserts them into a new ets table, whose identifier is stored in the
returned
httpdbrecord in a new field namedcredentials.Then, if during replication there's a HTTP status code 401, the
replicator does a POST-request to the remote DB's
/_sessionendpoint,in which the request body contains the user name and password from the
basic auth credentials stored in the ets table. If that request is
successful, the session cookie is inserted into the ets table together
with the current timestamp returned by
os:timestamp/0. From thatpoint on,
couch_replicator_httpc:send_req/3always retrieves thecookie from the ets table and sets it as value in the "Cookie" request
header. When a new session cookie is set, the cookie value, together
with a new timestamp, will be updated in the ets table accordingly.
Now, after having started a new session, the request that generated the
response status code 401 is repeated.
In order to prevent the replicator from wrongly starting a new session
after getting a 401 status code and then repeating the request again
(and thereby causing an infinite loop), it is checked whether the
difference between the current timestamp returned by
os:timestamp/0and the timestamp stored together with the session cookie is greater
than one minute. A new session is started and the request is repeated
only if the difference is greater than one minute. Otherwise it is
proceeded as ever, i.e., the response with the 401 status code is
returned.