lkrg-org / lkrg

Linux Kernel Runtime Guard
https://lkrg.org
Other
404 stars 72 forks source link

Consider updating shadow credentials on commit_creds, etc. #220

Open solardiz opened 1 year ago

solardiz commented 1 year ago

This is item 4 in https://github.com/lkrg-org/lkrg/issues/215#issuecomment-1195744061 and alternative to #219:

Unfortunately, exploits can currently make us keep our off flag set for too long (over some credentials-using code in and calls into the kernel) by abusing override_creds. So maybe the option of accepting that and "weakening" "off must be off" checks (replacing them with resetting of off) doesn't make things much worse (they're pretty bad anyhow). Similarly, maybe even updating shadow credentials on commit_creds is within consideration (and then ditto on override_creds and revert_creds instead of having the off flag at all), which would let us simplify LKRG a lot. As I now recall @Adam-pi3 and I had discussed privately before, this is something we could do if we accept relying only on pCFI to protect from exploits' calls to (or ROP'ing into) those functions. (Our decision not to update on commit_creds pre-dates the introduction of pCFI into LKRG.) Maybe that's the way to go, if we can't reasonably do much better anyway?

Right now, we're paying the price of complexity of hooking many other interfaces instead and we have to match specific kernel versions' logic in updating our off flag and we get false positives if we got anything wrong or if we haven't yet adapted to a kernel change. Maybe that complexity and price is not justified - maybe there is not a corresponding increase in protection compared to simply hooking commit_creds, override_creds, and revert_creds, checking pCFI, and updating our shadow credentials there.

Presumably, an exploit that would bypass pCFI could also have abused override_creds setting of off with our current approach. Such exploit would currently need to do its job without triggering an "off must be off" code path and quickly revert its credentials override afterwards. Is that enough exploitation complexity increase to justify our current protection complexity (compared to the simpler alternative suggested above)?

solardiz commented 1 year ago

instead of having the off flag at all

I was wrong about this part - we would still need that flag for its short-lived uses across (sys)calls such as setuid (edit: would be across commit_creds then), where the credentials are briefly inconsistent, as long as we can sometimes perform asynchronous validation of task credentials (that is, not only when the task itself attempts to use the credentials, but also e.g. as part of our system+tasks integrity checking on timer).