[Tarantool-patches] [PATCH 1/1] replication: use empty password by default

Vladislav Shpilevoy v.shpilevoy at tarantool.org
Mon Nov 4 18:10:16 MSK 2019


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 at 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 at 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)



More information about the Tarantool-patches mailing list