2017-06-11 11:45:00

by Mickaël Salaün

[permalink] [raw]
Subject: Re: [PATCH v1] shebang: restrict python interactive prompt/interpreter


On 10/06/2017 07:27, Tetsuo Handa wrote:
> Kees Cook wrote:
>> On Fri, Jun 9, 2017 at 10:23 AM, Matt Brown <[email protected]> wrote:
>>> what does everyone thing about a envp_blacklist option that is a list of
>>> environmental variables that will be stripped from exec calls. This can
>>> be done in the LSM hook bprm_check_security.
>>>
>>> Is there any reason on a hardened system why you would need the
>>> PYTHONINSPECT environmental variable?
>>
>> As part of shebang, it likely makes sense to whitelist (rather than
>> blacklist) the env of the restricted interpreters. Though this is
>> starting to get complex. :P
>
> Blacklisting environment variables is dangerous. I think that
> administrators can afford whitelisting environment variable names.
> I think that implementing whitelist of environment variable names
> as an independent LSM module would be fine.
>
> While it is true that things starts getting complex if we check environment
> variables, shebang will already become complex if it starts worrying about
> updating inode number list in order to close the race window between doing
> creat()+write()+close()+chmod()+rename() by the package manager and teaching
> the kernel the new inode number determined by creat(). We will need an
> interface for allowing the package manager to teach the kernel the new inode
> number and modification of the package manager, for the kernel side is doing
> inode number based blacklisting while user side can execute it before rename().
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

Using filesystem xattr seems like a good idea for this kind of
exceptions and instead of a hardcoded interpreter path. Something like
"security.tpe.interpreter=1|2" (bitmask for interpreter-only and/or CLI)
and "security.tpe.environment=HOME,LOGNAME" would be quite flexible to
configure a security policy for some binaries. This could also be
protected by IMA/EVM, if needed.

This kind of xattr should be writable by the owner of the file. The TPE
LSM [1] could then take these xattr into account according to the TPE
policy.

The "security.tpe.environment" could also be set on a script file to be
part of the union with the interpreter's environment whitelist. This may
be needed to be able to use environment variables as configuration in a
script.

In the future, a "security.tpe.memory" could contain a set of flags as
PaX uses for mprotect-like exceptions (user.pax.flags).

Userland daemons such as paxctld or paxrat could be used (with some
tweaks) to keep a consistent TPE policy over time.

Micka?l


[1] https://lkml.kernel.org/r/[email protected]


Attachments:
signature.asc (488.00 B)
OpenPGP digital signature

2017-06-12 02:33:40

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH v1] shebang: restrict python interactive prompt/interpreter

On Sun, 2017-06-11 at 13:44 +0200, Mickaël Salaün wrote:
> On 10/06/2017 07:27, Tetsuo Handa wrote:
> > Kees Cook wrote:
> >> On Fri, Jun 9, 2017 at 10:23 AM, Matt Brown <[email protected]> wrote:
> >>> what does everyone thing about a envp_blacklist option that is a list of
> >>> environmental variables that will be stripped from exec calls. This can
> >>> be done in the LSM hook bprm_check_security.
> >>>
> >>> Is there any reason on a hardened system why you would need the
> >>> PYTHONINSPECT environmental variable?
> >>
> >> As part of shebang, it likely makes sense to whitelist (rather than
> >> blacklist) the env of the restricted interpreters. Though this is
> >> starting to get complex. :P
> >
> > Blacklisting environment variables is dangerous. I think that
> > administrators can afford whitelisting environment variable names.
> > I think that implementing whitelist of environment variable names
> > as an independent LSM module would be fine.
> >
> > While it is true that things starts getting complex if we check environment
> > variables, shebang will already become complex if it starts worrying about
> > updating inode number list in order to close the race window between doing
> > creat()+write()+close()+chmod()+rename() by the package manager and teaching
> > the kernel the new inode number determined by creat(). We will need an
> > interface for allowing the package manager to teach the kernel the new inode
> > number and modification of the package manager, for the kernel side is doing
> > inode number based blacklisting while user side can execute it before rename().

I don't think we're trying to protect against executing the
interpreter prior to the rename.  Rename, itself, would trigger
associating the interpreter name with an inode number.

> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
>
> Using filesystem xattr seems like a good idea for this kind of
> exceptions and instead of a hardcoded interpreter path. Something like
> "security.tpe.interpreter=1|2" (bitmask for interpreter-only and/or CLI)
> and "security.tpe.environment=HOME,LOGNAME" would be quite flexible to
> configure a security policy for some binaries. This could also be
> protected by IMA/EVM, if needed.

