linux-kernel - Re: [PATCH] Fix dmesg_restrict build failure with CONFIG_EMBEDDED=y and CONFIG

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <AANLkTi=4AQst9bNXrvwN6sZc2CM9rccYbERYYQkVK99q@mail.gmail.com>
Date:	Sat, 13 Nov 2010 12:22:15 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Dan Rosenberg <drosenberg@...curity.com>,
	James Morris <jmorris@...ei.org>
Cc:	Joe Perches <joe@...ches.com>, LKML <linux-kernel@...r.kernel.org>,
	Ingo Molnar <mingo@...e.hu>, Eugene Teo <eugeneteo@...nel.org>,
	Kees Cook <kees.cook@...onical.com>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] Fix dmesg_restrict build failure with CONFIG_EMBEDDED=y
 and CONFIG_PRINTK=n

On Sat, Nov 13, 2010 at 10:25 AM, Dan Rosenberg
<drosenberg@...curity.com> wrote:
>
>>
>> Anyway, suggested replacement patch attached. Comments?
>>
>
> The desired behavior was to allow a reader with CAP_SYS_ADMIN to open
> the syslog via /proc/kmsg and continue reading it even after dropping
> capabilities, which is why it was placed where it was.  I see no problem
> with moving it back out to do_syslog, but ideally the same behavior
> should be replicated.

Hmm. No wonder I missed that. The security interface is totally
idiotic. If the intention is for /proc/kmsg security checks to be done
at open time, then dammit, that logic should _not_ be inside some
random security policy.

So I missed the intention, because the code is written in such an odd
way. Those security hooks were obviously done as a
"search-and-replace" kind of thing, rather than trying to make sense.

I suspect "from_file" should never be passed to the security hook,
since the only point would be exactly that "do security checks of
/proc/kmsg at open time" - which I think is better done totally
independent of the security model - otherwise the security models just
inevitably just always do fundamentally different things.

Security people should be the ones to know that the way to do security
is to make it obvious, instead of having totally crazy interfaces for
hooks that make no sense. "Not making sense" is how obvious patches
then miss the point of the check.

So what happens now is that the capability-based logic thinks the
rules are about "open time", while the _other_ security rules seem to
think it's about read time (_and_ open time - they just ignore the
whole from_file).

So which one is right? Making it a case of "random security models can
implement totally random semantics" is just stupid.

So my suspicion is that the intent was to just do the check at open
time, and the confusing interface just means that selinux and others
didn't even realize what the whole intent of that "from_file" thing
was. Why not just fix that. How does this (UNTESTED!) patch look?

I've added James Morris to the recipients list. Comments?

(The diffstat says that this adds more lines than it removes, but that
is misleading: it is due to actually commenting the rule that checks
are done open-time for /proc/kmsg)

                         Linus

View attachment "patch.diff" of type "text/x-patch" (6188 bytes)