Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH 1/1] replication: use empty password by default
@ 2019-11-04 15:10 Vladislav Shpilevoy
  2019-11-04 15:40 ` Konstantin Osipov
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Vladislav Shpilevoy @ 2019-11-04 15:10 UTC (permalink / raw)
  To: tarantool-patches, kostja.osipov

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)

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-11-21 18:40 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-04 15:10 [Tarantool-patches] [PATCH 1/1] replication: use empty password by default Vladislav Shpilevoy
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

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