kdb: Conditionally permit breaking to DDB with a raised securelevel

[markj]

Summary

The mac_ddb policy permits only explicitly flagged DDB commands. Since
these cannot be used to lower the system's securelevel, we can enable
toggling of KDB-specific sysctls even when securelevel > 0.

The implementation adds a new sysctl flag, CTLFLAG_KDB_SECURE, which has
the same semantics as CTLFLAG_SECURE except that a MAC policy may be
used to override the securelevel check. This way, mac_ddb can enable
use of the sysctls, so a privileged user can break into DDB and run
allowed commands when mac_ddb is loaded and securelevel > 0.

[mhorne]

Don't you also want to change the flags for debug.kdb.enter, debug.kdb.panic, etc?

Thinking more broadly about this.. doesn't it make more sense to have the securelevel check at the entry point of the debugger, rather than for each of these sysctls? As it stands, CTLFLAG_SECURE prevents one from purposefully entering the debugger, or modifying the sysctl values, but crucially it does not prevent entering the debugger altogether on a system with raised securelevel if e.g. debug.debugger_on_panic is already set to 1. I think such a restructuring would prevent the need for a new CTLFLAG, but maybe there is a reason things are not done this way.

share/man/man9/sysctl.9

909

Perhaps, work in a cross-reference to security(7), where securelevel is described.

sys/kern/kern_sysctl.c

2252

Or, just have the added condition match the existing style.

sys/security/mac/mac_policy.h

1028

Looks like we are good here.

sys/sys/sysctl.h

115

Just noting that this leaves... 6 free bits?

[mhorne]

In D37106#842717, @mhorne wrote:

I think such a restructuring would prevent the need for a new CTLFLAG, but maybe there is a reason things are not done this way.

Forget that part; it would not. What you have is a good solution to one of the shortcomings of mac_ddb.

[markj]

In D37106#842717, @mhorne wrote:

Don't you also want to change the flags for debug.kdb.enter, debug.kdb.panic, etc?

Thinking more broadly about this.. doesn't it make more sense to have the securelevel check at the entry point of the debugger, rather than for each of these sysctls? As it stands, CTLFLAG_SECURE prevents one from purposefully entering the debugger, or modifying the sysctl values, but crucially it does not prevent entering the debugger altogether on a system with raised securelevel if e.g. debug.debugger_on_panic is already set to 1.

Hmm, that's true, but securelevel goes out of the window once the kernel has panicked, and debugger_on_panic is generally going to be set to 0 on any kind of release build of FreeBSD.

The problem at hand is that one might want to set securelevel > 0 _and_ still be able to enter DDB (with debug.kdb.enter, for instance) or toggle DDB settings. Normally that's not safe, but mac_ddb prevents all commands that could be abused to lower securelevel and then return to a running system. In the absence of mac_ddb, it made sense for debug.kdb.enter etc. to be flagged with CTLFLAG_SECURE, but now we can relax the restriction on KDB/DDB sysctls and allow them to be frobbed so long as mac_ddb says it's ok. In other words, I'm assuming here that MEMSAFE commands cannot be abused to lower securelevel.

Checking the securelevel at the entry point of the debugger doesn't solve this problem since we want to permit use of the debugger while securelevel > 0.

Note also that with securelevel > 0, kldunload is not permitted, so one can't remove the mac_ddb policy.

[mhorne]

In D37106#842736, @markj wrote:

In D37106#842717, @mhorne wrote:

Don't you also want to change the flags for debug.kdb.enter, debug.kdb.panic, etc?

Thinking more broadly about this.. doesn't it make more sense to have the securelevel check at the entry point of the debugger, rather than for each of these sysctls? As it stands, CTLFLAG_SECURE prevents one from purposefully entering the debugger, or modifying the sysctl values, but crucially it does not prevent entering the debugger altogether on a system with raised securelevel if e.g. debug.debugger_on_panic is already set to 1.

Hmm, that's true, but securelevel goes out of the window once the kernel has panicked, and debugger_on_panic is generally going to be set to 0 on any kind of release build of FreeBSD.

It goes out the window in the sense that no kernel state is truly reliable after a panic. Practically though, it seems just fine to me. With mac_ddb we already trust certain kernel state post-panic, such as the DB_CMD_MEMSAFE status of commands, or the strcmp in mac_ddb_check_backend() to verify that the debugger backend matches "ddb".

The problem at hand is that one might want to set securelevel > 0 _and_ still be able to enter DDB (with debug.kdb.enter, for instance) or toggle DDB settings. Normally that's not safe, but mac_ddb prevents all commands that could be abused to lower securelevel and then return to a running system. In the absence of mac_ddb, it made sense for debug.kdb.enter etc. to be flagged with CTLFLAG_SECURE, but now we can relax the restriction on KDB/DDB sysctls and allow them to be frobbed so long as mac_ddb says it's ok. In other words, I'm assuming here that MEMSAFE commands cannot be abused to lower securelevel.

Checking the securelevel at the entry point of the debugger doesn't solve this problem since we want to permit use of the debugger while securelevel > 0.

Note also that with securelevel > 0, kldunload is not permitted, so one can't remove the mac_ddb policy.

No no, I understand the problem at hand. I totally agree that in order to play nicely with a raised securelevel, a loaded mac_kdb policy must be allowed to say "I will ensure that access to the debugger is secure", and override the decision to block.

Here's what I was getting at described a little more clearly:
Presently, kdb has a loosely defined security policy w.r.t. securelevel, which we could describe as: "when securelevel is set (> 0), forbid access/modification to kdb-related sysctls". I am questioning if it is sensible for the kdb-level policy to be implemented at the sysctl level. If we are thinking more seriously about the notion of security policies for the kernel debugger, then I believe what we are really hoping to implement is: "access to the kernel debugger is forbidden when securelevel is set". But per my unlikely example, it shows that there is already a hole, due to how it is implemented today.

