Skip to content

Commit cbd3aa5

Browse files
committed
Merge rpccookieperms_octal_compat-28+knots
2 parents 66b8c66 + e57bd40 commit cbd3aa5

File tree

4 files changed

+87
-9
lines changed

4 files changed

+87
-9
lines changed

src/httprpc.cpp

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -300,12 +300,18 @@ static bool InitRPCAuthentication()
300300
std::optional<fs::perms> cookie_perms{std::nullopt};
301301
auto cookie_perms_arg{gArgs.GetArg("-rpccookieperms")};
302302
if (cookie_perms_arg) {
303-
auto perm_opt = InterpretPermString(*cookie_perms_arg);
304-
if (!perm_opt) {
305-
LogError("Invalid -rpccookieperms=%s; must be one of 'owner', 'group', or 'all'.", *cookie_perms_arg);
306-
return false;
303+
if (*cookie_perms_arg == "0") {
304+
cookie_perms = std::nullopt;
305+
} else if (cookie_perms_arg->empty() || *cookie_perms_arg == "1") {
306+
// leave at default
307+
} else {
308+
auto perm_opt = InterpretPermString(*cookie_perms_arg);
309+
if (!perm_opt) {
310+
LogError("Invalid -rpccookieperms=%s; must be one of 'owner', 'group', or 'all'.", *cookie_perms_arg);
311+
return false;
312+
}
313+
cookie_perms = *perm_opt;
307314
}
308-
cookie_perms = *perm_opt;
309315
}
310316

311317
assert(strRPCUserColonPass.empty()); // Only support initializing once

src/rpc/request.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,11 @@ bool GenerateAuthCookie(std::string* cookie_out, const std::pair<std::optional<f
113113
if (filepath_tmp.empty()) {
114114
return true; // -norpccookiefile
115115
}
116+
try {
117+
fs::remove(filepath_tmp);
118+
} catch (const fs::filesystem_error& e) {
119+
// ignore
120+
}
116121
file.open(filepath_tmp);
117122
if (!file.is_open()) {
118123
LogWarning("Unable to open cookie authentication file %s for writing", fs::PathToString(filepath_tmp));
@@ -132,6 +137,11 @@ bool GenerateAuthCookie(std::string* cookie_out, const std::pair<std::optional<f
132137
file.close();
133138

134139
fs::path filepath = GetAuthCookieFile(false);
140+
try {
141+
fs::remove(filepath);
142+
} catch (const fs::filesystem_error& e) {
143+
// ignore
144+
}
135145
if (!RenameOver(filepath_tmp, filepath)) {
136146
LogWarning("Unable to rename cookie authentication file %s to %s", fs::PathToString(filepath_tmp), fs::PathToString(filepath));
137147
return false;

src/util/fs_helpers.cpp

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,22 @@ std::string PermsToSymbolicString(fs::perms p)
360360
return perm_str;
361361
}
362362

363+
static std::optional<unsigned> StringToOctal(const std::string& str)
364+
{
365+
unsigned ret = 0;
366+
for (char c : str) {
367+
if (c < '0' || c > '7') return std::nullopt;
368+
ret = (ret << 3) | (c - '0');
369+
}
370+
return ret;
371+
}
372+
373+
static auto ConvertPermsToOctal(const std::string& str) noexcept -> std::optional<unsigned>
374+
{
375+
if ((str.length() == 3) || (str.length() == 4)) return StringToOctal(str);
376+
return std::nullopt;
377+
}
378+
363379
std::optional<fs::perms> InterpretPermString(const std::string& s)
364380
{
365381
if (s == "owner") {
@@ -371,6 +387,8 @@ std::optional<fs::perms> InterpretPermString(const std::string& s)
371387
return fs::perms::owner_read | fs::perms::owner_write |
372388
fs::perms::group_read |
373389
fs::perms::others_read;
390+
} else if (auto octal_perms = ConvertPermsToOctal(s)) {
391+
return static_cast<fs::perms>(*octal_perms);
374392
} else {
375393
return std::nullopt;
376394
}

test/functional/rpc_users.py

Lines changed: 48 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,15 @@ def test_auth(self, node, user, password, wallet_restrictions=None):
168168
assert b'"Requested wallet does not exist or is not loaded"' in resp.data
169169

170170
def test_rpccookieperms(self):
171-
p = {"owner": 0o600, "group": 0o640, "all": 0o644}
171+
p = {
172+
"owner": 0o600,
173+
"group": 0o640,
174+
"all": 0o644,
175+
"440": 0o440,
176+
"0640": 0o640,
177+
"444": 0o444,
178+
"1660": 0o1660,
179+
}
172180

173181
if platform.system() == 'Windows':
174182
self.log.info(f"Skip cookie file permissions checks as OS detected as: {platform.system()=}")
@@ -177,7 +185,7 @@ def test_rpccookieperms(self):
177185
self.log.info('Check cookie file permissions can be set using -rpccookieperms')
178186

179187
cookie_file_path = self.nodes[1].chain_path / '.cookie'
180-
PERM_BITS_UMASK = 0o777
188+
PERM_BITS_UMASK = 0o7777
181189

182190
def test_perm(perm: Optional[str]):
183191
if not perm:
@@ -190,17 +198,38 @@ def test_perm(perm: Optional[str]):
190198
actual_perms = file_stat.st_mode & PERM_BITS_UMASK
191199
expected_perms = p[perm]
192200
assert_equal(expected_perms, actual_perms)
201+
return actual_perms
193202

194203
# Remove any leftover rpc{user|password} config options from previous tests
195204
self.nodes[1].replace_in_config([("rpcuser", "#rpcuser"), ("rpcpassword", "#rpcpassword")])
196205

197206
self.log.info('Check default cookie permission')
198-
test_perm(None)
207+
default_perms = test_perm(None)
199208

200209
self.log.info('Check custom cookie permissions')
201-
for perm in ["owner", "group", "all"]:
210+
for perm in p.keys():
202211
test_perm(perm)
203212

213+
self.log.info('Check leaving cookie permissions alone')
214+
unassigned_perms = os.stat(self.nodes[1].chain_path / 'debug.log').st_mode & PERM_BITS_UMASK
215+
self.restart_node(1, extra_args=["-rpccookieperms=0"])
216+
actual_perms = os.stat(cookie_file_path).st_mode & PERM_BITS_UMASK
217+
assert_equal(unassigned_perms, actual_perms)
218+
self.restart_node(1, extra_args=["-norpccookieperms"])
219+
actual_perms = os.stat(cookie_file_path).st_mode & PERM_BITS_UMASK
220+
assert_equal(unassigned_perms, actual_perms)
221+
222+
self.log.info('Check -norpccookieperms -rpccookieperms')
223+
self.restart_node(1, extra_args=["-rpccookieperms=0", "-rpccookieperms=1"])
224+
actual_perms = os.stat(cookie_file_path).st_mode & PERM_BITS_UMASK
225+
assert_equal(default_perms, actual_perms)
226+
self.restart_node(1, extra_args=["-norpccookieperms", "-rpccookieperms"])
227+
actual_perms = os.stat(cookie_file_path).st_mode & PERM_BITS_UMASK
228+
assert_equal(default_perms, actual_perms)
229+
self.restart_node(1, extra_args=["-rpccookieperms=1660", "-norpccookieperms", "-rpccookieperms"])
230+
actual_perms = os.stat(cookie_file_path).st_mode & PERM_BITS_UMASK
231+
assert_equal(default_perms, actual_perms)
232+
204233
def test_norpccookiefile(self, node0_cookie_path):
205234
assert self.nodes[0].is_node_stopped(), "We expect previous test to stopped the node"
206235
assert not node0_cookie_path.exists()
@@ -293,13 +322,28 @@ def run_test(self):
293322
cookie_path = self.nodes[0].chain_path / ".cookie"
294323
cookie_path_tmp = self.nodes[0].chain_path / ".cookie.tmp"
295324
cookie_path_tmp.mkdir()
325+
cookie_path_tmp_subdir = cookie_path_tmp / "subdir"
326+
cookie_path_tmp_subdir.mkdir()
296327
self.nodes[0].assert_start_raises_init_error(expected_msg=init_error)
328+
cookie_path_tmp_subdir.rmdir()
297329
cookie_path_tmp.rmdir()
298330
assert not cookie_path.exists()
299331
self.restart_node(0)
300332
assert cookie_path.exists()
301333
self.stop_node(0)
302334

335+
cookie_path.mkdir()
336+
cookie_path_subdir = cookie_path / "subdir"
337+
cookie_path_subdir.mkdir()
338+
self.nodes[0].assert_start_raises_init_error(expected_msg=init_error)
339+
cookie_path_subdir.rmdir()
340+
cookie_path.rmdir()
341+
342+
self.log.info('Check that a non-writable cookie file will get replaced gracefully')
343+
cookie_path.mkdir(mode=1)
344+
self.restart_node(0)
345+
self.stop_node(0)
346+
303347
self.test_rpccookieperms()
304348

305349
self.test_norpccookiefile(cookie_path)

0 commit comments

Comments
 (0)