Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: tarantool-patches@dev.tarantool.org, kostja.osipov@gmail.com
Subject: [Tarantool-patches] [PATCH 1/1] replication: use empty password by default
Date: Mon,  4 Nov 2019 18:10:16 +0300	[thread overview]
Message-ID: <c83b11bf88f7249fbce6902e768047dcbcc55401.1572880138.git.v.shpilevoy@tarantool.org> (raw)

Replication's applier encoded an auth request with exactly the
same parameters as extracted by the URI parser. I.e. when no
password was specified, the parser returned it as NULL, and it was
not encoded. The relay, received such an auth request, complained
that IPROTO_TUPLE field is not specified (this is password).

Such an error confuses - a user didn't do anything illegal, he
just used URI like 'login@host:port', without a password after the
login.

The patch makes the applier use an empty string as a default
password.

An alternative was to force a user always set a password even if
it is an empty string, like that: 'login:@host:port'. And if a
password was not found in an auth request, then reject it with a
password mismatch error. But in that case a URI of kind
'login@host:port' becomes useless - it can never pass. In
addition, netbox already uses an empty string as a default
password. So the only way to make it consistent, and don't break
anything - repeat netbox logic for replication URIs.

Closes #4605
---
Issue: https://github.com/tarantool/tarantool/issues/4605
Branch: https://github.com/tarantool/tarantool/tree/gerold103/gh-4605-empty-password-replication

 src/box/applier.cc                            |  4 +-
 .../replication/gh-4605-empty-password.result | 52 +++++++++++++++++++
 .../gh-4605-empty-password.test.lua           | 17 ++++++
 test/replication/suite.cfg                    |  1 +
 4 files changed, 73 insertions(+), 1 deletion(-)
 create mode 100644 test/replication/gh-4605-empty-password.result
 create mode 100644 test/replication/gh-4605-empty-password.test.lua

diff --git a/src/box/applier.cc b/src/box/applier.cc
index a04d13564..9467718d7 100644
--- a/src/box/applier.cc
+++ b/src/box/applier.cc
@@ -373,7 +373,9 @@ applier_connect(struct applier *applier)
 	/* Authenticate */
 	applier_set_state(applier, APPLIER_AUTH);
 	xrow_encode_auth_xc(&row, greeting.salt, greeting.salt_len, uri->login,
-			    uri->login_len, uri->password, uri->password_len);
+			    uri->login_len,
+			    uri->password != NULL ? uri->password : "",
+			    uri->password_len);
 	coio_write_xrow(coio, &row);
 	coio_read_xrow(coio, ibuf, &row);
 	applier->last_row_time = ev_monotonic_now(loop());
diff --git a/test/replication/gh-4605-empty-password.result b/test/replication/gh-4605-empty-password.result
new file mode 100644
index 000000000..ec33c4914
--- /dev/null
+++ b/test/replication/gh-4605-empty-password.result
@@ -0,0 +1,52 @@
+-- test-run result file version 2
+test_run = require('test_run').new()
+ | ---
+ | ...
+box.schema.user.create('test_user', {password = ''})
+ | ---
+ | ...
+box.schema.user.grant('test_user', 'replication')
+ | ---
+ | ...
+
+test_run:cmd("create server replica_auth with rpl_master=default, script='replication/replica_auth.lua'")
+ | ---
+ | - true
+ | ...
+test_run:cmd("start server replica_auth with wait=True, wait_load=True, args='test_user 0.1'")
+ | ---
+ | - true
+ | ...
+
+test_run:switch('replica_auth')
+ | ---
+ | - true
+ | ...
+i = box.info
+ | ---
+ | ...
+i.replication[(i.id + 1) % 2].upstream.status == 'follow' or i
+ | ---
+ | - true
+ | ...
+
+test_run:switch('default')
+ | ---
+ | - true
+ | ...
+test_run:cmd("stop server replica_auth")
+ | ---
+ | - true
+ | ...
+test_run:cmd("cleanup server replica_auth")
+ | ---
+ | - true
+ | ...
+test_run:cmd("delete server replica_auth")
+ | ---
+ | - true
+ | ...
+
+box.schema.user.drop('test_user')
+ | ---
+ | ...
diff --git a/test/replication/gh-4605-empty-password.test.lua b/test/replication/gh-4605-empty-password.test.lua
new file mode 100644
index 000000000..0e178e15a
--- /dev/null
+++ b/test/replication/gh-4605-empty-password.test.lua
@@ -0,0 +1,17 @@
+test_run = require('test_run').new()
+box.schema.user.create('test_user', {password = ''})
+box.schema.user.grant('test_user', 'replication')
+
+test_run:cmd("create server replica_auth with rpl_master=default, script='replication/replica_auth.lua'")
+test_run:cmd("start server replica_auth with wait=True, wait_load=True, args='test_user 0.1'")
+
+test_run:switch('replica_auth')
+i = box.info
+i.replication[(i.id + 1) % 2].upstream.status == 'follow' or i
+
+test_run:switch('default')
+test_run:cmd("stop server replica_auth")
+test_run:cmd("cleanup server replica_auth")
+test_run:cmd("delete server replica_auth")
+
+box.schema.user.drop('test_user')
diff --git a/test/replication/suite.cfg b/test/replication/suite.cfg
index eb25077d8..dcf52f247 100644
--- a/test/replication/suite.cfg
+++ b/test/replication/suite.cfg
@@ -11,6 +11,7 @@
     "on_schema_init.test.lua": {},
     "long_row_timeout.test.lua": {},
     "join_without_snap.test.lua": {},
+    "gh-4605-empty-password.test.lua": {},
     "*": {
         "memtx": {"engine": "memtx"},
         "vinyl": {"engine": "vinyl"}
-- 
2.21.0 (Apple Git-122.2)

             reply	other threads:[~2019-11-04 15:04 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-04 15:10 Vladislav Shpilevoy [this message]
2019-11-04 15:40 ` Konstantin Osipov
2019-11-05  9:45   ` Vladislav Shpilevoy
2019-11-05 12:42 ` Vladislav Shpilevoy
2019-11-05 12:51 ` Vladislav Shpilevoy
2019-11-05 18:13   ` Konstantin Osipov
2019-11-21 18:40 ` Kirill Yukhin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c83b11bf88f7249fbce6902e768047dcbcc55401.1572880138.git.v.shpilevoy@tarantool.org \
    --to=v.shpilevoy@tarantool.org \
    --cc=kostja.osipov@gmail.com \
    --cc=tarantool-patches@dev.tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH 1/1] replication: use empty password by default' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox