2024-05-23 15:43:26

by James Bottomley

[permalink] [raw]
Subject: Re: [PATCH RESEND] KEYS: trusted: Use ASN.1 encoded OID

On Thu, 2024-05-23 at 18:37 +0300, Jarkko Sakkinen wrote:
> On Thu May 23, 2024 at 6:30 PM EEST, James Bottomley wrote:
> > On Thu, 2024-05-23 at 16:54 +0300, Jarkko Sakkinen wrote:
> > > On Thu May 23, 2024 at 4:38 PM EEST, James Bottomley wrote:
> > > > On Thu, 2024-05-23 at 16:19 +0300, Jarkko Sakkinen wrote:
> > > > > There's no reason to encode OID_TPMSealedData at run-time, as
> > > > > it never changes.
> > > > >
> > > > > Replace it with the encoded version, which has exactly the
> > > > > same size:
> > > > >
> > > > >         67 81 05 0A 01 05
> > > > >
> > > > > Include OBJECT IDENTIFIER (0x06) tag and length as the
> > > > > epilogue so that the OID can be simply copied to the blob.
> > > >
> > > > This is true, but if we're going to do this, we should expand
> > > > the OID registry functions (in lib/oid_registry.c) to do
> > > > something like encode_OID.  The registry already contains the
> > > > hex above minus the two prefixes (which are easy to add).
> > >
> > > Yes, I do agree with this idea, and I named variable the I named
> > > it to make it obvious that generation is possible.
> > >
> > > It would be best to have a single source, which could be just
> > > a CSV file with entries like:
> > >
> > > <Name>,<OID number>
> > >
> > > And then in scripts/ there should be a script that takes this
> > > source and generates oid_registry.gen.{h,c}. The existing
> > > oid_registry.h should really just include oid_registry.gen.h
> > > then to make this transparent change.
> > >
> > > And then in the series where OID's are encoded per-subsystem
> > > patch that takes pre-encoded OID into use.
> > >
> > > Happy to review such patch set if it is pushed forward.
> >
> > Heh, OK, since I'm the one who thinks it's quite easy, I'll give it
> > a go.
>
> I guess if it cleaned up multiple sites in kernel then it could
> be considered useful. I'd guess that there is at least a few
> locations that also encode OID.

This should be the only one currently. The ASN.1 encoding was added to
the kernel to support the trusted keys pipe use case. However, if you
want the kernel to construct and pipe out asymmetric keys, that would
be the second use case.

James



2024-05-23 15:55:40

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH RESEND] KEYS: trusted: Use ASN.1 encoded OID

On Thu May 23, 2024 at 6:43 PM EEST, James Bottomley wrote:
> On Thu, 2024-05-23 at 18:37 +0300, Jarkko Sakkinen wrote:
> > On Thu May 23, 2024 at 6:30 PM EEST, James Bottomley wrote:
> > > On Thu, 2024-05-23 at 16:54 +0300, Jarkko Sakkinen wrote:
> > > > On Thu May 23, 2024 at 4:38 PM EEST, James Bottomley wrote:
> > > > > On Thu, 2024-05-23 at 16:19 +0300, Jarkko Sakkinen wrote:
> > > > > > There's no reason to encode OID_TPMSealedData at run-time, as
> > > > > > it never changes.
> > > > > >
> > > > > > Replace it with the encoded version, which has exactly the
> > > > > > same size:
> > > > > >
> > > > > >         67 81 05 0A 01 05
> > > > > >
> > > > > > Include OBJECT IDENTIFIER (0x06) tag and length as the
> > > > > > epilogue so that the OID can be simply copied to the blob.
> > > > >
> > > > > This is true, but if we're going to do this, we should expand
> > > > > the OID registry functions (in lib/oid_registry.c) to do
> > > > > something like encode_OID.  The registry already contains the
> > > > > hex above minus the two prefixes (which are easy to add).
> > > >
> > > > Yes, I do agree with this idea, and I named variable the I named
> > > > it to make it obvious that generation is possible.
> > > >
> > > > It would be best to have a single source, which could be just
> > > > a CSV file with entries like:
> > > >
> > > > <Name>,<OID number>
> > > >
> > > > And then in scripts/ there should be a script that takes this
> > > > source and generates oid_registry.gen.{h,c}. The existing
> > > > oid_registry.h should really just include oid_registry.gen.h
> > > > then to make this transparent change.
> > > >
> > > > And then in the series where OID's are encoded per-subsystem
> > > > patch that takes pre-encoded OID into use.
> > > >
> > > > Happy to review such patch set if it is pushed forward.
> > >
> > > Heh, OK, since I'm the one who thinks it's quite easy, I'll give it
> > > a go.
> >
> > I guess if it cleaned up multiple sites in kernel then it could
> > be considered useful. I'd guess that there is at least a few
> > locations that also encode OID.
>
> This should be the only one currently. The ASN.1 encoding was added to
> the kernel to support the trusted keys pipe use case. However, if you
> want the kernel to construct and pipe out asymmetric keys, that would
> be the second use case.