The above description would be amended to say "access to the kernel debugger is forbidden when securelevel is set, unless a loaded mac_kdb policy allows it".

I think how you have chosen to implement this is just fine given how things are done already. If there is more work in this area planned, then I am saying we should consider shedding CTLFLAG_SECURE from kdb sysctls, and have the kdb code itself handle the securelevel and MAC checks.

[markj]

In D37106#842915, @mhorne wrote:

No no, I understand the problem at hand. I totally agree that in order to play nicely with a raised securelevel, a loaded mac_kdb policy must be allowed to say "I will ensure that access to the debugger is secure", and override the decision to block.

Here's what I was getting at described a little more clearly:
Presently, kdb has a loosely defined security policy w.r.t. securelevel, which we could describe as: "when securelevel is set (> 0), forbid access/modification to kdb-related sysctls". I am questioning if it is sensible for the kdb-level policy to be implemented at the sysctl level. If we are thinking more seriously about the notion of security policies for the kernel debugger, then I believe what we are really hoping to implement is: "access to the kernel debugger is forbidden when securelevel is set". But per my unlikely example, it shows that there is already a hole, due to how it is implemented today.

The above description would be amended to say "access to the kernel debugger is forbidden when securelevel is set, unless a loaded mac_kdb policy allows it".

I think how you have chosen to implement this is just fine given how things are done already. If there is more work in this area planned, then I am saying we should consider shedding CTLFLAG_SECURE from kdb sysctls, and have the kdb code itself handle the securelevel and MAC checks.

Ok, I think I see what you mean now, sorry. Restating what you wrote, we could perhaps remove CTLFLAG_SECURE from some of the kdb sysctls and simply refuse to enter the kernel debugger if securelevel is raised unless mac_ddb says it's ok. That's a cleaner approach than what I did, and I'll try to implement it. The outline looks like this:

• Remove CTLFLAG_SECURE from debug.{debugger_on_panic,debugger_on_recursive_panic,debugger_on_trap} and debug.kdb.{enter,break_to_debugger,alt_break_to_debugger}.
• Add a new function, mac_kdb_grant_backend(), which returns 0 if a MAC policy explicitly permits the specified backend.
• Add a new CTLFLAG_SECURE sysctl, debug.kdb.enter_securelevel, default 0, which lets you enter kdb so long as the securelevel is less than or equal to the value. (So that developers can override the seatbelt if they really want to use the debugger with raised securelevel.)
• Replace the existing mac_kdb_check_backend() check with the following: if securelevel > kdb_enter_securelevel then return mac_kdb_grant_backend(), else return mac_kdb_check_backend().
• Not strictly required, but modify kdb_enter() to perform the same check.

This way, there's no need for a new MAC hook nor a new sysctl flag. Does that look right?

[mhorne]

In D37106#842931, @markj wrote:

• Remove CTLFLAG_SECURE from debug.{debugger_on_panic,debugger_on_recursive_panic,debugger_on_trap} and debug.kdb.{enter,break_to_debugger,alt_break_to_debugger}.

I think this is the right subset.. we still forbid intentionally panicking the system, which is desirable.

• Add a new function, mac_kdb_grant_backend(), which returns 0 if a MAC policy explicitly permits the specified backend.
• Add a new CTLFLAG_SECURE sysctl, debug.kdb.enter_securelevel, default 0, which lets you enter kdb so long as the securelevel is less than or equal to the value. (So that developers can override the seatbelt if they really want to use the debugger with raised securelevel.)
• Replace the existing mac_kdb_check_backend() check with the following: if securelevel > kdb_enter_securelevel then return mac_kdb_grant_backend(), else return mac_kdb_check_backend().
• Not strictly required, but modify kdb_enter() to perform the same check.

This way, there's no need for a new MAC hook nor a new sysctl flag. Does that look right?

Well, you are still proposing a new MAC hook, but it is the sysctl flag that is the important savings. Yes, this sketch looks right to me.

[markj]

In D37106#842959, @mhorne wrote:

In D37106#842931, @markj wrote:

• Remove CTLFLAG_SECURE from debug.{debugger_on_panic,debugger_on_recursive_panic,debugger_on_trap} and debug.kdb.{enter,break_to_debugger,alt_break_to_debugger}.

I think this is the right subset.. we still forbid intentionally panicking the system, which is desirable.

• Add a new function, mac_kdb_grant_backend(), which returns 0 if a MAC policy explicitly permits the specified backend.
• Add a new CTLFLAG_SECURE sysctl, debug.kdb.enter_securelevel, default 0, which lets you enter kdb so long as the securelevel is less than or equal to the value. (So that developers can override the seatbelt if they really want to use the debugger with raised securelevel.)
• Replace the existing mac_kdb_check_backend() check with the following: if securelevel > kdb_enter_securelevel then return mac_kdb_grant_backend(), else return mac_kdb_check_backend().
• Not strictly required, but modify kdb_enter() to perform the same check.

This way, there's no need for a new MAC hook nor a new sysctl flag. Does that look right?

Well, you are still proposing a new MAC hook, but it is the sysctl flag that is the important savings. Yes, this sketch looks right to me.

Maybe I'm using the wrong terminology. Indeed, I added a new MAC function, mac_kdb_grant_backend(), but it uses the same MAC hook, kdb_check_backend, as mac_kdb_check_backend().

[markj]

This was abandoned in favour of an alternate approach, committed in cab1056105e3fdadf6f2b8cc745cfecfb0411755.