From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp63.i.mail.ru (smtp63.i.mail.ru [217.69.128.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id D215542F4C3 for ; Mon, 4 Nov 2019 18:04:31 +0300 (MSK) From: Vladislav Shpilevoy Date: Mon, 4 Nov 2019 18:10:16 +0300 Message-Id: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH 1/1] replication: use empty password by default List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@dev.tarantool.org, kostja.osipov@gmail.com 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)