Linux Security Modules development
 help / color / mirror / Atom feed
* Re: Should mprotect(..., PROT_EXEC) be checked by IMA?
From: Matthew Garrett @ 2019-04-02 22:31 UTC (permalink / raw)
  To: Igor Zhbanov
  Cc: Stephen Smalley, Mimi Zohar, Kees Cook, Casey Schaufler,
	Paul Moore, John Johansen, linux-integrity, Jann Horn,
	linux-security-module
In-Reply-To: <cb46d5ba-30de-664f-67d4-646da4592a6f@omprussia.ru>

On Fri, Mar 29, 2019 at 5:50 AM Igor Zhbanov <i.zhbanov@omprussia.ru> wrote:
> I want to be sure that no unsigned code page could be executed. So exploits
> could only be of ROP kind and not being able to download any extra code
> from their servers. That's why I found that disabling of anonymous executable
> pages could be useful for that (as well as disabling of making executable
> pages writable to modify already mapped code). In conjunction with IMA it
> should guarantee that no untrusted code could be executed.

Remember that many interpreted languages allow execution of code
provided to them on the command line (eg, python -c) and also grant
access to arbitrary syscalls, so there's still no guarantee that
you're only executing trusted code.

^ permalink raw reply

* Re: Should mprotect(..., PROT_EXEC) be checked by IMA?
From: Igor Zhbanov @ 2019-04-03  9:59 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: Stephen Smalley, Mimi Zohar, Kees Cook, Casey Schaufler,
	Paul Moore, John Johansen, linux-integrity, Jann Horn,
	linux-security-module
In-Reply-To: <CACdnJusPpis1JJgfNOrQoqPcVk0q2OGbosrXOVtZmG0cSqkotw@mail.gmail.com>

On 03.04.2019 1:31, Matthew Garrett wrote:
> On Fri, Mar 29, 2019 at 5:50 AM Igor Zhbanov <i.zhbanov@omprussia.ru> wrote:
>> I want to be sure that no unsigned code page could be executed. So
>> exploits could only be of ROP kind and not being able to download
>> any extra code from their servers. That's why I found that
>> disabling of anonymous executable pages could be useful for that
>> (as well as disabling of making executable pages writable to modify
>> already mapped code). In conjunction with IMA it should guarantee
>> that no untrusted code could be executed.
> 
> Remember that many interpreted languages allow execution of code
> provided to them on the command line (eg, python -c) and also grant
> access to arbitrary syscalls, so there's still no guarantee that
> you're only executing trusted code.

Yes. But in some installations you can get rid of interpreters at all or limit
the number of scripts they can open. For example you can require that all
scripts have to be signed.

And having this feature as a per-process you could still limit the attack
surface by restricting e.g. network services as they are constantly attacked.

So are you saying that this feature doesn't worth to make it?

^ permalink raw reply

* Re: Should mprotect(..., PROT_EXEC) be checked by IMA?
From: Mimi Zohar @ 2019-04-03 11:57 UTC (permalink / raw)
  To: Igor Zhbanov, Stephen Smalley, Matthew Garrett, Kees Cook,
	Casey Schaufler, Paul Moore, John Johansen
  Cc: linux-integrity, Jann Horn, linux-security-module
In-Reply-To: <cb46d5ba-30de-664f-67d4-646da4592a6f@omprussia.ru>

