From: Petr Tesarik <petr.tesarik1@huawei-partners.com>

While I started working on my development branch to illustrate how
SandBox Mode could be enhanced to allow dynamic memory allocation and
other features necessary to convert some existing code, my colleague
Roberto Sassu set out and adapted a PGP key parser to run in a sandbox.

Disclaimer:

The code had to be rearranged in order to avoid memory allocations
and crypto operations in the sandbox. The code might contain errors.

David Howells (4):
  PGPLIB: PGP definitions (RFC 4880)
  PGPLIB: Basic packet parser
  PGPLIB: Signature parser
  KEYS: PGP data parser

Roberto Sassu (4):
  mpi: Introduce mpi_key_length()
  rsa: add parser of raw format
  KEYS: Run PGP key parser in a sandbox
  KEYS: Add intentional fault injection

 crypto/asymmetric_keys/Kconfig          |  17 +
 crypto/asymmetric_keys/Makefile         |  20 +
 crypto/asymmetric_keys/pgp.h            | 206 +++++++++
 crypto/asymmetric_keys/pgp_library.c    | 556 ++++++++++++++++++++++++
 crypto/asymmetric_keys/pgp_parser.h     |  18 +
 crypto/asymmetric_keys/pgp_public_key.c | 441 +++++++++++++++++++
 crypto/asymmetric_keys/pgplib.h         |  58 +++
 crypto/rsa.c                            |  14 +-
 crypto/rsa_helper.c                     |  69 +++
 include/crypto/internal/rsa.h           |   6 +
 include/linux/mpi.h                     |   2 +
 lib/crypto/mpi/mpicoder.c               |  33 +-
 12 files changed, 1429 insertions(+), 11 deletions(-)
 create mode 100644 crypto/asymmetric_keys/pgp.h
 create mode 100644 crypto/asymmetric_keys/pgp_library.c
 create mode 100644 crypto/asymmetric_keys/pgp_parser.h
 create mode 100644 crypto/asymmetric_keys/pgp_public_key.c
 create mode 100644 crypto/asymmetric_keys/pgplib.h

-- 
2.34.1

On 2/16/24 07:24, Petr Tesarik wrote:
> While I started working on my development branch to illustrate how
> SandBox Mode could be enhanced to allow dynamic memory allocation and
> other features necessary to convert some existing code, my colleague
> Roberto Sassu set out and adapted a PGP key parser to run in a sandbox.
> 
> Disclaimer:
> 
> The code had to be rearranged in order to avoid memory allocations
> and crypto operations in the sandbox. The code might contain errors.

I'm confused by this.  The kernel doesn't (appear to) have a PGP parser
today.  So are you saying that it *should* have one and it's only
feasible if its confined in a sandbox?

A much more powerful example would be to take something that the kernel
has already and put it in a sandbox.  That would show us how difficult
it is to sandbox something versus just doing it _normally_ in the kernel.

As it stands, I fear this was just the largest chunk of sandbox code
that was laying around and it seemed like a good idea to just chuck
~1400 lines of code over the wall at a huge cc list.

I'm not sure I want to see any more SandBox mode filling up my inbox.

On Fri, 16 Feb 2024 07:38:30 -0800
Dave Hansen <dave.hansen@intel.com> wrote:

> On 2/16/24 07:24, Petr Tesarik wrote:
> > While I started working on my development branch to illustrate how
> > SandBox Mode could be enhanced to allow dynamic memory allocation and
> > other features necessary to convert some existing code, my colleague
> > Roberto Sassu set out and adapted a PGP key parser to run in a sandbox.
> > 
> > Disclaimer:
> > 
> > The code had to be rearranged in order to avoid memory allocations
> > and crypto operations in the sandbox. The code might contain errors.  
> 
> I'm confused by this.  The kernel doesn't (appear to) have a PGP parser
> today.  So are you saying that it *should* have one and it's only
> feasible if its confined in a sandbox?

I'm sorry if this is confusing. Yes, your understanding is correct.
This patch series demonstrates that SBM (even in the initial version
that was submitted) allows to write a PGP parser which can survive
memory safety bugs withoug compromising the rest of the kernel.