Yes, we definitely need tpm2_key_ecdsa, and that is actually probably
more important than RSA but I think both are needed. It was easier to
use RSA to carve stuff to fit as there some off-the-shelf code that
could be modified for the purpose.

I was already considering do we need the encoder at all but I think
for dynamic assets like octect strings and variable size integers
it has its place. Obviously is not very mature at this point.

I think I keep the "dump" strategy for RSA keys at least for first
to keep the series as tight as possible but at least v3 included
already a patch to make asn1_encode_integer() eat variable size
stuff:

https://lore.kernel.org/linux-integrity/[email protected]/

So my proposal here is that I land RSA keys without using encoder
but that said it can take them into use in ECDSA keys when the
encoder has been carved up properly. There's already bunch of
things changed in v4, so thus I did not want to keep this
in that series:

https://lore.kernel.org/linux-integrity/[email protected]/

I.e. the dump can be considered as first iteration. There's
bunch of uses for these keys, e.g. WIFI passwords, Ethereum
keypairs (requires ECDSA) and many others. It is really awesome
application feature for TPM2.

BR, Jarkko

2024-05-23 17:03:51

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH RESEND] KEYS: trusted: Use ASN.1 encoded OID

On Thu May 23, 2024 at 6:55 PM EEST, Jarkko Sakkinen wrote:
> I was already considering do we need the encoder at all but I think
> for dynamic assets like octect strings and variable size integers
> it has its place. Obviously is not very mature at this point.

Also, I've been opening up discussion of opt-in and *experimental*
ASN1_RUST feature.

I think it is inevident that it needs to be done at some point because
for ASN.1 like format this would have benefits, and also given that it
used to process security sensitive data.

So metrics for this would something along the lines:

- Depending on ASN1_RUST setting, the C API's would be implemented
either C or Rust.
- OID database could be shared from C-side to Rust simply with
bindgen.
- C API should be streamline and matured a bit to cover mostly
dynamic assets (integers, octect strings and such). Since the
number of call sites is small improving should be easy.
- After tpm2_key_rsa is landed as it is now as per how buffer
processing goes it can be brought to use encoder.
- I'd consider have just a single ASN1 flag instead of a separate
ASN1_ENCODER flag. It simplifies thing and is not significant
cost for vmlinux size so not worth it IMHO.

As for sending patches for e.g. improving OID database, I'd like to
land the current tpm2_key_rsa first because then in possible OID
series that can be also applied to it (and encoder). It does stuff
that affects all this work. And as said we need also tpm2_key_ecdsa.

Right, there's also

https://datatracker.ietf.org/doc/draft-woodhouse-cert-best-practice/

I checked from David that this TPM2 asymmetric key work is relevant
for this spec although I readily know some applications for it, and
he acknowledged that. I should probably link that to the next
version.

Asymmetric keys essentially make TPM2 a peer in x.509 ecosystem,
which has bunch of especially enterprise and data center type
of use cases.

I guess this summarizes the big picture. I've been messing around
mailing lists and developed these thoughts but this along the
lines how I see big picture, including integration to the Rust
ecosystem (in non-intrusive way).

But yeah, tpm2_key_rsa needs to be the first step.

I don't have an employer for kernel development at the moment (probably
at some point I do, my contract researcher sabbatical ends at end of
Sep) no money to come to the plumbers to discuss about all this at the
boot-time security mc so I need to spam my input for that I guess ;-)

BR, Jarkko