On Fri, 2019-03-29 at 15:50 +0300, Igor Zhbanov wrote:
> On 29.03.2019 15:28, Stephen Smalley wrote:
> > On 3/29/19 6:59 AM, Mimi Zohar wrote:
> >> [Cc'ing the LSM mailing list and others]
> >>
> >> On Fri, 2019-03-29 at 13:00 +0300, Igor Zhbanov wrote:
> >>> Hi Mimi,On 28.03.2019 20:17, Mimi Zohar wrote:
> >>
> >>>> I just came across the grsecurity article on mprotect.[1]
> >>>>   Has anyone looked at it? Would it make sense to make it a minor LSM?
> >>>>
> >>>> [1]https://pax.grsecurity.net/docs/mprotect.txt
> >>>
> >>> Interesting article. It is almost exactly of what I wanted to be
> >>> implemented.
> >>>
> >>> If this minor LSM would be stackable to allow combining with e.g. SELinux
> >>> then why not.
> >>
> >> Stacking shouldn't be a problem.  Other LSMs are already on the
> >> mprotect hook.  Let's hear what others think.
> > 
> > SELinux already provides a set of controls over executable mappings;
> > see selinux_mmap_file and selinux_file_mprotect. Other major security
> > modules may do likewise but I can't speak to that. Is there some gap
> > you are trying to address that isn't already covered, or are you just
> > trying to provide such restrictions without requiring one of the
> > major modules?
> 
> I want to be sure that no unsigned code page could be executed. So exploits
> could only be of ROP kind and not being able to download any extra code
> from their servers. That's why I found that disabling of anonymous executable
> pages could be useful for that (as well as disabling of making executable
> pages writable to modify already mapped code). In conjunction with IMA it
> should guarantee that no untrusted code could be executed.

Let's separate the different types of attacks.  From an IMA
perspective, memory attacks are out of scope.  That leaves mmap'ed
files, possibly just mmap'ed shared files.  Currently IMA can be
configured to verify a file's integrity, based on signatures, being
mmap'ed execute.  Assuming that not all files opened require a file
signature, a file could be mmap'ed read/write and later changed to
execute to circumvent the mmap'ed execute signature requirement.  If
the existing LSMs are able to prevent this sort of attack, we could
just document this requirement.

Mimi


^ permalink raw reply

* Re: Should mprotect(..., PROT_EXEC) be checked by IMA?
From: Mimi Zohar @ 2019-04-03 12:11 UTC (permalink / raw)
  To: Matthew Garrett, Igor Zhbanov
  Cc: Stephen Smalley, Kees Cook, Casey Schaufler, Paul Moore,
	John Johansen, linux-integrity, Jann Horn, linux-security-module
In-Reply-To: <CACdnJusPpis1JJgfNOrQoqPcVk0q2OGbosrXOVtZmG0cSqkotw@mail.gmail.com>

On Tue, 2019-04-02 at 15:31 -0700, Matthew Garrett wrote:
> On Fri, Mar 29, 2019 at 5:50 AM Igor Zhbanov <i.zhbanov@omprussia.ru> wrote:
> > I want to be sure that no unsigned code page could be executed. So exploits
> > could only be of ROP kind and not being able to download any extra code
> > from their servers. That's why I found that disabling of anonymous executable
> > pages could be useful for that (as well as disabling of making executable
> > pages writable to modify already mapped code). In conjunction with IMA it
> > should guarantee that no untrusted code could be executed.
> 
> Remember that many interpreted languages allow execution of code
> provided to them on the command line (eg, python -c) and also grant
> access to arbitrary syscalls, so there's still no guarantee that
> you're only executing trusted code.

Interpreters are a known concern, as Yves-Alexis Perez pointed out in
his LSS-2018 Europe talk[1].

Mimi

[1] https://events.linuxfoundation.org/wp-content/uploads/2017/12/Linu
x-Kernel-Security-Contributions-by-ANSSI-Yves-Alexis-Perez-ANSSI.pdf


^ permalink raw reply

* Re: Should mprotect(..., PROT_EXEC) be checked by IMA?
From: Stephen Smalley @ 2019-04-03 13:10 UTC (permalink / raw)
  To: Mimi Zohar, Igor Zhbanov, Matthew Garrett, Kees Cook,
	Casey Schaufler, Paul Moore, John Johansen
  Cc: linux-integrity, Jann Horn, linux-security-module
In-Reply-To: <1554292678.7309.47.camel@linux.ibm.com>

On 4/3/19 7:57 AM, Mimi Zohar wrote:
> On Fri, 2019-03-29 at 15:50 +0300, Igor Zhbanov wrote:
>> On 29.03.2019 15:28, Stephen Smalley wrote:
>>> On 3/29/19 6:59 AM, Mimi Zohar wrote:
>>>> [Cc'ing the LSM mailing list and others]
>>>>
>>>> On Fri, 2019-03-29 at 13:00 +0300, Igor Zhbanov wrote:
>>>>> Hi Mimi,On 28.03.2019 20:17, Mimi Zohar wrote:
>>>>
>>>>>> I just came across the grsecurity article on mprotect.[1]
>>>>>>    Has anyone looked at it? Would it make sense to make it a minor LSM?
>>>>>>
>>>>>> [1]https://pax.grsecurity.net/docs/mprotect.txt
>>>>>
>>>>> Interesting article. It is almost exactly of what I wanted to be
>>>>> implemented.
>>>>>
>>>>> If this minor LSM would be stackable to allow combining with e.g. SELinux
>>>>> then why not.
>>>>
>>>> Stacking shouldn't be a problem.  Other LSMs are already on the
>>>> mprotect hook.  Let's hear what others think.
>>>
>>> SELinux already provides a set of controls over executable mappings;
>>> see selinux_mmap_file and selinux_file_mprotect. Other major security
>>> modules may do likewise but I can't speak to that. Is there some gap
>>> you are trying to address that isn't already covered, or are you just
>>> trying to provide such restrictions without requiring one of the
>>> major modules?
>>
>> I want to be sure that no unsigned code page could be executed. So exploits
>> could only be of ROP kind and not being able to download any extra code
>> from their servers. That's why I found that disabling of anonymous executable
>> pages could be useful for that (as well as disabling of making executable
>> pages writable to modify already mapped code). In conjunction with IMA it
>> should guarantee that no untrusted code could be executed.
> 
> Let's separate the different types of attacks.  From an IMA
> perspective, memory attacks are out of scope.  That leaves mmap'ed
> files, possibly just mmap'ed shared files.  Currently IMA can be
> configured to verify a file's integrity, based on signatures, being
> mmap'ed execute.  Assuming that not all files opened require a file
> signature, a file could be mmap'ed read/write and later changed to
> execute to circumvent the mmap'ed execute signature requirement.  If
> the existing LSMs are able to prevent this sort of attack, we could
> just document this requirement.

I guess I don't understand why IMA isn't already being called from 
security_file_mprotect(). security_file_mprotect() could just call 
ima_file_mmap(vma->vm_file, prot) if all of the security hooks pass.

SELinux can be used to prevent unauthorized mprotect PROT_EXEC but it 
won't perform a measurement of the file if it is allowed by policy.

^ permalink raw reply

* Re: Should mprotect(..., PROT_EXEC) be checked by IMA?
From: Perez Yves-Alexis @ 2019-04-03 13:18 UTC (permalink / raw)
  To: Mimi Zohar, Matthew Garrett, Igor Zhbanov
  Cc: Stephen Smalley, Kees Cook, Casey Schaufler, Paul Moore,
	John Johansen, linux-integrity, Jann Horn, linux-security-module,
	Mickaël Salaün, Yves-Alexis Perez,
	Mickaël Salaün
In-Reply-To: <1554293478.7309.54.camel@linux.ibm.com>

Le 03/04/2019 à 14:11, Mimi Zohar a écrit :
> On Tue, 2019-04-02 at 15:31 -0700, Matthew Garrett wrote:
>> On Fri, Mar 29, 2019 at 5:50 AM Igor Zhbanov <i.zhbanov@omprussia.ru> wrote:
>>> I want to be sure that no unsigned code page could be executed. So exploits
>>> could only be of ROP kind and not being able to download any extra code
>>> from their servers. That's why I found that disabling of anonymous executable
>>> pages could be useful for that (as well as disabling of making executable
>>> pages writable to modify already mapped code). In conjunction with IMA it
>>> should guarantee that no untrusted code could be executed.
>>
>> Remember that many interpreted languages allow execution of code
>> provided to them on the command line (eg, python -c) and also grant
>> access to arbitrary syscalls, so there's still no guarantee that
>> you're only executing trusted code.
>
> Interpreters are a known concern, as Yves-Alexis Perez pointed out in
> his LSS-2018 Europe talk[1].
>
> Mimi
>
> [1] https://events.linuxfoundation.org/wp-content/uploads/2017/12/Linu
> x-Kernel-Security-Contributions-by-ANSSI-Yves-Alexis-Perez-ANSSI.pdf
>
And Mickaël Salaün posted the O_MAYEXEC patch RFC back in December
(https://lore.kernel.org/lkml/20181212081712.32347-1-mic@digikod.net/)

Regards,
--
Yves-Alexis Perez
ANSSI/SDE/ST/LAM
Les données à caractère personnel recueillies et traitées dans le cadre de cet échange, le sont à seule fin d’exécution d’une relation professionnelle et s’opèrent dans cette seule finalité et pour la durée nécessaire à cette relation. Si vous souhaitez faire usage de vos droits de consultation, de rectification et de suppression de vos données, veuillez contacter contact.rgpd@sgdsn.gouv.fr. Si vous avez reçu ce message par erreur, nous vous remercions d’en informer l’expéditeur et de détruire le message. The personal data collected and processed during this exchange aims solely at completing a business relationship and is limited to the necessary duration of that relationship. If you wish to use your rights of consultation, rectification and deletion of your data, please contact: contact.rgpd@sgdsn.gouv.fr. If you have received this message in error, we thank you for informing the sender and destroying the message.

^ permalink raw reply

* RE: [PATCH 1/2] efi: Fix cast to pointer from integer of different size in TPM log code
From: David Laight @ 2019-04-03 13:42 UTC (permalink / raw)
  To: 'Matthew Garrett', linux-integrity@vger.kernel.org
  Cc: peterhuewe@gmx.de, jarkko.sakkinen@linux.intel.com, jgg@ziepe.ca,
	roberto.sassu@huawei.com, linux-efi@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	linux-kernel@vger.kernel.org, tweek@google.com, Matthew Garrett
In-Reply-To: <20190402215556.257406-2-matthewgarrett@google.com>

From: Matthew Garrett
> Sent: 02 April 2019 22:56
> 
> 8bfcff4a6a1d9d7226bb63a7da758b82d9ab4373 introduced a cast from
> efi_physical_address_t to (void *), which are different sizes on 32-bit.
> Fix that. Caught by the 0-day test bot.

Casting a physical address to 'void *' seems completely wrong.
Also you'd need a guarantee that the address was below 4G or the result
is meaningless.
Looks to me like something is using the wrong types somewhere.

	David

> Signed-off-by: Matthew Garrett <mjg59@google.com>
> ---
>  drivers/firmware/efi/libstub/tpm.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/firmware/efi/libstub/tpm.c b/drivers/firmware/efi/libstub/tpm.c
> index b6e93e14fcf1..6b3b507a54eb 100644
> --- a/drivers/firmware/efi/libstub/tpm.c
> +++ b/drivers/firmware/efi/libstub/tpm.c
> @@ -114,8 +114,8 @@ void efi_retrieve_tpm2_eventlog(efi_system_table_t *sys_table_arg)
>  			 */
>  			last_entry_size =
>  				__calc_tpm2_event_size((void *)last_entry_addr,
> -						       (void *)log_location,
> -						       false);
> +						    (void *)(long)log_location,
> +						    false);
>  		} else {
>  			last_entry_size = sizeof(struct tcpa_event) +
>  			   ((struct tcpa_event *) last_entry_addr)->event_size;
> --
> 2.21.0.392.gf8f6787159e-goog

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

^ permalink raw reply

* Re: Should mprotect(..., PROT_EXEC) be checked by IMA?
From: Mimi Zohar @ 2019-04-03 14:33 UTC (permalink / raw)
  To: Stephen Smalley, Igor Zhbanov, Matthew Garrett, Kees Cook,
	Casey Schaufler, Paul Moore, John Johansen
  Cc: linux-integrity, Jann Horn, linux-security-module
In-Reply-To: <648f1a64-e95d-d929-62dc-06decaf5a14b@tycho.nsa.gov>

On Wed, 2019-04-03 at 09:10 -0400, Stephen Smalley wrote:
> On 4/3/19 7:57 AM, Mimi Zohar wrote:

> > Let's separate the different types of attacks.  From an IMA
> > perspective, memory attacks are out of scope.  That leaves mmap'ed
> > files, possibly just mmap'ed shared files.  Currently IMA can be
> > configured to verify a file's integrity, based on signatures, being
> > mmap'ed execute.  Assuming that not all files opened require a file
> > signature, a file could be mmap'ed read/write and later changed to
> > execute to circumvent the mmap'ed execute signature requirement.  If
> > the existing LSMs are able to prevent this sort of attack, we could
> > just document this requirement.
> 
> I guess I don't understand why IMA isn't already being called from 
> security_file_mprotect(). security_file_mprotect() could just call 
> ima_file_mmap(vma->vm_file, prot) if all of the security hooks pass.
> 
> SELinux can be used to prevent unauthorized mprotect PROT_EXEC but it 
> won't perform a measurement of the file if it is allowed by policy.

From a measurement perspective, this will at least measure the file,
but the call to ima_file_mmap() will verify the file signature against
the file, not what is currently in memory, right?

Mimi


^ permalink raw reply

* Re: Should mprotect(..., PROT_EXEC) be checked by IMA?
From: Stephen Smalley @ 2019-04-03 14:33 UTC (permalink / raw)
  To: Mimi Zohar, Igor Zhbanov, Matthew Garrett, Kees Cook,
	Casey Schaufler, Paul Moore, John Johansen
  Cc: linux-integrity, Jann Horn, linux-security-module
In-Reply-To: <1554301990.7309.71.camel@linux.ibm.com>

On 4/3/19 10:33 AM, Mimi Zohar wrote:
> On Wed, 2019-04-03 at 09:10 -0400, Stephen Smalley wrote:
>> On 4/3/19 7:57 AM, Mimi Zohar wrote:
> 
>>> Let's separate the different types of attacks.  From an IMA
>>> perspective, memory attacks are out of scope.  That leaves mmap'ed
>>> files, possibly just mmap'ed shared files.  Currently IMA can be
>>> configured to verify a file's integrity, based on signatures, being
>>> mmap'ed execute.  Assuming that not all files opened require a file
>>> signature, a file could be mmap'ed read/write and later changed to
>>> execute to circumvent the mmap'ed execute signature requirement.  If
>>> the existing LSMs are able to prevent this sort of attack, we could
>>> just document this requirement.
>>
>> I guess I don't understand why IMA isn't already being called from
>> security_file_mprotect(). security_file_mprotect() could just call
>> ima_file_mmap(vma->vm_file, prot) if all of the security hooks pass.
>>
>> SELinux can be used to prevent unauthorized mprotect PROT_EXEC but it
>> won't perform a measurement of the file if it is allowed by policy.
> 
>  From a measurement perspective, this will at least measure the file,
> but the call to ima_file_mmap() will verify the file signature against
> the file, not what is currently in memory, right?

Yes, but you can use SELinux to prevent that (don't allow execmem or 
execmod permissions for that domain).




^ permalink raw reply

* Re: Should mprotect(..., PROT_EXEC) be checked by IMA?
From: Mimi Zohar @ 2019-04-03 16:21 UTC (permalink / raw)
  To: Stephen Smalley, Igor Zhbanov, Matthew Garrett, Kees Cook,
	Casey Schaufler, Paul Moore, John Johansen
  Cc: linux-integrity, Jann Horn, linux-security-module
In-Reply-To: <d40b94ed-b9c8-d877-b109-d45c5472ae15@tycho.nsa.gov>

On Wed, 2019-04-03 at 10:33 -0400, Stephen Smalley wrote:
> On 4/3/19 10:33 AM, Mimi Zohar wrote:
> > On Wed, 2019-04-03 at 09:10 -0400, Stephen Smalley wrote:
> >> On 4/3/19 7:57 AM, Mimi Zohar wrote:
> > 
> >>> Let's separate the different types of attacks.  From an IMA
> >>> perspective, memory attacks are out of scope.  That leaves mmap'ed
> >>> files, possibly just mmap'ed shared files.  Currently IMA can be
> >>> configured to verify a file's integrity, based on signatures, being
> >>> mmap'ed execute.  Assuming that not all files opened require a file
> >>> signature, a file could be mmap'ed read/write and later changed to
> >>> execute to circumvent the mmap'ed execute signature requirement.  If
> >>> the existing LSMs are able to prevent this sort of attack, we could
> >>> just document this requirement.
> >>
> >> I guess I don't understand why IMA isn't already being called from
> >> security_file_mprotect(). security_file_mprotect() could just call
> >> ima_file_mmap(vma->vm_file, prot) if all of the security hooks pass.
> >>
> >> SELinux can be used to prevent unauthorized mprotect PROT_EXEC but it
> >> won't perform a measurement of the file if it is allowed by policy.
> > 
> >  From a measurement perspective, this will at least measure the file,
> > but the call to ima_file_mmap() will verify the file signature against
> > the file, not what is currently in memory, right?
> 
> Yes, but you can use SELinux to prevent that (don't allow execmem or 
> execmod permissions for that domain).

Ok, let's start with at least adding the ima_file_mmap() call as you
suggested.  For LSMs which don't have this capability, they might need
to be stacked with a minor LSM such as S.A.R.A, once it is upstreamed,
or similar.

Mimi


^ permalink raw reply

* Re: [PATCH 1/2] efi: Fix cast to pointer from integer of different size in TPM log code
From: Matthew Garrett @ 2019-04-03 16:51 UTC (permalink / raw)
  To: David Laight
  Cc: linux-integrity@vger.kernel.org, peterhuewe@gmx.de,
	jarkko.sakkinen@linux.intel.com, jgg@ziepe.ca,
	roberto.sassu@huawei.com, linux-efi@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	linux-kernel@vger.kernel.org, tweek@google.com
In-Reply-To: <89e1cf4093d7405987c55f682519f9fc@AcuMS.aculab.com>

On Wed, Apr 3, 2019 at 6:41 AM David Laight <David.Laight@aculab.com> wrote:
>
> From: Matthew Garrett
> > Sent: 02 April 2019 22:56
> >
> > 8bfcff4a6a1d9d7226bb63a7da758b82d9ab4373 introduced a cast from
> > efi_physical_address_t to (void *), which are different sizes on 32-bit.
> > Fix that. Caught by the 0-day test bot.
>
> Casting a physical address to 'void *' seems completely wrong.
> Also you'd need a guarantee that the address was below 4G or the result
> is meaningless.
> Looks to me like something is using the wrong types somewhere.

We're in UEFI here, not the kernel proper - the firmware functions we
call give us back physical addresses, and we're operating with a 1:1
mapping. long is 64 bit on 64 bit systems, and on 32 bit systems we've
already asserted that all firmware resources are under 4GB (obviously
we're going to have a bad time if they're not, but there's not really
anything we can do about that)

^ permalink raw reply

* Re: Should mprotect(..., PROT_EXEC) be checked by IMA?
From: Matthew Garrett @ 2019-04-03 16:58 UTC (permalink / raw)
  To: Igor Zhbanov
  Cc: Stephen Smalley, Mimi Zohar, Kees Cook, Casey Schaufler,
	Paul Moore, John Johansen, linux-integrity, Jann Horn,
	linux-security-module
In-Reply-To: <11fcd3b2-afb9-74d2-100d-449df61433d1@omprussia.ru>

On Wed, Apr 3, 2019 at 2:59 AM Igor Zhbanov <i.zhbanov@omprussia.ru> wrote:
>
> On 03.04.2019 1:31, Matthew Garrett wrote:
> > Remember that many interpreted languages allow execution of code
> > provided to them on the command line (eg, python -c) and also grant
> > access to arbitrary syscalls, so there's still no guarantee that
> > you're only executing trusted code.
>
> Yes. But in some installations you can get rid of interpreters at all or limit
> the number of scripts they can open. For example you can require that all
> scripts have to be signed.

No, that's my point - if you're able to pass code directly to the
interpreter then you're not protected by file signatures. python -c
'print("hello")' will execute without the user-provided code touching
IMA.

> And having this feature as a per-process you could still limit the attack
> surface by restricting e.g. network services as they are constantly attacked.
>
> So are you saying that this feature doesn't worth to make it?

I'm saying that before adding more security code you should describe
the attack that you're actually trying to prevent. What you're doing
prevents applications from opening a file read-only and then
mprotect()ing it to being executable without IMA noticing, but what
attack are you actually protecting against as a result? You block one
avenue of obtaining code execution that isn't measured by IMA, but
there are many more. Most (but not all) are blocked by requiring that
all files be signed, but if all files are signed we don't need to
change the behaviour here - opening the file read-only will be enough
to trigger an appraisal.

^ permalink raw reply

* Re: Should mprotect(..., PROT_EXEC) be checked by IMA?
From: Igor Zhbanov @ 2019-04-03 17:31 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: Stephen Smalley, Mimi Zohar, Kees Cook, Casey Schaufler,
	Paul Moore, John Johansen, linux-integrity, Jann Horn,
	linux-security-module
In-Reply-To: <CACdnJusgwpcSxzwNEieqsa54_3cTYMEfuXpAeP0cW81TQ29E+w@mail.gmail.com>

On 03.04.2019 19:58, Matthew Garrett wrote:
> On Wed, Apr 3, 2019 at 2:59 AM Igor Zhbanov <i.zhbanov@omprussia.ru> wrote:
> No, that's my point - if you're able to pass code directly to the
> interpreter then you're not protected by file signatures. python -c
> 'print("hello")' will execute without the user-provided code touching
> IMA.
> 
>> And having this feature as a per-process you could still limit the attack
>> surface by restricting e.g. network services as they are constantly attacked.
>>
>> So are you saying that this feature doesn't worth to make it?
> 
> I'm saying that before adding more security code you should describe
> the attack that you're actually trying to prevent. What you're doing
> prevents applications from opening a file read-only and then
> mprotect()ing it to being executable without IMA noticing, but what
> attack are you actually protecting against as a result? You block one
> avenue of obtaining code execution that isn't measured by IMA, but
> there are many more. Most (but not all) are blocked by requiring that
> all files be signed, but if all files are signed we don't need to
> change the behaviour here - opening the file read-only will be enough
> to trigger an appraisal.

I'm trying to reduce attacker's possibilities to inject any new unauthorized
code. Currently it could be:
1) Downloaded binaries (on disk) which is validated by IMA,

2) Downloaded scripts (which need some combined approach like removing
    unneeded interpreters and checking the they are open and disabling passing
    the script as command line argument),

