mac: add new mac_ddb(4) policy

[mhorne]

Summary

Generally, access to the kernel debugger is considered to be unsafe from
a security perspective since it presents an unrestricted interface to
inspect or modify the system state, including sensitive data such as
signing keys.

However, having some access to debugger functionality on production
systems may be useful in determining the cause of a panic or hang.
Therefore, it is desirable to have an optional policy which allows
limited use of ddb(4) while disabling the functionality which could
reveal system secrets.

This loadable MAC module provides allow-lists for some ddb(4) commands
while preventing the execution of others. The commands have been broadly
grouped into three categories:

• Those which are 'safe' and will not emit sensitive data (e.g. backtrace). Generally, these commands are deterministic and don't accept arguments.
• Those which are definitively unsafe (e.g. examine <addr>, search <addr> <value>)
• Commands which may be safe to execute depending on the arguments provided (e.g. show proc <addr>).

Safe commands will be flagged as such and their execution allowed, while
all unrecognized commands will be prevented from executing.

Commands requiring extra validation can provide a function to do so.
For example, 'show proc <addr>' can be used as long as addr can be
checked against the system's list of process structures. The machinery
for this validation is added in this patch while some simple validation
functions will be added in the next.

The policy also prevents debugger backends other than ddb(4) from
executing, for example gdb(4).

TODO: man page

[markj]

Without having looked deeply at the patch set yet, it looks like the approach is generally to disable DDB commands by default and explicitly enable only those that we deem to be safe by some criteria, and the criteria is effectively, "doesn't permit arbitrary memory accesses"?

I'm not sure about the approach of maintaining a separate table of allowed commands rather than somehow embedding that info into the command definition. Over time it'll become stale and potentially incorrect, e.g., if a developer "fixes" a command to let it cast an arbitrary memory address to a structure.

[mhorne]

In D35371#802163, @markj wrote:

Without having looked deeply at the patch set yet, it looks like the approach is generally to disable DDB commands by default and explicitly enable only those that we deem to be safe by some criteria, and the criteria is effectively, "doesn't permit arbitrary memory accesses"?

For the large part, yes, this is the criteria and intent of the changes. I haven't attempted to be completely exhaustive, rather just identifying a large part of the most common/useful/default commands.

I'm not sure about the approach of maintaining a separate table of allowed commands rather than somehow embedding that info into the command definition. Over time it'll become stale and potentially incorrect, e.g., if a developer "fixes" a command to let it cast an arbitrary memory address to a structure.

Yes, this is the major downside/weakness of the current implementation. There is some room to improve it, for example we might be strict in validating the arguments of every command that is allowed, which would provide a little protection from such changes.

I'll say that I initially approached this feature attempting to modify the command definitions, e.g. having DB_COMMAND_SAFE() variants. It was quite messy and difficult to express some of what was required -- not to say it can't be done. If we do decide to proceed that way however, we effectively tie this particular security policy to becoming a permanent piece of ddb. The big reason why I went with the current approach of adding MAC hooks and a loadable module is that it puts the entire policy in one place, and keeps it separate from the actual debugger code.

The particular security requirements here are what Juniper wants, and it is believed to be generally useful enough to upstream. But if someone wanted something different (e.g. command filtering based on different criteria) the approach we take to implementing this feature might not allow for that. Is it likely that there will be other use-cases of a 'secure ddb'? 2+ decades without any such requirement suggest maybe not, but this is where I could really use some input.

[mhorne]

In D35371#803351, @mhorne wrote:

In D35371#802163, @markj wrote:

I'm not sure about the approach of maintaining a separate table of allowed commands rather than somehow embedding that info into the command definition. Over time it'll become stale and potentially incorrect, e.g., if a developer "fixes" a command to let it cast an arbitrary memory address to a structure.

Yes, this is the major downside/weakness of the current implementation. There is some room to improve it, for example we might be strict in validating the arguments of every command that is allowed, which would provide a little protection from such changes.

I'll say that I initially approached this feature attempting to modify the command definitions, e.g. having DB_COMMAND_SAFE() variants. It was quite messy and difficult to express some of what was required -- not to say it can't be done. If we do decide to proceed that way however, we effectively tie this particular security policy to becoming a permanent piece of ddb. The big reason why I went with the current approach of adding MAC hooks and a loadable module is that it puts the entire policy in one place, and keeps it separate from the actual debugger code.

The particular security requirements here are what Juniper wants, and it is believed to be generally useful enough to upstream. But if someone wanted something different (e.g. command filtering based on different criteria) the approach we take to implementing this feature might not allow for that. Is it likely that there will be other use-cases of a 'secure ddb'? 2+ decades without any such requirement suggest maybe not, but this is where I could really use some input.

I thought a bit more about this, and I think it would be reasonable to embed a DB_CMD_MEMSAFE flag in the command definitions, but keep all the logic confined to the MAC module. Memory-safe is a property of the command, so it is sensible that it be specified there, and this reduces the maintenance risk/burden. Within the module we can maintain the smaller list of commands that are allowed to execute with the help a function that validates the arguments, e.g. show thread.

I will wait for some feedback before making any changes.

[mhorne]

Update to use the new DB_CMD_MEMSAFE flag -- added and applied in D35581, D35583, D35584.

@markj I believe these updates should address your main concern.

[markj]

In D35371#806811, @mhorne wrote:

Update to use the new DB_CMD_MEMSAFE flag -- added and applied in D35581, D35583, D35584.

@markj I believe these updates should address your main concern.

Thanks! I like this version better.

[jhb]

Are you still planning on updating this? It seems to still reject all but 'show thread' currently and I suspect you don't mean to do that?

[mhorne]

Add a man_ddb(4) man page and xrefs in relevant places.

Fix building this module statically in the kernel.

Catch up with changes the DB_CMD_MEMSAFE flag changes.

[markj]

share/man/man4/mac_bsdextended.4

120 ↗

(On Diff #108075)

Seems rather silly to be maintaining these lists everywhere instead of only in mac.4...

share/man/man4/mac_ddb.4

63 ↗

(On Diff #108075)

Consider dropping "basic" and/or "usefully".

66 ↗

(On Diff #108075)

Would it be worth mentioning the CMD_MEMSAFE mechanism here?

78 ↗

(On Diff #108075)

Maybe explicitly reference gdb(4) here?

[pauamma_gundo.com]

Manual page changes are in good English but most of what they say is beyond me.