Linux Security Modules development
 help / color / mirror / Atom feed
* LSM policy options for new GPIO kernel driver interface
From: Weber, Matthew L Collins @ 2021-08-02 17:08 UTC (permalink / raw)
  To: selinux@vger.kernel.org, linux-security-module@vger.kernel.org
  Cc: Graziano, David D Collins

All,

Since the 5.10 kernel, the GPIO subsystem has migrated from a sysfs based GPIO export method [1] (everything is a file) to a character device[2] + library[3].  The new framework[2] provides users with signal debouncing and other features that benefit embedded products.  The legacy method[1] allowed fine policy control of who can export / set / get the GPIO state.  We have not found a similar security policy path with the new approach.  Has anyone brainstormed strategies for the new character device-based interface without adding a userspace broker to enforce another level of rules?  The ideal case would be to keep all the controls within the SELinux refpolicy such that testing can be all-inclusive.

I'd be interested in what people think, such that I can prepare a university research project submission for Fall 2021 to build a prototype. 

Best Regards,
--
Matt Weber

[1] https://www.kernel.org/doc/html/latest/driver-api/gpio/legacy.html#sysfs-interface-for-userspace-optional
[2] https://www.kernel.org/doc/html/latest/driver-api/gpio/using-gpio.html
[3] https://git.kernel.org/pub/scm/libs/libgpiod/libgpiod.git/about/

^ permalink raw reply

* RE: [RFC][PATCH v2 06/12] diglim: Interfaces - digest_list_add, digest_list_del
From: Roberto Sassu @ 2021-08-02 16:54 UTC (permalink / raw)
  To: Mimi Zohar, gregkh@linuxfoundation.org, mchehab+huawei@kernel.org
  Cc: linux-integrity@vger.kernel.org,
	linux-security-module@vger.kernel.org, linux-doc@vger.kernel.org,
	linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <3e6a54d4be87a3eafc45c85d013250d17aa0835e.camel@linux.ibm.com>

> From: Roberto Sassu
> Sent: Monday, August 2, 2021 5:13 PM
> > From: Mimi Zohar [mailto:zohar@linux.ibm.com]
> > Sent: Monday, August 2, 2021 4:42 PM
> > Hi Roberto,
> >
> > On Fri, 2021-07-30 at 13:16 +0000, Roberto Sassu wrote:
> >
> > > The reason of storing the actions performed by IMA on the
> > > digest lists helps to determine for which purpose they can be
> > > used. If digest lists are used only for measurement purpose,
> > > it should be sufficient that digest lists are measured. The
> > > same applies for appraisal.
> >
> > Is that assumption correct?   How would you know if the digests lists
> > are only being used one way and not the other.  For example, assuming
> > that the digest lists are stored on protected media, the digest lists
> > could be measured, but would not necessarily be appraised.
> 
> Hi Mimi
> 
> the actions performed by IMA on the digest lists are recorded
> in the digest_list_item structure. These can be retrieved when
> IMA calls diglim_digest_get_info() (actually it is the OR of the
> actions for the digest lists that contain the digest passed as a
> query).
> 
> At the moment, DIGLIM can only know whether a digest list
> has been measured or not (with the return value of
> ima_measure_critical_data()). In the next patch set, I add the
> changes to get the actions from the integrity_iint_cache().
> 
> > > > Adding the kernel_read_file() "READING_DIGEST_LIST" support in IMA
> > does
> > > > not seem to be optional.  IMA would then be calculating the digest list
> > > > file hash twice, once in kernel_read_file() and then, again, in
> > > > ima_measure_critical_data().
> > >
> > > I didn't include also this part: I retrieve the integrity_iint_cache for
> > > the opened file descriptor and I get the flags from there. If the
> > > IMA_MEASURED flag is set, it is not necessary to call also
> > > ima_measure_critical_data().
> >
> > Right, assuming the file is in policy, the digest would already be
> > stored in the iint cache.
> >
> > > > > > I understand that with your changes to ima_measure_critical_data(),
> > > > > > which are now in next-integrity-testing branch, allow IMA to calculate
> > > > > > the file data hash.
> > > > >
> > > > > Yes, correct. But actually there is another useful use case.
> > > > > If digest lists are not in the format supported by the kernel,
> > > > > the user space parser has to convert them before uploading
> > > > > them to the kernel.
> > > > >
> > > > > ima_measure_critical_data() would in this case measure
> > > > > the converted digest list (it is written directly, without
> > > > > sending the file path). It is easier to attest the result,
> > > > > instead of determining whether the user space parser
> > > > > produced the expected result (by checking the files it
> > > > > read).
> > > >
> > > > The application to properly convert the digest list file data into the
> > > > appropriate format would need to be trusted.  I'm concerned that not
> > > > requiring the converted data to be signed and the signature verified is
> > > > introducing a new integrity gap.  Perhaps between an LSM policy,
> > > > limiting which files may be read by the application, and an IMA policy,
> > > > requiring all files read by this application to be measured and the
> > > > signature verified, this integrity gap could be averted.
> > >
> > > It is the weakest point in the chain, yes. Relying on existing LSMs
> > > didn't seem to me a good idea, as:
> > > - a new policy must be installed
> > > - we must be sure that the policy is really enforced
> > > - we need to support different LSMs (SELinux for Fedora,
> > >   Apparmor for SUSE)
> > > - there might be no LSM we can rely on
> > >
> > > For these reasons, I developed a new LSM. Its purpose is to
> > > identify the user space parser and for each file it opens, ensure
> > > that the file has been measured or appraised by IMA. If one of
> > > these actions are missing, it will not be set in the digest list the
> > > user space parser uploads to the kernel (which means that IMA
> > > will ignore the digest list for that specific action).
> >
> > Properly identifying (all) user space parser(s) would be critical.  It
> > would be simpler and  safer to require the converted data be signed.

When a process directly uploads a buffer to the kernel, the actions are
added to a digest list depending on the result of ima_measure_critical_data()
and from the actions attached to the process credentials and set by the
new LSM.

If a process fails the identification, the actions in the process credentials
remain zero and the digest lists the process uploads will be ignored by IMA.

The actions in the process credentials are set with the actions performed
on the executable by IMA, only if the digest of the executable is found in
a digest list and the digest list type is COMPACT_PARSER. The parser is
statically linked.

The digest list for the parser can be generated at the end of the
building process and signed similarly to kernel modules (for SUSE,
with pesign-obs-integration). This is the only exception to handle,
other packages are not affected.

After the parser has been identified, each file operation is monitored.
The LSM has to explicitly perform a second open to ensure that
the file is measured/appraised before the integrity_iint_cache structure
is retrieved (because IMA is called after all LSMs).

If an action is missing from the integrity_iint_cache structure, it
will be cleared by the LSM in the actions attached to the process
credentials, and will not be added to the digest list being uploaded.

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Li Jian, Shi Yanli

> I agree, it would be much easier. However, it would require changes
> to the building infrastructure of Linux distribution vendors, which
> might limit the applicability of DIGLIM.
> 
> With the user space parser taking care of the conversion, distributions
> can do appraisal of executables and shared libraries with an update of:
> - the kernel: to add DIGLIM
> - dracut: to add required digest lists in the initial ram disk
> - rpm (plugin): to extract the RPM header and its signature and write
>   them to a file that is uploaded to the kernel by the user space parser
> 
> I'm planning to append the signature at the end of the RPM header
> (and use appraise_type=modsig) to avoid the dependency on the
> 'initramfs: add support for xattrs in the initial ram disk' patch set
> (which I might try to resume in the future).
> 
> Thanks
> 
> Roberto
> 
> HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
> Managing Director: Li Peng, Li Jian, Shi Yanli
> 
> > thanks,
> >
> > Mimi


^ permalink raw reply

* RE: [RFC][PATCH v2 06/12] diglim: Interfaces - digest_list_add, digest_list_del
From: Roberto Sassu @ 2021-08-02 15:12 UTC (permalink / raw)
  To: Mimi Zohar, gregkh@linuxfoundation.org, mchehab+huawei@kernel.org
  Cc: linux-integrity@vger.kernel.org,
	linux-security-module@vger.kernel.org, linux-doc@vger.kernel.org,
	linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <3e6a54d4be87a3eafc45c85d013250d17aa0835e.camel@linux.ibm.com>

> From: Mimi Zohar [mailto:zohar@linux.ibm.com]
> Sent: Monday, August 2, 2021 4:42 PM
> Hi Roberto,
> 
> On Fri, 2021-07-30 at 13:16 +0000, Roberto Sassu wrote:
> 
> > The reason of storing the actions performed by IMA on the
> > digest lists helps to determine for which purpose they can be
> > used. If digest lists are used only for measurement purpose,
> > it should be sufficient that digest lists are measured. The
> > same applies for appraisal.
> 
> Is that assumption correct?   How would you know if the digests lists
> are only being used one way and not the other.  For example, assuming
> that the digest lists are stored on protected media, the digest lists
> could be measured, but would not necessarily be appraised.

Hi Mimi

the actions performed by IMA on the digest lists are recorded
in the digest_list_item structure. These can be retrieved when
IMA calls diglim_digest_get_info() (actually it is the OR of the
actions for the digest lists that contain the digest passed as a
query).

At the moment, DIGLIM can only know whether a digest list
has been measured or not (with the return value of
ima_measure_critical_data()). In the next patch set, I add the
changes to get the actions from the integrity_iint_cache().

> > > Adding the kernel_read_file() "READING_DIGEST_LIST" support in IMA
> does
> > > not seem to be optional.  IMA would then be calculating the digest list
> > > file hash twice, once in kernel_read_file() and then, again, in
> > > ima_measure_critical_data().
> >
> > I didn't include also this part: I retrieve the integrity_iint_cache for
> > the opened file descriptor and I get the flags from there. If the
> > IMA_MEASURED flag is set, it is not necessary to call also
> > ima_measure_critical_data().
> 
> Right, assuming the file is in policy, the digest would already be
> stored in the iint cache.
> 
> > > > > I understand that with your changes to ima_measure_critical_data(),
> > > > > which are now in next-integrity-testing branch, allow IMA to calculate
> > > > > the file data hash.
> > > >
> > > > Yes, correct. But actually there is another useful use case.
> > > > If digest lists are not in the format supported by the kernel,
> > > > the user space parser has to convert them before uploading
> > > > them to the kernel.
> > > >
> > > > ima_measure_critical_data() would in this case measure
> > > > the converted digest list (it is written directly, without
> > > > sending the file path). It is easier to attest the result,
> > > > instead of determining whether the user space parser
> > > > produced the expected result (by checking the files it
> > > > read).
> > >
> > > The application to properly convert the digest list file data into the
> > > appropriate format would need to be trusted.  I'm concerned that not
> > > requiring the converted data to be signed and the signature verified is
> > > introducing a new integrity gap.  Perhaps between an LSM policy,
> > > limiting which files may be read by the application, and an IMA policy,
> > > requiring all files read by this application to be measured and the
> > > signature verified, this integrity gap could be averted.
> >
> > It is the weakest point in the chain, yes. Relying on existing LSMs
> > didn't seem to me a good idea, as:
> > - a new policy must be installed
> > - we must be sure that the policy is really enforced
> > - we need to support different LSMs (SELinux for Fedora,
> >   Apparmor for SUSE)
> > - there might be no LSM we can rely on
> >
> > For these reasons, I developed a new LSM. Its purpose is to
> > identify the user space parser and for each file it opens, ensure
> > that the file has been measured or appraised by IMA. If one of
> > these actions are missing, it will not be set in the digest list the
> > user space parser uploads to the kernel (which means that IMA
> > will ignore the digest list for that specific action).
> 
> Properly identifying (all) user space parser(s) would be critical.  It
> would be simpler and  safer to require the converted data be signed.

I agree, it would be much easier. However, it would require changes
to the building infrastructure of Linux distribution vendors, which
might limit the applicability of DIGLIM.

With the user space parser taking care of the conversion, distributions
can do appraisal of executables and shared libraries with an update of:
- the kernel: to add DIGLIM
- dracut: to add required digest lists in the initial ram disk
- rpm (plugin): to extract the RPM header and its signature and write
  them to a file that is uploaded to the kernel by the user space parser

I'm planning to append the signature at the end of the RPM header
(and use appraise_type=modsig) to avoid the dependency on the
'initramfs: add support for xattrs in the initial ram disk' patch set
(which I might try to resume in the future).

Thanks

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Li Jian, Shi Yanli

> thanks,
> 
> Mimi


^ permalink raw reply

* Re: [RFC][PATCH v2 06/12] diglim: Interfaces - digest_list_add, digest_list_del
From: Mimi Zohar @ 2021-08-02 15:01 UTC (permalink / raw)
  To: Roberto Sassu, gregkh@linuxfoundation.org,
	mchehab+huawei@kernel.org
  Cc: linux-integrity@vger.kernel.org,
	linux-security-module@vger.kernel.org, linux-doc@vger.kernel.org,
	linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org,
	Igor Stoppa
In-Reply-To: <96c7cd3d19254e84a6cb45b2a940e944@huawei.com>

On Mon, 2021-08-02 at 08:14 +0000, Roberto Sassu wrote:
> > From: Roberto Sassu [mailto:roberto.sassu@huawei.com]
> > Sent: Friday, July 30, 2021 4:25 PM
> > > From: Mimi Zohar [mailto:zohar@linux.ibm.com]
> > > Sent: Friday, July 30, 2021 4:03 PM
> > > Hi Roberto,
> > >
> > > On Fri, 2021-07-30 at 13:16 +0000, Roberto Sassu wrote:
> > > > > From: Mimi Zohar [mailto:zohar@linux.ibm.com]
> > > > > Sent: Friday, July 30, 2021 2:40 PM
> > >
> > > > > "critical data", in this context, should probably be used for verifying
> > > > > the in memory file digests and other state information haven't been
> > > > > compromised.
> > > >
> > > > Actually, this is what we are doing currently. To keep the
> > > > implementation simple, once the file or the buffer are uploaded
> > > > to the kernel, they will not be modified, just accessed through
> > > > the indexes.
> > >
> > > My main concern about digest lists is their integrity, from loading the
> > > digest lists to their being stored in memory.  A while back, there was
> > > some work on defining a write once memory allocator.  I don't recall
> > > whatever happened to it.  This would be a perfect usecase for that
> > > memory allocator.
> > 
> > Adding Igor in CC.
> > 
> > Regarding loading, everything uploaded to the kernel is carefully
> > evaluated. This should not be a concern. Regarding making them
> > read-only, probably if you can subvert digest lists you can also
> > remove the read-only protection (unless you use an hypervisor).
> 
> I briefly talked with Igor. He also agreed with that, and added that
> it could make it more difficult for an attacker to also disable the
> protection. However, he is not planning to submit an update soon,
> so I wouldn't consider this an option for now.

Hi Roberto, Greg,

As long as others understand and agree to the risk, the IMA details can
be worked out.

thanks,

Mimi


^ permalink raw reply

* Re: [RFC][PATCH v2 06/12] diglim: Interfaces - digest_list_add, digest_list_del
From: Mimi Zohar @ 2021-08-02 14:42 UTC (permalink / raw)
  To: Roberto Sassu, gregkh@linuxfoundation.org,
	mchehab+huawei@kernel.org
  Cc: linux-integrity@vger.kernel.org,
	linux-security-module@vger.kernel.org, linux-doc@vger.kernel.org,
	linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <555bf01bee4b4ea7a9bee658366d535a@huawei.com>

Hi Roberto,

On Fri, 2021-07-30 at 13:16 +0000, Roberto Sassu wrote:

> The reason of storing the actions performed by IMA on the
> digest lists helps to determine for which purpose they can be
> used. If digest lists are used only for measurement purpose,
> it should be sufficient that digest lists are measured. The
> same applies for appraisal.

Is that assumption correct?   How would you know if the digests lists
are only being used one way and not the other.  For example, assuming
that the digest lists are stored on protected media, the digest lists
could be measured, but would not necessarily be appraised.

> > Adding the kernel_read_file() "READING_DIGEST_LIST" support in IMA does
> > not seem to be optional.  IMA would then be calculating the digest list
> > file hash twice, once in kernel_read_file() and then, again, in
> > ima_measure_critical_data().
> 
> I didn't include also this part: I retrieve the integrity_iint_cache for
> the opened file descriptor and I get the flags from there. If the
> IMA_MEASURED flag is set, it is not necessary to call also
> ima_measure_critical_data().

Right, assuming the file is in policy, the digest would already be
stored in the iint cache.

> > > > I understand that with your changes to ima_measure_critical_data(),
> > > > which are now in next-integrity-testing branch, allow IMA to calculate
> > > > the file data hash.
> > >
> > > Yes, correct. But actually there is another useful use case.
> > > If digest lists are not in the format supported by the kernel,
> > > the user space parser has to convert them before uploading
> > > them to the kernel.
> > >
> > > ima_measure_critical_data() would in this case measure
> > > the converted digest list (it is written directly, without
> > > sending the file path). It is easier to attest the result,
> > > instead of determining whether the user space parser
> > > produced the expected result (by checking the files it
> > > read).
> > 
> > The application to properly convert the digest list file data into the
> > appropriate format would need to be trusted.  I'm concerned that not
> > requiring the converted data to be signed and the signature verified is
> > introducing a new integrity gap.  Perhaps between an LSM policy,
> > limiting which files may be read by the application, and an IMA policy,
> > requiring all files read by this application to be measured and the
> > signature verified, this integrity gap could be averted.
> 
> It is the weakest point in the chain, yes. Relying on existing LSMs
> didn't seem to me a good idea, as:
> - a new policy must be installed
> - we must be sure that the policy is really enforced
> - we need to support different LSMs (SELinux for Fedora,
>   Apparmor for SUSE)
> - there might be no LSM we can rely on
> 
> For these reasons, I developed a new LSM. Its purpose is to
> identify the user space parser and for each file it opens, ensure
> that the file has been measured or appraised by IMA. If one of
> these actions are missing, it will not be set in the digest list the
> user space parser uploads to the kernel (which means that IMA
> will ignore the digest list for that specific action).

Properly identifying (all) user space parser(s) would be critical.  It
would be simpler and  safer to require the converted data be signed.

thanks,

Mimi


^ permalink raw reply