3) Return-Oriented-Programming exploits. Compiler support for validating
    return addresses is needed.

4) Anonymous executable pages (either new or existing changing to writable).
    ^ This is what I'm talking about. Because it's relatively easy to create
    anonymous executable page to stay below the radar. Because even if you
    enable signature checking for all opened files it would be possible to
    simply download the code and execute it directly from the anonymous pages.

By closing one by one these possibilities one can be (relatively) sure that
only trusted code and scripts could be executed.

^ permalink raw reply

* Re: Add support for TCG2 log format on UEFI systems
From: Jarkko Sakkinen @ 2019-04-03 17:50 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: tomas.winkler, linux-integrity, Peter Huewe, Jason Gunthorpe,
	Roberto Sassu, linux-efi, LSM List, Linux Kernel Mailing List,
	Thiébaud Weksteen
In-Reply-To: <CACdnJuvFxKCwP3XftqacFiLTmTZzCGHkc9DzrJu2u6vwy5_o-w@mail.gmail.com>

On Tue, Apr 02, 2019 at 10:15:39AM -0700, Matthew Garrett wrote:
> On Tue, Apr 2, 2019 at 6:07 AM Jarkko Sakkinen
> <jarkko.sakkinen@linux.intel.com> wrote:
> > Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> >
> > I'll apply all patches soonish and include them to the next PR.
> 
> Thanks! Looks like I need some fixes to deal with non-x86
> architectures, I'll get on that today.

