mirrors/core-lightning
1
0
Fork 0
mirror of https://github.com/ElementsProject/lightning.git synced 2025-02-22 06:41:44 +01:00
Code Issues Projects Releases Wiki Activity

lightningd: check for writability before allowing setconfig.

Browse source 
If we actually can't write it, we crash (to avoid an inconsistent
state), so sanity check FIRST.

Fixes: https://github.com/ElementsProject/lightning/issues/7964
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This commit is contained in:
Rusty Russell 2025-02-18 17:11:32 +10:30
parent ee690665eb
commit 70e28f767a
2 changed files with 147 additions and 2 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

51
lightningd/configs.c
View file

@ -579,6 +579,47 @@ static struct command_result *setconfig_success(struct command *cmd,
return command_success(cmd, response);
}
static bool file_writable(const char *fname)
{
return access(fname, W_OK) == 0;
}
static bool dir_writable(const char *fname)
{
return access(path_dirname(tmpctx, fname), W_OK) == 0;
}
/* Returns config file name if not writable */
static const char *config_not_writable(const tal_t *ctx,
struct command *cmd,
const struct opt_table *ot)
{
struct lightningd *ld = cmd->ld;
struct configvar *oldcv;
const char *fname;
/* If it exists before, we will need to replace that file (rename) */
oldcv = configvar_first(ld->configvars, opt_names_arr(tmpctx, ot));
if (oldcv && oldcv->file) {
/* We will rename */
if (!dir_writable(oldcv->file))
return oldcv->file;
}
/* If we don't have a setconfig file we'll have to create it, and
* amend the config file. */
if (!ld->setconfig_file) {
fname = base_conf_file(tmpctx, ld, NULL);
if (!dir_writable(fname))
return tal_steal(ctx, fname);
} else {
/* We will try to append config.setconfig */
if (!file_writable(ld->setconfig_file))
return tal_strdup(ctx, ld->setconfig_file);
}
return NULL;
}
static struct command_result *json_setconfig(struct command *cmd,
const char *buffer,
const jsmntok_t *obj UNNEEDED,
@ -597,11 +638,17 @@ static struct command_result *json_setconfig(struct command *cmd,
NULL))
return command_param_failed();
log_debug(cmd->ld->log, "setconfig!");
/* We don't handle DYNAMIC MULTI, at least yet! */
assert(!(ot->type & OPT_MULTI));
if (!*transient) {
const char *fname = config_not_writable(cmd, cmd, ot);
if (fname)
return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
"Cannot write to config file %s",
fname);
}
/* We use arg = NULL to tell callback it's only for testing */
if (command_check_only(cmd))
arg = NULL;

98
tests/test_misc.py
View file

@ -4068,6 +4068,104 @@ def test_setconfig(node_factory, bitcoind):
assert lines == ["# Created and update by setconfig, but you can edit this manually", "min-capacity-sat=400000"]
def test_setconfig_access(node_factory, bitcoind):
"""Test that we correctly fail (not crash) if config file/dir not writable"""
l1 = node_factory.get_node()
netconfigfile = os.path.join(l1.daemon.opts.get("lightning-dir"), TEST_NETWORK, 'config')
# It's OK if the config file doesn't exist.
l1.rpc.check("setconfig", config="min-capacity-sat", val=1000000)
# But not if we can't create it.
os.chmod(os.path.dirname(netconfigfile), 0o550)
with pytest.raises(RpcError, match=f'Cannot write to config file {netconfigfile}'):
l1.rpc.check("setconfig", config="min-capacity-sat", val=1000000)
with pytest.raises(RpcError, match=f'Cannot write to config file {netconfigfile}'):
l1.rpc.setconfig(config="min-capacity-sat", val=1000000)
# Empty config file (we need to be able to write dir)
os.chmod(os.path.dirname(netconfigfile), 0o750)
with open(netconfigfile, 'w') as file:
pass
l1.restart()
# check will fail
os.chmod(os.path.dirname(netconfigfile), 0o550)
with pytest.raises(RpcError, match=f'Cannot write to config file {netconfigfile}'):
l1.rpc.check("setconfig", config="min-capacity-sat", val=1000000)
# real write will definitely fail
with pytest.raises(RpcError, match=f'Cannot write to config file {netconfigfile}'):
l1.rpc.setconfig(config="min-capacity-sat", val=1000000)
# Transient? Don't care that we can't change it.
ret = l1.rpc.setconfig(config='min-capacity-sat', val=400001, transient=True)
assert ret == {'config':
{'config': 'min-capacity-sat',
'source': 'setconfig transient',
'value_int': 400001,
'dynamic': True}}
# db also needs to write directory!
os.chmod(os.path.dirname(netconfigfile), 0o750)
# Now put a setting in the main config file
l1.stop()
mainconfigfile = os.path.join(l1.daemon.opts.get("lightning-dir"), 'config')
with open(mainconfigfile, 'w') as file:
file.write("min-capacity-sat=100")
l1.start()
# We don't actually need to write file, just directoty.
os.chmod(mainconfigfile, 0o400)
l1.rpc.check("setconfig", config="min-capacity-sat", val=9999)
l1.rpc.setconfig(config="min-capacity-sat", val=9999)
# setconfig file exists, and its permissions matter!
setconfigfile = netconfigfile + ".setconfig"
os.chmod(setconfigfile, 0o400)
with pytest.raises(RpcError, match=f'Cannot write to config file {setconfigfile}'):
l1.rpc.check("setconfig", config="min-capacity-sat", val=1000000)
with pytest.raises(RpcError, match=f'Cannot write to config file {setconfigfile}'):
l1.rpc.setconfig(config="min-capacity-sat", val=1000000)
# Change location of setconfig file in another sub directory.
l1.stop()
includedir = os.path.join(os.path.dirname(netconfigfile), "include")
os.mkdir(includedir)
os.unlink(setconfigfile)
setconfigfile = os.path.join(includedir, "special.setconfig")
with open(netconfigfile, 'w') as file:
file.write(f"include {setconfigfile}")
with open(setconfigfile, 'w') as file:
pass
l1.start()
# Needs to be writable, to append.
os.chmod(setconfigfile, 0o400)
with pytest.raises(RpcError, match=f'Cannot write to config file {setconfigfile}'):
l1.rpc.check("setconfig", config="min-capacity-sat", val=1000000)
with pytest.raises(RpcError, match=f'Cannot write to config file {setconfigfile}'):
l1.rpc.setconfig(config="min-capacity-sat", val=1000000)
# But directory doesn't!
os.chmod(includedir, 0o500)
os.chmod(setconfigfile, 0o700)
assert l1.rpc.setconfig(config="min-capacity-sat", val=1000000) == {'config':
{'config': 'min-capacity-sat',
'source': f'{setconfigfile}:1',
'value_int': 1000000,
'dynamic': True}}
# Don't break pytest cleanup!
os.chmod(includedir, 0o700)
@unittest.skipIf(os.getenv('TEST_DB_PROVIDER', 'sqlite3') != 'sqlite3', "deletes database, which is assumed sqlite3")
def test_recover_command(node_factory, bitcoind):
l1, l2 = node_factory.get_nodes(2)