Checking for the existence of an xattr without caching is relatively
slow.  I'm not sure that we would want to go this route.

> This kind of xattr should be writable by the owner of the file. The TPE
> LSM [1] could then take these xattr into account according to the TPE
> policy.

Security xattrs are only writable by root.

Mimi

> The "security.tpe.environment" could also be set on a script file to be
> part of the union with the interpreter's environment whitelist. This may
> be needed to be able to use environment variables as configuration in a
> script.
>
> In the future, a "security.tpe.memory" could contain a set of flags as
> PaX uses for mprotect-like exceptions (user.pax.flags).
>
> Userland daemons such as paxctld or paxrat could be used (with some
> tweaks) to keep a consistent TPE policy over time.
>
> Mickaël
>
>
> [1] https://lkml.kernel.org/r/[email protected]
>

2017-06-12 14:27:49

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH v1] shebang: restrict python interactive prompt/interpreter

On Sun, 2017-06-11 at 22:32 -0400, Mimi Zohar wrote:
> On Sun, 2017-06-11 at 13:44 +0200, Mickaël Salaün wrote:

> > Using filesystem xattr seems like a good idea for this kind of
> > exceptions and instead of a hardcoded interpreter path. Something like
> > "security.tpe.interpreter=1|2" (bitmask for interpreter-only and/or CLI)
> > and "security.tpe.environment=HOME,LOGNAME" would be quite flexible to
> > configure a security policy for some binaries. This could also be
> > protected by IMA/EVM, if needed.
>
> Checking for the existence of an xattr without caching is relatively
> slow.  I'm not sure that we would want to go this route.
 
For identifying interpreters, xattrs would be too slow (without
caching results), but once identified, using xattrs as you suggested,
for specifying how interpreters can be invoked and limiting
environment variables, is a good idea.  Perhaps the two xattrs could
be combined?

Mimi


2017-06-13 21:00:10

by Mickaël Salaün

[permalink] [raw]
Subject: Re: [PATCH v1] shebang: restrict python interactive prompt/interpreter


On 12/06/2017 04:32, Mimi Zohar wrote:
> On Sun, 2017-06-11 at 13:44 +0200, Mickaël Salaün wrote:
>> On 10/06/2017 07:27, Tetsuo Handa wrote:
>>> Kees Cook wrote:
>>>> On Fri, Jun 9, 2017 at 10:23 AM, Matt Brown <[email protected]> wrote:
>>>>> what does everyone thing about a envp_blacklist option that is a list of
>>>>> environmental variables that will be stripped from exec calls. This can
>>>>> be done in the LSM hook bprm_check_security.
>>>>>
>>>>> Is there any reason on a hardened system why you would need the
>>>>> PYTHONINSPECT environmental variable?
>>>>
>>>> As part of shebang, it likely makes sense to whitelist (rather than
>>>> blacklist) the env of the restricted interpreters. Though this is
>>>> starting to get complex. :P
>>>
>>> Blacklisting environment variables is dangerous. I think that
>>> administrators can afford whitelisting environment variable names.
>>> I think that implementing whitelist of environment variable names
>>> as an independent LSM module would be fine.
>>>
>>> While it is true that things starts getting complex if we check environment
>>> variables, shebang will already become complex if it starts worrying about
>>> updating inode number list in order to close the race window between doing
>>> creat()+write()+close()+chmod()+rename() by the package manager and teaching
>>> the kernel the new inode number determined by creat(). We will need an
>>> interface for allowing the package manager to teach the kernel the new inode
>>> number and modification of the package manager, for the kernel side is doing
>>> inode number based blacklisting while user side can execute it before rename().
>
> I don't think we're trying to protect against executing the
> interpreter prior to the rename. Rename, itself, would trigger
> associating the interpreter name with an inode number.
>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
>>> the body of a message to [email protected]
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>>
>> Using filesystem xattr seems like a good idea for this kind of
>> exceptions and instead of a hardcoded interpreter path. Something like
>> "security.tpe.interpreter=1|2" (bitmask for interpreter-only and/or CLI)
>> and "security.tpe.environment=HOME,LOGNAME" would be quite flexible to
>> configure a security policy for some binaries. This could also be
>> protected by IMA/EVM, if needed.
>
> Checking for the existence of an xattr without caching is relatively
> slow. I'm not sure that we would want to go this route.
>
>> This kind of xattr should be writable by the owner of the file. The TPE
>> LSM [1] could then take these xattr into account according to the TPE
>> policy.
>
> Security xattrs are only writable by root.