Great thanks. I'll check them tomorrow (Thu).

/Jarkko

^ permalink raw reply

* Re: Should mprotect(..., PROT_EXEC) be checked by IMA?
From: Matthew Garrett @ 2019-04-03 18:19 UTC (permalink / raw)
  To: Igor Zhbanov
  Cc: Stephen Smalley, Mimi Zohar, Kees Cook, Casey Schaufler,
	Paul Moore, John Johansen, linux-integrity, Jann Horn,
	linux-security-module
In-Reply-To: <bca4eb5b-2d76-5df8-6e63-573b3a057985@omprussia.ru>

On Wed, Apr 3, 2019 at 10:31 AM Igor Zhbanov <i.zhbanov@omprussia.ru> wrote:
> I'm trying to reduce attacker's possibilities to inject any new unauthorized
> code. Currently it could be:

(snip)

> 4) Anonymous executable pages (either new or existing changing to writable).
>     ^ This is what I'm talking about. Because it's relatively easy to create
>     anonymous executable page to stay below the radar. Because even if you
>     enable signature checking for all opened files it would be possible to
>     simply download the code and execute it directly from the anonymous pages.

There's two possible cases here:

1) The application is legitimate but can be convinced to open and
execute malicious code. There should be no such applications that
download code from the internet and execute it directly, so this can
be prevented by requiring that files be signed (which has to be done
to protect against attackers just using an interpreted language
instead)
2) The application is actively malicious. In this case this approach
is insufficient - an actively malicious application can interpret code
rather than executing it directly. This can only be prevented by not
signing malicious applications.

