From 64902055e850ee0b5954ef9dbab7498eb4c6b08d Mon Sep 17 00:00:00 2001 From: Alexander Turenko <alexander.turenko@tarantool.org> Date: Fri, 23 Jun 2023 16:59:20 +0300 Subject: [PATCH] config: forbid incorrect/senseless advertise URIs The following values shouldn't be accepted as an advertise URI: * Not an URI. * Comma separated list of URIs. * An URI with IPv4/IPv6 INADDR_ANY host (`0.0.0.0` or `::`). * An URI with zero TCP port. Part of #8810 NO_DOC=the old behavior was not released, the documentation request will be registered manually NO_CHANGELOG=see NO_DOC --- src/box/lua/config/instance_config.lua | 59 +++++++++ .../instance_config_schema_test.lua | 124 ++++++++++++++++++ 2 files changed, 183 insertions(+) diff --git a/src/box/lua/config/instance_config.lua b/src/box/lua/config/instance_config.lua index 89aa8f5013..d756d051d5 100644 --- a/src/box/lua/config/instance_config.lua +++ b/src/box/lua/config/instance_config.lua @@ -2,6 +2,7 @@ local schema = require('internal.config.utils.schema') local tarantool = require('tarantool') local compat = require('compat') local uuid = require('uuid') +local urilib = require('uri') -- List of annotations: -- @@ -118,6 +119,47 @@ local function validate_uuid_str(data, w) end end +-- Accepts an uri object (one produced by urilib.parse()). +-- +-- Performs several checks regarding ability to use the URI to +-- create a client socket. IOW, to call connect() on it. +-- +-- The function returns `true` if the URI is OK to connect and +-- `false, err` otherwise. +-- +-- If the URI doesn't fit the given criteria an error is raised. +-- The function returns nothing otherwise. +-- +-- The following checks are performed: +-- +-- * INADDR_ANY IPv4 address (0.0.0.0) or in6addr_any IPv6 address +-- (::) in the host part of the URI. +-- +-- It means 'bind to all interfaces' for the bind() call, but it +-- has no meaning at the connect() call on a client. +-- * Zero TCP port (service part of the URI). +-- +-- It means 'bind to a random free port' for the bind() call, +-- but it has no meaning at the connect() call on a client. +local function uri_is_suitable_to_connect(uri) + assert(uri ~= nil) + + if uri.ipv4 == '0.0.0.0' then + return false, 'INADDR_ANY (0.0.0.0) cannot be used to create ' .. + 'a client socket' + end + if uri.ipv6 == '::' then + return false, 'in6addr_any (::) cannot be used to create a client ' .. + 'socket' + end + if uri.service == '0' then + return false, 'An URI with zero port cannot be used to create ' .. + 'a client socket' + end + + return true +end + return schema.new('instance_config', schema.record({ config = schema.record({ version = schema.enum({ @@ -398,6 +440,23 @@ return schema.new('instance_config', schema.record({ advertise = schema.scalar({ type = 'string', default = box.NULL, + validate = function(data, w) + -- Substitute variables with placeholders to don't + -- confuse the URI parser with the curly brackets. + data = data:gsub('{{ *.- *}}', 'placeholder') + + local uri, err = urilib.parse(data) + if uri == nil then + if data:find(',') then + w.error('A single URI is expected, not a list of URIs') + end + w.error('Unable to parse an URI: %s', err) + end + local ok, err = uri_is_suitable_to_connect(uri) + if not ok then + w.error(err) + end + end, }), threads = schema.scalar({ type = 'integer', diff --git a/test/config-luatest/instance_config_schema_test.lua b/test/config-luatest/instance_config_schema_test.lua index ae7c694277..faf7d13d08 100644 --- a/test/config-luatest/instance_config_schema_test.lua +++ b/test/config-luatest/instance_config_schema_test.lua @@ -310,6 +310,130 @@ g.test_iproto = function() t.assert_equals(res, exp) end +-- Verify iproto.advertise validation, bad cases. +for case_name, case in pairs({ + incorrect_uri = { + advertise = ':3301', + exp_err_msg = table.concat({ + '[instance_config] iproto.advertise', + 'Unable to parse an URI', + 'Incorrect URI', + 'expected host:service or /unix.socket' + }, ': '), + }, + multiple_uris = { + advertise = 'localhost:3301,localhost:3302', + exp_err_msg = table.concat({ + '[instance_config] iproto.advertise', + 'A single URI is expected, not a list of URIs', + }, ': '), + }, + inaddr_any_ipv4 = { + advertise = '0.0.0.0:3301', + exp_err_msg = table.concat({ + '[instance_config] iproto.advertise', + 'INADDR_ANY (0.0.0.0) cannot be used to create a client socket', + }, ': '), + }, + inaddr_any_ipv4_user = { + advertise = 'user@0.0.0.0:3301', + exp_err_msg = table.concat({ + '[instance_config] iproto.advertise', + 'INADDR_ANY (0.0.0.0) cannot be used to create a client socket', + }, ': '), + }, + inaddr_any_ipv4_user_pass = { + advertise = 'user:pass@0.0.0.0:3301', + exp_err_msg = table.concat({ + '[instance_config] iproto.advertise', + 'INADDR_ANY (0.0.0.0) cannot be used to create a client socket', + }, ': '), + }, + inaddr_any_ipv6 = { + advertise = '[::]:3301', + exp_err_msg = table.concat({ + '[instance_config] iproto.advertise', + 'in6addr_any (::) cannot be used to create a client socket', + }, ': '), + }, + inaddr_any_ipv6_user = { + advertise = 'user@[::]:3301', + exp_err_msg = table.concat({ + '[instance_config] iproto.advertise', + 'in6addr_any (::) cannot be used to create a client socket', + }, ': '), + }, + inaddr_any_ipv6_user_pass = { + advertise = 'user:pass@[::]:3301', + exp_err_msg = table.concat({ + '[instance_config] iproto.advertise', + 'in6addr_any (::) cannot be used to create a client socket', + }, ': '), + }, + zero_port = { + advertise = 'localhost:0', + exp_err_msg = table.concat({ + '[instance_config] iproto.advertise', + 'An URI with zero port cannot be used to create a client socket', + }, ': '), + }, + zero_port_user = { + advertise = 'user@localhost:0', + exp_err_msg = table.concat({ + '[instance_config] iproto.advertise', + 'An URI with zero port cannot be used to create a client socket', + }, ': '), + }, + zero_port_user_pass = { + advertise = 'user:pass@localhost:0', + exp_err_msg = table.concat({ + '[instance_config] iproto.advertise', + 'An URI with zero port cannot be used to create a client socket', + }, ': '), + }, +}) do + g[('test_bad_iproto_advertise_%s'):format(case_name)] = function() + t.assert_error_msg_equals(case.exp_err_msg, function() + instance_config:validate({ + iproto = { + advertise = case.advertise, + }, + }) + end) + end +end + +-- Successful cases for iproto.advertise. +for case_name, case in pairs({ + inet_socket = { + advertise = 'localhost:3301', + }, + inet_socket_user = { + advertise = 'user@localhost:3301', + }, + inet_socket_user_pass = { + advertise = 'user:pass@localhost:3301', + }, + unix_socket = { + advertise = 'unix/:/foo/bar.iproto', + }, + unix_socket_user = { + advertise = 'user@unix/:/foo/bar.iproto', + }, + unix_socket_user_pass = { + advertise = 'user:pass@unix/:/foo/bar.iproto', + }, +}) do + g[('test_good_iproto_advertise_%s'):format(case_name)] = function() + assert(case.advertise ~= nil) + instance_config:validate({ + iproto = { + advertise = case.advertise, + }, + }) + end +end + g.test_database = function() local iconfig = { database = { -- GitLab