Date: Thu, 30 Mar 2023 14:57:31 GMT From: Mark Johnston <markj@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: cab1056105e3 - main - kdb: Modify securelevel policy Message-ID: <202303301457.32UEvV26011463@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=cab1056105e3fdadf6f2b8cc745cfecfb0411755 commit cab1056105e3fdadf6f2b8cc745cfecfb0411755 Author: Mark Johnston <markj@FreeBSD.org> AuthorDate: 2022-10-25 17:57:44 +0000 Commit: Mark Johnston <markj@FreeBSD.org> CommitDate: 2023-03-30 14:45:00 +0000 kdb: Modify securelevel policy Currently, sysctls which enable KDB in some way are flagged with CTLFLAG_SECURE, meaning that you can't modify them if securelevel > 0. This is so that KDB cannot be used to lower a running system's securelevel, see commit 3d7618d8bf0b7. However, the newer mac_ddb(4) restricts DDB operations which could be abused to lower securelevel while retaining some ability to gather useful debugging information. To enable the use of KDB (specifically, DDB) on systems with a raised securelevel, change the KDB sysctl policy: rather than relying on CTLFLAG_SECURE, add a check of the current securelevel to kdb_trap(). If the securelevel is raised, only pass control to the backend if MAC specifically grants access; otherwise simply check to see if mac_ddb vetoes the request, as before. Add a new secure sysctl, debug.kdb.enter_securelevel, to override this behaviour. That is, the sysctl lets one enter a KDB backend even with a raised securelevel, so long as it is set before the securelevel is raised. Reviewed by: mhorne, stevek MFC after: 1 month Sponsored by: Juniper Networks Sponsored by: Klara, Inc. Differential Revision: https://reviews.freebsd.org/D37122 --- share/man/man7/security.7 | 7 ++++-- sys/kern/kern_shutdown.c | 12 +++++------ sys/kern/subr_kdb.c | 46 +++++++++++++++++++++++++++++++++------- sys/security/mac/mac_framework.h | 1 + sys/security/mac/mac_kdb.c | 9 ++++++++ 5 files changed, 59 insertions(+), 16 deletions(-) diff --git a/share/man/man7/security.7 b/share/man/man7/security.7 index f87833ece112..e3f11765d083 100644 --- a/share/man/man7/security.7 +++ b/share/man/man7/security.7 @@ -28,7 +28,7 @@ .\" .\" $FreeBSD$ .\" -.Dd September 29, 2022 +.Dd March 30, 2023 .Dt SECURITY 7 .Os .Sh NAME @@ -537,7 +537,10 @@ kernel modules (see may not be loaded or unloaded. The kernel debugger may not be entered using the .Va debug.kdb.enter -sysctl. +sysctl unless a +.Xr MAC 9 +policy grants access, for example using +.Xr mac_ddb 4 . A panic or trap cannot be forced using the .Va debug.kdb.panic , .Va debug.kdb.panic_str diff --git a/sys/kern/kern_shutdown.c b/sys/kern/kern_shutdown.c index 70de6b691fd3..4544e217802d 100644 --- a/sys/kern/kern_shutdown.c +++ b/sys/kern/kern_shutdown.c @@ -129,18 +129,18 @@ int debugger_on_panic = 0; int debugger_on_panic = 1; #endif SYSCTL_INT(_debug, OID_AUTO, debugger_on_panic, - CTLFLAG_RWTUN | CTLFLAG_SECURE, - &debugger_on_panic, 0, "Run debugger on kernel panic"); + CTLFLAG_RWTUN, &debugger_on_panic, 0, + "Run debugger on kernel panic"); static bool debugger_on_recursive_panic = false; SYSCTL_BOOL(_debug, OID_AUTO, debugger_on_recursive_panic, - CTLFLAG_RWTUN | CTLFLAG_SECURE, - &debugger_on_recursive_panic, 0, "Run debugger on recursive kernel panic"); + CTLFLAG_RWTUN, &debugger_on_recursive_panic, 0, + "Run debugger on recursive kernel panic"); int debugger_on_trap = 0; SYSCTL_INT(_debug, OID_AUTO, debugger_on_trap, - CTLFLAG_RWTUN | CTLFLAG_SECURE, - &debugger_on_trap, 0, "Run debugger on kernel trap before panic"); + CTLFLAG_RWTUN, &debugger_on_trap, 0, + "Run debugger on kernel trap before panic"); #ifdef KDB_TRACE static int trace_on_panic = 1; diff --git a/sys/kern/subr_kdb.c b/sys/kern/subr_kdb.c index b1bf197be3dc..ff981cdfe47c 100644 --- a/sys/kern/subr_kdb.c +++ b/sys/kern/subr_kdb.c @@ -77,6 +77,7 @@ struct trapframe *kdb_frame = NULL; static int kdb_break_to_debugger = KDB_BREAK_TO_DEBUGGER; static int kdb_alt_break_to_debugger = KDB_ALT_BREAK_TO_DEBUGGER; +static int kdb_enter_securelevel = 0; KDB_BACKEND(null, NULL, NULL, NULL, NULL); @@ -103,7 +104,7 @@ SYSCTL_PROC(_debug_kdb, OID_AUTO, current, "currently selected KDB backend"); SYSCTL_PROC(_debug_kdb, OID_AUTO, enter, - CTLTYPE_INT | CTLFLAG_RW | CTLFLAG_SECURE | CTLFLAG_MPSAFE, NULL, 0, + CTLTYPE_INT | CTLFLAG_RW | CTLFLAG_MPSAFE, NULL, 0, kdb_sysctl_enter, "I", "set to enter the debugger"); @@ -133,13 +134,18 @@ SYSCTL_PROC(_debug_kdb, OID_AUTO, stack_overflow, "set to cause a stack overflow"); SYSCTL_INT(_debug_kdb, OID_AUTO, break_to_debugger, - CTLFLAG_RWTUN | CTLFLAG_SECURE, + CTLFLAG_RWTUN, &kdb_break_to_debugger, 0, "Enable break to debugger"); SYSCTL_INT(_debug_kdb, OID_AUTO, alt_break_to_debugger, - CTLFLAG_RWTUN | CTLFLAG_SECURE, + CTLFLAG_RWTUN, &kdb_alt_break_to_debugger, 0, "Enable alternative break to debugger"); +SYSCTL_INT(_debug_kdb, OID_AUTO, enter_securelevel, + CTLFLAG_RWTUN | CTLFLAG_SECURE, + &kdb_enter_securelevel, 0, + "Maximum securelevel to enter a KDB backend"); + /* * Flag to indicate to debuggers why the debugger was entered. */ @@ -489,6 +495,34 @@ kdb_dbbe_select(const char *name) return (EINVAL); } +static bool +kdb_backend_permitted(struct kdb_dbbe *be, struct ucred *cred) +{ + int error; + + error = securelevel_gt(cred, kdb_enter_securelevel); +#ifdef MAC + /* + * Give MAC a chance to weigh in on the policy: if the securelevel is + * not raised, then MAC may veto the backend, otherwise MAC may + * explicitly grant access. + */ + if (error == 0) { + error = mac_kdb_check_backend(be); + if (error != 0) { + printf("MAC prevented execution of KDB backend: %s\n", + be->dbbe_name); + return (false); + } + } else if (mac_kdb_grant_backend(be) == 0) { + error = 0; + } +#endif + if (error != 0) + printf("refusing to enter KDB with elevated securelevel\n"); + return (error == 0); +} + /* * Enter the currently selected debugger. If a message has been provided, * it is printed first. If the debugger does not support the enter method, @@ -733,15 +767,11 @@ kdb_trap(int type, int code, struct trapframe *tf) cngrab(); for (;;) { -#ifdef MAC - if (mac_kdb_check_backend(be) != 0) { - printf("MAC prevented execution of KDB backend: %s\n", - be->dbbe_name); + if (!kdb_backend_permitted(be, curthread->td_ucred)) { /* Unhandled breakpoint traps are fatal. */ handled = 1; break; } -#endif handled = be->dbbe_trap(type, code); if (be == kdb_dbbe) break; diff --git a/sys/security/mac/mac_framework.h b/sys/security/mac/mac_framework.h index 31951c97a69e..8a1de6fe13e1 100644 --- a/sys/security/mac/mac_framework.h +++ b/sys/security/mac/mac_framework.h @@ -214,6 +214,7 @@ void mac_ipq_reassemble(struct ipq *q, struct mbuf *m); void mac_ipq_update(struct mbuf *m, struct ipq *q); int mac_kdb_check_backend(struct kdb_dbbe *be); +int mac_kdb_grant_backend(struct kdb_dbbe *be); int mac_kenv_check_dump(struct ucred *cred); int mac_kenv_check_get(struct ucred *cred, char *name); diff --git a/sys/security/mac/mac_kdb.c b/sys/security/mac/mac_kdb.c index 9082ec7d4580..08bb3eb743f1 100644 --- a/sys/security/mac/mac_kdb.c +++ b/sys/security/mac/mac_kdb.c @@ -39,6 +39,15 @@ #include <security/mac/mac_internal.h> #include <security/mac/mac_policy.h> +int +mac_kdb_grant_backend(struct kdb_dbbe *be) +{ + int error = 0; + + MAC_POLICY_GRANT_NOSLEEP(kdb_check_backend, be); + return (error); +} + int mac_kdb_check_backend(struct kdb_dbbe *be) {
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202303301457.32UEvV26011463>