When you talk about "staying below the radar" it implies that you're
talking about case 2, but the proposed solution is only a speed bump
rather than a blocker.

^ permalink raw reply

* Re: Should mprotect(..., PROT_EXEC) be checked by IMA?
From: Igor Zhbanov @ 2019-04-03 18:47 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: Stephen Smalley, Mimi Zohar, Kees Cook, Casey Schaufler,
	Paul Moore, John Johansen, linux-integrity, Jann Horn,
	linux-security-module
In-Reply-To: <CACdnJutv4O9bPwvddrasz2up06P6OtBbHjsr6JGSOwAtryMTpg@mail.gmail.com>

On 03.04.2019 21:19, Matthew Garrett wrote:
> On Wed, Apr 3, 2019 at 10:31 AM Igor Zhbanov <i.zhbanov@omprussia.ru> wrote:
>> I'm trying to reduce attacker's possibilities to inject any new unauthorized
>> code. Currently it could be:
> 
> (snip)
> 
>> 4) Anonymous executable pages (either new or existing changing to writable).
>>    ^ This is what I'm talking about. Because it's relatively easy to create
>>    anonymous executable page to stay below the radar. Because even if you
>>    enable signature checking for all opened files it would be possible to
>>    simply download the code and execute it directly from the anonymous pages.
> 
> There's two possible cases here:
> 
> 1) The application is legitimate but can be convinced to open and
> execute malicious code. There should be no such applications that
> download code from the internet and execute it directly, so this can
> be prevented by requiring that files be signed (which has to be done
> to protect against attackers just using an interpreted language
> instead)
> 2) The application is actively malicious. In this case this approach
> is insufficient - an actively malicious application can interpret code
> rather than executing it directly. This can only be prevented by not
> signing malicious applications.
> 
> When you talk about "staying below the radar" it implies that you're
> talking about case 2, but the proposed solution is only a speed bump
> rather than a blocker.

