linux-hardening - Re: [PATCH v2 1/2] landlock: Extend documentation for kernel support

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <ZfgOf2dHBVT4WUcp@debian>
Date: Mon, 18 Mar 2024 10:50:42 +0100
From: Alejandro Colomar <alx@...nel.org>
To: Mickaël Salaün <mic@...ikod.net>
Cc: Günther Noack <gnoack@...gle.com>,
	Paul Moore <paul@...l-moore.com>,
	"Serge E . Hallyn" <serge@...lyn.com>,
	Konstantin Meskhidze <konstantin.meskhidze@...wei.com>,
	Shervin Oloumi <enlightened@...omium.org>,
	linux-hardening@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-security-module@...r.kernel.org,
	Kees Cook <keescook@...omium.org>
Subject: Re: [PATCH v2 1/2] landlock: Extend documentation for kernel support

Hi Mickaël, Günther,

Sorry for the delay!

On Thu, Mar 07, 2024 at 11:21:57AM +0100, Mickaël Salaün wrote:
> CCing Alejandro
> 
> On Tue, Feb 27, 2024 at 05:32:20PM +0100, Günther Noack wrote:
> > On Tue, Feb 27, 2024 at 12:05:49PM +0100, Mickaël Salaün wrote:
> > > Extend the kernel support section with one subsection for build time
> > > configuration and another for boot time configuration.
> > > 
> > > Extend the boot time subsection with a concrete example.
> > > 
> > > Update the journalctl command to include the boot option.
> > > 
> > > Cc: Günther Noack <gnoack@...gle.com>
> > > Cc: Kees Cook <keescook@...omium.org>
> > > Signed-off-by: Mickaël Salaün <mic@...ikod.net>
> > > ---
> > > 
> > > Changes since v1:
> > > * New patch, suggested by Kees Cook.
> > > ---
> > >  Documentation/userspace-api/landlock.rst | 57 +++++++++++++++++++++---
> > >  1 file changed, 51 insertions(+), 6 deletions(-)

[...]

> > > +
> > > +  lsm=landlock,lockdown,yama,integrity,apparmor
> > > +
> > > +After a reboot, we can check that Landlock is up and running by looking at
> > > +kernel logs:
> > > +
> > > +.. code-block:: console
> > > +
> > > +    # dmesg | grep landlock || journalctl -kb -g landlock
> > > +    [    0.000000] Command line: [...] lsm=landlock,lockdown,yama,integrity,apparmor
> > > +    [    0.000000] Kernel command line: [...] lsm=landlock,lockdown,yama,integrity,apparmor
> > > +    [    0.000000] LSM: initializing lsm=lockdown,capability,landlock,yama,integrity,apparmor
> > > +    [    0.000000] landlock: Up and running.
> > > +
> > > +Note that according to the built time kernel configuration,
> > 
> > s/built time/build time/
> >                  ^
> 
> OK

Here, this should actually be "build-time" since it works as an
adjective.

> 
> > 
> > It feels like the phrase "according to" could be slightly more specific here.
> > 
> > To paraphrase Alejandro Colomar, "Note that" is usually redundant.
> > https://lore.kernel.org/all/0aafcdd6-4ac7-8501-c607-9a24a98597d7@gmail.com/
> > 
> > I'd suggest:
> > 
> >   The kernel may be configured at build time to always load the ``lockdown`` and
> >   ``capability`` LSMs.  In that case, these LSMs will appear at the beginning of
> >   the ``LSM: initializing`` log line as well, even if they are not configured in
> >   the boot loader.

LGTM

> 
> OK, I integrated your suggestion.  I guess `capability` is not really
> considered an LSM but it would be too confusing and out of scope for an
> user documentation to explain that.
> 
> > 
> > > +``lockdown,capability,`` may always stay at the beginning of the ``LSM:
> > > +initializing lsm=`` list even if they are not configured with the bootloader,
> > 
> > Nit: The man pages spell this in two words as "boot loader".
> 
> OK, I'll use "boot loader" too.
> 
> > 
> > 
> > > +which is OK.
> > > +
> > > +Network support
> > > +---------------
> > > +
> > >  To be able to explicitly allow TCP operations (e.g., adding a network rule with
> > >  ``LANDLOCK_ACCESS_NET_BIND_TCP``), the kernel must support TCP
> > >  (``CONFIG_INET=y``).  Otherwise, sys_landlock_add_rule() returns an
> > > 
> > > base-commit: b4007fd27206c478a4b76e299bddf4a71787f520
> > > -- 
> > > 2.44.0
> > > 
> > 
> > Reviewed-by: Günther Noack <gnoack@...gle.com>
> 
> Thanks!

Reviewed-by: Alejandro Colomar <alx@...nel.org>

Have a lovely day!
Alex

-- 
<https://www.alejandro-colomar.es/>
Looking for a remote C programming job at the moment.

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)