This is currently the case but an exception for this kind of LSM could
be allowed, or another dedicated prefix could be used.


Attachments:
signature.asc (488.00 B)
OpenPGP digital signature

2017-06-13 21:00:29

by Mickaël Salaün

[permalink] [raw]
Subject: Re: [PATCH v1] shebang: restrict python interactive prompt/interpreter


On 12/06/2017 16:27, Mimi Zohar wrote:
> On Sun, 2017-06-11 at 22:32 -0400, Mimi Zohar wrote:
>> On Sun, 2017-06-11 at 13:44 +0200, Mickaël Salaün wrote:
>
>>> Using filesystem xattr seems like a good idea for this kind of
>>> exceptions and instead of a hardcoded interpreter path. Something like
>>> "security.tpe.interpreter=1|2" (bitmask for interpreter-only and/or CLI)
>>> and "security.tpe.environment=HOME,LOGNAME" would be quite flexible to
>>> configure a security policy for some binaries. This could also be
>>> protected by IMA/EVM, if needed.
>>
>> Checking for the existence of an xattr without caching is relatively
>> slow. I'm not sure that we would want to go this route.
>
> For identifying interpreters, xattrs would be too slow (without
> caching results), but once identified, using xattrs as you suggested,
> for specifying how interpreters can be invoked and limiting
> environment variables, is a good idea. Perhaps the two xattrs could
> be combined?

Yes, caching results is definitely interesting. I think using one
variable per usage is cleaner, though.


Attachments:
signature.asc (488.00 B)
OpenPGP digital signature

2017-06-13 21:44:36

by Casey Schaufler

[permalink] [raw]
Subject: Re: [kernel-hardening] Re: [PATCH v1] shebang: restrict python interactive prompt/interpreter

On 6/13/2017 1:59 PM, Mickaël Salaün wrote:
> On 12/06/2017 04:32, Mimi Zohar wrote:
>> On Sun, 2017-06-11 at 13:44 +0200, Mickaël Salaün wrote:
>>> On 10/06/2017 07:27, Tetsuo Handa wrote:
>>>> Kees Cook wrote:
>>>>> On Fri, Jun 9, 2017 at 10:23 AM, Matt Brown <[email protected]> wrote:
>>>>>> what does everyone thing about a envp_blacklist option that is a list of
>>>>>> environmental variables that will be stripped from exec calls. This can
>>>>>> be done in the LSM hook bprm_check_security.
>>>>>>
>>>>>> Is there any reason on a hardened system why you would need the
>>>>>> PYTHONINSPECT environmental variable?
>>>>> As part of shebang, it likely makes sense to whitelist (rather than
>>>>> blacklist) the env of the restricted interpreters. Though this is
>>>>> starting to get complex. :P
>>>> Blacklisting environment variables is dangerous. I think that
>>>> administrators can afford whitelisting environment variable names.
>>>> I think that implementing whitelist of environment variable names
>>>> as an independent LSM module would be fine.
>>>>
>>>> While it is true that things starts getting complex if we check environment
>>>> variables, shebang will already become complex if it starts worrying about
>>>> updating inode number list in order to close the race window between doing
>>>> creat()+write()+close()+chmod()+rename() by the package manager and teaching
>>>> the kernel the new inode number determined by creat(). We will need an
>>>> interface for allowing the package manager to teach the kernel the new inode
>>>> number and modification of the package manager, for the kernel side is doing
>>>> inode number based blacklisting while user side can execute it before rename().
>> I don't think we're trying to protect against executing the
>> interpreter prior to the rename. Rename, itself, would trigger
>> associating the interpreter name with an inode number.
>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
>>>> the body of a message to [email protected]
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>>
>>> Using filesystem xattr seems like a good idea for this kind of
>>> exceptions and instead of a hardcoded interpreter path. Something like
>>> "security.tpe.interpreter=1|2" (bitmask for interpreter-only and/or CLI)
>>> and "security.tpe.environment=HOME,LOGNAME" would be quite flexible to
>>> configure a security policy for some binaries. This could also be
>>> protected by IMA/EVM, if needed.
>> Checking for the existence of an xattr without caching is relatively
>> slow. I'm not sure that we would want to go this route.
>>
>>> This kind of xattr should be writable by the owner of the file. The TPE
>>> LSM [1] could then take these xattr into account according to the TPE
>>> policy.
>> Security xattrs are only writable by root.
> This is currently the case but an exception for this kind of LSM could
> be allowed, or another dedicated prefix could be used.