But what about buffer/stack overflow? The application doesn't need to be
malicious. It could be just a web-browser or e-mail client processing
some evil file.

^ permalink raw reply

* Re: Should mprotect(..., PROT_EXEC) be checked by IMA?
From: Matthew Garrett @ 2019-04-03 19:25 UTC (permalink / raw)
  To: Igor Zhbanov
  Cc: Stephen Smalley, Mimi Zohar, Kees Cook, Casey Schaufler,
	Paul Moore, John Johansen, linux-integrity, Jann Horn,
	linux-security-module
In-Reply-To: <3ebff410-f1bd-654e-7cd0-486ea4768ebe@omprussia.ru>

On Wed, Apr 3, 2019 at 11:47 AM Igor Zhbanov <i.zhbanov@omprussia.ru> wrote:
> On 03.04.2019 21:19, Matthew Garrett wrote:
> > There's two possible cases here:
> >
> > 1) The application is legitimate but can be convinced to open and
> > execute malicious code. There should be no such applications that
> > download code from the internet and execute it directly, so this can
> > be prevented by requiring that files be signed (which has to be done
> > to protect against attackers just using an interpreted language
> > instead)
> > 2) The application is actively malicious. In this case this approach
> > is insufficient - an actively malicious application can interpret code
> > rather than executing it directly. This can only be prevented by not
> > signing malicious applications.
> >
> > When you talk about "staying below the radar" it implies that you're
> > talking about case 2, but the proposed solution is only a speed bump
> > rather than a blocker.
>
> But what about buffer/stack overflow? The application doesn't need to be
> malicious. It could be just a web-browser or e-mail client processing
> some evil file.

Executable pages shouldn't be writable?

^ permalink raw reply

* [PATCH] Smack: Fix IPv6 handling of 0 secmark
From: Casey Schaufler @ 2019-04-03 22:01 UTC (permalink / raw)
  To: casey, linux-security-module, SMACK-discuss

Handle the case where the skb for an IPv6 packet contains
a 0 in the secmark for a packet generated locally. This
can only happen for system packets, so allow the access.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 security/smack/smack_lsm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index bd45c9139d34..b9abcdb36a73 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -3907,6 +3907,8 @@ static int smack_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
 #ifdef SMACK_IPV6_SECMARK_LABELING
 		if (skb && skb->secmark != 0)
 			skp = smack_from_secid(skb->secmark);
+		else if (smk_ipv6_localhost(&sadd))
+			break;
 		else
 			skp = smack_ipv6host_label(&sadd);
 		if (skp == NULL)
-- 
2.19.1


^ permalink raw reply related

