[tarantool-patches] Re: [PATCH] Check for not logging password added

[Georgy Kirichenko]

Thanks for the patch.

It looks good except the test your attached which is not even working.
I updated your branch with a proper version of the test and corresponding 
result file.
Also, don't forget about issue and branch references in your email.

Please review the fix and resent updated email.

On Monday, September 16, 2019 8:09:57 PM MSK Maria wrote:
> It was possible to leak user password through
> setting 'replication' configuration option in
> box.cfg. Logging in load_cfg function used to
> be unconditional unlike reload_cfg.
> 
> Closes #4493
> ---
>  src/box/lua/load_cfg.lua   |  3 +++
>  test/box/cfg.test.lua      |  9 +++++++++
>  test/box/lua/cfg_test6.lua | 10 ++++++++++
>  3 files changed, 22 insertions(+)
>  create mode 100644 test/box/lua/cfg_test6.lua
> 
> diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
> index 79bde4d23..e051322db 100644
> --- a/src/box/lua/load_cfg.lua
> +++ b/src/box/lua/load_cfg.lua
> @@ -514,6 +514,9 @@ local function load_cfg(cfg)
>                  fun()
>              end
>              if not compare_cfg(val, default_cfg[key]) then
> +                if log_cfg_option[key] ~= nil then
> +                    val = log_cfg_option[key](val)
> +                end
>                  log.info("set '%s' configuration option to %s", key,
> json.encode(val)) end
>          end
> diff --git a/test/box/cfg.test.lua b/test/box/cfg.test.lua
> index 56ccb6767..ca505808c 100644
> --- a/test/box/cfg.test.lua
> +++ b/test/box/cfg.test.lua
> @@ -141,3 +141,12 @@ test_run:cmd("start server cfg_tester6")
>  test_run:grep_log('cfg_tester6', 'set \'vinyl_memory\' configuration option
> to 1073741824', 1000) test_run:cmd("stop server cfg_tester6")
>  test_run:cmd("cleanup server cfg_tester6")
> +
> +--
> +-- gh-4493: Replication user password may leak to logs
> +--
> +test_run:cmd('create server cfg_tester7 with script =
> "box/lua/cfg_test6.lua"') +test_run:cmd("start server cfg_tester7")
> +test_run:grep_log('cfg_tester6', 'set \'replication\' configuration option
> to "admin:test-cluster-cookie at localhost:33003"') +test_run:cmd("stop server
> cfg_tester7")
> +test_run:cmd("cleanup server cfg_tester7")
> diff --git a/test/box/lua/cfg_test6.lua b/test/box/lua/cfg_test6.lua
> new file mode 100644
> index 000000000..ca8dba63a
> --- /dev/null
> +++ b/test/box/lua/cfg_test6.lua
> @@ -0,0 +1,10 @@
> +#!/usr/bin/env tarantool
> +os = require('os')
> +
> +box.cfg{
> +    listen              = os.getenv("LISTEN"),
> +    replication         = "admin:test-cluster-cookie at localhost:33003",
> +}
> +
> +require('console').listen(os.getenv('ADMIN'))
> +box.schema.user.grant('guest', 'read,write,execute', 'universe')
> \ No newline at end of file