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 3d7618d8bf.  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

@@ -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

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,
+    CTLFLAG_RWTUN, &debugger_on_panic, 0,
-    &debugger_on_panic, 0, "Run debugger on kernel panic");
+    "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,
+    CTLFLAG_RWTUN, &debugger_on_recursive_panic, 0,
-    &debugger_on_recursive_panic, 0, "Run debugger on recursive kernel panic");
+    "Run debugger on recursive kernel panic");
 
 int debugger_on_trap = 0;
 SYSCTL_INT(_debug, OID_AUTO, debugger_on_trap,
-    CTLFLAG_RWTUN | CTLFLAG_SECURE,
+    CTLFLAG_RWTUN, &debugger_on_trap, 0,
-    &debugger_on_trap, 0, "Run debugger on kernel trap before panic");
+    "Run debugger on kernel trap before panic");
 
 #ifdef KDB_TRACE
 static int trace_on_panic = 1;

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 (!kdb_backend_permitted(be, curthread->td_ucred)) {
-		if (mac_kdb_check_backend(be) != 0) {
-			printf("MAC prevented execution of KDB backend: %s\n",
-			    be->dbbe_name);
 			/* Unhandled breakpoint traps are fatal. */
 			handled = 1;
 			break;
 		}
-#endif
 		handled = be->dbbe_trap(type, code);
 		if (be == kdb_dbbe)
 			break;

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);

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)
 {