diff mbox series

[v3] KEYS: trusted: Use ASN.1 encoded OID

Message ID 20240523142056.17159-1-jarkko@kernel.org (mailing list archive)
State Not Applicable
Delegated to: Herbert Xu
Headers show
Series [v3] KEYS: trusted: Use ASN.1 encoded OID | expand

Commit Message

Jarkko Sakkinen May 23, 2024, 2:20 p.m. UTC 
There's no reason to encode OID_TPMSealedData at run-time, as it never
changes.

Replace it with an encoded u8-array, which has the same number of
elements:

	67 81 05 0A 01 05

Include OBJECT IDENTIFIER (0x06) tag and length as the prologue so that
the OID can be simply copied to the blob leading to:

	06 06 67 81 05 0A 01 05

Since this in stationary place in the buffer it is guaranteed to always
fit and not further checks are required.

Reviewed-by: David Howells <dhowells@redhat.com>
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
v3:
* Addressed Ben's suggestions:
  https://lore.kernel.org/linux-integrity/Zk9Hb-whVYvJrfLY@farprobe/
* Added David's R-b, as the only change in code was to add const
  to the declaration.
v2:
* Not my day I guess. This one has print_hex_dump() taken away.
  Apologies for spamming. The patch is however tested properly
  with run-tests.sh in https://gitlab.com/jarkkojs/linux-tpmdd-test.
---
 include/linux/asn1_encoder.h              |  4 -
 lib/asn1_encoder.c                        | 91 -----------------------
 security/keys/trusted-keys/trusted_tpm2.c |  6 +-
 3 files changed, 3 insertions(+), 98 deletions(-)

Comments

Jarkko Sakkinen May 23, 2024, 2:26 p.m. UTC | #1 
On Thu May 23, 2024 at 5:20 PM EEST, Jarkko Sakkinen wrote:
> There's no reason to encode OID_TPMSealedData at run-time, as it never
> changes.
>
> Replace it with an encoded u8-array, which has the same number of
> elements:
>
> 	67 81 05 0A 01 05
>
> Include OBJECT IDENTIFIER (0x06) tag and length as the prologue so that
> the OID can be simply copied to the blob leading to:
>
> 	06 06 67 81 05 0A 01 05
>
> Since this in stationary place in the buffer it is guaranteed to always
> fit and not further checks are required.
>
> Reviewed-by: David Howells <dhowells@redhat.com>
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>

Does not really substitute distribution kernel testing, which is
IMHO essential for something like TPM2 boot in systemd but for
simple patches like this, the following does a trivial smoke
test:

export LINUX_OVERRIDE_SRCDIR=<path to a kernel tree with a trusted keys patch>
git clone https://gitlab.com/jarkkojs/linux-tpmdd-test.git
cd linux-tpmdd-test
cmake -Bbuild && make -Cbuild buildroot-prepare
make -Cbuild/buildroot/build
build/buildroot/build/images/run-tests.sh

I'm planning to migrate at some point to systemd and make it
appear more like distribution tho..

For recompiling just kernel only thing needed is:

rm -rf build/buildroot/build/build/linux-custom
make -Cbuild/buildroot/build

I've put this also to the MAINTAINERS entry of TPM driver although
I use it also for keyrings etc. Also it is open for contributions
via Gitlab merge requests (not requesting them per se but I'm open
to such possibility).

BR, Jarkko
Jarkko Sakkinen May 23, 2024, 2:29 p.m. UTC | #2 
On Thu May 23, 2024 at 5:26 PM EEST, Jarkko Sakkinen wrote:
> On Thu May 23, 2024 at 5:20 PM EEST, Jarkko Sakkinen wrote:
> > There's no reason to encode OID_TPMSealedData at run-time, as it never
> > changes.
> >
> > Replace it with an encoded u8-array, which has the same number of
> > elements:
> >
> > 	67 81 05 0A 01 05
> >
> > Include OBJECT IDENTIFIER (0x06) tag and length as the prologue so that
> > the OID can be simply copied to the blob leading to:
> >
> > 	06 06 67 81 05 0A 01 05
> >
> > Since this in stationary place in the buffer it is guaranteed to always
> > fit and not further checks are required.
> >
> > Reviewed-by: David Howells <dhowells@redhat.com>
> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
>
> Does not really substitute distribution kernel testing, which is
> IMHO essential for something like TPM2 boot in systemd but for
> simple patches like this, the following does a trivial smoke
> test:
>
> export LINUX_OVERRIDE_SRCDIR=<path to a kernel tree with a trusted keys patch>
> git clone https://gitlab.com/jarkkojs/linux-tpmdd-test.git
> cd linux-tpmdd-test
> cmake -Bbuild && make -Cbuild buildroot-prepare
> make -Cbuild/buildroot/build
> build/buildroot/build/images/run-tests.sh
>
> I'm planning to migrate at some point to systemd and make it
> appear more like distribution tho..
>
> For recompiling just kernel only thing needed is:
>
> rm -rf build/buildroot/build/build/linux-custom
> make -Cbuild/buildroot/build
>
> I've put this also to the MAINTAINERS entry of TPM driver although
> I use it also for keyrings etc. Also it is open for contributions
> via Gitlab merge requests (not requesting them per se but I'm open
> to such possibility).

Right and this is fully CI compatible with both Github and Gitlab
with only a single tweak: BR2_PACKAGE_HOST_QEMU needs to be added
to the qemu config. It is CI agnostic test environment to put 
short.

I've had this also deployed to the CI in the past. It can run both
x86 and aarch64 based runners and even emulates hardware TPM in
three different modes (TPM 1.2, TPM 2.0 FIFO, TPM 2.0 CRB).