* RE: [RFC][PATCH v2 06/12] diglim: Interfaces - digest_list_add, digest_list_del
From: Roberto Sassu @ 2021-08-02  8:14 UTC (permalink / raw)
  To: Roberto Sassu, Mimi Zohar, gregkh@linuxfoundation.org,
	mchehab+huawei@kernel.org
  Cc: linux-integrity@vger.kernel.org,
	linux-security-module@vger.kernel.org, linux-doc@vger.kernel.org,
	linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org,
	Igor Stoppa
In-Reply-To: <bd0787e0ee4f47baa41abf47976e536c@huawei.com>

> From: Roberto Sassu [mailto:roberto.sassu@huawei.com]
> Sent: Friday, July 30, 2021 4:25 PM
> > From: Mimi Zohar [mailto:zohar@linux.ibm.com]
> > Sent: Friday, July 30, 2021 4:03 PM
> > Hi Roberto,
> >
> > On Fri, 2021-07-30 at 13:16 +0000, Roberto Sassu wrote:
> > > > From: Mimi Zohar [mailto:zohar@linux.ibm.com]
> > > > Sent: Friday, July 30, 2021 2:40 PM
> >
> > > > "critical data", in this context, should probably be used for verifying
> > > > the in memory file digests and other state information haven't been
> > > > compromised.
> > >
> > > Actually, this is what we are doing currently. To keep the
> > > implementation simple, once the file or the buffer are uploaded
> > > to the kernel, they will not be modified, just accessed through
> > > the indexes.
> >
> > My main concern about digest lists is their integrity, from loading the
> > digest lists to their being stored in memory.  A while back, there was
> > some work on defining a write once memory allocator.  I don't recall
> > whatever happened to it.  This would be a perfect usecase for that
> > memory allocator.
> 
> Adding Igor in CC.
> 
> Regarding loading, everything uploaded to the kernel is carefully
> evaluated. This should not be a concern. Regarding making them
> read-only, probably if you can subvert digest lists you can also
> remove the read-only protection (unless you use an hypervisor).

I briefly talked with Igor. He also agreed with that, and added that
it could make it more difficult for an attacker to also disable the
protection. However, he is not planning to submit an update soon,
so I wouldn't consider this an option for now.

Thanks

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Li Jian, Shi Yanli

> Thanks
> 
> Roberto
> 
> HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
> Managing Director: Li Peng, Li Jian, Shi Yanli
> 
> > thanks,
> >
> > Mimi


^ permalink raw reply

* Re: [PATCH v2 1/4] landlock.7: Add a new page to introduce Landlock
From: G. Branden Robinson @ 2021-07-31 13:48 UTC (permalink / raw)
  To: Alejandro Colomar (man-pages)
  Cc: Mickaël Salaün, Jann Horn, Jonathan Corbet, Kees Cook,
	Randy Dunlap, Vincent Dagonneau, landlock, linux-kernel,
	linux-man, linux-security-module, Mickaël Salaün,
	Michael Kerrisk
In-Reply-To: <e88fe0d9-1330-3de4-53e1-4b72360ce7d3@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 3649 bytes --]

Hi, Alex,

[man, that CC list makes me cringe--this is all style issues and groff
release history, skip freely]

At 2021-07-31T12:51:30+0200, Alejandro Colomar (man-pages) wrote:
> Hi Branden, Mickaël,
> > One of the things I did after the groff 1.22.4 release (December
> > 2018) was to split groff_man(7) into two pages.  The one you've
> > linked is the terser reference for seasoned (perhaps salty) man page
> > writers.  Near the top of it you'll find this.
[...]
> Hmmmm, I can't find that text on my Debian Sid (with a bit of experimental)
> groff_man(7).  Not even in the SEE ALSO.

That's because Debian is still shipping groff 1.22.4, even in unstable.
That's not shocking; I think Colin Watson was hesitant to ship a release
candidate and that was all the groff team had ready at the time.  (I'm
the most active developer but not the project lead or release manager;
I've shied away from those responsibilities.)

> Re-reading this, we've been doing it wrong (as you pointed out in
> another thread) with macro names with variable part.

I do think it is wise to have a markup distinction between constant and
variable parts of C (or C preprocessor) symbol names.  Admittedly, font
style distinctions can get lost in terminal copy-and-paste, but we can't
solve everything in plain text alone.

> I wasn't very convinced by the current usage of the man pages, but it
> was already current, so I just followed it :/
> 
> I'll try to follow this from now.

The man-pages project has some style rules for visible output that are
not in alignment with what groff does, but the only one that comes to
mind is the style used for man page names (man-pages: bold; groff:
italics).  I have a plan for resolving that on the rendering end[1].

> Ahh, I missed this text, as it was neither under .I nor .IR, and only
> had a fast read of the page.  The 3rd paragraph hints that you should
> only use .IR when really needed, and use .I otherwise.  But someone
> new to man-pages, who probably did never read this page (or read some
> specific paragraphs of it when needed), and found the extensive (and
> somewhat incorrect) usage of .IR in place of .I in the current man
> pages, might be confused by all of that inconsistency and hard-to-find
> (except by those who already know where it is (reference to Pirates of
> the Caribbean intended :) )) information.

Yes, it would really help if we (groff) could do a release.  :-/

> > I'd appreciate feedback from anyone on how I can improve the above.
> 
> I think it's great.  But unless one reads the page with some time (and
> not only the bullets), one might think that the man page is
> incomplete.  Maybe groff_man_style(7) is better suited for newbies,
> but I can't tell...  I couldn't find it.

It's in groff Git and in the man-pages curated collection,
<https://man7.org/linux/man-pages/man7/groff_man_style.7.html>.

Michael apparently re-pulls from groff Git HEAD every time he does a
man-pages release, which makes the groff man pages massively more up to
date (by 2.5 years and counting) than what most distributions have.

> I'm not sure, but maybe (I always get confused by these things)?:
> 
> [
> each _of the_ OverlayFS layers and merge hierarchies _is_ standalone and
> _contains_
> _its_ own set of files and directories,
> which is different from bind mounts.
> ]
> 
> And still I'm not sure about the last "mounts".

Yes, I think that's an improvement, and "bind mounts" could be made
singular: "a bind mount".

Regards,
Branden

[1] https://lists.gnu.org/archive/html/groff/2020-08/msg00068.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH v2 1/4] landlock.7: Add a new page to introduce Landlock
From: Alejandro Colomar (man-pages) @ 2021-07-31 11:02 UTC (permalink / raw)
  To: G. Branden Robinson
  Cc: Mickaël Salaün, Jann Horn, Jonathan Corbet, Kees Cook,
	Randy Dunlap, Vincent Dagonneau, landlock, linux-kernel,
	linux-man, linux-security-module, Mickaël Salaün,
	Michael Kerrisk
In-Reply-To: <20210731001529.ggiknccl74akaahk@localhost.localdomain>

Hi Branden,

On 7/31/21 2:15 AM, G. Branden Robinson wrote:
> Hi, Alex!
> 
> At 2021-07-30T14:59:52+0200, Alejandro Colomar (man-pages) wrote:
>> Yes, they were because of semantic newlines.
>>
>> The "rules" are:
>>
>> Follow mainly "semantic newlines" style (forgetting about the line
>> length), which will give you a text that (mostly) fits into 75 or 80
>> columns.
>>
>> If after doing that there are some lines that exceed the 75 or 80
>> column right margin, consider fixing that line by breaking it at a
>> different point or maybe breaking it further.  The 80 column limit is
>> a hard limit (I can't read anything past the 80 col), while the 75
>> limit is a bit softer (that's for allowing quotes in reviews) (if
>> fitting a line into col 75 would break it in a weird way, don't do
>> it).
>>
>> If I didn't explain myself enough, please tell me.
> 
> I'm a little puzzled by the above.  Semantic newlines have little to do
> with the output line length in *roff systems.  They arose due to a Bell
> Labs Unix Room practice, popularized by Brian Kernighan.  Brandon Rhodes
> has a backgrounder on this[1].
> 
> Man pages tend to be really flexible with respect to output line length.
> This is one reason the groff man macros expose a user-settable LL
> register.  The main limitations on line length are people using tbl(1)
> tables or disabling filling (with the .nf request or in .EX/.EE
> examples).  Another limitation is that as lines get shorter, it becomes
> hard to set the page headers and footers without them overlapping.
> 
> For the first two points there is not much the macro package can do;
> both tbl(1) and filling disablement leave the placement of line breaks
> in the hands of the document author, and they can abuse that power by
> "oversetting" a line; that is, making it longer than the configured line
> length.
> 
> The third point is a problem the macro package can overcome with some
> effort, by measuring the lengths of the components that go into a header
> or footer an abbreviating them.  This is not a theoretical concern;
> Erlang supplies some man pages with insanely long names[2], and you can
> see the problem in footers today on the man-pages site for any page
> groff ships, because Michael pulls from our Git repository (to my great
> relief, because I fix documentation errors and make other improvements
> all the time) and our version identifier has gotten crazily long because
> we're on the order of one thousand commits since the last release
> candidate, and gnulib's git-version-gen uses release tags, commit count
> since the tag, _and_ an abbreviated commit ID to generate the version
> string.  For my tree right now that's "1.23.0.rc1.999-7ae6d".
> 
> Here's a specimen of how that works out in a rendered page:
> <https://man7.org/linux/man-pages/man1/neqn.1.html> (scroll to the
> bottom).
> 
> I've fixed the problem for long page names for the next groff release,
> but it involved some string-manipulation gymnastics[3].  I haven't yet
> factored those out into their own (private) macro which I can also call
> when preparing the page footer.

I think you misunderstood the context here.

I meant all of that about input, i.e., the text of the patch itself, 
text to be added to a man page source text.

What I meant is that when you break lines semantically (when writing a 
patch),
and initially forget about the 80 (or 75) column right margin,
most of the lines you write will already (as a side effect of breaking 
lines semantically) be within the 80 (or 75) right margin.

For those that still don't fit into 80 characters after doing that, 
break further (or at some other point that may also break nicely 
semantically); otherwise, I won't see the text when editing the man page 
on my 80-col terminal.

For those that still don't fit into 75 characters after doing that,
consider breaking further, but only if doing so seems easy and lines 
break nicely.  I can still see after line 75, and I can do some effort 
to scroll an email a few columns if needed (when many quotes move the 
text further to the right).  So if the source code would break in a 
weird way because of forcing a 75 col right margin, please ignore that 
margin.


I hope I was clear this time.


BTW, thanks for your mail.  It wasn't related to what I meant, but was 
interesting :=)

Regards,

Alex


> 
> Regards,
> Branden
> 
> [1] https://rhodesmill.org/brandon/2012/one-sentence-per-line/
> [2] CosNotifyChannelAdmin_StructuredProxyPushSupplier(3erl)
> [3] https://git.savannah.gnu.org/cgit/groff.git/commit/?id=b7f38e8a1d698e1078d7c215d08fde57d8e919b9
> 


-- 
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/

^ permalink raw reply

* Re: [PATCH v2 1/4] landlock.7: Add a new page to introduce Landlock
From: Alejandro Colomar (man-pages) @ 2021-07-31 10:51 UTC (permalink / raw)
  To: G. Branden Robinson, Mickaël Salaün
  Cc: Jann Horn, Jonathan Corbet, Kees Cook, Randy Dunlap,
	Vincent Dagonneau, landlock, linux-kernel, linux-man,
	linux-security-module, Mickaël Salaün, Michael Kerrisk
In-Reply-To: <20210730233931.lbtq67esmeuo4o6d@localhost.localdomain>

Hi Branden, Mickaël,

