From 92013a9c34b2a667ccb59095709805d97243fe79 Mon Sep 17 00:00:00 2001 From: Renuka Manavalan <47282725+renukamanavalan@users.noreply.github.com> Date: Mon, 26 Apr 2021 14:10:44 -0700 Subject: [PATCH 1/3] disk_check: Check & mount RO as RW using tmpfs (#1569) What I did There is a bug that occasionally turn root-overlay as RO. This makes /etc & /home as RO. This blocks any new remote user login, as that needs to write into /etc & /home. This tool scans /etc & /home (or given dirs) as in RW or RO state. If RO, it could create a writable overlay using tmpfs. This is transient and stays until next reboot. Any write after the overlay will be lost upon reboot. But this allows new remote users login. How I did it Create upper & work dirs in /run/mount (tmpfs). Mount /etc & /home as lowerdirs and use the same name for final merge. This allows anyone opening a file in /etc or /home to operate on the merged overlay, transparently. How to verify it Mount any dir on tmpfs ( mount -t tmpfs tmpfs test_dir) remount as RO (mount -o remount,ro test_dir) Pass that dir to this script. (disk_check.py -d ./test_dir) Now it should be RW --- scripts/disk_check.py | 151 ++++++++++++++++++++++++++++++++++++ setup.py | 1 + tests/disk_check_test.py | 161 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 313 insertions(+) create mode 100644 scripts/disk_check.py create mode 100644 tests/disk_check_test.py diff --git a/scripts/disk_check.py b/scripts/disk_check.py new file mode 100644 index 0000000000..94959bfa1a --- /dev/null +++ b/scripts/disk_check.py @@ -0,0 +1,151 @@ +#!/usr/bin/env python3 +# -*- coding: utf-8 -*- + +""" +What: + There have been cases, where disk turns Read-only due to kernel bug. + In Read-only state, system blocks new remote user login via TACACS. + This utility is to check & make transient recovery as needed. + +How: + check for Read-Write permission. If Read-only, create writable overlay using tmpfs. + + By default "/etc" & "/home" are checked and if in Read-only state, make them Read-Write + using overlay on top of tmpfs. + + Making /etc & /home as writable lets successful new remote user login. + + If in Read-only state or in Read-Write state with the help of tmpfs overlay, + syslog ERR messages are written, to help raise alerts. + + Monit may be used to invoke it periodically, to help scan & fix and + report via syslog. + +""" + +import argparse +import os +import sys +import syslog +import subprocess + +UPPER_DIR = "/run/mount/upper" +WORK_DIR = "/run/mount/work" +MOUNTS_FILE = "/proc/mounts" + +def log_err(m): + print("Err: {}".format(m), file=sys.stderr) + syslog.syslog(syslog.LOG_ERR, m) + + +def log_info(m): + print("Info: {}".format(m)) + syslog.syslog(syslog.LOG_INFO, m) + + +def log_debug(m): + print("debug: {}".format(m)) + syslog.syslog(syslog.LOG_DEBUG, m) + + +def test_writable(dirs): + for d in dirs: + rw = os.access(d, os.W_OK) + if not rw: + log_err("{} is not read-write".format(d)) + return False + else: + log_debug("{} is Read-Write".format(d)) + return True + + +def run_cmd(cmd): + proc = subprocess.run(cmd, shell=True, text=True, capture_output=True) + ret = proc.returncode + if ret: + log_err("failed: ret={} cmd={}".format(ret, cmd)) + else: + log_info("ret={} cmd: {}".format(ret, cmd)) + + if proc.stdout: + log_info("stdout: {}".format(str(proc.stdout))) + if proc.stderr: + log_info("stderr: {}".format(str(proc.stderr))) + return ret + + +def get_dname(path_name): + return os.path.basename(os.path.normpath(path_name)) + + +def do_mnt(dirs): + if os.path.exists(UPPER_DIR): + log_err("Already mounted") + return 1 + + for i in (UPPER_DIR, WORK_DIR): + try: + os.mkdir(i) + except OSError as error: + log_err("Failed to create {}".format(i)) + return 1 + + for d in dirs: + ret = run_cmd("mount -t overlay overlay_{} -o lowerdir={}," + "upperdir={},workdir={} {}".format( + get_dname(d), d, UPPER_DIR, WORK_DIR, d)) + if ret: + break + + if ret: + log_err("Failed to mount {} as Read-Write".format(dirs)) + else: + log_info("{} are mounted as Read-Write".format(dirs)) + return ret + + +def is_mounted(dirs): + if not os.path.exists(UPPER_DIR): + return False + + onames = set() + for d in dirs: + onames.add("overlay_{}".format(get_dname(d))) + + with open(MOUNTS_FILE, "r") as s: + for ln in s.readlines(): + n = ln.strip().split()[0] + if n in onames: + log_debug("Mount exists for {}".format(n)) + return True + return False + + +def do_check(skip_mount, dirs): + ret = 0 + if not test_writable(dirs): + if not skip_mount: + ret = do_mnt(dirs) + + # Check if mounted + if (not ret) and is_mounted(dirs): + log_err("READ-ONLY: Mounted {} to make Read-Write".format(dirs)) + + return ret + + +def main(): + parser=argparse.ArgumentParser( + description="check disk for Read-Write and mount etc & home as Read-Write") + parser.add_argument('-s', "--skip-mount", action='store_true', default=False, + help="Skip mounting /etc & /home as Read-Write") + parser.add_argument('-d', "--dirs", default="/etc,/home", + help="dirs to mount") + args = parser.parse_args() + + ret = do_check(args.skip_mount, args.dirs.split(",")) + return ret + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/setup.py b/setup.py index e1bb9528fb..9b3efdb52b 100644 --- a/setup.py +++ b/setup.py @@ -65,6 +65,7 @@ def get_test_suite(): 'scripts/db_migrator.py', 'scripts/decode-syseeprom', 'scripts/dropcheck', + 'scripts/disk_check.py', 'scripts/ecnconfig', 'scripts/mmuconfig', 'scripts/fast-reboot', diff --git a/tests/disk_check_test.py b/tests/disk_check_test.py new file mode 100644 index 0000000000..40bc55f0f5 --- /dev/null +++ b/tests/disk_check_test.py @@ -0,0 +1,161 @@ +import sys +import syslog +from unittest.mock import patch +import pytest + +sys.path.append("scripts") +import disk_check + +disk_check.MOUNTS_FILE = "/tmp/proc_mounts" + +test_data = { + "0": { + "desc": "All good as /tmp is read-write", + "args": ["", "-d", "/tmp"], + "err": "" + }, + "1": { + "desc": "Not good as /tmpx is not read-write; But fix skipped", + "args": ["", "-d", "/tmpx", "-s"], + "err": "/tmpx is not read-write" + }, + "2": { + "desc": "Not good as /tmpx is not read-write; expect mount", + "args": ["", "-d", "/tmpx"], + "upperdir": "/tmp/tmpx", + "workdir": "/tmp/tmpy", + "mounts": "overlay_tmpx blahblah", + "err": "/tmpx is not read-write|READ-ONLY: Mounted ['/tmpx'] to make Read-Write", + "cmds": ['mount -t overlay overlay_tmpx -o lowerdir=/tmpx,upperdir=/tmp/tmpx,workdir=/tmp/tmpy /tmpx'] + }, + "3": { + "desc": "Not good as /tmpx is not read-write; mount fail as create of upper fails", + "args": ["", "-d", "/tmpx"], + "upperdir": "/tmpx", + "expect_ret": 1 + }, + "4": { + "desc": "Not good as /tmpx is not read-write; mount fail as upper exist", + "args": ["", "-d", "/tmpx"], + "upperdir": "/tmp", + "err": "/tmpx is not read-write|Already mounted", + "expect_ret": 1 + }, + "5": { + "desc": "/tmp is read-write, but as well mount exists; hence report", + "args": ["", "-d", "/tmp"], + "upperdir": "/tmp", + "mounts": "overlay_tmp blahblah", + "err": "READ-ONLY: Mounted ['/tmp'] to make Read-Write" + }, + "6": { + "desc": "Test another code path for good case", + "args": ["", "-d", "/tmp"], + "upperdir": "/tmp" + } +} + +err_data = "" +cmds = [] +current_tc = None + +def mount_file(d): + with open(disk_check.MOUNTS_FILE, "w") as s: + s.write(d) + + +def report_err_msg(lvl, m): + global err_data + if lvl == syslog.LOG_ERR: + if err_data: + err_data += "|" + err_data += m + + +class proc: + returncode = 0 + stdout = None + stderr = None + + def __init__(self, proc_upd = None): + if proc_upd: + self.returncode = proc_upd.get("ret", 0) + self.stdout = proc_upd.get("stdout", None) + self.stderr = proc_upd.get("stderr", None) + + +def mock_subproc_run(cmd, shell, text, capture_output): + global cmds + + upd = (current_tc["proc"][len(cmds)] + if len(current_tc.get("proc", [])) > len(cmds) else None) + cmds.append(cmd) + + return proc(upd) + + +def init_tc(tc): + global err_data, cmds, current_tc + + err_data = "" + cmds = [] + mount_file(tc.get("mounts", "")) + current_tc = tc + + +def swap_upper(tc): + tmp_u = tc["upperdir"] + tc["upperdir"] = disk_check.UPPER_DIR + disk_check.UPPER_DIR = tmp_u + + +def swap_work(tc): + tmp_w = tc["workdir"] + tc["upperdir"] = disk_check.WORK_DIR + disk_check.WORK_DIR = tmp_w + + +class TestDiskCheck(object): + def setup(self): + pass + + + @patch("disk_check.syslog.syslog") + @patch("disk_check.subprocess.run") + def test_readonly(self, mock_proc, mock_log): + global err_data, cmds + + mock_proc.side_effect = mock_subproc_run + mock_log.side_effect = report_err_msg + + for i, tc in test_data.items(): + print("-----------Start tc {}---------".format(i)) + init_tc(tc) + + with patch('sys.argv', tc["args"]): + if "upperdir" in tc: + swap_upper(tc) + + if "workdir" in tc: + # restore + swap_work(tc) + + ret = disk_check.main() + + if "upperdir" in tc: + # restore + swap_upper(tc) + + if "workdir" in tc: + # restore + swap_work(tc) + + print("ret = {}".format(ret)) + print("err_data={}".format(err_data)) + print("cmds: {}".format(cmds)) + + assert ret == tc.get("expect_ret", 0) + if "err" in tc: + assert err_data == tc["err"] + assert cmds == tc.get("cmds", []) + print("-----------End tc {}-----------".format(i)) From 64cc14e42e70feac63f531b6671ab1ac8ce52786 Mon Sep 17 00:00:00 2001 From: Renuka Manavalan <47282725+renukamanavalan@users.noreply.github.com> Date: Mon, 2 Aug 2021 09:13:15 -0700 Subject: [PATCH 2/3] disk_check updates: (#1736) 1) Set default loglevel to SYSLOG_ERR 2) Make log level configurable via args --- scripts/disk_check.py | 21 +++++++++++++++------ tests/disk_check_test.py | 17 ++++++++++++++++- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/scripts/disk_check.py b/scripts/disk_check.py index 94959bfa1a..b13f1335f0 100644 --- a/scripts/disk_check.py +++ b/scripts/disk_check.py @@ -33,19 +33,23 @@ WORK_DIR = "/run/mount/work" MOUNTS_FILE = "/proc/mounts" +chk_log_level = syslog.LOG_ERR + +def _log_msg(lvl, pfx, msg): + if lvl <= chk_log_level: + print("{}: {}".format(pfx, msg)) + syslog.syslog(lvl, msg) + def log_err(m): - print("Err: {}".format(m), file=sys.stderr) - syslog.syslog(syslog.LOG_ERR, m) + _log_msg(syslog.LOG_ERR, "Err", m) def log_info(m): - print("Info: {}".format(m)) - syslog.syslog(syslog.LOG_INFO, m) + _log_msg(syslog.LOG_INFO, "Info", m) def log_debug(m): - print("debug: {}".format(m)) - syslog.syslog(syslog.LOG_DEBUG, m) + _log_msg(syslog.LOG_DEBUG, "Debug", m) def test_writable(dirs): @@ -135,14 +139,19 @@ def do_check(skip_mount, dirs): def main(): + global chk_log_level + parser=argparse.ArgumentParser( description="check disk for Read-Write and mount etc & home as Read-Write") parser.add_argument('-s', "--skip-mount", action='store_true', default=False, help="Skip mounting /etc & /home as Read-Write") parser.add_argument('-d', "--dirs", default="/etc,/home", help="dirs to mount") + parser.add_argument('-l', "--loglvl", default=syslog.LOG_ERR, type=int, + help="log level") args = parser.parse_args() + chk_log_level = args.loglvl ret = do_check(args.skip_mount, args.dirs.split(",")) return ret diff --git a/tests/disk_check_test.py b/tests/disk_check_test.py index 40bc55f0f5..b5ad7a489c 100644 --- a/tests/disk_check_test.py +++ b/tests/disk_check_test.py @@ -56,6 +56,7 @@ } err_data = "" +max_log_lvl = -1 cmds = [] current_tc = None @@ -66,6 +67,11 @@ def mount_file(d): def report_err_msg(lvl, m): global err_data + global max_log_lvl + + if lvl > max_log_lvl: + max_log_lvl = lvl + if lvl == syslog.LOG_ERR: if err_data: err_data += "|" @@ -123,11 +129,16 @@ def setup(self): @patch("disk_check.syslog.syslog") @patch("disk_check.subprocess.run") def test_readonly(self, mock_proc, mock_log): - global err_data, cmds + global err_data, cmds, max_log_lvl mock_proc.side_effect = mock_subproc_run mock_log.side_effect = report_err_msg + with patch('sys.argv', ["", "-l", "7", "-d", "/tmp"]): + disk_check.main() + assert max_log_lvl == syslog.LOG_DEBUG + max_log_lvl = -1 + for i, tc in test_data.items(): print("-----------Start tc {}---------".format(i)) init_tc(tc) @@ -159,3 +170,7 @@ def test_readonly(self, mock_proc, mock_log): assert err_data == tc["err"] assert cmds == tc.get("cmds", []) print("-----------End tc {}-----------".format(i)) + + + assert max_log_lvl == syslog.LOG_ERR + From 688922bdaae1f5bb47a37a8b5e930ae53e1ef764 Mon Sep 17 00:00:00 2001 From: Renuka Manavalan <47282725+renukamanavalan@users.noreply.github.com> Date: Mon, 23 Aug 2021 20:50:21 -0700 Subject: [PATCH 3/3] disk_check: Script updated to run good in 201811 & 201911 (#1747) What I did Have independent subdirs for each mounted dir to avoid any collisions of files/dirs by same name. Adopt for older version of python3 How I did it Changes: Individual subdirs for each dir to be mounted subprocess args made compatible with older version of python3 (tested in version 3.5.3) How to verify it Simulate read-only state Run this script Test ssh via new tacacs user (who had not logged in earlier) --- scripts/disk_check.py | 19 +++++++++++++++---- tests/disk_check_test.py | 8 ++++++-- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/scripts/disk_check.py b/scripts/disk_check.py index b13f1335f0..4fa8d69746 100644 --- a/scripts/disk_check.py +++ b/scripts/disk_check.py @@ -21,6 +21,11 @@ Monit may be used to invoke it periodically, to help scan & fix and report via syslog. +Tidbit: + If you would like to test this script, you could simulate a RO disk + with the following command. Reboot will revert the effect. + sudo bash -c "echo u > /proc/sysrq-trigger" + """ import argparse @@ -64,7 +69,7 @@ def test_writable(dirs): def run_cmd(cmd): - proc = subprocess.run(cmd, shell=True, text=True, capture_output=True) + proc = subprocess.run(cmd, shell=True, stdout=subprocess.PIPE) ret = proc.returncode if ret: log_err("failed: ret={} cmd={}".format(ret, cmd)) @@ -72,9 +77,9 @@ def run_cmd(cmd): log_info("ret={} cmd: {}".format(ret, cmd)) if proc.stdout: - log_info("stdout: {}".format(str(proc.stdout))) + log_info("stdout: {}".format(proc.stdout.decode("utf-8"))) if proc.stderr: - log_info("stderr: {}".format(str(proc.stderr))) + log_info("stderr: {}".format(proc.stderr.decode("utf-8"))) return ret @@ -95,9 +100,15 @@ def do_mnt(dirs): return 1 for d in dirs: + d_name = get_dname(d) + d_upper = os.path.join(UPPER_DIR, d_name) + d_work = os.path.join(WORK_DIR, d_name) + os.mkdir(d_upper) + os.mkdir(d_work) + ret = run_cmd("mount -t overlay overlay_{} -o lowerdir={}," "upperdir={},workdir={} {}".format( - get_dname(d), d, UPPER_DIR, WORK_DIR, d)) + d_name, d, d_upper, d_work, d)) if ret: break diff --git a/tests/disk_check_test.py b/tests/disk_check_test.py index b5ad7a489c..ce4faad900 100644 --- a/tests/disk_check_test.py +++ b/tests/disk_check_test.py @@ -2,6 +2,7 @@ import syslog from unittest.mock import patch import pytest +import subprocess sys.path.append("scripts") import disk_check @@ -26,7 +27,7 @@ "workdir": "/tmp/tmpy", "mounts": "overlay_tmpx blahblah", "err": "/tmpx is not read-write|READ-ONLY: Mounted ['/tmpx'] to make Read-Write", - "cmds": ['mount -t overlay overlay_tmpx -o lowerdir=/tmpx,upperdir=/tmp/tmpx,workdir=/tmp/tmpy /tmpx'] + "cmds": ['mount -t overlay overlay_tmpx -o lowerdir=/tmpx,upperdir=/tmp/tmpx/tmpx,workdir=/tmp/tmpy/tmpx /tmpx'] }, "3": { "desc": "Not good as /tmpx is not read-write; mount fail as create of upper fails", @@ -90,9 +91,12 @@ def __init__(self, proc_upd = None): self.stderr = proc_upd.get("stderr", None) -def mock_subproc_run(cmd, shell, text, capture_output): +def mock_subproc_run(cmd, shell, stdout): global cmds + assert shell == True + assert stdout == subprocess.PIPE + upd = (current_tc["proc"][len(cmds)] if len(current_tc.get("proc", [])) > len(cmds) else None) cmds.append(cmd)