I've been just wondering why we need kernel tree changes for Gitlab
CI when you can pretty easily just bootstrap toolchain and qemu
and call it a day (and with trimmed builds it is quite fast too).

BR, Jarkko
diff mbox series

Patch

diff --git a/include/linux/asn1_encoder.h b/include/linux/asn1_encoder.h
index 08cd0c2ad34f..afeefdfe2525 100644
--- a/include/linux/asn1_encoder.h
+++ b/include/linux/asn1_encoder.h
@@ -8,14 +8,10 @@ 
 #include <linux/asn1_ber_bytecode.h>
 #include <linux/bug.h>
 
-#define asn1_oid_len(oid) (sizeof(oid)/sizeof(u32))
 unsigned char *
 asn1_encode_integer(unsigned char *data, const unsigned char *end_data,
 		    s64 integer);
 unsigned char *
-asn1_encode_oid(unsigned char *data, const unsigned char *end_data,
-		u32 oid[], int oid_len);
-unsigned char *
 asn1_encode_tag(unsigned char *data, const unsigned char *end_data,
 		u32 tag, const unsigned char *string, int len);
 unsigned char *
diff --git a/lib/asn1_encoder.c b/lib/asn1_encoder.c
index 0fd3c454a468..c0db3cbebe89 100644
--- a/lib/asn1_encoder.c
+++ b/lib/asn1_encoder.c
@@ -85,97 +85,6 @@  asn1_encode_integer(unsigned char *data, const unsigned char *end_data,
 }
 EXPORT_SYMBOL_GPL(asn1_encode_integer);
 
-/* calculate the base 128 digit values setting the top bit of the first octet */
-static int asn1_encode_oid_digit(unsigned char **_data, int *data_len, u32 oid)
-{
-	unsigned char *data = *_data;
-	int start = 7 + 7 + 7 + 7;
-	int ret = 0;
-
-	if (*data_len < 1)
-		return -EINVAL;
-
-	/* quick case */
-	if (oid == 0) {
-		*data++ = 0x80;
-		(*data_len)--;
-		goto out;
-	}
-
-	while (oid >> start == 0)
-		start -= 7;
-
-	while (start > 0 && *data_len > 0) {
-		u8 byte;
-
-		byte = oid >> start;
-		oid = oid - (byte << start);
-		start -= 7;
-		byte |= 0x80;
-		*data++ = byte;
-		(*data_len)--;
-	}
-
-	if (*data_len > 0) {
-		*data++ = oid;
-		(*data_len)--;
-	} else {
-		ret = -EINVAL;
-	}
-
- out:
-	*_data = data;
-	return ret;
-}
-
-/**
- * asn1_encode_oid() - encode an oid to ASN.1
- * @data:	position to begin encoding at
- * @end_data:	end of data pointer, points one beyond last usable byte in @data
- * @oid:	array of oids
- * @oid_len:	length of oid array
- *
- * this encodes an OID up to ASN.1 when presented as an array of OID values
- */
-unsigned char *
-asn1_encode_oid(unsigned char *data, const unsigned char *end_data,
-		u32 oid[], int oid_len)
-{
-	int data_len = end_data - data;
-	unsigned char *d = data + 2;
-	int i, ret;
-
-	if (WARN(oid_len < 2, "OID must have at least two elements"))
-		return ERR_PTR(-EINVAL);
-
-	if (WARN(oid_len > 32, "OID is too large"))
-		return ERR_PTR(-EINVAL);
-
-	if (IS_ERR(data))
-		return data;
-
-
-	/* need at least 3 bytes for tag, length and OID encoding */
-	if (data_len < 3)
-		return ERR_PTR(-EINVAL);
-
-	data[0] = _tag(UNIV, PRIM, OID);
-	*d++ = oid[0] * 40 + oid[1];
-
-	data_len -= 3;
-
-	for (i = 2; i < oid_len; i++) {
-		ret = asn1_encode_oid_digit(&d, &data_len, oid[i]);
-		if (ret < 0)
-			return ERR_PTR(ret);
-	}
-
-	data[1] = d - data - 2;
-
-	return d;
-}
-EXPORT_SYMBOL_GPL(asn1_encode_oid);
-
 /**
  * asn1_encode_length() - encode a length to follow an ASN.1 tag
  * @data: pointer to encode at
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 8b7dd73d94c1..d478096cb5c1 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -26,7 +26,7 @@  static struct tpm2_hash tpm2_hash_map[] = {
 	{HASH_ALGO_SM3_256, TPM_ALG_SM3_256},
 };
 
-static u32 tpm2key_oid[] = { 2, 23, 133, 10, 1, 5 };
+static const u8 OID_TPMSealedData_ASN1[] = {0x06, 0x06, 0x67, 0x81, 0x05, 0x0a, 0x01, 0x05};
 
 static int tpm2_key_encode(struct trusted_key_payload *payload,
 			   struct trusted_key_options *options,
@@ -51,8 +51,8 @@  static int tpm2_key_encode(struct trusted_key_payload *payload,
 	if (!scratch)
 		return -ENOMEM;
 
-	work = asn1_encode_oid(work, end_work, tpm2key_oid,
-			       asn1_oid_len(tpm2key_oid));
+	work = memcpy(work, OID_TPMSealedData_ASN1, sizeof(OID_TPMSealedData_ASN1));
+	work += sizeof(OID_TPMSealedData_ASN1);
 
 	if (options->blobauth_len == 0) {
 		unsigned char bool[3], *w = bool;