* [PATCH V32 0/27] Lockdown patches for 5.2
From: Matthew Garrett @ 2019-04-04  0:32 UTC (permalink / raw)
  To: jmorris; +Cc: linux-security-module, linux-kernel, dhowells, linux-api, luto

Fairly minimal changes since the last set: tracefs is restricted at
Steven's suggestion (but could do with a once-over, I'm very much not a
vfs person), debugfs is back to Dave's original implementation. I've
also fixed up a malformed patch that resulted from me getting confused
during rebase, and added some further documentation to the initial patch
in order to give a reference for the design goals.



^ permalink raw reply

* [PATCH V32 04/27] kexec_load: Disable at runtime if the kernel is locked down
From: Matthew Garrett @ 2019-04-04  0:32 UTC (permalink / raw)
  To: jmorris
  Cc: linux-security-module, linux-kernel, dhowells, linux-api, luto,
	Matthew Garrett, Matthew Garrett, Dave Young, kexec
In-Reply-To: <20190404003249.14356-1-matthewgarrett@google.com>

From: Matthew Garrett <mjg59@srcf.ucam.org>

The kexec_load() syscall permits the loading and execution of arbitrary
code in ring 0, which is something that lock-down is meant to prevent. It
makes sense to disable kexec_load() in this situation.

This does not affect kexec_file_load() syscall which can check for a
signature on the image to be booted.

Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Matthew Garrett <mjg59@google.com>
Acked-by: Dave Young <dyoung@redhat.com>
cc: kexec@lists.infradead.org
---
 kernel/kexec.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/kernel/kexec.c b/kernel/kexec.c