> A much more powerful example would be to take something that the kernel
> has already and put it in a sandbox.  That would show us how difficult
> it is to sandbox something versus just doing it _normally_ in the kernel.

That's what I have also tested as a PoC with an earlier version of my
patch series and a few quick hacks on top. As it happens, that code
on top needs to be adapted for the current patch series, so I cannot
post it just yet. Please, stay tuned.

> As it stands, I fear this was just the largest chunk of sandbox code
> that was laying around and it seemed like a good idea to just chuck
> ~1400 lines of code over the wall at a huge cc list.

You asked for some real-world scenarios, and this should be one.
Another is on the way, but see above why it takes a bit of time.

I am not trying to claim I'm the smartest person on this planet. I can
accept it if there is a fundamental flaw in my approach. Yet, I'd be
glad if somebody can make me a favor and at least hint at what exactly
is the issue with it. I have to admit this thing is still not quite
clear to me. I would be sad if I couldn't learn from this experience.

Petr T

Petr Tesařík <petr@tesarici.cz> writes:

> On Fri, 16 Feb 2024 07:38:30 -0800
> Dave Hansen <dave.hansen@intel.com> wrote:
>> I'm confused by this.  The kernel doesn't (appear to) have a PGP parser
>> today.  So are you saying that it *should* have one and it's only
>> feasible if its confined in a sandbox?
>
> I'm sorry if this is confusing. Yes, your understanding is correct.
> This patch series demonstrates that SBM (even in the initial version
> that was submitted) allows to write a PGP parser which can survive
> memory safety bugs withoug compromising the rest of the kernel.

So I have a different question: some years ago we added the "usermode
blob" feature for just this kind of use case - parsing firewall rules at
the time.  It has never been used for that, but it's still there in
kernel/usermode_driver.c.  Is there a reason why this existing
functionality can't be used for tasks like PGP parsing as well?

Thanks,

jon

On 2/16/2024 6:21 PM, Jonathan Corbet wrote:
> Petr Tesařík <petr@tesarici.cz> writes:
> 
>> On Fri, 16 Feb 2024 07:38:30 -0800
>> Dave Hansen <dave.hansen@intel.com> wrote:
>>> I'm confused by this.  The kernel doesn't (appear to) have a PGP parser
>>> today.  So are you saying that it *should* have one and it's only
>>> feasible if its confined in a sandbox?
>>
>> I'm sorry if this is confusing. Yes, your understanding is correct.
>> This patch series demonstrates that SBM (even in the initial version
>> that was submitted) allows to write a PGP parser which can survive
>> memory safety bugs withoug compromising the rest of the kernel.
> 
> So I have a different question: some years ago we added the "usermode
> blob" feature for just this kind of use case - parsing firewall rules at
> the time.  It has never been used for that, but it's still there in
> kernel/usermode_driver.c.  Is there a reason why this existing
> functionality can't be used for tasks like PGP parsing as well?

Yes, it was an option I explored last year (briefly talked about it as a 
BoF at LSS NA 2023).

You are right, there is such feature that seemed to fit well.

User space blob embedded in a kernel module, so signed. User space 
process connected only to the kernel through a pipe.

I even went ahead, and created a framework:

https://lore.kernel.org/linux-kernel/20230317145240.363908-1-roberto.sassu@huaweicloud.com/

so that anyone can implement similar use cases.

The further step is: how can I ensure that the process launched by the 
kernel is not attacked by root (which I assumed to be less powerful than 
the kernel in a locked-down system).

I handled this in both directions:

- The process launched by the kernel is under a seccomp strict profile,
   and can only read/write through the pipe created by the kernel (and
   call few other system calls, such as exit()). Otherwise it is killed.
   Cannot create any new communication channel.

- I created an LSM that denies any attempt to ptrace/signal to the
   process launched by the kernel. Jann Horn also suggested to make the
   process non-swappable.

However, despite these attempts, security people don't feel confident on 
offloading a kernel workload outside the kernel.

This is why this work started.

Roberto