From 6c01ca48e05d325914024f6823d88ca8ff53b0af Mon Sep 17 00:00:00 2001
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Date: Mon, 4 Nov 2019 16:58:46 +0300
Subject: [PATCH] replication: use empty password by default
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
Conflicts:
test/replication/suite.cfg
---
src/box/applier.cc | 4 +-
.../replication/gh-4605-empty-password.result | 62 +++++++++++++++++++
.../gh-4605-empty-password.test.lua | 27 ++++++++
test/replication/suite.cfg | 1 +
4 files changed, 93 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 a374ff50d7..42374f8866 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 0000000000..defdfcfcdf
--- /dev/null
+++ b/test/replication/gh-4605-empty-password.result
@@ -0,0 +1,62 @@
+-- test-run result file version 2
+test_run = require('test_run').new()
+ | ---
+ | ...
+
+--
+-- gh-4605: replication and netbox both use URI as a remote
+-- resource identifier. If URI does not contain a password, netbox
+-- assumes it is an empty string - ''. But replication's applier
+-- wasn't assuming the same, and just didn't send a password at
+-- all, when it was not specified in the URI. It led to a strange
+-- error message and inconsistent behaviour. The test checks, that
+-- replication now also uses an empty string password by default.
+
+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 % 2 + 1].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 0000000000..f42a55f81c
--- /dev/null
+++ b/test/replication/gh-4605-empty-password.test.lua
@@ -0,0 +1,27 @@
+test_run = require('test_run').new()
+
+--
+-- gh-4605: replication and netbox both use URI as a remote
+-- resource identifier. If URI does not contain a password, netbox
+-- assumes it is an empty string - ''. But replication's applier
+-- wasn't assuming the same, and just didn't send a password at
+-- all, when it was not specified in the URI. It led to a strange
+-- error message and inconsistent behaviour. The test checks, that
+-- replication now also uses an empty string password by default.
+
+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 % 2 + 1].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 0848eecd61..cd686a0e2c 100644
--- a/test/replication/suite.cfg
+++ b/test/replication/suite.cfg
@@ -12,6 +12,7 @@
"long_row_timeout.test.lua": {},
"join_without_snap.test.lua": {},
"gh-4402-info-errno.test.lua": {},
+ "gh-4605-empty-password.test.lua": {},
"gh-4606-admin-creds.test.lua": {},
"*": {
"memtx": {"engine": "memtx"},
--
GitLab