index 68559808fdfa..57047acc9a36 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -207,6 +207,14 @@ static inline int kexec_load_check(unsigned long nr_segments,
 	if (result < 0)
 		return result;
 
+	/*
+	 * kexec can be used to circumvent module loading restrictions, so
+	 * prevent loading in that case
+	 */
+	if (kernel_is_locked_down("kexec of unsigned images",
+				  LOCKDOWN_INTEGRITY))
+		return -EPERM;
+
 	/*
 	 * Verify we have a legal set of flags
 	 * This leaves us room for future extensions.
-- 
2.21.0.392.gf8f6787159e-goog


^ permalink raw reply related

* [PATCH V32 09/27] uswsusp: Disable when the kernel is locked down
From: Matthew Garrett @ 2019-04-04  0:32 UTC (permalink / raw)
  To: jmorris
  Cc: linux-security-module, linux-kernel, dhowells, linux-api, luto,
	Matthew Garrett, Matthew Garrett, linux-pm, pavel, rjw
In-Reply-To: <20190404003249.14356-1-matthewgarrett@google.com>

From: Matthew Garrett <mjg59@srcf.ucam.org>

uswsusp allows a user process to dump and then restore kernel state, which
makes it possible to modify the running kernel.  Disable this if the kernel
is locked down.

Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Matthew Garrett <mjg59@google.com>
cc: linux-pm@vger.kernel.org
Cc: pavel@ucw.cz
Cc: rjw@rjwysocki.net
---
 kernel/power/user.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/power/user.c b/kernel/power/user.c
index 2d8b60a3c86b..99e13fd13237 100644
--- a/kernel/power/user.c
+++ b/kernel/power/user.c
@@ -52,6 +52,9 @@ static int snapshot_open(struct inode *inode, struct file *filp)
 	if (!hibernation_available())
 		return -EPERM;
 
+	if (kernel_is_locked_down("/dev/snapshot", LOCKDOWN_INTEGRITY))
+		return -EPERM;
+
 	lock_system_sleep();
 
 	if (!atomic_add_unless(&snapshot_device_available, -1, 0)) {
-- 
2.21.0.392.gf8f6787159e-goog


^ permalink raw reply related

* [PATCH V32 13/27] ACPI: Limit access to custom_method when the kernel is locked down
From: Matthew Garrett @ 2019-04-04  0:32 UTC (permalink / raw)
  To: jmorris
  Cc: linux-security-module, linux-kernel, dhowells, linux-api, luto,
	Matthew Garrett, Matthew Garrett, linux-acpi
In-Reply-To: <20190404003249.14356-1-matthewgarrett@google.com>

From: Matthew Garrett <mjg59@srcf.ucam.org>

custom_method effectively allows arbitrary access to system memory, making
it possible for an attacker to circumvent restrictions on module loading.
Disable it if the kernel is locked down.

Signed-off-by: Matthew Garrett <mjg59@google.com>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: linux-acpi@vger.kernel.org
---
 drivers/acpi/custom_method.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/acpi/custom_method.c b/drivers/acpi/custom_method.c
index 4451877f83b6..37de3cd84493 100644
--- a/drivers/acpi/custom_method.c
+++ b/drivers/acpi/custom_method.c
@@ -29,6 +29,9 @@ static ssize_t cm_write(struct file *file, const char __user * user_buf,
 	struct acpi_table_header table;
 	acpi_status status;
 
+	if (kernel_is_locked_down("ACPI custom methods", LOCKDOWN_INTEGRITY))
+		return -EPERM;
+
 	if (!(*ppos)) {
 		/* parse the table header to get the table length */
 		if (count <= sizeof(struct acpi_table_header))
-- 
2.21.0.392.gf8f6787159e-goog


^ permalink raw reply related

* [PATCH V32 17/27] Lock down TIOCSSERIAL
From: Matthew Garrett @ 2019-04-04  0:32 UTC (permalink / raw)
  To: jmorris
  Cc: linux-security-module, linux-kernel, dhowells, linux-api, luto,
	Greg Kroah-Hartman, Matthew Garrett, Jiri Slaby, linux-serial
In-Reply-To: <20190404003249.14356-1-matthewgarrett@google.com>

From: David Howells <dhowells@redhat.com>

Lock down TIOCSSERIAL as that can be used to change the ioport and irq
settings on a serial port.  This only appears to be an issue for the serial
drivers that use the core serial code.  All other drivers seem to either
ignore attempts to change port/irq or give an error.

Reported-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Matthew Garrett <mjg59@google.com>
cc: Jiri Slaby <jslaby@suse.com>
Cc: linux-serial@vger.kernel.org
---
 drivers/tty/serial/serial_core.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index d4cca5bdaf1c..65b67f0d4386 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -842,6 +842,12 @@ static int uart_set_info(struct tty_struct *tty, struct tty_port *port,
 	new_flags = (__force upf_t)new_info->flags;
 	old_custom_divisor = uport->custom_divisor;
 
+	if ((change_port || change_irq) &&
+	    kernel_is_locked_down("Using TIOCSSERIAL to change device addresses, irqs and dma channels", LOCKDOWN_INTEGRITY)) {
+		retval = -EPERM;
+		goto exit;
+	}
+
 	if (!capable(CAP_SYS_ADMIN)) {
 		retval = -EPERM;
 		if (change_irq || change_port ||
-- 
2.21.0.392.gf8f6787159e-goog


^ permalink raw reply related

* [PATCH V32 20/27] Lock down /proc/kcore
From: Matthew Garrett @ 2019-04-04  0:32 UTC (permalink / raw)
  To: jmorris
  Cc: linux-security-module, linux-kernel, dhowells, linux-api, luto,
	Matthew Garrett
In-Reply-To: <20190404003249.14356-1-matthewgarrett@google.com>

From: David Howells <dhowells@redhat.com>

Disallow access to /proc/kcore when the kernel is locked down to prevent
access to cryptographic data. This is limited to lockdown
confidentiality mode and is still permitted in integrity mode.

Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Matthew Garrett <mjg59@google.com>
---
 fs/proc/kcore.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/proc/kcore.c b/fs/proc/kcore.c
index bbcc185062bb..1c556a453569 100644
--- a/fs/proc/kcore.c
+++ b/fs/proc/kcore.c
@@ -518,6 +518,8 @@ read_kcore(struct file *file, char __user *buffer, size_t buflen, loff_t *fpos)
 
 static int open_kcore(struct inode *inode, struct file *filp)
 {
+	if (kernel_is_locked_down("/proc/kcore", LOCKDOWN_CONFIDENTIALITY))
+		return -EPERM;
 	if (!capable(CAP_SYS_RAWIO))
 		return -EPERM;
 
-- 
2.21.0.392.gf8f6787159e-goog


^ permalink raw reply related

* [PATCH V32 22/27] bpf: Restrict bpf when kernel lockdown is in confidentiality mode
From: Matthew Garrett @ 2019-04-04  0:32 UTC (permalink / raw)
  To: jmorris
  Cc: linux-security-module, linux-kernel, dhowells, linux-api, luto,
	Alexei Starovoitov, Matthew Garrett, netdev, Chun-Yi Lee,
	Daniel Borkmann
In-Reply-To: <20190404003249.14356-1-matthewgarrett@google.com>

From: David Howells <dhowells@redhat.com>

There are some bpf functions can be used to read kernel memory:
bpf_probe_read, bpf_probe_write_user and bpf_trace_printk.  These allow
private keys in kernel memory (e.g. the hibernation image signing key) to
be read by an eBPF program and kernel memory to be altered without
restriction. Disable them if the kernel has been locked down in
confidentiality mode.

Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Matthew Garrett <mjg59@google.com>
cc: netdev@vger.kernel.org
cc: Chun-Yi Lee <jlee@suse.com>
cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
---
 kernel/trace/bpf_trace.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 8b068adb9da1..9e8eda605b5e 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -137,6 +137,9 @@ BPF_CALL_3(bpf_probe_read, void *, dst, u32, size, const void *, unsafe_ptr)
 {
 	int ret;
 
+	if (kernel_is_locked_down("BPF", LOCKDOWN_CONFIDENTIALITY))
+		return -EINVAL;
+
 	ret = probe_kernel_read(dst, unsafe_ptr, size);
 	if (unlikely(ret < 0))
 		memset(dst, 0, size);
@@ -156,6 +159,8 @@ static const struct bpf_func_proto bpf_probe_read_proto = {
 BPF_CALL_3(bpf_probe_write_user, void *, unsafe_ptr, const void *, src,
 	   u32, size)
 {
+	if (kernel_is_locked_down("BPF", LOCKDOWN_CONFIDENTIALITY))
+		return -EINVAL;
 	/*
 	 * Ensure we're in user context which is safe for the helper to
 	 * run. This helper has no business in a kthread.
@@ -207,6 +212,9 @@ BPF_CALL_5(bpf_trace_printk, char *, fmt, u32, fmt_size, u64, arg1,
 	char buf[64];
 	int i;
 
+	if (kernel_is_locked_down("BPF", LOCKDOWN_CONFIDENTIALITY))
+		return -EINVAL;
+
 	/*
 	 * bpf_check()->check_func_arg()->check_stack_boundary()
 	 * guarantees that fmt points to bpf program stack,
@@ -535,6 +543,9 @@ BPF_CALL_3(bpf_probe_read_str, void *, dst, u32, size,
 {
 	int ret;
 
+	if (kernel_is_locked_down("BPF", LOCKDOWN_CONFIDENTIALITY))
+		return -EINVAL;
+
 	/*
 	 * The strncpy_from_unsafe() call will likely not fill the entire
 	 * buffer, but that's okay in this circumstance as we're probing
-- 
2.21.0.392.gf8f6787159e-goog


^ permalink raw reply related


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