Better yet, use "user.tpe.whatever" and explicitly look for that in your
xattr hooks, denying access based on whatever criteria you like.
You could allow it to be set, but never deleted, for example.
You can do it all within shebang without creating exceptions or new
prefixes.

2017-06-14 14:10:45

by Alan Cox

[permalink] [raw]
Subject: Re: [PATCH v1] shebang: restrict python interactive prompt/interpreter

On Mon, 12 Jun 2017 10:27:24 -0400
Mimi Zohar <[email protected]> wrote:

> On Sun, 2017-06-11 at 22:32 -0400, Mimi Zohar wrote:
> > On Sun, 2017-06-11 at 13:44 +0200, Mickaël Salaün wrote:
>
> > > Using filesystem xattr seems like a good idea for this kind of
> > > exceptions and instead of a hardcoded interpreter path. Something like
> > > "security.tpe.interpreter=1|2" (bitmask for interpreter-only and/or CLI)
> > > and "security.tpe.environment=HOME,LOGNAME" would be quite flexible to
> > > configure a security policy for some binaries. This could also be
> > > protected by IMA/EVM, if needed.
> >
> > Checking for the existence of an xattr without caching is relatively
> > slow.  I'm not sure that we would want to go this route.
>  
> For identifying interpreters, xattrs would be too slow (without
> caching results), but once identified, using xattrs as you suggested,
> for specifying how interpreters can be invoked and limiting
> environment variables, is a good idea.  Perhaps the two xattrs could
> be combined?

It's not just #! you need to cover. If I can run ld.so for my arch format
then ld.so will helpfully let me load any ELF binary I like and run it.

Alan

2017-06-14 20:37:30

by Boris Lukashev

[permalink] [raw]
Subject: Re: [kernel-hardening] Re: [PATCH v1] shebang: restrict python interactive prompt/interpreter

On Wed, Jun 14, 2017 at 10:10 AM, Alan Cox <[email protected]> wrote:
>
> On Mon, 12 Jun 2017 10:27:24 -0400
> Mimi Zohar <[email protected]> wrote:
>
> > On Sun, 2017-06-11 at 22:32 -0400, Mimi Zohar wrote:
> > > On Sun, 2017-06-11 at 13:44 +0200, Mickaël Salaün wrote:
> >
> > > > Using filesystem xattr seems like a good idea for this kind of
> > > > exceptions and instead of a hardcoded interpreter path. Something like
> > > > "security.tpe.interpreter=1|2" (bitmask for interpreter-only and/or CLI)
> > > > and "security.tpe.environment=HOME,LOGNAME" would be quite flexible to
> > > > configure a security policy for some binaries. This could also be
> > > > protected by IMA/EVM, if needed.
> > >
> > > Checking for the existence of an xattr without caching is relatively
> > > slow. I'm not sure that we would want to go this route.
> >
> > For identifying interpreters, xattrs would be too slow (without
> > caching results), but once identified, using xattrs as you suggested,
> > for specifying how interpreters can be invoked and limiting
> > environment variables, is a good idea. Perhaps the two xattrs could
> > be combined?
>
> It's not just #! you need to cover. If I can run ld.so for my arch format
> then ld.so will helpfully let me load any ELF binary I like and run it.
>
> Alan

That depends on the threat model. Interpreted payloads are beneficial
to attackers for their light forensic footprint along with implicit
code-gen needs/powers - they dont require writing files to disk in
order to affect their goals (staging is implicitly handled by the
runtime, and behavior is tough to track). Anything going to disk is
subject to forensic recovery, malware analysis, and a host of other
concerns nobody wants to deal with when trying to compromise things
quietly. While attackers can abuse the linker to their heart's content
under certain contexts, they generally need to write files to disk in
a place where the linker can get to them, leaving more footprints or
being deterred altogether. At the payload delivery stage, remote
attackers are effectively blind and unlikely to know if their exploit
failed or the payload execution was mitigated. It has real world
value.

To go along with the notion of "perfect can be the enemy of good"
(please pardon the likely incorrect paraphrasing, second language and
all), rejecting a protection which offers coverage for a subset of
potential vectors, because it does not provide coverage for others,
leads to dead code and open holes. Is it feasible to adopt protections
which cover specific threat models with annotations on what they may
leave open such as to cover those areas in later commits? This would
also allow common work to be converged, such as LSM code dealing with
mprotect issues, path resolution as a security validator, etc.

-Boris