On 7/31/21 1:39 AM, G. Branden Robinson wrote:
> Hi, Mickaël!
> 
> I'm going to rearrange your message to reply to it to put the shortest
> point first, as I am nervous of people tiring of my info dumps,
> especially with such an efflorescent CC list.
> 
> At 2021-07-30T14:15:48+0200, Mickaël Salaün wrote:
>>>> +The rule will only allow reading the file hierarchy
>>>> +.IR /usr .
>>
>> Why ".IR" is correct here?
> 
> Because you don't want a space or a line break in the output between
> "/usr" and the period.
> 
> Line breaks in *roff input usually mean "insert a word break here".[1]
> 
> [the long version]
>> When do we really need .IR? Isn't `.I "foo bar"` the same as `.IR foo
>> bar`? What do you use roman for?
>>
>> Where can we find these preferences? The best I found was
>> https://www.man7.org/linux/man-pages/man7/groff_man.7.html but it
>> doesn't explain what to use. The current man pages seems to use both
>> interchangeably.
> 
> This is a good news/bad news situation for me.  As the maintainer of
> that man page, I'm delighted to hear that you found it the best resource
> of its type.  But that you came away still not knowing when or why to
> use .IR tells me I still have work to do.
> 
> One of the things I did after the groff 1.22.4 release (December 2018)
> was to split groff_man(7) into two pages.  The one you've linked is the
> terser reference for seasoned (perhaps salty) man page writers.  Near
> the top of it you'll find this.
> 
>         This document presents the macros thematically; for those needing
>         only a quick reference, the following table lists them
>         alphabetically, with cross-references to appropriate subsections
>         below.
> 
>         Man page authors and maintainers who are not already experienced
>         groff users should consult groff_man_style(7), an expanded
>         version of this document, for additional explanations and advice.
>         It covers only those concepts required for man page document
>         maintenance, and not the full breadth of the groff typesetting
>         system.

Hmmmm, I can't find that text on my Debian Sid (with a bit of 
experimental) groff_man(7).  Not even in the SEE ALSO.


SEE ALSO
        Groff:  The GNU Implementation of troff, by Trent A. Fisher
        and Werner Lemberg, is the main groff  documentation.   You
        can browse it interactively with “info groff”.

        tbl(1),  eqn(1),  and  refer(1) are preprocessors used with
        man pages.

        man(1) describes the man page formatter on your system.

        groff_mdoc(7) describes the groff version of the BSD‐origi‐
        nated alternative macro package for man pages.

        groff(7), groff_char(7), man(7)

groff 1.22.4              27 January 2021             GROFF_MAN(7)

> 
> There, at <https://www.man7.org/linux/man-pages/man7/groff_man.7.html>,
> I'd direct you to the following.
> 
>     Font style macros
>         The man macro package is limited in its font styling options,
>         offering only bold (.B), italic (.I), and roman.  Italic text is
>         usually set underscored instead on terminal devices.  The .SM and
>         .SB macros set text in roman or bold, respectively, at a smaller
>         point size; these differ visually from regular-sized roman or
>         bold text only on typesetter devices.  It is often necessary to
>         set text in different styles without intervening space.  The
>         macros .BI, .BR, .IB, .IR, .RB, and .RI, where “B”, “I”, and “R”
>         indicate bold, italic, and roman, respectively, set their odd-
>         and even-numbered arguments in alternating styles, with no space
>         separating them.
> [...]
>         .I [text]
>                Set text in italics.  If the macro is given no arguments,
>                the text of the next input line is set in italics.
> 
>                Use italics for file and path names, for environment
>                variables, for enumeration or preprocessor constants in C,
>                for variable (user-determined) portions of syntax
>                synopses, for the first occurrence (only) of a technical
>                concept being introduced, for names of works of software
>                (including commands and functions, but excluding names of
>                operating systems or their kernels), and anywhere a
>                parameter requiring replacement by the user is
>                encountered.  An exception involves variable text in a
>                context that is already marked up in italics, such as file
>                or path names with variable components; in such cases,
>                follow the convention of mathematical typography: set the
>                file or path name in italics as usual but use roman for
>                the variable part (see .IR and .RI below), and italics
>                again in running roman text when referring to the variable
>                material.

Re-reading this, we've been doing it wrong (as you pointed out in 
another thread) with macro names with variable part.

I wasn't very convinced by the current usage of the man pages, but it 
was already current, so I just followed it :/

I'll try to follow this from now.

> [...]
>         Note what is not prescribed for setting in bold or italics above:
>         elements of “synopsis language” such as ellipses and brackets
>         around options; proper names and adjectives; titles of anything
>         other than works of literature or software; identifiers for
>         standards documents or technical reports such as CSTR #54,
>         RFC 1918, Unicode 13.0, or POSIX.1-2017; acronyms; and
>         occurrences after the first of a technical term or piece of
>         jargon.  Again, the names of operating systems and their kernels
>         are, by practically universal convention, set in roman.
> 
>         Be frugal with italics for emphasis, and particularly with bold.
>         Brief runs of literal text, such as references to individual
>         characters or short strings, including section and subsection
>         headings of man pages, are suitable objects for quotation; see
>         the \(lq, \(rq, \(oq, and \(cq escapes in subsection
>         “Portability” below.
> 
>         Unlike the above font style macros, the font style alternation
>         macros below accept only arguments on the same line as the macro
>         call.  Italic corrections are applied as appropriate.  If space
>         is required within one of the arguments, first consider whether
>         the same result could be achieved with as much clarity by using
>         the single-style macros on separate input lines.  When it cannot,
>         double-quote an argument containing embedded space characters.
>         Setting all three different styles within a word presents
>         challenges; it is possible with the \c and/or \f escapes, but see
>         subsection “Portability” below for caveats.

Ahh, I missed this text, as it was neither under .I nor .IR, and only 
had a fast read of the page.
The 3rd paragraph hints that you should only use .IR when really needed, 
and use .I otherwise.  But someone new to man-pages, who probably did 
never read this page (or read some specific paragraphs of it when 
needed), and found the extensive (and somewhat incorrect) usage of .IR 
in place of .I in the current man pages, might be confused by all of 
that inconsistency and hard-to-find (except by those who already know 
where it is (reference to Pirates of the Caribbean intended :) )) 
information.

> [...]
>         .IR italic-text roman-text ...
>                Set each argument in italics and roman, alternately.
> 
>                       This is the first command of the
>                       .IR prologue .
> 
> I'd appreciate feedback from anyone on how I can improve the above.

I think it's great.  But unless one reads the page with some time (and 
not only the bullets), one might think that the man page is incomplete. 
  Maybe groff_man_style(7) is better suited for newbies, but I can't 
tell...  I couldn't find it.

> 
>>>> +upper layer.
>>>> +From a Landlock policy point of view,
>>>> +each OverlayFS layers and merge hierarchies are standalone and contains
>>>> +their own set of files and directories,
>>>> +which is different from bind mounts.
>>>
>>>
>>> Incorrect mix of singular and plural, I think.
>>
>> Is it OK with s/contains/contain/?
> 
> That's correct, but you also need s/their/its/.

I'm not sure, but maybe (I always get confused by these things)?:

[
each _of the_ OverlayFS layers and merge hierarchies _is_ standalone and 
_contains_
_its_ own set of files and directories,
which is different from bind mounts.
]

And still I'm not sure about the last "mounts".


Without s/each/each of the/ we still have layers and hierarchies 
(plurals), which don't mix well with each (singular).


Regards,

Alex

> 
> A handy technique for resolving inflection/agreement problems in English
> is to drop phrases from the sentence in a way that preserves its
> structure; this usually makes clear what should be done.
> 
> In this case, "... Each ... contains its own set."
> 
> Native speakers screw this up even in simpler cases; e.g.,
> 
>    *"The spaces in between leave room for you and I to grow."
> 
> We get thrown by the conjunction "and", which makes the language organ
> in our brain think of plural number.  But no native speaker ever commits
> the error
> 
>    *"Will you buy a hamburger for I?"
> 
> unless for deliberate effect.  (Though there is probably some hamlet in
> the West Midlands of England or something where this is standard. :-| )
> 
> Regards,
> Branden
> 
> [1] "Usually."  In roff terms, this generalization applies to text lines
>      (not control lines; that is, lines starting with a control
>      character) that do not end with the output line continuation escape
>      sequence, '\c'.
> 


-- 
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/

^ permalink raw reply

* Re: [PATCH] security: remove unneeded subdir-$(CONFIG_...)
From: Masahiro Yamada @ 2021-07-31  6:02 UTC (permalink / raw)
  To: James Morris, Serge E . Hallyn
  Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	John Fastabend, KP Singh, Martin KaFai Lau,
	Mickaël Salaün, Song Liu, Yonghong Song, bpf,
	Linux Kernel Mailing List, linux-security-module, Networking
In-Reply-To: <20210528180140.176257-1-masahiroy@kernel.org>

On Sat, May 29, 2021 at 3:02 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> All of these are unneeded. The directories to descend are specified
> by obj-$(CONFIG_...).
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>


Ping?




> ---
>
>  security/Makefile | 11 -----------
>  1 file changed, 11 deletions(-)
>
> diff --git a/security/Makefile b/security/Makefile
> index 47e432900e24..18121f8f85cd 100644
> --- a/security/Makefile
> +++ b/security/Makefile
> @@ -4,16 +4,6 @@
>  #
>
>  obj-$(CONFIG_KEYS)                     += keys/
> -subdir-$(CONFIG_SECURITY_SELINUX)      += selinux
> -subdir-$(CONFIG_SECURITY_SMACK)                += smack
> -subdir-$(CONFIG_SECURITY_TOMOYO)        += tomoyo
> -subdir-$(CONFIG_SECURITY_APPARMOR)     += apparmor
> -subdir-$(CONFIG_SECURITY_YAMA)         += yama
> -subdir-$(CONFIG_SECURITY_LOADPIN)      += loadpin
> -subdir-$(CONFIG_SECURITY_SAFESETID)    += safesetid
> -subdir-$(CONFIG_SECURITY_LOCKDOWN_LSM) += lockdown
> -subdir-$(CONFIG_BPF_LSM)               += bpf
> -subdir-$(CONFIG_SECURITY_LANDLOCK)     += landlock
>
>  # always enable default capabilities
>  obj-y                                  += commoncap.o
> @@ -36,5 +26,4 @@ obj-$(CONFIG_BPF_LSM)                 += bpf/
>  obj-$(CONFIG_SECURITY_LANDLOCK)                += landlock/
>
>  # Object integrity file lists
> -subdir-$(CONFIG_INTEGRITY)             += integrity
>  obj-$(CONFIG_INTEGRITY)                        += integrity/
> --
> 2.27.0
>


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply

* Re: [PATCH v2 1/4] landlock.7: Add a new page to introduce Landlock
From: G. Branden Robinson @ 2021-07-31  0:15 UTC (permalink / raw)
  To: Alejandro Colomar (man-pages)
  Cc: Mickaël Salaün, Jann Horn, Jonathan Corbet, Kees Cook,
	Randy Dunlap, Vincent Dagonneau, landlock, linux-kernel,
	linux-man, linux-security-module, Mickaël Salaün,
	Michael Kerrisk
In-Reply-To: <1a698059-d9dd-5aa0-2765-42e704c3a697@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 3267 bytes --]

Hi, Alex!

At 2021-07-30T14:59:52+0200, Alejandro Colomar (man-pages) wrote:
> Yes, they were because of semantic newlines.
> 
> The "rules" are:
> 
> Follow mainly "semantic newlines" style (forgetting about the line
> length), which will give you a text that (mostly) fits into 75 or 80
> columns.
> 
> If after doing that there are some lines that exceed the 75 or 80
> column right margin, consider fixing that line by breaking it at a
> different point or maybe breaking it further.  The 80 column limit is
> a hard limit (I can't read anything past the 80 col), while the 75
> limit is a bit softer (that's for allowing quotes in reviews) (if
> fitting a line into col 75 would break it in a weird way, don't do
> it).
> 
> If I didn't explain myself enough, please tell me.

I'm a little puzzled by the above.  Semantic newlines have little to do
with the output line length in *roff systems.  They arose due to a Bell
Labs Unix Room practice, popularized by Brian Kernighan.  Brandon Rhodes
has a backgrounder on this[1].

Man pages tend to be really flexible with respect to output line length.
This is one reason the groff man macros expose a user-settable LL
register.  The main limitations on line length are people using tbl(1)
tables or disabling filling (with the .nf request or in .EX/.EE
examples).  Another limitation is that as lines get shorter, it becomes
hard to set the page headers and footers without them overlapping.

For the first two points there is not much the macro package can do;
both tbl(1) and filling disablement leave the placement of line breaks
in the hands of the document author, and they can abuse that power by
"oversetting" a line; that is, making it longer than the configured line
length.

The third point is a problem the macro package can overcome with some
effort, by measuring the lengths of the components that go into a header
or footer an abbreviating them.  This is not a theoretical concern;
Erlang supplies some man pages with insanely long names[2], and you can
see the problem in footers today on the man-pages site for any page
groff ships, because Michael pulls from our Git repository (to my great
relief, because I fix documentation errors and make other improvements
all the time) and our version identifier has gotten crazily long because
we're on the order of one thousand commits since the last release
candidate, and gnulib's git-version-gen uses release tags, commit count
since the tag, _and_ an abbreviated commit ID to generate the version
string.  For my tree right now that's "1.23.0.rc1.999-7ae6d".

Here's a specimen of how that works out in a rendered page:
<https://man7.org/linux/man-pages/man1/neqn.1.html> (scroll to the
bottom).

I've fixed the problem for long page names for the next groff release,
but it involved some string-manipulation gymnastics[3].  I haven't yet
factored those out into their own (private) macro which I can also call
when preparing the page footer.

Regards,
Branden

[1] https://rhodesmill.org/brandon/2012/one-sentence-per-line/
[2] CosNotifyChannelAdmin_StructuredProxyPushSupplier(3erl)
[3] https://git.savannah.gnu.org/cgit/groff.git/commit/?id=b7f38e8a1d698e1078d7c215d08fde57d8e919b9

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH v2 1/4] landlock.7: Add a new page to introduce Landlock
From: G. Branden Robinson @ 2021-07-30 23:39 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: Alejandro Colomar (man-pages), Jann Horn, Jonathan Corbet,
	Kees Cook, Randy Dunlap, Vincent Dagonneau, landlock,
	linux-kernel, linux-man, linux-security-module,
	Mickaël Salaün, Michael Kerrisk
In-Reply-To: <c5036c5c-37a1-57d2-e5dc-1f41a5ed0d31@digikod.net>

[-- Attachment #1: Type: text/plain, Size: 7506 bytes --]

Hi, Mickaël!

I'm going to rearrange your message to reply to it to put the shortest
point first, as I am nervous of people tiring of my info dumps,
especially with such an efflorescent CC list.

At 2021-07-30T14:15:48+0200, Mickaël Salaün wrote:
> >> +The rule will only allow reading the file hierarchy
> >> +.IR /usr .
> 
> Why ".IR" is correct here?

Because you don't want a space or a line break in the output between
"/usr" and the period.

Line breaks in *roff input usually mean "insert a word break here".[1]

[the long version]
> When do we really need .IR? Isn't `.I "foo bar"` the same as `.IR foo
> bar`? What do you use roman for?
> 
> Where can we find these preferences? The best I found was
> https://www.man7.org/linux/man-pages/man7/groff_man.7.html but it
> doesn't explain what to use. The current man pages seems to use both
> interchangeably.

This is a good news/bad news situation for me.  As the maintainer of
that man page, I'm delighted to hear that you found it the best resource
of its type.  But that you came away still not knowing when or why to
use .IR tells me I still have work to do.

One of the things I did after the groff 1.22.4 release (December 2018)
was to split groff_man(7) into two pages.  The one you've linked is the
terser reference for seasoned (perhaps salty) man page writers.  Near
the top of it you'll find this.

       This document presents the macros thematically; for those needing
       only a quick reference, the following table lists them
       alphabetically, with cross-references to appropriate subsections
       below.

       Man page authors and maintainers who are not already experienced
       groff users should consult groff_man_style(7), an expanded
       version of this document, for additional explanations and advice.
       It covers only those concepts required for man page document
       maintenance, and not the full breadth of the groff typesetting
       system.

There, at <https://www.man7.org/linux/man-pages/man7/groff_man.7.html>,
I'd direct you to the following.

   Font style macros
       The man macro package is limited in its font styling options,
       offering only bold (.B), italic (.I), and roman.  Italic text is
       usually set underscored instead on terminal devices.  The .SM and
       .SB macros set text in roman or bold, respectively, at a smaller
       point size; these differ visually from regular-sized roman or
       bold text only on typesetter devices.  It is often necessary to
       set text in different styles without intervening space.  The
       macros .BI, .BR, .IB, .IR, .RB, and .RI, where “B”, “I”, and “R”
       indicate bold, italic, and roman, respectively, set their odd-
       and even-numbered arguments in alternating styles, with no space
       separating them.
[...]
       .I [text]
              Set text in italics.  If the macro is given no arguments,
              the text of the next input line is set in italics.

              Use italics for file and path names, for environment
              variables, for enumeration or preprocessor constants in C,
              for variable (user-determined) portions of syntax
              synopses, for the first occurrence (only) of a technical
              concept being introduced, for names of works of software
              (including commands and functions, but excluding names of
              operating systems or their kernels), and anywhere a
              parameter requiring replacement by the user is
              encountered.  An exception involves variable text in a
              context that is already marked up in italics, such as file
              or path names with variable components; in such cases,
              follow the convention of mathematical typography: set the
              file or path name in italics as usual but use roman for
              the variable part (see .IR and .RI below), and italics
              again in running roman text when referring to the variable
              material.
[...]
       Note what is not prescribed for setting in bold or italics above:
       elements of “synopsis language” such as ellipses and brackets
       around options; proper names and adjectives; titles of anything
       other than works of literature or software; identifiers for
       standards documents or technical reports such as CSTR #54,
       RFC 1918, Unicode 13.0, or POSIX.1-2017; acronyms; and
       occurrences after the first of a technical term or piece of
       jargon.  Again, the names of operating systems and their kernels
       are, by practically universal convention, set in roman.

       Be frugal with italics for emphasis, and particularly with bold.
       Brief runs of literal text, such as references to individual
       characters or short strings, including section and subsection
       headings of man pages, are suitable objects for quotation; see
       the \(lq, \(rq, \(oq, and \(cq escapes in subsection
       “Portability” below.

       Unlike the above font style macros, the font style alternation
       macros below accept only arguments on the same line as the macro
       call.  Italic corrections are applied as appropriate.  If space
       is required within one of the arguments, first consider whether
       the same result could be achieved with as much clarity by using
       the single-style macros on separate input lines.  When it cannot,
       double-quote an argument containing embedded space characters.
       Setting all three different styles within a word presents
       challenges; it is possible with the \c and/or \f escapes, but see
       subsection “Portability” below for caveats.
[...]
       .IR italic-text roman-text ...
              Set each argument in italics and roman, alternately.

                     This is the first command of the
                     .IR prologue .

I'd appreciate feedback from anyone on how I can improve the above.

> >> +upper layer.
> >> +From a Landlock policy point of view,
> >> +each OverlayFS layers and merge hierarchies are standalone and contains
> >> +their own set of files and directories,
> >> +which is different from bind mounts.
> > 
> > 
> > Incorrect mix of singular and plural, I think.
> 
> Is it OK with s/contains/contain/?

That's correct, but you also need s/their/its/.

A handy technique for resolving inflection/agreement problems in English
is to drop phrases from the sentence in a way that preserves its
structure; this usually makes clear what should be done.

In this case, "... Each ... contains its own set."

Native speakers screw this up even in simpler cases; e.g.,

  *"The spaces in between leave room for you and I to grow."

We get thrown by the conjunction "and", which makes the language organ
in our brain think of plural number.  But no native speaker ever commits
the error

  *"Will you buy a hamburger for I?"

unless for deliberate effect.  (Though there is probably some hamlet in
the West Midlands of England or something where this is standard. :-| )

Regards,
Branden

[1] "Usually."  In roff terms, this generalization applies to text lines
    (not control lines; that is, lines starting with a control
    character) that do not end with the output line continuation escape
    sequence, '\c'.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH] apparmor: use per file locks for transactional queries
From: John Johansen @ 2021-07-30 23:24 UTC (permalink / raw)
  To: Hamza Mahfooz, linux-kernel
  Cc: James Morris, Serge E. Hallyn, linux-security-module
In-Reply-To: <20210730052355.77289-1-someguy@effective-light.com>

On 7/29/21 10:23 PM, Hamza Mahfooz wrote:
> As made mention of in commit 1dea3b41e84c5 ("apparmor: speed up
> transactional queries"), a single lock is currently used to synchronize
> transactional queries. We can, use the lock allocated for each file by
> VFS instead.
> 
> Signed-off-by: Hamza Mahfooz <someguy@effective-light.com>

Acked-by: John Johansen <john.johansen@canonical.com>

I'll pull this into my tree asap

> ---
>  security/apparmor/apparmorfs.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
> index 2ee3b3d29f10..c0b626a271a0 100644
> --- a/security/apparmor/apparmorfs.c
> +++ b/security/apparmor/apparmorfs.c
> @@ -812,8 +812,6 @@ struct multi_transaction {
>  };
>  
>  #define MULTI_TRANSACTION_LIMIT (PAGE_SIZE - sizeof(struct multi_transaction))
> -/* TODO: replace with per file lock */
> -static DEFINE_SPINLOCK(multi_transaction_lock);
>  
>  static void multi_transaction_kref(struct kref *kref)
>  {
> @@ -847,10 +845,10 @@ static void multi_transaction_set(struct file *file,
>  	AA_BUG(n > MULTI_TRANSACTION_LIMIT);
>  
>  	new->size = n;
> -	spin_lock(&multi_transaction_lock);
> +	spin_lock(&file->f_lock);
>  	old = (struct multi_transaction *) file->private_data;
>  	file->private_data = new;
> -	spin_unlock(&multi_transaction_lock);
> +	spin_unlock(&file->f_lock);
>  	put_multi_transaction(old);
>  }
>  
> @@ -879,9 +877,10 @@ static ssize_t multi_transaction_read(struct file *file, char __user *buf,
>  	struct multi_transaction *t;
>  	ssize_t ret;
>  
> -	spin_lock(&multi_transaction_lock);
> +	spin_lock(&file->f_lock);
>  	t = get_multi_transaction(file->private_data);
> -	spin_unlock(&multi_transaction_lock);
> +	spin_unlock(&file->f_lock);
> +
>  	if (!t)
>  		return 0;
>  
> 


^ permalink raw reply

* [PATCH v3 4/4] landlock_restrict_self.2: Document new syscall
From: Mickaël Salaün @ 2021-07-30 14:41 UTC (permalink / raw)
  To: Alejandro Colomar, Michael Kerrisk
  Cc: Mickaël Salaün, Jann Horn, Jonathan Corbet, Kees Cook,
	Randy Dunlap, Vincent Dagonneau, landlock, linux-kernel,
	linux-man, linux-security-module, Mickaël Salaün
In-Reply-To: <20210730144116.332091-1-mic@digikod.net>

From: Mickaël Salaün <mic@linux.microsoft.com>

This is an adaptation of
https://www.kernel.org/doc/html/v5.13/userspace-api/landlock.html

Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
Link: https://lore.kernel.org/r/20210730144116.332091-5-mic@digikod.net
---

Changes since v2:
* Add an EXAMPLES section referring to landlock(7).
* Change list order in the SEE ALSO section.
* Fix .IR and .BR use as explained by Alejandro Colomar.

Changes since v1:
* Replace all ".I" with ".IR", except when used for titles.
* Append punctuation to ".IR" and ".BR" when it makes sense (requested
  by Alejandro Colomar).
* Cut lines according to the semantic newline rules (requested by
  Alejandro Colomar).
* Remove roman style from ".TP" section titles (requested by Alejandro
  Colomar).
* Add comma after "i.e." and "e.g.".
* Add a "CONFORMING TO" section.
* Replace "(2)" with "()" for the described syscall name.
---
 man2/landlock_restrict_self.2 | 133 ++++++++++++++++++++++++++++++++++
 1 file changed, 133 insertions(+)
 create mode 100644 man2/landlock_restrict_self.2

diff --git a/man2/landlock_restrict_self.2 b/man2/landlock_restrict_self.2
new file mode 100644
index 000000000000..4b10997e2fb6
--- /dev/null
+++ b/man2/landlock_restrict_self.2
@@ -0,0 +1,133 @@
+.\" Copyright © 2017-2020 Mickaël Salaün <mic@digikod.net>
+.\" Copyright © 2019-2020 ANSSI
+.\" Copyright © 2021 Microsoft Corporation
+.\"
+.\" %%%LICENSE_START(VERBATIM)
+.\" Permission is granted to make and distribute verbatim copies of this
+.\" manual provided the copyright notice and this permission notice are
+.\" preserved on all copies.
+.\"
+.\" Permission is granted to copy and distribute modified versions of this
+.\" manual under the conditions for verbatim copying, provided that the
+.\" entire resulting derived work is distributed under the terms of a
+.\" permission notice identical to this one.
+.\"
+.\" Since the Linux kernel and libraries are constantly changing, this
+.\" manual page may be incorrect or out-of-date.  The author(s) assume no
+.\" responsibility for errors or omissions, or for damages resulting from
+.\" the use of the information contained herein.  The author(s) may not
+.\" have taken the same level of care in the production of this manual,
+.\" which is licensed free of charge, as they might when working
+.\" professionally.
+.\"
+.\" Formatted or processed versions of this manual, if unaccompanied by
+.\" the source, must acknowledge the copyright and authors of this work.
+.\" %%%LICENSE_END
+.\"
+.TH LANDLOCK_RESTRICT_SELF 2 2021-06-27 Linux "Linux Programmer's Manual"
+.SH NAME
+landlock_restrict_self \- enforce a Landlock ruleset
+.SH SYNOPSIS
+.nf
+.BR "#include <linux/landlock.h>" "  /* Definition of " LANDLOCK_* " constants */"
+.BR "#include <sys/syscall.h>" "     /* Definition of " SYS_* " constants */"
+.PP
+.BI "int syscall(SYS_landlock_restrict_self, int " ruleset_fd ,
+.BI "            __u32 " flags );
+.SH DESCRIPTION
+Once a Landlock ruleset is populated with the desired rules, the
+.BR landlock_restrict_self ()
+system call enables enforcing this ruleset on the calling thread.
+See
+.BR landlock (7)
+for a global overview.
+.PP
+A thread can be restricted with multiple rulesets that are then composed
+together to form the thread's Landlock domain.
+This can be seen as a stack of rulesets but it is implemented in a more
+efficient way.
+A domain can only be updated in such a way that the constraints of each
+past and future composed rulesets will restrict the thread and its future
+children for their entire life.
+It is then possible to gradually enforce tailored access control policies
+with multiple independant rulesets coming from different sources
+(e.g., init system configuration, user session policy,
+built-in application policy).
+However, most applications should only need one call to
+.BR landlock_restrict_self ()
+and they should avoid arbitrary numbers of such calls because of the
+composed rulesets limit.
+Instead, developers are encouraged to build a tailored ruleset thanks to
+multiple calls to
+.BR landlock_add_rule (2).
+.PP
+In order to enforce a ruleset, either the caller must have the
+.B CAP_SYS_ADMIN
+capability in its user namespace, or the thread must already have the
+.I no_new_privs
+bit set.
+As for
+.BR seccomp (2),
+this avoids scenarios where unprivileged processes can affect the behavior
+of privileged children (e.g., because of set-user-ID binaries).
+If that bit was not already set by an ancestor of this thread,
+the thread must make the following call:
+.IP
+.EX
+prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0);
+.EE
+.PP
+.I ruleset_fd
+is a Landlock ruleset file descriptor obtained with
+.BR landlock_create_ruleset (2)
+and fully populated with a set of calls to
+.BR landlock_add_rule (2).
+.PP
+.I flags
+must be 0.
+.SH RETURN VALUE
+On success,
+.BR landlock_restrict_self ()
+returns 0.
+.SH ERRORS
+.BR landlock_restrict_self ()
+can failed for the following reasons:
+.TP
+.B EOPNOTSUPP
+Landlock is supported by the kernel but disabled at boot time.
+.TP
+.B EINVAL
+.I flags
+is not 0.
+.TP
+.B EBADF
+.I ruleset_fd
+is not a file descriptor for the current thread.
+.TP
+.B EBADFD
+.I ruleset_fd
+is not a ruleset file descriptor.
+.TP
+.B EPERM
+.I ruleset_fd
+has no read access to the underlying ruleset,
+or the calling thread is not running with
+.IR no_new_privs ,
+or it doesn't have the
+.B CAP_SYS_ADMIN
+in its user namespace.
+.TP
+.B E2BIG
+The maximum number of composed rulesets is reached for the calling thread.
+This limit is currently 64.
+.SH VERSIONS
+Landlock was added in Linux 5.13.
+.SH CONFORMING TO
+This system call is Linux-specific.
+.SH EXAMPLES
+See
+.BR landlock (7).
+.SH SEE ALSO
+.BR landlock_create_ruleset (2),
+.BR landlock_add_rule (2),
+.BR landlock (7)
-- 
2.32.0


^ permalink raw reply related

* [PATCH v3 3/4] landlock_add_rule.2: Document new syscall
From: Mickaël Salaün @ 2021-07-30 14:41 UTC (permalink / raw)
  To: Alejandro Colomar, Michael Kerrisk
  Cc: Mickaël Salaün, Jann Horn, Jonathan Corbet, Kees Cook,
	Randy Dunlap, Vincent Dagonneau, landlock, linux-kernel,
	linux-man, linux-security-module, Mickaël Salaün
In-Reply-To: <20210730144116.332091-1-mic@digikod.net>

From: Mickaël Salaün <mic@linux.microsoft.com>

This is an adaptation of
https://www.kernel.org/doc/html/v5.13/userspace-api/landlock.html

Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
Link: https://lore.kernel.org/r/20210730144116.332091-4-mic@digikod.net
---

Changes since v2:
* Fix syscall's rule_attr pointer.
* Add an EXAMPLES section referring to landlock(7).
* Change list order in the SEE ALSO section.
* Fix .IR and .BR use as explained by Alejandro Colomar.

Changes since v1:
* Replace all ".I" with ".IR", except when used for titles.
* Append punctuation to ".IR" and ".BR" when it makes sense (requested
  by Alejandro Colomar).
* Cut lines according to the semantic newline rules (requested by
  Alejandro Colomar).
* Remove roman style from ".TP" section titles (requested by Alejandro
  Colomar).
* Add comma after "i.e." and "e.g.".
* Add a "CONFORMING TO" section.
* Replace "(2)" with "()" for the described syscall name.
---
 man2/landlock_add_rule.2 | 144 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 144 insertions(+)
 create mode 100644 man2/landlock_add_rule.2

diff --git a/man2/landlock_add_rule.2 b/man2/landlock_add_rule.2
new file mode 100644
index 000000000000..eafb8f8201b7
--- /dev/null
+++ b/man2/landlock_add_rule.2
@@ -0,0 +1,144 @@
+.\" Copyright © 2017-2020 Mickaël Salaün <mic@digikod.net>
+.\" Copyright © 2019-2020 ANSSI
+.\" Copyright © 2021 Microsoft Corporation
+.\"
+.\" %%%LICENSE_START(VERBATIM)
+.\" Permission is granted to make and distribute verbatim copies of this
+.\" manual provided the copyright notice and this permission notice are
+.\" preserved on all copies.
+.\"
+.\" Permission is granted to copy and distribute modified versions of this
+.\" manual under the conditions for verbatim copying, provided that the
+.\" entire resulting derived work is distributed under the terms of a
+.\" permission notice identical to this one.
+.\"
+.\" Since the Linux kernel and libraries are constantly changing, this
+.\" manual page may be incorrect or out-of-date.  The author(s) assume no
+.\" responsibility for errors or omissions, or for damages resulting from
+.\" the use of the information contained herein.  The author(s) may not
+.\" have taken the same level of care in the production of this manual,
+.\" which is licensed free of charge, as they might when working
+.\" professionally.
+.\"
+.\" Formatted or processed versions of this manual, if unaccompanied by
+.\" the source, must acknowledge the copyright and authors of this work.
+.\" %%%LICENSE_END
+.\"
+.TH LANDLOCK_ADD_RULE 2 2021-06-27 Linux "Linux Programmer's Manual"
+.SH NAME
+landlock_add_rule \- add a new Landlock rule to a ruleset
+.SH SYNOPSIS
+.nf
+.BR "#include <linux/landlock.h>" "  /* Definition of " LANDLOCK_* " constants */"
+.BR "#include <sys/syscall.h>" "     /* Definition of " SYS_* " constants */"
+.PP
+.BI "int syscall(SYS_landlock_add_rule, int " ruleset_fd ,
+.BI "            enum landlock_rule_type " rule_type ,
+.BI "            const void *" rule_attr ", __u32 " flags );
+.SH DESCRIPTION
+A Landlock rule describes an action on an object.
+An object is currently a file hierarchy, and the related filesystem actions
+are defined with a set of access rights.
+This
+.BR landlock_add_rule ()
+system call enables adding a new Landlock rule to an existing ruleset
+created with
+.BR landlock_create_ruleset (2).
+See
+.BR landlock (7)
+for a global overview.
+.PP
+.I ruleset_fd
+is a Landlock ruleset file descriptor obtained with
+.BR landlock_create_ruleset (2).
+.PP
+.I rule_type
+identifies the structure type pointed to by
+.IR rule_attr .
+Currently, Linux supports the following
+.I rule_type
+value:
+.TP
+.B LANDLOCK_RULE_PATH_BENEATH
+This defines the object type as a file hierarchy.
+In this case,
+.I rule_attr
+points to the following structure:
+.IP
+.in +4n
+.EX
+struct landlock_path_beneath_attr {
+    __u64 allowed_access;
+    __s32 parent_fd;
+} __attribute__((packed));
+.EE
+.in
+.IP
+.I allowed_access
+contains a bitmask of allowed filesystem actions for this file hierarchy
+(see
+.B Filesystem actions
+in
+.BR landlock (7)).
+.IP
+.I parent_fd
+is an opened file descriptor, preferably with the
+.I O_PATH
+flag,
+which identifies the parent directory of the file hierarchy or
+just a file.
+.PP
+.I flags
+must be 0.
+.SH RETURN VALUE
+On success,
+.BR landlock_add_rule ()
+returns 0.
+.SH ERRORS
+.BR landlock_add_rule ()
+can failed for the following reasons:
+.TP
+.B EOPNOTSUPP
+Landlock is supported by the kernel but disabled at boot time.
+.TP
+.B EINVAL
+.I flags
+is not 0, or the rule accesses are inconsistent (i.e.,
+.I rule_attr->allowed_access
+is not a subset of the ruleset handled accesses).
+.TP
+.B ENOMSG
+Empty accesses (i.e.,
+.I rule_attr->allowed_access
+is 0).
+.TP
+.B EBADF
+.I ruleset_fd
+is not a file descriptor for the current thread, or a member of
+.I rule_attr
+is not a file descriptor as expected.
+.TP
+.B EBADFD
+.I ruleset_fd
+is not a ruleset file descriptor, or a member of
+.I rule_attr
+is not the expected file descriptor type.
+.TP
+.B EPERM
+.I ruleset_fd
+has no write access to the underlying ruleset.
+.TP
+.B EFAULT
+.I rule_attr
+was not a valid address.
+.SH VERSIONS
+Landlock was added in Linux 5.13.
+.SH CONFORMING TO
+This system call is Linux-specific.
+.SH EXAMPLES
+See
+.BR landlock (7).
+.SH SEE ALSO
+.BR landlock_create_ruleset (2),
+.BR landlock_restrict_self (2),
+.BR landlock (7)
-- 
2.32.0


^ permalink raw reply related

* [PATCH v3 2/4] landlock_create_ruleset.2: Document new syscall
From: Mickaël Salaün @ 2021-07-30 14:41 UTC (permalink / raw)
  To: Alejandro Colomar, Michael Kerrisk
  Cc: Mickaël Salaün, Jann Horn, Jonathan Corbet, Kees Cook,
	Randy Dunlap, Vincent Dagonneau, landlock, linux-kernel,
	linux-man, linux-security-module, Mickaël Salaün
In-Reply-To: <20210730144116.332091-1-mic@digikod.net>

From: Mickaël Salaün <mic@linux.microsoft.com>

This is an adaptation of
https://www.kernel.org/doc/html/v5.13/userspace-api/landlock.html

Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
Link: https://lore.kernel.org/r/20210730144116.332091-3-mic@digikod.net
---

Changes since v2:
* Fix syscall signature (attr pointer).
* Add an EXAMPLES section referring to landlock(7).
* Change list order in the SEE ALSO section.
* Fix .IR and .BR use as explained by Alejandro Colomar.

Changes since v1:
* Replace all ".I" with ".IR", except when used for titles.
* Append punctuation to ".IR" and ".BR" when it makes sense (requested
  by Alejandro Colomar).
* Cut lines according to the semantic newline rules (requested by
  Alejandro Colomar).
* Remove roman style from ".TP" section titles (requested by Alejandro
  Colomar).
* Add comma after "i.e." and "e.g.".
* Add a "CONFORMING TO" section.
* Replace "(2)" with "()" for the described syscall name.
---
 man2/landlock_create_ruleset.2 | 139 +++++++++++++++++++++++++++++++++
 1 file changed, 139 insertions(+)
 create mode 100644 man2/landlock_create_ruleset.2

diff --git a/man2/landlock_create_ruleset.2 b/man2/landlock_create_ruleset.2
new file mode 100644
index 000000000000..e1ca4bcf8c86
--- /dev/null
+++ b/man2/landlock_create_ruleset.2
@@ -0,0 +1,139 @@
+.\" Copyright © 2017-2020 Mickaël Salaün <mic@digikod.net>
+.\" Copyright © 2019-2020 ANSSI
+.\" Copyright © 2021 Microsoft Corporation
+.\"
+.\" %%%LICENSE_START(VERBATIM)
+.\" Permission is granted to make and distribute verbatim copies of this
+.\" manual provided the copyright notice and this permission notice are
+.\" preserved on all copies.
+.\"
+.\" Permission is granted to copy and distribute modified versions of this
+.\" manual under the conditions for verbatim copying, provided that the
+.\" entire resulting derived work is distributed under the terms of a
+.\" permission notice identical to this one.
+.\"
+.\" Since the Linux kernel and libraries are constantly changing, this
+.\" manual page may be incorrect or out-of-date.  The author(s) assume no
+.\" responsibility for errors or omissions, or for damages resulting from
+.\" the use of the information contained herein.  The author(s) may not
+.\" have taken the same level of care in the production of this manual,
+.\" which is licensed free of charge, as they might when working
+.\" professionally.
+.\"
+.\" Formatted or processed versions of this manual, if unaccompanied by
+.\" the source, must acknowledge the copyright and authors of this work.
+.\" %%%LICENSE_END
+.\"
+.TH LANDLOCK_CREATE_RULESET 2 2021-06-27 Linux "Linux Programmer's Manual"
+.SH NAME
+landlock_create_ruleset \- create a new Landlock ruleset
+.SH SYNOPSIS
+.nf
+.BR "#include <linux/landlock.h>" "  /* Definition of " LANDLOCK_* " constants */"
+.BR "#include <sys/syscall.h>" "     /* Definition of " SYS_* " constants */"
+.PP
+.BI "int syscall(SYS_landlock_create_ruleset,
+.BI "            const struct landlock_ruleset_attr *" attr ,
+.BI "            size_t " size " , __u32 " flags );
+.SH DESCRIPTION
+A Landlock ruleset identifies a set of rules (i.e., actions on objects).
+This
+.BR landlock_create_ruleset ()
+system call enables creating a new file descriptor identifying a ruleset.
+This file descriptor can then be used by
+.BR landlock_add_rule (2)
+and
+.BR landlock_restrict_self (2).
+See
+.BR landlock (7)
+for a global overview.
+.PP
+.I attr
+specifies the properties of the new ruleset.
+It points to the following structure:
+.IP
+.in +4n
+.EX
+struct landlock_ruleset_attr {
+	__u64 handled_access_fs;
+};
+.EE
+.in
+.IP
+.I handled_access_fs
+is a bitmask of actions that is handled by this ruleset and should then be
+forbidden if no rule explicitly allow them
+(see
+.B Filesystem actions
+in
+.BR landlock (7)).
+This enables simply restricting ambient rights
+(e.g., global filesystem access) and is needed for compatibility reasons.
+.PP
+.I size
+must be specified as
+.I sizeof(struct landlock_ruleset_attr)
+for compatibility reasons.
+.PP
+.I flags
+must be 0 if
+.I attr
+is used.
+Otherwise,
+.I flags
+can be set to:
+.TP
+.B LANDLOCK_CREATE_RULESET_VERSION
+If
+.I attr
+is NULL and
+.I size
+is 0, then the returned value is the highest supported Landlock ABI version
+(starting at 1).
+This version can be used for a best-effort security approach,
+which is encouraged when user space is not pinned to a specific kernel
+version.
+All features documented in these man pages are available with the version
+1.
+.SH RETURN VALUE
+On success,
+.BR landlock_create_ruleset ()
+returns a new Landlock ruleset file descriptor, or a Landlock ABI version
+according to
+.IR flags .
+.SH ERRORS
+.BR landlock_create_ruleset ()
+can failed for the following reasons:
+.TP
+.B EOPNOTSUPP
+Landlock is supported by the kernel but disabled at boot time.
+.TP
+.B EINVAL
+Unknown
+.IR flags ,
+or unknown access, or too small
+.IR size .
+.TP
+.B E2BIG
+.I size
+is too big.
+.TP
+.B EFAULT
+.I attr
+was not a valid address.
+.TP
+.B ENOMSG
+Empty accesses (i.e.,
+.I attr->handled_access_fs
+is 0).
+.SH VERSIONS
+Landlock was added in Linux 5.13.
+.SH CONFORMING TO
+This system call is Linux-specific.
+.SH EXAMPLES
+See
+.BR landlock (7).
+.SH SEE ALSO
+.BR landlock_add_rule (2),
+.BR landlock_restrict_self (2),
+.BR landlock (7)
-- 
2.32.0


^ permalink raw reply related

* [PATCH v3 1/4] landlock.7: Add a new page to introduce Landlock
From: Mickaël Salaün @ 2021-07-30 14:41 UTC (permalink / raw)
  To: Alejandro Colomar, Michael Kerrisk
  Cc: Mickaël Salaün, Jann Horn, Jonathan Corbet, Kees Cook,
	Randy Dunlap, Vincent Dagonneau, landlock, linux-kernel,
	linux-man, linux-security-module, Mickaël Salaün
In-Reply-To: <20210730144116.332091-1-mic@digikod.net>

From: Mickaël Salaün <mic@linux.microsoft.com>

From the user point of view, Landlock is a set of system calls enabling
to build and enforce a set of access-control rules.  A ruleset can be
created with landlock_create_ruleset(2), populated with
landlock_add_rule(2) and enforced with landlock_restrict_self(2).  This
man page gives an overview of the whole mechanism.  Details of these
system calls are documented in their respective man pages.

This is an adaptation of
https://www.kernel.org/doc/html/v5.13/userspace-api/landlock.html

Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
Link: https://lore.kernel.org/r/20210730144116.332091-2-mic@digikod.net
---

Changes since v2:
* Fix semantic newlines according to Alejandro Colomar.
* Fix minor grammar issues spotted by Alejandro.
* Fix .IR and .BR use as explained by Alejandro.

Changes since v1:
* Replace all ".I" with ".IR", except when used for titles.
* Append punctuation to ".IR" and ".BR" when it makes sense (requested
  by Alejandro Colomar).
* Cut lines according to the semantic newline rules (requested by
  Alejandro Colomar).
* Remove roman style from ".TP" section titles (requested by Alejandro
  Colomar).
* Add comma after "i.e." and "e.g.".
* Move the example in a new EXAMPLES section.
* Improve title.
* Explain the LSM acronym at first use.
---
 man7/landlock.7 | 361 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 361 insertions(+)
 create mode 100644 man7/landlock.7

diff --git a/man7/landlock.7 b/man7/landlock.7
new file mode 100644
index 000000000000..f6a9b02d434a
--- /dev/null
+++ b/man7/landlock.7
@@ -0,0 +1,361 @@
+.\" Copyright © 2017-2020 Mickaël Salaün <mic@digikod.net>
+.\" Copyright © 2019-2020 ANSSI
+.\" Copyright © 2021 Microsoft Corporation
+.\"
+.\" %%%LICENSE_START(VERBATIM)
+.\" Permission is granted to make and distribute verbatim copies of this
+.\" manual provided the copyright notice and this permission notice are
+.\" preserved on all copies.
+.\"
+.\" Permission is granted to copy and distribute modified versions of this
+.\" manual under the conditions for verbatim copying, provided that the
+.\" entire resulting derived work is distributed under the terms of a
+.\" permission notice identical to this one.
+.\"
+.\" Since the Linux kernel and libraries are constantly changing, this
+.\" manual page may be incorrect or out-of-date.  The author(s) assume no
+.\" responsibility for errors or omissions, or for damages resulting from
+.\" the use of the information contained herein.  The author(s) may not
+.\" have taken the same level of care in the production of this manual,
+.\" which is licensed free of charge, as they might when working
+.\" professionally.
+.\"
+.\" Formatted or processed versions of this manual, if unaccompanied by
+.\" the source, must acknowledge the copyright and authors of this work.
+.\" %%%LICENSE_END
+.\"
+.TH LANDLOCK 7 2021-06-27 Linux "Linux Programmer's Manual"
+.SH NAME
+Landlock \- unprivileged access-control
+.SH DESCRIPTION
+Landlock is an access-control system that enables any processes to
+securely restrict themselves and their future children.
+Because Landlock is a stackable Linux Security Module (LSM),
+it makes it possible to create safe security sandboxes as new security
+layers in addition to the existing system-wide access-controls.
+This kind of sandbox is expected to help mitigate
+the security impact of bugs,
+and unexpected or malicious behaviors in applications.
+.PP
+A Landlock security policy is a set of access rights
+(e.g., open a file in read-only, make a directory, etc.)
+tied to a file hierarchy.
+Such policy can be configured and enforced by processes for themselves
+using three system calls:
+.IP \(bu 2
+.BR landlock_create_ruleset (2)
+creates a new ruleset;
+.IP \(bu
+.BR landlock_add_rule (2)
+adds a new rule to a ruleset;
+.IP \(bu
+.BR landlock_restrict_self (2)
+enforces a ruleset on the calling thread.
+.PP
+To be able to use these system calls,
+the running kernel must support Landlock and
+it must be enabled at boot time.
+.\"
+.SS Landlock rules
+A Landlock rule describes an action on an object.
+An object is currently a file hierarchy,
+and the related filesystem actions are defined with access rights (see
+.BR landlock_add_rule (2)).
+A set of rules is aggregated in a ruleset,
+which can then restrict the thread enforcing it,
+and its future children.
+.\"
+.SS Filesystem actions
+These flags enable to restrict a sandboxed process to a
+set of actions on files and directories.
+Files or directories opened before the sandboxing
+are not subject to these restrictions.
+See
+.BR landlock_add_rule (2)
+and
+.BR landlock_create_ruleset (2)
+for more context.
+.PP
+A file can only receive these access rights:
+.TP
+.B LANDLOCK_ACCESS_FS_EXECUTE
+Execute a file.
+.TP
+.B LANDLOCK_ACCESS_FS_WRITE_FILE
+Open a file with write access.
+.TP
+.B LANDLOCK_ACCESS_FS_READ_FILE
+Open a file with read access.
+.PP
+A directory can receive access rights related to files or directories.
+The following access right is applied to the directory itself,
+and the directories beneath it:
+.TP
+.B LANDLOCK_ACCESS_FS_READ_DIR
+Open a directory or list its content.
+.PP
+However,
+the following access rights only apply to the content of a directory,
+not the directory itself:
+.TP
+.B LANDLOCK_ACCESS_FS_REMOVE_DIR
+Remove an empty directory or rename one.
+.TP
+.B LANDLOCK_ACCESS_FS_REMOVE_FILE
+Unlink (or rename) a file.
+.TP
+.B LANDLOCK_ACCESS_FS_MAKE_CHAR
+Create (or rename or link) a character device.
+.TP
+.B LANDLOCK_ACCESS_FS_MAKE_DIR
+Create (or rename) a directory.
+.TP
+.B LANDLOCK_ACCESS_FS_MAKE_REG
+Create (or rename or link) a regular file.
+.TP
+.B LANDLOCK_ACCESS_FS_MAKE_SOCK
+Create (or rename or link) a UNIX domain socket.
+.TP
+.B LANDLOCK_ACCESS_FS_MAKE_FIFO
+Create (or rename or link) a named pipe.
+.TP
+.B LANDLOCK_ACCESS_FS_MAKE_BLOCK
+Create (or rename or link) a block device.
+.TP
+.B LANDLOCK_ACCESS_FS_MAKE_SYM
+Create (or rename or link) a symbolic link.
+.\"
+.SS Layers of file path access rights
+Each time a thread enforces a ruleset on itself,
+it updates its Landlock domain with a new layer of policy.
+Indeed, this complementary policy is composed with the potentially other
+rulesets already restricting this thread.
+A sandboxed thread can then safely add more constraints to itself with a
+new enforced ruleset.
+.PP
+One policy layer grants access to a file path
+if at least one of its rules encountered on the path grants the access.
+A sandboxed thread can only access a file path
+if all its enforced policy layers grant the access
+as well as all the other system access controls
+(e.g., filesystem DAC, other LSM policies, etc.).
+.\"
+.SS Bind mounts and OverlayFS
+Landlock enables restricting access to file hierarchies,
+which means that these access rights can be propagated with bind mounts
+(cf.
+.BR mount_namespaces (7))
+but not with OverlayFS.
+.PP
+A bind mount mirrors a source file hierarchy to a destination.
+The destination hierarchy is then composed of the exact same files,
+on which Landlock rules can be tied,
+either via the source or the destination path.
+These rules restrict access when they are encountered on a path,
+which means that they can restrict access to
+multiple file hierarchies at the same time,
+whether these hierarchies are the result of bind mounts or not.
+.PP
+An OverlayFS mount point consists of upper and lower layers.
+These layers are combined in a merge directory, result of the mount point.
+This merge hierarchy may include files from the upper and lower layers,
+but modifications performed on the merge hierarchy
+only reflect on the upper layer.
+From a Landlock policy point of view,
+each OverlayFS layers and merge hierarchies are standalone and contain
+their own set of files and directories,
+which is different from bind mounts.
+A policy restricting an OverlayFS layer will not restrict the resulted
+merged hierarchy, and vice versa.
+Landlock users should then only think about file hierarchies they want to
+allow access to, regardless of the underlying filesystem.
+.\"
+.SS Inheritance
+Every new thread resulting from a
+.BR clone (2)
+inherits Landlock domain restrictions from its parent.
+This is similar to the
+.BR seccomp (2)
+inheritance or any other LSM dealing with tasks'
+.BR credentials (7).
+For instance, one process's thread may apply Landlock rules to itself,
+but they will not be automatically applied to other sibling threads
+(unlike POSIX thread credential changes, cf.
+.BR nptl (7)).
+.PP
+When a thread sandboxes itself,
+we have the guarantee that the related security policy
+will stay enforced on all this thread's descendants.
+This allows creating standalone and modular security policies
+per application,
+which will automatically be composed between themselves
+according to their runtime parent policies.
+.\"
+.SS Ptrace restrictions
+A sandboxed process has less privileges than a non-sandboxed process and
+must then be subject to additional restrictions
+when manipulating another process.
+To be allowed to use
+.BR ptrace (2)
+and related syscalls on a target process,
+a sandboxed process should have a subset of the target process rules,
+which means the tracee must be in a sub-domain of the tracer.
+.SH VERSIONS
+Landlock was added in Linux 5.13.
+.SH NOTES
+Landlock is enabled by
+.BR CONFIG_SECURITY_LANDLOCK .
+The
+.I lsm=lsm1,...,lsmN
+command line parameter controls the sequence of the initialization of
+Linux Security Modules.
+It must contain the string
+.I landlock
+to enable Landlock.
+If the command line parameter is not specified,
+the initialization falls back to the value of the deprecated
+.I security=
+command line parameter and further to the value of CONFIG_LSM.
+We can check that Landlock is enabled by looking for
+.I landlock: Up and running.
+in kernel logs.
+.PP
+It is currently not possible to restrict some file-related actions
+accessible through these system call families:
+.BR chdir (2),
+.BR truncate (2),
+.BR stat (2),
+.BR flock (2),
+.BR chmod (2),
+.BR chown (2),
+.BR setxattr (2),
+.BR utime (2),
+.BR ioctl (2),
+.BR fcntl (2),
+.BR access (2).
+Future Landlock evolutions will enable to restrict them.
+.SH EXAMPLES
+We first need to create the ruleset that will contain our rules.
+For this example,
+the ruleset will contain rules that only allow read actions,
+but write actions will be denied.
+The ruleset then needs to handle both of these kind of actions.
+See below for the description of filesystem actions.
+.PP
+.in +4n
+.EX
+int ruleset_fd;
+struct landlock_ruleset_attr ruleset_attr = {
+    .handled_access_fs =
+        LANDLOCK_ACCESS_FS_EXECUTE |
+        LANDLOCK_ACCESS_FS_WRITE_FILE |
+        LANDLOCK_ACCESS_FS_READ_FILE |
+        LANDLOCK_ACCESS_FS_READ_DIR |
+        LANDLOCK_ACCESS_FS_REMOVE_DIR |
+        LANDLOCK_ACCESS_FS_REMOVE_FILE |
+        LANDLOCK_ACCESS_FS_MAKE_CHAR |
+        LANDLOCK_ACCESS_FS_MAKE_DIR |
+        LANDLOCK_ACCESS_FS_MAKE_REG |
+        LANDLOCK_ACCESS_FS_MAKE_SOCK |
+        LANDLOCK_ACCESS_FS_MAKE_FIFO |
+        LANDLOCK_ACCESS_FS_MAKE_BLOCK |
+        LANDLOCK_ACCESS_FS_MAKE_SYM,
+};
+
+ruleset_fd = landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+if (ruleset_fd < 0) {
+    perror("Failed to create a ruleset");
+    return 1;
+}
+.EE
+.in
+.PP
+We can now add a new rule to this ruleset thanks to the returned file
+descriptor referring to this ruleset.
+The rule will only allow reading the file hierarchy
+.IR /usr .
+Without another rule, write actions would then be denied by the ruleset.
+To add
+.I /usr
+to the ruleset, we open it with the
+.I O_PATH
+flag and fill the
+.I struct landlock_path_beneath_attr
+with this file descriptor.
+.PP
+.in +4n
+.EX
+int err;
+struct landlock_path_beneath_attr path_beneath = {
+    .allowed_access =
+        LANDLOCK_ACCESS_FS_EXECUTE |
+        LANDLOCK_ACCESS_FS_READ_FILE |
+        LANDLOCK_ACCESS_FS_READ_DIR,
+};
+
+path_beneath.parent_fd = open("/usr", O_PATH | O_CLOEXEC);
+if (path_beneath.parent_fd < 0) {
+    perror("Failed to open file");
+    close(ruleset_fd);
+    return 1;
+}
+err = landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
+                        &path_beneath, 0);
+close(path_beneath.parent_fd);
+if (err) {
+    perror("Failed to update ruleset");
+    close(ruleset_fd);
+    return 1;
+}
+.EE
+.in
+.PP
+We now have a ruleset with one rule allowing read access to
+.I /usr
+while denying all other handled accesses for the filesystem.
+The next step is to restrict the current thread from gaining more
+privileges
+(e.g., thanks to a set-user-ID binary).
+.PP
+.in +4n
+.EX
+if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0)) {
+    perror("Failed to restrict privileges");
+    close(ruleset_fd);
+    return 1;
+}
+.EE
+.in
+.PP
+The current thread is now ready to sandbox itself with the ruleset.
+.PP
+.in +4n
+.EX
+if (landlock_restrict_self(ruleset_fd, 0)) {
+    perror("Failed to enforce ruleset");
+    close(ruleset_fd);
+    return 1;
+}
+close(ruleset_fd);
+.EE
+.in
+.PP
+If the
+.BR landlock_restrict_self (2)
+system call succeeds, the current thread is now restricted and this policy
+will be enforced on all its subsequently created children as well.
+Once a thread is landlocked, there is no way to remove its security policy;
+only adding more restrictions is allowed.
+These threads are now in a new Landlock domain,
+merge of their parent one (if any) with the new ruleset.
+.PP
+Full working code can be found in
+.UR https://git.kernel.org\:/pub\:/scm\:/linux\:/kernel\:/git\:/stable\:/linux.git\:/tree\:/samples\:/landlock\:/sandboxer.c
+.UE
+.SH SEE ALSO
+.BR landlock_create_ruleset (2),
+.BR landlock_add_rule (2),
+.BR landlock_restrict_self (2)
+.PP
+.UR https://landlock.io\:/
+.UE
-- 
2.32.0


^ permalink raw reply related

* [PATCH v3 0/4] Add Landlock man pages
From: Mickaël Salaün @ 2021-07-30 14:41 UTC (permalink / raw)
  To: Alejandro Colomar, Michael Kerrisk
  Cc: Mickaël Salaün, Jann Horn, Jonathan Corbet, Kees Cook,
	Randy Dunlap, Vincent Dagonneau, landlock, linux-kernel,
	linux-man, linux-security-module, Mickaël Salaün

From: Mickaël Salaün <mic@linux.microsoft.com>

Hi,

These four documents give a global overview of Landlock and explain each
system calls.  This is mainly a formatting of the current kernel
documentation with some new additional details.

This third patch series mostly fixes formatting pointed out by Alejandro
Colomar.

This patch series can be found in a Git repository:
https://github.com/landlock-lsm/man-pages/commits/landlock-v3

Previous version:
https://lore.kernel.org/linux-man/20210712155745.831580-1-mic@digikod.net/

Regards,

Mickaël Salaün (4):
  landlock.7: Add a new page to introduce Landlock
  landlock_create_ruleset.2: Document new syscall
  landlock_add_rule.2: Document new syscall
  landlock_restrict_self.2: Document new syscall

 man2/landlock_add_rule.2       | 144 +++++++++++++
 man2/landlock_create_ruleset.2 | 139 +++++++++++++
 man2/landlock_restrict_self.2  | 133 ++++++++++++
 man7/landlock.7                | 361 +++++++++++++++++++++++++++++++++
 4 files changed, 777 insertions(+)
 create mode 100644 man2/landlock_add_rule.2
 create mode 100644 man2/landlock_create_ruleset.2
 create mode 100644 man2/landlock_restrict_self.2
 create mode 100644 man7/landlock.7


base-commit: fbe71b1b79e72be3b9afc44b5d479e7fd84b598a
-- 
2.32.0


^ permalink raw reply

* Re: [PATCH v2 1/4] landlock.7: Add a new page to introduce Landlock
From: Mickaël Salaün @ 2021-07-30 14:32 UTC (permalink / raw)
  To: Alejandro Colomar (man-pages)
  Cc: Jann Horn, Jonathan Corbet, Kees Cook, Randy Dunlap,
	Vincent Dagonneau, landlock, linux-kernel, linux-man,
	linux-security-module, Mickaël Salaün, Michael Kerrisk,
	G. Branden Robinson
In-Reply-To: <1a698059-d9dd-5aa0-2765-42e704c3a697@gmail.com>

Thanks Alejandro for the detailed explanations, that's useful!

On 30/07/2021 14:59, Alejandro Colomar (man-pages) wrote:
> Hi Mickaël,
> 
> On 7/30/21 2:41 PM, Alejandro Colomar (man-pages) wrote:
> [...]
>>
>>
>>
>> I hope this was helpful :)
>>
>> Cheers,
>>
>> Alex
>>
>>
>>
>>
>> But, let's use .I
> 
> I hit send too soon.  Let's continue.
> 
> As for current usage, yes, there are many uses of .IR to mean .I, but
> for new code, we're only using .I (or .B) when possible.
> 
>>
>>
>> CC: Branden
>>
>>>
>>>>
>>>> If there was a misunderstanding about this, I'm sorry.
>>>>
>>>>> * Append punctuation to ".IR" and ".BR" when it makes sense (requested
>>>>>     by Alejandro Colomar).
>>>>> * Cut lines according to the semantic newline rules (requested by
>>>>>     Alejandro Colomar).
>>>>> * Remove roman style from ".TP" section titles (requested by Alejandro
>>>>>     Colomar).
>>>>> * Add comma after "i.e." and "e.g.".
>>>>> * Move the example in a new EXAMPLES section.
>>>>> * Improve title.
>>>>> * Explain the LSM acronym at first use.
>>>>> ---
>>>>>    man7/landlock.7 | 356
>>>>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>    1 file changed, 356 insertions(+)
>>>>>    create mode 100644 man7/landlock.7
>>>>>
>>>>> diff --git a/man7/landlock.7 b/man7/landlock.7
>>>>> new file mode 100644
>>>>> index 000000000000..c89f5b1cabb6
>>>>> --- /dev/null
>>>>> +++ b/man7/landlock.7
>>>>> @@ -0,0 +1,356 @@
>>>>> +.\" Copyright © 2017-2020 Mickaël Salaün <mic@digikod.net>
>>>>> +.\" Copyright © 2019-2020 ANSSI
>>>>> +.\" Copyright © 2021 Microsoft Corporation
>>>>> +.\"
>>>>> +.\" %%%LICENSE_START(VERBATIM)
>>>>> +.\" Permission is granted to make and distribute verbatim copies
>>>>> of this
>>>>> +.\" manual provided the copyright notice and this permission
>>>>> notice are
>>>>> +.\" preserved on all copies.
>>>>> +.\"
>>>>> +.\" Permission is granted to copy and distribute modified versions of
>>>>> this
>>>>> +.\" manual under the conditions for verbatim copying, provided
>>>>> that the
>>>>> +.\" entire resulting derived work is distributed under the terms of a
>>>>> +.\" permission notice identical to this one.
>>>>> +.\"
>>>>> +.\" Since the Linux kernel and libraries are constantly changing,
>>>>> this
>>>>> +.\" manual page may be incorrect or out-of-date.  The author(s)
>>>>> assume no
>>>>> +.\" responsibility for errors or omissions, or for damages resulting
>>>>> from
>>>>> +.\" the use of the information contained herein.  The author(s)
>>>>> may not
>>>>> +.\" have taken the same level of care in the production of this
>>>>> manual,
>>>>> +.\" which is licensed free of charge, as they might when working
>>>>> +.\" professionally.
>>>>> +.\"
>>>>> +.\" Formatted or processed versions of this manual, if
>>>>> unaccompanied by
>>>>> +.\" the source, must acknowledge the copyright and authors of this
>>>>> work.
>>>>> +.\" %%%LICENSE_END
>>>>> +.\"
>>>>> +.TH LANDLOCK 7 2021-06-27 Linux "Linux Programmer's Manual"
>>>>> +.SH NAME
>>>>> +Landlock \- unprivileged access-control
>>>>> +.SH DESCRIPTION
>>>>> +Landlock is an access-control system that enables any processes to //
>>>>> securely /J/
>>>
>>> Why adding a line break here? This line is 75 columns as stated by the
>>> documentation: https://man7.org/linux/man-pages/man7/man-pages.7.html
>>> I guess it helps for semantic newlines, right? If so, what are the
>>> rules?
> 
> Yes, they were because of semantic newlines.
> 
> The "rules" are:
> 
> Follow mainly "semantic newlines" style (forgetting about the line
> length), which will give you a text that (mostly) fits into 75 or 80
> columns.
> 
> If after doing that there are some lines that exceed the 75 or 80 column
> right margin, consider fixing that line by breaking it at a different
> point or maybe breaking it further.  The 80 column limit is a hard limit
> (I can't read anything past the 80 col), while the 75 limit is a bit
> softer (that's for allowing quotes in reviews) (if fitting a line into
> col 75 would break it in a weird way, don't do it).
> 
> If I didn't explain myself enough, please tell me.
> 
>>>>> +upper layer.
>>>>> +From a Landlock policy point of view,
>>>>> +each OverlayFS layers and merge hierarchies are standalone and
>>>>> contains
>>>>> +their own set of files and directories,
>>>>> +which is different from bind mounts.
>>>>
>>>>
>>>> Incorrect mix of singular and plural, I think.
>>> >> Is it OK with s/contains/contain/?
> 
> I think so.
> 
>>>
>>>>
>>>>> +A policy restricting an OverlayFS layer will not restrict the
>>>>> resulted
>>>>> +merged hierarchy, and vice versa.
>>>>> +Landlock users should then only think about file hierarchies they
>>>>> want to
>>>>> +allow access to, regardless of the underlying filesystem.
>>>>> +.\"
>>>>> +.SS Inheritance
>>>>> +Every new thread resulting from a
>>>>> +.BR clone (2)
>>>>> +inherits Landlock domain restrictions from its parent.
>>>>> +This is similar to the
>>>>> +.BR seccomp (2)
>>>>> +inheritance or any other LSM dealing with task's
>>>>
>>>> Not sure:
>>>>
>>>> s/task/a task/
>>>> or
>>>> s/task's/tasks'/
>>>
>>> I'll take "tasks'".
> 
> Okay.
> 
>>>
>>>>
>>>>> +.BR credentials (7).
>>>>> +For instance, one process's thread may apply Landlock rules to
>>>>> itself,
>>>>
>>>> s/process's/process'/
>>>
>>> As pointed out by Branden, this is correct.
> 
> That's right. :)
> 
>>>
>>>>
>>>>> +.BR chdir (2),
>>>>> +.BR truncate (2),
>>>>> +.BR stat (2),
>>>>> +.BR flock (2),
>>>>> +.BR chmod (2),
>>>>> +.BR chown (2),
>>>>> +.BR setxattr (2),
>>>>> +.BR utime (2),
>>>>> +.BR ioctl (2),
>>>>> +.BR fcntl (2),
>>>>> +.BR access (2).
>>>>> +Future Landlock evolutions will enable to restrict them.
>>>>> +.SH EXAMPLES
>>>> I'd prefer a complete example with a main function, if you can come up
>>>> with such one.  If not, this will be ok.
>>>
>>> I think it is clearer to not to use a full main to explain these basic
>>> steps. A full working example is linked though.
> 
> Ahh sorry, I didn't see the link.
> I'll have a look at it.
> 
>>>
>>>>
>>>>
>>>>> +We first need to create the ruleset that will contain our rules.
>>>>> +For this example,
>>>>> +the ruleset will contain rules that only allow read actions,
>>>>> +but write actions will be denied.
>>>>> +The ruleset then needs to handle both of these kind of actions.
>>>>> +See below for the description of filesystem actions.
>>>>> +.PP
>>>>> +.in +4n
>>>>> +.EX
>>>>> +int ruleset_fd;
>>>>> +struct landlock_ruleset_attr ruleset_attr = {
>>>>> +    .handled_access_fs =
>>>>> +        LANDLOCK_ACCESS_FS_EXECUTE |
>>>>> +        LANDLOCK_ACCESS_FS_WRITE_FILE |
>>>>> +        LANDLOCK_ACCESS_FS_READ_FILE |
>>>>> +        LANDLOCK_ACCESS_FS_READ_DIR |
>>>>> +        LANDLOCK_ACCESS_FS_REMOVE_DIR |
>>>>> +        LANDLOCK_ACCESS_FS_REMOVE_FILE |
>>>>> +        LANDLOCK_ACCESS_FS_MAKE_CHAR |
>>>>> +        LANDLOCK_ACCESS_FS_MAKE_DIR |
>>>>> +        LANDLOCK_ACCESS_FS_MAKE_REG |
>>>>> +        LANDLOCK_ACCESS_FS_MAKE_SOCK |
>>>>> +        LANDLOCK_ACCESS_FS_MAKE_FIFO |
>>>>> +        LANDLOCK_ACCESS_FS_MAKE_BLOCK |
>>>>> +        LANDLOCK_ACCESS_FS_MAKE_SYM,
>>>>> +};
>>>>> +
>>>>> +ruleset_fd = landlock_create_ruleset(&ruleset_attr,
>>>>> sizeof(ruleset_attr), 0);
>>>>> +if (ruleset_fd < 0) {
>>>>> +    perror("Failed to create a ruleset");
>>>>> +    return 1;
>>>>> +}
>>>>> +.EE
>>>>> +.in
>>>>> +.PP
>>>>> +We can now add a new rule to this ruleset thanks to the returned file
>>>>> +descriptor referring to this ruleset.
>>>>> +The rule will only allow reading the file hierarchy
>>>>> +.IR /usr .
>>>
>>> Why ".IR" is correct here?
> 
> "/usr" needs to be formatted, but "." not.
> 
> [
> .I /usr
> .
> ]
> 
> Would add a space: /usr .
> So we need a solution that formats only part of a space-separated token;
> that's what .IR does.  I hope the last email explained that well.
> 
> Thanks,
> 
> Alex
> 
> 

^ permalink raw reply

* RE: [RFC][PATCH v2 06/12] diglim: Interfaces - digest_list_add, digest_list_del
From: Roberto Sassu @ 2021-07-30 14:24 UTC (permalink / raw)
  To: Mimi Zohar, gregkh@linuxfoundation.org, mchehab+huawei@kernel.org
  Cc: linux-integrity@vger.kernel.org,
	linux-security-module@vger.kernel.org, linux-doc@vger.kernel.org,
	linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org,
	Igor Stoppa
In-Reply-To: <2c731f07bd08f01f2a3e032814bc65ae9a8494ad.camel@linux.ibm.com>

> From: Mimi Zohar [mailto:zohar@linux.ibm.com]
> Sent: Friday, July 30, 2021 4:03 PM
> Hi Roberto,
> 
> On Fri, 2021-07-30 at 13:16 +0000, Roberto Sassu wrote:
> > > From: Mimi Zohar [mailto:zohar@linux.ibm.com]
> > > Sent: Friday, July 30, 2021 2:40 PM
> 
> > > "critical data", in this context, should probably be used for verifying
> > > the in memory file digests and other state information haven't been
> > > compromised.
> >
> > Actually, this is what we are doing currently. To keep the
> > implementation simple, once the file or the buffer are uploaded
> > to the kernel, they will not be modified, just accessed through
> > the indexes.
> 
> My main concern about digest lists is their integrity, from loading the
> digest lists to their being stored in memory.  A while back, there was
> some work on defining a write once memory allocator.  I don't recall
> whatever happened to it.  This would be a perfect usecase for that
> memory allocator.

Adding Igor in CC.

Regarding loading, everything uploaded to the kernel is carefully
evaluated. This should not be a concern. Regarding making them
read-only, probably if you can subvert digest lists you can also
remove the read-only protection (unless you use an hypervisor).

Thanks

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Li Jian, Shi Yanli

> thanks,
> 
> Mimi


^ permalink raw reply

* Re: [RFC][PATCH v2 06/12] diglim: Interfaces - digest_list_add, digest_list_del
From: Mimi Zohar @ 2021-07-30 14:03 UTC (permalink / raw)
  To: Roberto Sassu, gregkh@linuxfoundation.org,
	mchehab+huawei@kernel.org
  Cc: linux-integrity@vger.kernel.org,
	linux-security-module@vger.kernel.org, linux-doc@vger.kernel.org,
	linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <555bf01bee4b4ea7a9bee658366d535a@huawei.com>

Hi Roberto,

On Fri, 2021-07-30 at 13:16 +0000, Roberto Sassu wrote:
> > From: Mimi Zohar [mailto:zohar@linux.ibm.com]
> > Sent: Friday, July 30, 2021 2:40 PM

> > "critical data", in this context, should probably be used for verifying
> > the in memory file digests and other state information haven't been
> > compromised.
> 
> Actually, this is what we are doing currently. To keep the
> implementation simple, once the file or the buffer are uploaded
> to the kernel, they will not be modified, just accessed through
> the indexes.

My main concern about digest lists is their integrity, from loading the
digest lists to their being stored in memory.  A while back, there was
some work on defining a write once memory allocator.  I don't recall
whatever happened to it.  This would be a perfect usecase for that
memory allocator.

thanks,

Mimi


^ permalink raw reply

* RE: [RFC][PATCH v2 06/12] diglim: Interfaces - digest_list_add, digest_list_del
From: Roberto Sassu @ 2021-07-30 13:16 UTC (permalink / raw)
  To: Mimi Zohar, gregkh@linuxfoundation.org, mchehab+huawei@kernel.org
  Cc: linux-integrity@vger.kernel.org,
	linux-security-module@vger.kernel.org, linux-doc@vger.kernel.org,
	linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <1ef95096bee13578b3f906dd9f708c6af9d6ff18.camel@linux.ibm.com>

> From: Mimi Zohar [mailto:zohar@linux.ibm.com]
> Sent: Friday, July 30, 2021 2:40 PM
> Hi Roberto,
> 
> On Fri, 2021-07-30 at 07:16 +0000, Roberto Sassu wrote:
> > > From: Mimi Zohar [mailto:zohar@linux.ibm.com]
> > > Sent: Thursday, July 29, 2021 11:21 PM
> > >
> > > On Mon, 2021-07-26 at 18:36 +0200, Roberto Sassu wrote:
> > > > /*
> > > > + * digest_list_read: read and parse the digest list from the path
> > > > + */
> > > > +static ssize_t digest_list_read(char *path, enum ops op)
> > > > +{
> > > > +       void *data = NULL;
> > > > +       char *datap;
> > > > +       size_t size;
> > > > +       u8 actions = 0;
> > > > +       struct file *file;
> > > > +       char event_name[NAME_MAX + 9 + 1];
> > > > +       u8 digest[IMA_MAX_DIGEST_SIZE] = { 0 };
> > > > +       enum hash_algo algo;
> > > > +       int rc, pathlen = strlen(path);
> > > > +
> > > > +       /* Remove \n. */
> > > > +       datap = path;
> > > > +       strsep(&datap, "\n");
> > > > +
> > > > +       file = filp_open(path, O_RDONLY, 0);
> > > > +       if (IS_ERR(file)) {
> > > > +               pr_err("unable to open file: %s (%ld)", path, PTR_ERR(file));
> > > > +               return PTR_ERR(file);
> > > > +       }
> > > > +
> > > > +       rc = kernel_read_file(file, 0, &data, INT_MAX, NULL,
> > > > +                             READING_DIGEST_LIST);
> > > > +       if (rc < 0) {
> > > > +               pr_err("unable to read file: %s (%d)", path, rc);
> > > > +               goto out;
> > > > +       }
> > > > +
> > > > +       size = rc;
> > > > +
> > > > +       snprintf(event_name, sizeof(event_name), "%s_file_%s",
> > > > +                op == DIGEST_LIST_ADD ? "add" : "del",
> > > > +                file_dentry(file)->d_name.name);
> > > > +
> > > > +       rc = ima_measure_critical_data("diglim", event_name, data, size,
> false,
> > > > +                                      digest, sizeof(digest));
> > > > +       if (rc < 0 && rc != -EEXIST)
> > > > +               goto out_vfree;
> > >
> > > The digest lists could easily be measured while reading the digest list
> > > file above in kernel_read_file().  What makes it "critical-data"?  In
> > > the SELinux case, the in memory SELinux policy is being measured and
> > > re-measured to make sure it hasn't been modified.  Is the digest list
> > > file data being measured more than once?
> >
> > Hi Mimi
> >
> > yes, the digest lists can be measured with kernel_read_file().
> > I didn't send the change yet, but I added a DIGEST_LIST_CHECK
> > hook mapped to READING_DIGEST_LIST, so that digest lists
> > can be easily measured or appraised.
> >
> > The point was that the digest of the digest list must be always
> > calculated, as it is added to the hash table. Instead of duplicating
> > the code, I preferred to use ima_measure_critical_data().
> >
> > The advantage is also that, if the use case is to just measure
> > digest lists, ima_measure_critical_data() could do both at the
> > same time.
> >
> > Digest lists can be seen as "critical data" in the sense that
> > they can affect the security decision on whether to grant
> > access to a file or not, assuming that an appropriate rule is
> > added in the IMA policy.
> 
> Of course the integrity of files containing the digest lists is
> important, but that doesn't make them "critical data".  If the
> integrity of these files is important, then the digest lists not only
> need to be measured, but they need to be signed and the resulting
> signature verified.  Without signature verification, there is no basis
> on which to trust the digest lists data.

The reason of storing the actions performed by IMA on the
digest lists helps to determine for which purpose they can be
used. If digest lists are used only for measurement purpose,
it should be sufficient that digest lists are measured. The
same applies for appraisal.

> Adding the kernel_read_file() "READING_DIGEST_LIST" support in IMA does
> not seem to be optional.  IMA would then be calculating the digest list
> file hash twice, once in kernel_read_file() and then, again, in
> ima_measure_critical_data().

I didn't include also this part: I retrieve the integrity_iint_cache for
the opened file descriptor and I get the flags from there. If the
IMA_MEASURED flag is set, it is not necessary to call also
ima_measure_critical_data().

> > > I understand that with your changes to ima_measure_critical_data(),
> > > which are now in next-integrity-testing branch, allow IMA to calculate
> > > the file data hash.
> >
> > Yes, correct. But actually there is another useful use case.
> > If digest lists are not in the format supported by the kernel,
> > the user space parser has to convert them before uploading
> > them to the kernel.
> >
> > ima_measure_critical_data() would in this case measure
> > the converted digest list (it is written directly, without
> > sending the file path). It is easier to attest the result,
> > instead of determining whether the user space parser
> > produced the expected result (by checking the files it
> > read).
> 
> The application to properly convert the digest list file data into the
> appropriate format would need to be trusted.  I'm concerned that not
> requiring the converted data to be signed and the signature verified is
> introducing a new integrity gap.  Perhaps between an LSM policy,
> limiting which files may be read by the application, and an IMA policy,
> requiring all files read by this application to be measured and the
> signature verified, this integrity gap could be averted.

It is the weakest point in the chain, yes. Relying on existing LSMs
didn't seem to me a good idea, as:
- a new policy must be installed
- we must be sure that the policy is really enforced
- we need to support different LSMs (SELinux for Fedora,
  Apparmor for SUSE)
- there might be no LSM we can rely on

For these reasons, I developed a new LSM. Its purpose is to
identify the user space parser and for each file it opens, ensure
that the file has been measured or appraised by IMA. If one of
these actions are missing, it will not be set in the digest list the
user space parser uploads to the kernel (which means that IMA
will ignore the digest list for that specific action).

> "critical data", in this context, should probably be used for verifying
> the in memory file digests and other state information haven't been
> compromised.

Actually, this is what we are doing currently. To keep the
implementation simple, once the file or the buffer are uploaded
to the kernel, they will not be modified, just accessed through
the indexes.

I could send the second part of the patch set, so that it becomes
more clear how digest lists are used by IMA and how the
integrity gap is filled.

Thanks

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Li Jian, Shi Yanli

> thanks,
> 
> Mimi


^ permalink raw reply

* Re: [PATCH v2 1/4] landlock.7: Add a new page to introduce Landlock
From: Alejandro Colomar (man-pages) @ 2021-07-30 12:59 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: Jann Horn, Jonathan Corbet, Kees Cook, Randy Dunlap,
	Vincent Dagonneau, landlock, linux-kernel, linux-man,
	linux-security-module, Mickaël Salaün, Michael Kerrisk,
	G. Branden Robinson
In-Reply-To: <a3b271e6-e03f-dab8-b04c-c76315cdd98e@gmail.com>

Hi Mickaël,

On 7/30/21 2:41 PM, Alejandro Colomar (man-pages) wrote:
[...]
> 
> 
> 
> I hope this was helpful :)
> 
> Cheers,
> 
> Alex
> 
> 
> 
> 
> But, let's use .I

I hit send too soon.  Let's continue.

As for current usage, yes, there are many uses of .IR to mean .I, but 
for new code, we're only using .I (or .B) when possible.

> 
> 
> CC: Branden
> 
>>
>>>
>>> If there was a misunderstanding about this, I'm sorry.
>>>
>>>> * Append punctuation to ".IR" and ".BR" when it makes sense (requested
>>>>     by Alejandro Colomar).
>>>> * Cut lines according to the semantic newline rules (requested by
>>>>     Alejandro Colomar).
>>>> * Remove roman style from ".TP" section titles (requested by Alejandro
>>>>     Colomar).
>>>> * Add comma after "i.e." and "e.g.".
>>>> * Move the example in a new EXAMPLES section.
>>>> * Improve title.
>>>> * Explain the LSM acronym at first use.
>>>> ---
>>>>    man7/landlock.7 | 356 
>>>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>>>    1 file changed, 356 insertions(+)
>>>>    create mode 100644 man7/landlock.7
>>>>
>>>> diff --git a/man7/landlock.7 b/man7/landlock.7
>>>> new file mode 100644
>>>> index 000000000000..c89f5b1cabb6
>>>> --- /dev/null
>>>> +++ b/man7/landlock.7
>>>> @@ -0,0 +1,356 @@
>>>> +.\" Copyright © 2017-2020 Mickaël Salaün <mic@digikod.net>
>>>> +.\" Copyright © 2019-2020 ANSSI
>>>> +.\" Copyright © 2021 Microsoft Corporation
>>>> +.\"
>>>> +.\" %%%LICENSE_START(VERBATIM)
>>>> +.\" Permission is granted to make and distribute verbatim copies of 
>>>> this
>>>> +.\" manual provided the copyright notice and this permission notice 
>>>> are
>>>> +.\" preserved on all copies.
>>>> +.\"
>>>> +.\" Permission is granted to copy and distribute modified versions of
>>>> this
>>>> +.\" manual under the conditions for verbatim copying, provided that 
>>>> the
>>>> +.\" entire resulting derived work is distributed under the terms of a
>>>> +.\" permission notice identical to this one.
>>>> +.\"
>>>> +.\" Since the Linux kernel and libraries are constantly changing, this
>>>> +.\" manual page may be incorrect or out-of-date.  The author(s)
>>>> assume no
>>>> +.\" responsibility for errors or omissions, or for damages resulting
>>>> from
>>>> +.\" the use of the information contained herein.  The author(s) may 
>>>> not
>>>> +.\" have taken the same level of care in the production of this 
>>>> manual,
>>>> +.\" which is licensed free of charge, as they might when working
>>>> +.\" professionally.
>>>> +.\"
>>>> +.\" Formatted or processed versions of this manual, if 
>>>> unaccompanied by
>>>> +.\" the source, must acknowledge the copyright and authors of this 
>>>> work.
>>>> +.\" %%%LICENSE_END
>>>> +.\"
>>>> +.TH LANDLOCK 7 2021-06-27 Linux "Linux Programmer's Manual"
>>>> +.SH NAME
>>>> +Landlock \- unprivileged access-control
>>>> +.SH DESCRIPTION
>>>> +Landlock is an access-control system that enables any processes to //
>>>> securely /J/
>>
>> Why adding a line break here? This line is 75 columns as stated by the
>> documentation: https://man7.org/linux/man-pages/man7/man-pages.7.html
>> I guess it helps for semantic newlines, right? If so, what are the rules?

Yes, they were because of semantic newlines.

The "rules" are:

Follow mainly "semantic newlines" style (forgetting about the line 
length), which will give you a text that (mostly) fits into 75 or 80 
columns.

If after doing that there are some lines that exceed the 75 or 80 column 
right margin, consider fixing that line by breaking it at a different 
point or maybe breaking it further.  The 80 column limit is a hard limit 
(I can't read anything past the 80 col), while the 75 limit is a bit 
softer (that's for allowing quotes in reviews) (if fitting a line into 
col 75 would break it in a weird way, don't do it).

If I didn't explain myself enough, please tell me.

>>>> +upper layer.
>>>> +From a Landlock policy point of view,
>>>> +each OverlayFS layers and merge hierarchies are standalone and 
>>>> contains
>>>> +their own set of files and directories,
>>>> +which is different from bind mounts.
>>>
>>>
>>> Incorrect mix of singular and plural, I think.
>> >> Is it OK with s/contains/contain/?

I think so.

>>
>>>
>>>> +A policy restricting an OverlayFS layer will not restrict the resulted
>>>> +merged hierarchy, and vice versa.
>>>> +Landlock users should then only think about file hierarchies they
>>>> want to
>>>> +allow access to, regardless of the underlying filesystem.
>>>> +.\"
>>>> +.SS Inheritance
>>>> +Every new thread resulting from a
>>>> +.BR clone (2)
>>>> +inherits Landlock domain restrictions from its parent.
>>>> +This is similar to the
>>>> +.BR seccomp (2)
>>>> +inheritance or any other LSM dealing with task's
>>>
>>> Not sure:
>>>
>>> s/task/a task/
>>> or
>>> s/task's/tasks'/
>>
>> I'll take "tasks'".

Okay.

>>
>>>
>>>> +.BR credentials (7).
>>>> +For instance, one process's thread may apply Landlock rules to itself,
>>>
>>> s/process's/process'/
>>
>> As pointed out by Branden, this is correct.

That's right. :)

>>
>>>
>>>> +.BR chdir (2),
>>>> +.BR truncate (2),
>>>> +.BR stat (2),
>>>> +.BR flock (2),
>>>> +.BR chmod (2),
>>>> +.BR chown (2),
>>>> +.BR setxattr (2),
>>>> +.BR utime (2),
>>>> +.BR ioctl (2),
>>>> +.BR fcntl (2),
>>>> +.BR access (2).
>>>> +Future Landlock evolutions will enable to restrict them.
>>>> +.SH EXAMPLES
>>> I'd prefer a complete example with a main function, if you can come up
>>> with such one.  If not, this will be ok.
>>
>> I think it is clearer to not to use a full main to explain these basic
>> steps. A full working example is linked though.

Ahh sorry, I didn't see the link.
I'll have a look at it.

>>
>>>
>>>
>>>> +We first need to create the ruleset that will contain our rules.
>>>> +For this example,
>>>> +the ruleset will contain rules that only allow read actions,
>>>> +but write actions will be denied.
>>>> +The ruleset then needs to handle both of these kind of actions.
>>>> +See below for the description of filesystem actions.
>>>> +.PP
>>>> +.in +4n
>>>> +.EX
>>>> +int ruleset_fd;
>>>> +struct landlock_ruleset_attr ruleset_attr = {
>>>> +    .handled_access_fs =
>>>> +        LANDLOCK_ACCESS_FS_EXECUTE |
>>>> +        LANDLOCK_ACCESS_FS_WRITE_FILE |
>>>> +        LANDLOCK_ACCESS_FS_READ_FILE |
>>>> +        LANDLOCK_ACCESS_FS_READ_DIR |
>>>> +        LANDLOCK_ACCESS_FS_REMOVE_DIR |
>>>> +        LANDLOCK_ACCESS_FS_REMOVE_FILE |
>>>> +        LANDLOCK_ACCESS_FS_MAKE_CHAR |
>>>> +        LANDLOCK_ACCESS_FS_MAKE_DIR |
>>>> +        LANDLOCK_ACCESS_FS_MAKE_REG |
>>>> +        LANDLOCK_ACCESS_FS_MAKE_SOCK |
>>>> +        LANDLOCK_ACCESS_FS_MAKE_FIFO |
>>>> +        LANDLOCK_ACCESS_FS_MAKE_BLOCK |
>>>> +        LANDLOCK_ACCESS_FS_MAKE_SYM,
>>>> +};
>>>> +
>>>> +ruleset_fd = landlock_create_ruleset(&ruleset_attr,
>>>> sizeof(ruleset_attr), 0);
>>>> +if (ruleset_fd < 0) {
>>>> +    perror("Failed to create a ruleset");
>>>> +    return 1;
>>>> +}
>>>> +.EE
>>>> +.in
>>>> +.PP
>>>> +We can now add a new rule to this ruleset thanks to the returned file
>>>> +descriptor referring to this ruleset.
>>>> +The rule will only allow reading the file hierarchy
>>>> +.IR /usr .
>>
>> Why ".IR" is correct here?

"/usr" needs to be formatted, but "." not.

[
.I /usr
.
]

Would add a space: /usr .
So we need a solution that formats only part of a space-separated token; 
that's what .IR does.  I hope the last email explained that well.

Thanks,

Alex


-- 
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/

^ permalink raw reply

* Re: [PATCH v2 1/4] landlock.7: Add a new page to introduce Landlock
From: Alejandro Colomar (man-pages) @ 2021-07-30 12:41 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: Jann Horn, Jonathan Corbet, Kees Cook, Randy Dunlap,
	Vincent Dagonneau, landlock, linux-kernel, linux-man,
	linux-security-module, Mickaël Salaün, Michael Kerrisk,
	G. Branden Robinson
In-Reply-To: <c5036c5c-37a1-57d2-e5dc-1f41a5ed0d31@digikod.net>

On 7/30/21 2:15 PM, Mickaël Salaün wrote:
> 
> On 29/07/2021 16:56, Alejandro Colomar (man-pages) wrote:
>> Hi Mickaël,
>>
>> On 7/12/21 5:57 PM, Mickaël Salaün wrote:
>>> From: Mickaël Salaün <mic@linux.microsoft.com>
>>>
>>>   From the user point of view, Landlock is a set of system calls enabling
>>> to build and enforce a set of access-control rules.  A ruleset can be
>>> created with landlock_create_ruleset(2), populated with
>>> landlock_add_rule(2) and enforced with landlock_restrict_self(2).  This
>>> man page gives an overview of the whole mechanism.  Details of these
>>> system calls are documented in their respective man pages.
>>>
>>> This is an adaptation of
>>> https://www.kernel.org/doc/html/v5.13/userspace-api/landlock.html
>>>
>>> Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
>>> Link: https://lore.kernel.org/r/20210712155745.831580-2-mic@digikod.net
>>
>> Please see some comments below, mostly about formatting.
>> The text looks good to me.
> 
> Thanks for the review.
> 
>>
>> Thanks,
>>
>> Alex
>>
>>> ---
>>>
>>> Changes since v1:
>>> * Replace all ".I" with ".IR", except when used for titles.
>>
>> Sorry, but I actually prefer the opposite: Use .I unless you really need
>> .IR
> 
> When do we really need .IR? Isn't `.I "foo bar"` the same as `.IR foo
> bar`? What do you use roman for?
> 
> Where can we find these preferences? The best I found was
> https://www.man7.org/linux/man-pages/man7/groff_man.7.html but it
> doesn't explain what to use. The current man pages seems to use both
> interchangeably.

I was going to point you to groff_man(7), but you're right that it 
doesn't explain it really well for someone new to groff.  Maybe some 
examples would help, but then the manual page would grow too much, so a 
bit of testing may be better than trying to have an extensive manual page.

So when to use each, and when any can fit:

.IR can do anything that .I can do.  It's a "superset" of .I in terms of 
output (ignoring that the input to get such output is different).
But .I is simpler and more readable when wanting .I behavior, so let's 
use .I for .I, and .IR only when roman is needed.

I means italics (which in the terminal is underscored, but ok);
R means roman (i.e., normal text)

For the following examples, I'll use uppercase to mean italics.

.I transforms everything to the right of it into italics, respecting 
spaces.  Therefore, .I doesn't require you to use quotes.

If you want two (or one) separate words in italics, you can use:
.I hello world
which will print:
HELLO WORLD

.IR on the other hand, alternates roman and bold breaking at spaces 
(which are not translated into the output).
Example:
.IR hello beautiful world
which will print:
HELLObeautifulWORLD

This is usually used when formatting a word that is followed by 
puntuation, as normally you don't want puntuation to be formatted, so 
you'll typically see (especially at the end of a sentence):
.IR size_t .
which will print:
SIZE_T.   (the period is not formatted)

You could use .IR (but don't) to imitate .I:
.IR hello
which will print:
HELLO

In the example above, as we didn't specify a roman part, it is just as an .I

When you need spaces to be respected with .IR, you need quotes:
.IR hello "beautiful world"
HELLObeautiful world

This is useful for formatting names consisting of two words next to 
punctuation:
.IR "hello world" .
HELLO WORLD.   (the period is not in italics)



I hope this was helpful :)

Cheers,

Alex




But, let's use .I


CC: Branden

> 
>>
>> If there was a misunderstanding about this, I'm sorry.
>>
>>> * Append punctuation to ".IR" and ".BR" when it makes sense (requested
>>>     by Alejandro Colomar).
>>> * Cut lines according to the semantic newline rules (requested by
>>>     Alejandro Colomar).
>>> * Remove roman style from ".TP" section titles (requested by Alejandro
>>>     Colomar).
>>> * Add comma after "i.e." and "e.g.".
>>> * Move the example in a new EXAMPLES section.
>>> * Improve title.
>>> * Explain the LSM acronym at first use.
>>> ---
>>>    man7/landlock.7 | 356 ++++++++++++++++++++++++++++++++++++++++++++++++
>>>    1 file changed, 356 insertions(+)
>>>    create mode 100644 man7/landlock.7
>>>
>>> diff --git a/man7/landlock.7 b/man7/landlock.7
>>> new file mode 100644
>>> index 000000000000..c89f5b1cabb6
>>> --- /dev/null
>>> +++ b/man7/landlock.7
>>> @@ -0,0 +1,356 @@
>>> +.\" Copyright © 2017-2020 Mickaël Salaün <mic@digikod.net>
>>> +.\" Copyright © 2019-2020 ANSSI
>>> +.\" Copyright © 2021 Microsoft Corporation
>>> +.\"
>>> +.\" %%%LICENSE_START(VERBATIM)
>>> +.\" Permission is granted to make and distribute verbatim copies of this
>>> +.\" manual provided the copyright notice and this permission notice are
>>> +.\" preserved on all copies.
>>> +.\"
>>> +.\" Permission is granted to copy and distribute modified versions of
>>> this
>>> +.\" manual under the conditions for verbatim copying, provided that the
>>> +.\" entire resulting derived work is distributed under the terms of a
>>> +.\" permission notice identical to this one.
>>> +.\"
>>> +.\" Since the Linux kernel and libraries are constantly changing, this
>>> +.\" manual page may be incorrect or out-of-date.  The author(s)
>>> assume no
>>> +.\" responsibility for errors or omissions, or for damages resulting
>>> from
>>> +.\" the use of the information contained herein.  The author(s) may not
>>> +.\" have taken the same level of care in the production of this manual,
>>> +.\" which is licensed free of charge, as they might when working
>>> +.\" professionally.
>>> +.\"
>>> +.\" Formatted or processed versions of this manual, if unaccompanied by
>>> +.\" the source, must acknowledge the copyright and authors of this work.
>>> +.\" %%%LICENSE_END
>>> +.\"
>>> +.TH LANDLOCK 7 2021-06-27 Linux "Linux Programmer's Manual"
>>> +.SH NAME
>>> +Landlock \- unprivileged access-control
>>> +.SH DESCRIPTION
>>> +Landlock is an access-control system that enables any processes to //
>>> securely /J/
> 
> Why adding a line break here? This line is 75 columns as stated by the
> documentation: https://man7.org/linux/man-pages/man7/man-pages.7.html
> I guess it helps for semantic newlines, right? If so, what are the rules?
> 
>>
>> I'll add some line breaks [//] and line joins [/J/] through the email.
>>
>>> +restrict themselves and their future children.
>>> +Because Landlock is a stackable Linux Security Module (LSM),
>>> +it makes possible to create safe security sandboxes as new security
>>> layers
>>
>> suggested wfix: "it makes it possible" or "it is possible"?
> 
> Ok
> 
>>
>>> +in addition to the existing system-wide access-controls.
>>> +This kind of sandbox is expected to help mitigate // the security
>>> impact of /J/ > +bugs, // and unexpected or malicious behaviors in
>>> applications.
>>
>> See line-break fixes above.
> 
> Ok
> 
>>
>>> +.PP
>>> +A Landlock security policy is a set of access rights
>>> +(e.g., open a file in read-only, make a directory, etc.)
>>> +tied to a file hierarchy.
>>> +Such policy can be configured and enforced by processes for themselves
>>> +using three system calls:
>>> +.IP \(bu 2
>>> +.BR landlock_create_ruleset (2)
>>> +creates a new ruleset;
>>> +.IP \(bu
>>> +.BR landlock_add_rule (2)
>>> +adds a new rule to a ruleset;
>>> +.IP \(bu
>>> +.BR landlock_restrict_self (2)
>>> +enforces a ruleset on the calling thread.
>>> +.PP
>>> +To be able to use these system calls,
>>> +the running kernel must support Landlock and // it must be enabled at
>>> boot /J/
>>> +time.
>>
>> See line-break fixes above
> 
> Ok
> 
>>
>>> +.\"
>>> +.SS Landlock rules
>>> +A Landlock rule describes an action on an object.
>>> +An object is currently a file hierarchy,
>>> +and the related filesystem actions are defined with access rights (see
>>> +.BR landlock_add_rule (2)).
>>> +A set of rules is aggregated in a ruleset, // which can /J/
>>> +then restrict the thread enforcing it, // and its future children.
>>
>> See line-break fixes above.
> 
> Ok
> 
>>
>>> +.\"
>>> +.SS Filesystem actions
>>> +These flags enable to restrict a sandboxed process to a // set of
>>> actions on /J/
>>> +files and directories. > +Files or directories opened before the
>>> sandboxing // are not subject
>> to these /J/
>>> +restrictions.
>>
>> See line-break fixes above.
> 
> Ok
> 
>>
>>> +See
>>> +.BR landlock_add_rule (2)
>>> +and
>>> +.BR landlock_create_ruleset (2)
>>> +for more context.
>>> +.PP
>>> +A file can only receive these access rights:
>>> +.TP
>>> +.B LANDLOCK_ACCESS_FS_EXECUTE
>>> +Execute a file.
>>> +.TP
>>> +.B LANDLOCK_ACCESS_FS_WRITE_FILE
>>> +Open a file with write access.
>>> +.TP
>>> +.B LANDLOCK_ACCESS_FS_READ_FILE
>>> +Open a file with read access.
>>> +.PP
>>> +A directory can receive access rights related to files or directories.
>>> +The following access right is applied to the directory itself,
>>> +and the directories beneath it:
>>> +.TP
>>> +.B LANDLOCK_ACCESS_FS_READ_DIR
>>> +Open a directory or list its content.
>>> +.PP
>>> +However,
>>> +the following access rights only apply to the content of a directory,
>>> +not the directory itself:
>>> +.TP
>>> +.B LANDLOCK_ACCESS_FS_REMOVE_DIR
>>> +Remove an empty directory or rename one.
>>> +.TP
>>> +.B LANDLOCK_ACCESS_FS_REMOVE_FILE
>>> +Unlink (or rename) a file.
>>> +.TP
>>> +.B LANDLOCK_ACCESS_FS_MAKE_CHAR
>>> +Create (or rename or link) a character device.
>>> +.TP
>>> +.B LANDLOCK_ACCESS_FS_MAKE_DIR
>>> +Create (or rename) a directory.
>>> +.TP
>>> +.B LANDLOCK_ACCESS_FS_MAKE_REG
>>> +Create (or rename or link) a regular file.
>>> +.TP
>>> +.B LANDLOCK_ACCESS_FS_MAKE_SOCK
>>> +Create (or rename or link) a UNIX domain socket.
>>> +.TP
>>> +.B LANDLOCK_ACCESS_FS_MAKE_FIFO
>>> +Create (or rename or link) a named pipe.
>>> +.TP
>>> +.B LANDLOCK_ACCESS_FS_MAKE_BLOCK
>>> +Create (or rename or link) a block device.
>>> +.TP
>>> +.B LANDLOCK_ACCESS_FS_MAKE_SYM
>>> +Create (or rename or link) a symbolic link.
>>> +.\"
>>> +.SS Layers of file path access rights
>>> +Each time a thread enforces a ruleset on itself, // it updates its
>>> Landlock /J/
>>
>> See line-break fixes above
> 
> Ok
> 
>>
>>> +domain with a new layer of policy.
>>> +Indeed, this complementary policy is composed with the potentially other
>>> +rulesets already restricting this thread.
>>> +A sandboxed thread can then safely add more constraints to itself with a
>>> +new enforced ruleset.
>>> +.PP
>>> +One policy layer grants access to a file path // if at least one of
>>> its rules /J/
>>> +encountered on the path grants the access.
>>> +A sandboxed thread can only access a file path // if all its enforced
>>> policy /J/
>>> +layers grant the access // as well as all the other system access
>>> controls
>>> +(e.g., filesystem DAC, other LSM policies, etc.).
>>
>> See line-break fixes above.
> 
> Ok
> 
>>
>>> +.\"
>>> +.SS Bind mounts and OverlayFS
>>> +Landlock enables restricting access to file hierarchies,
>>> +which means that these access rights can be propagated with bind mounts
>>> +(cf.
>>> +.BR mount_namespaces (7))
>>> +but not with OverlayFS.
>>> +.PP
>>> +A bind mount mirrors a source file hierarchy to a destination.
>>> +The destination hierarchy is then composed of the exact same files,
>>> +on which Landlock rules can be tied, // either via the source or the /J/
>>> +destination path.
>>> +These rules restrict access when they are encountered on a path,
>>> +which means that they can restrict access to // multiple file
>>> hierarchies at /J/
>>> +the same time,
>>> +whether these hierarchies are the result of bind mounts or not.
>>
>>
>> See line-break fixes above.
> 
> Ok
> 
>>
>>> +.PP
>>> +An OverlayFS mount point consists of upper and lower layers.
>>> +These layers are combined in a merge directory, result of the mount
>>> point.
>>> +This merge hierarchy may include files from the upper and lower layers,
>>> +but modifications performed on the merge hierarchy // only reflects
>>> on the /J/
>>
>> s/reflects/reflect/
> 
> Ok
> 
>>
>>> +upper layer.
>>> +From a Landlock policy point of view,
>>> +each OverlayFS layers and merge hierarchies are standalone and contains
>>> +their own set of files and directories,
>>> +which is different from bind mounts.
>>
>>
>> Incorrect mix of singular and plural, I think.
> 
> Is it OK with s/contains/contain/?
> 
>>
>>> +A policy restricting an OverlayFS layer will not restrict the resulted
>>> +merged hierarchy, and vice versa.
>>> +Landlock users should then only think about file hierarchies they
>>> want to
>>> +allow access to, regardless of the underlying filesystem.
>>> +.\"
>>> +.SS Inheritance
>>> +Every new thread resulting from a
>>> +.BR clone (2)
>>> +inherits Landlock domain restrictions from its parent.
>>> +This is similar to the
>>> +.BR seccomp (2)
>>> +inheritance or any other LSM dealing with task's
>>
>> Not sure:
>>
>> s/task/a task/
>> or
>> s/task's/tasks'/
> 
> I'll take "tasks'".
> 
>>
>>> +.BR credentials (7).
>>> +For instance, one process's thread may apply Landlock rules to itself,
>>
>> s/process's/process'/
> 
> As pointed out by Branden, this is correct.
> 
>>
>>> +but they will not be automatically applied to other sibling threads
>>> +(unlike POSIX thread credential changes, cf.
>>> +.BR nptl (7)).
>>> +.PP
>>> +When a thread sandboxes itself, // we have the guarantee that the
>>> related /J/
>>> +security policy // will stay enforced on all this thread's descendants.
>>> +This allows creating standalone and modular security policies // per /J/
>>> +application,
>>> +which will automatically be composed between themselves // according
>>> to their /J/
>>> +runtime parent policies.
>>> +.\"
>>> +.SS Ptrace restrictions
>>> +A sandboxed process has less privileges than a non-sandboxed process and
>>> +must then be subject to additional restrictions // when manipulating
>>> another /J/
>>> +process.
>>> +To be allowed to use
>>> +.BR ptrace (2)
>>> +and related syscalls on a target process,
>>> +a sandboxed process should have a subset of the target process rules,
>>> +which means the tracee must be in a sub-domain of the tracer.
>>> +.SH VERSIONS
>>> +Landlock was added in Linux 5.13.
>>> +.SH NOTES
>>> +Landlock is enabled by CONFIG_SECURITY_LANDLOCK.
>>
>> .BR CONFIG_SECURITY_LANDLOCK .
> 
> Ok
> 
>>
>>> +The
>>> +.IR lsm=lsm1,...,lsmN
>>
>> s/.IR/.I/
> 
> Ok
> 
>>
>>> +command line parameter controls the sequence of the initialization of
>>> +Linux Security Modules.
>>> +It must contain the string
>>> +.IR landlock
>>
>> s/.IR/.I
> 
> Ok
> 
>>
>>> +to enable Landlock.
>>> +If the command line parameter is not specified,
>>> +the initialization falls back to the value of the deprecated
>>> +.IR security=
>>
>> s/.IR/.I/
> 
> Ok
> 
>>
>>> +command line parameter and further to the value of CONFIG_LSM.
>>> +We can check that Landlock is enabled by looking for
>>> +.IR "landlock: Up and running."
>>
>> s/.IR/.I/
> 
> Ok
> 
>>
>>> +in kernel logs.
>>> +.PP
>>> +It is currently not possible to restrict some file-related actions
>>> +accessible through these syscall families:
>>
>> When using syscall to refer to system call (not the function syscall(2)),
>> we use the extended form "system call".
> 
> Ok
> 
>>
>>> +.BR chdir (2),
>>> +.BR truncate (2),
>>> +.BR stat (2),
>>> +.BR flock (2),
>>> +.BR chmod (2),
>>> +.BR chown (2),
>>> +.BR setxattr (2),
>>> +.BR utime (2),
>>> +.BR ioctl (2),
>>> +.BR fcntl (2),
>>> +.BR access (2).
>>> +Future Landlock evolutions will enable to restrict them.
>>> +.SH EXAMPLES
>> I'd prefer a complete example with a main function, if you can come up
>> with such one.  If not, this will be ok.
> 
> I think it is clearer to not to use a full main to explain these basic
> steps. A full working example is linked though.
> 
>>
>>
>>> +We first need to create the ruleset that will contain our rules.
>>> +For this example,
>>> +the ruleset will contain rules that only allow read actions,
>>> +but write actions will be denied.
>>> +The ruleset then needs to handle both of these kind of actions.
>>> +See below for the description of filesystem actions.
>>> +.PP
>>> +.in +4n
>>> +.EX
>>> +int ruleset_fd;
>>> +struct landlock_ruleset_attr ruleset_attr = {
>>> +    .handled_access_fs =
>>> +        LANDLOCK_ACCESS_FS_EXECUTE |
>>> +        LANDLOCK_ACCESS_FS_WRITE_FILE |
>>> +        LANDLOCK_ACCESS_FS_READ_FILE |
>>> +        LANDLOCK_ACCESS_FS_READ_DIR |
>>> +        LANDLOCK_ACCESS_FS_REMOVE_DIR |
>>> +        LANDLOCK_ACCESS_FS_REMOVE_FILE |
>>> +        LANDLOCK_ACCESS_FS_MAKE_CHAR |
>>> +        LANDLOCK_ACCESS_FS_MAKE_DIR |
>>> +        LANDLOCK_ACCESS_FS_MAKE_REG |
>>> +        LANDLOCK_ACCESS_FS_MAKE_SOCK |
>>> +        LANDLOCK_ACCESS_FS_MAKE_FIFO |
>>> +        LANDLOCK_ACCESS_FS_MAKE_BLOCK |
>>> +        LANDLOCK_ACCESS_FS_MAKE_SYM,
>>> +};
>>> +
>>> +ruleset_fd = landlock_create_ruleset(&ruleset_attr,
>>> sizeof(ruleset_attr), 0);
>>> +if (ruleset_fd < 0) {
>>> +    perror("Failed to create a ruleset");
>>> +    return 1;
>>> +}
>>> +.EE
>>> +.in
>>> +.PP
>>> +We can now add a new rule to this ruleset thanks to the returned file
>>> +descriptor referring to this ruleset.
>>> +The rule will only allow reading the file hierarchy
>>> +.IR /usr .
> 
> Why ".IR" is correct here?
> 
> 
>>> +Without another rule, write actions would then be denied by the ruleset.
>>> +To add
>>> +.IR /usr
>>> +to the ruleset, we open it with the
>>> +.IR O_PATH
>>> +flag and fill the
>>> +.IR "struct landlock_path_beneath_attr"
>>> +with this file descriptor.
>>> +.PP
>>> +.in +4n
>>> +.EX
>>> +int err;
>>> +struct landlock_path_beneath_attr path_beneath = {
>>> +    .allowed_access =
>>> +        LANDLOCK_ACCESS_FS_EXECUTE |
>>> +        LANDLOCK_ACCESS_FS_READ_FILE |
>>> +        LANDLOCK_ACCESS_FS_READ_DIR,
>>> +};
>>> +
>>> +path_beneath.parent_fd = open("/usr", O_PATH | O_CLOEXEC);
>>> +if (path_beneath.parent_fd < 0) {
>>> +    perror("Failed to open file");
>>> +    close(ruleset_fd);
>>> +    return 1;
>>> +}
>>> +err = landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
>>> +                        &path_beneath, 0);
>>> +close(path_beneath.parent_fd);
>>> +if (err) {
>>> +    perror("Failed to update ruleset");
>>> +    close(ruleset_fd);
>>> +    return 1;
>>> +}
>>> +.EE
>>> +.in
>>> +.PP
>>> +We now have a ruleset with one rule allowing read access to
>>> +.IR /usr
>>> +while denying all other handled accesses for the filesystem.
>>> +The next step is to restrict the current thread from gaining more
>>> +privileges
>>> +(e.g., thanks to a set-user-ID binary).
>>> +.PP
>>> +.in +4n
>>> +.EX
>>> +if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0)) {
>>> +    perror("Failed to restrict privileges");
>>> +    close(ruleset_fd);
>>> +    return 1;
>>> +}
>>> +.EE
>>> +.in
>>> +.PP
>>> +The current thread is now ready to sandbox itself with the ruleset.
>>> +.PP
>>> +.in +4n
>>> +.EX
>>> +if (landlock_restrict_self(ruleset_fd, 0)) {
>>> +    perror("Failed to enforce ruleset");
>>> +    close(ruleset_fd);
>>> +    return 1;
>>> +}
>>> +close(ruleset_fd);
>>> +.EE
>>> +.in
>>> +.PP
>>> +If the
>>> +.BR landlock_restrict_self (2)
>>> +system call succeeds, the current thread is now restricted and this
>>> policy
>>> +will be enforced on all its subsequently created children as well.
>>> +Once a thread is landlocked, there is no way to remove its security
>>> policy;
>>> +only adding more restrictions is allowed.
>>> +These threads are now in a new Landlock domain, // merge of their
>>> parent one /J/
>>> +(if any) with the new ruleset.
>>> +.PP
>>> +Full working code can be found in
>>> +.UR
>>> https://git.kernel.org\:/pub\:/scm\:/linux\:/kernel\:/git\:/stable\:/linux.git\:/tree\:/samples\:/landlock\:/sandboxer.c
>>>
>>> +.UE
>>> +.SH SEE ALSO
>>> +.BR landlock_create_ruleset (2),
>>> +.BR landlock_add_rule (2),
>>> +.BR landlock_restrict_self (2)
>>> +.PP
>>> +.UR https://landlock.io\:/
>>> +.UE
>>>
>>
>>


-- 
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/

^ permalink raw reply

* Re: [RFC][PATCH v2 06/12] diglim: Interfaces - digest_list_add, digest_list_del
From: Mimi Zohar @ 2021-07-30 12:39 UTC (permalink / raw)
  To: Roberto Sassu, gregkh@linuxfoundation.org,
	mchehab+huawei@kernel.org
  Cc: linux-integrity@vger.kernel.org,
	linux-security-module@vger.kernel.org, linux-doc@vger.kernel.org,
	linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <ef7c85dcb096479e95c8c60ccda4d700@huawei.com>

Hi Roberto,

On Fri, 2021-07-30 at 07:16 +0000, Roberto Sassu wrote:
> > From: Mimi Zohar [mailto:zohar@linux.ibm.com]
> > Sent: Thursday, July 29, 2021 11:21 PM
> > 
> > On Mon, 2021-07-26 at 18:36 +0200, Roberto Sassu wrote:
> > > /*
> > > + * digest_list_read: read and parse the digest list from the path
> > > + */
> > > +static ssize_t digest_list_read(char *path, enum ops op)
> > > +{
> > > +       void *data = NULL;
> > > +       char *datap;
> > > +       size_t size;
> > > +       u8 actions = 0;
> > > +       struct file *file;
> > > +       char event_name[NAME_MAX + 9 + 1];
> > > +       u8 digest[IMA_MAX_DIGEST_SIZE] = { 0 };
> > > +       enum hash_algo algo;
> > > +       int rc, pathlen = strlen(path);
> > > +
> > > +       /* Remove \n. */
> > > +       datap = path;
> > > +       strsep(&datap, "\n");
> > > +
> > > +       file = filp_open(path, O_RDONLY, 0);
> > > +       if (IS_ERR(file)) {
> > > +               pr_err("unable to open file: %s (%ld)", path, PTR_ERR(file));
> > > +               return PTR_ERR(file);
> > > +       }
> > > +
> > > +       rc = kernel_read_file(file, 0, &data, INT_MAX, NULL,
> > > +                             READING_DIGEST_LIST);
> > > +       if (rc < 0) {
> > > +               pr_err("unable to read file: %s (%d)", path, rc);
> > > +               goto out;
> > > +       }
> > > +
> > > +       size = rc;
> > > +
> > > +       snprintf(event_name, sizeof(event_name), "%s_file_%s",
> > > +                op == DIGEST_LIST_ADD ? "add" : "del",
> > > +                file_dentry(file)->d_name.name);
> > > +
> > > +       rc = ima_measure_critical_data("diglim", event_name, data, size, false,
> > > +                                      digest, sizeof(digest));
> > > +       if (rc < 0 && rc != -EEXIST)
> > > +               goto out_vfree;
> > 
> > The digest lists could easily be measured while reading the digest list
> > file above in kernel_read_file().  What makes it "critical-data"?  In
> > the SELinux case, the in memory SELinux policy is being measured and
> > re-measured to make sure it hasn't been modified.  Is the digest list
> > file data being measured more than once?
> 
> Hi Mimi
> 
> yes, the digest lists can be measured with kernel_read_file().
> I didn't send the change yet, but I added a DIGEST_LIST_CHECK
> hook mapped to READING_DIGEST_LIST, so that digest lists
> can be easily measured or appraised.
> 
> The point was that the digest of the digest list must be always
> calculated, as it is added to the hash table. Instead of duplicating
> the code, I preferred to use ima_measure_critical_data().
> 
> The advantage is also that, if the use case is to just measure
> digest lists, ima_measure_critical_data() could do both at the
> same time.
> 
> Digest lists can be seen as "critical data" in the sense that
> they can affect the security decision on whether to grant
> access to a file or not, assuming that an appropriate rule is
> added in the IMA policy.

Of course the integrity of files containing the digest lists is
important, but that doesn't make them "critical data".  If the
integrity of these files is important, then the digest lists not only
need to be measured, but they need to be signed and the resulting
signature verified.  Without signature verification, there is no basis
on which to trust the digest lists data.

Adding the kernel_read_file() "READING_DIGEST_LIST" support in IMA does
not seem to be optional.  IMA would then be calculating the digest list
file hash twice, once in kernel_read_file() and then, again, in
ima_measure_critical_data().

> 
> > I understand that with your changes to ima_measure_critical_data(),
> > which are now in next-integrity-testing branch, allow IMA to calculate
> > the file data hash.
> 
> Yes, correct. But actually there is another useful use case.
> If digest lists are not in the format supported by the kernel,
> the user space parser has to convert them before uploading
> them to the kernel.
> 
> ima_measure_critical_data() would in this case measure
> the converted digest list (it is written directly, without
> sending the file path). It is easier to attest the result,
> instead of determining whether the user space parser
> produced the expected result (by checking the files it
> read).

The application to properly convert the digest list file data into the
appropriate format would need to be trusted.  I'm concerned that not
requiring the converted data to be signed and the signature verified is
introducing a new integrity gap.  Perhaps between an LSM policy,
limiting which files may be read by the application, and an IMA policy,
requiring all files read by this application to be measured and the
signature verified, this integrity gap could be averted.

"critical data", in this context, should probably be used for verifying
the in memory file digests and other state information haven't been
compromised.

thanks,

Mimi


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox