Merge ~danfuhry/ubuntu/+source/tpm2-pkcs11:backport-der-encoding-patch into ubuntu/+source/tpm2-pkcs11:ubuntu/devel

[Dan Fuhry]

Backport upstream commit: [dbd7704d74ee] tpm2_ptool: do not re-encode the signed data when importing a certificate

Patch source: https://github.com/tpm2-software/tpm2-pkcs11/commit/dbd7704d74eeeb56ad116c073fdeb0a79be7460d.patch

Patch author: Nicolas Iooss <email address hidden>

Signed-Off-By: Dan Fuhry <email address hidden>

This is backport of a patch which fixes a rather serious certificate importation bug, which results in the signature being invalidated and/or the certificate being unusable when imported with `tpm2_ptool`.

[Robie Basak (racb)]

Thank you for contributing this and helping to make Ubuntu better.

For the Ubuntu development release, it looks like this fix is incorporated into 1.9.0-0.1 and needs merging into Ubuntu, which would be preferable to a cherry-pick of the fix.

If you need the fix backported to a stable release in Ubuntu, please follow https://wiki.ubuntu.com/StableReleaseUpdates#Procedure

[Robie Basak (racb)]

This MP is unsuitable for the development release because it should be a merge of 1.9.0-0.1 instead, and unsuitable for Jammy because it targets the wrong branch, so I'm marking it as Rejected.

However, once you've followed the SRU steps to prepare an appropriate bug, you can resubmit nearly exactly this branch but against pkg/ubuntu/jammy-devel instead if you wish. Please modify the changelog to close the bug (that you will need to create), and the package version string would need to be 1.7.0-1ubuntu1.22.04.1 to avoid collisions with the package versions Kinetic, Lunar and Mantic. We'd also expect to have the same change submitted to Lunar to avoid users regressing in behaviour if they upgrade from Jammy to Lunar.

[Dimitri John Ledkov (xnox)]

https://launchpad.net/ubuntu/+source/tpm2-pkcs11/1.9.0-0.1 is now available in mantic.

Unmerged commits

6a9d857... by Dan Fuhry on 2023-02-10

Backport upstream pull request #729: "Do not re-encode the signed data..."

Patch source: https://github.com/tpm2-software/tpm2-pkcs11/commit/dbd7704d74eeeb56ad116c073fdeb0a79be7460d.patch

Patch author: Nicolas Iooss <email address hidden>

diff --git a/debian/changelog b/debian/changelog
index b3eb916..e244672 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+tpm2-pkcs11 (1.7.0-2ubuntu1) jammy; urgency=medium
+
+  * Do not re-encode the signed data when importing a certificate (#729,
+    upstream commit dbd7704d74eeeb56ad116c073fdeb0a79be7460d)
+
+ -- Dan Fuhry <[e-mail]>  Fri, 10 Feb 2021 17:55:50 -0500
+
 tpm2-pkcs11 (1.7.0-1ubuntu1) jammy; urgency=medium
 
   * Do not build with Werror, many deprecation warnings from openssl v3
diff --git a/debian/patches/do-not-re-encode-the-signed-data-when-importing.patch b/debian/patches/do-not-re-encode-the-signed-data-when-importing.patch
new file mode 100644
index 0000000..48e3fc0
--- /dev/null
+++ b/debian/patches/do-not-re-encode-the-signed-data-when-importing.patch
@@ -0,0 +1,185 @@
+From dbd7704d74eeeb56ad116c073fdeb0a79be7460d Mon Sep 17 00:00:00 2001
+From: Nicolas Iooss <[e-mail]>
+Date: Thu, 23 Sep 2021 21:34:03 +0200
+Subject: [PATCH] tpm2_ptool: do not re-encode the signed data when importing a
+ certificate
+
+When using `tpm2_ptool addcert`, several users experienced issues
+because the signed data of the certificate was re-encoded when being
+added to the database. More precisely, the encoded certificate data is
+encoded using a BER encoder which encodes booleans using 1 of True (cf.
+https://github.com/etingof/pyasn1/blob/v0.4.8/pyasn1/codec/ber/encoder.py#L164
+). But in DER, the encoding of "True" is 0xff, and changing the signed
+data made the signature of the certificate no longer valid.
+
+To fix this issue:
+
+- Directly use the result of `pem.readPemFromFile(f)` in attribute
+  `CKA_VALUE`: this is directly the encoded form of the certificate.
+- Remove `pyasn1.codec.ber`, as this encoder is no longer used.
+- Rename the DER decoder from `decoder` to `derdecoder` and the encoder
+  from `derenc` to `derencoder`, to make the code easier to read.
+
+While at it:
+
+- Reindent the code to 4-space indentation
+- Use `hashlib.sha1(bercert).digest()` directly to compute a SHA1
+  digest, instead of using `m.update()`.
+
+Fixes: https://github.com/tpm2-software/tpm2-pkcs11/issues/700
+Signed-off-by: Nicolas Iooss <[e-mail]>
+---
+ tools/tpm2_pkcs11/utils.py | 126 ++++++++++++++++++-------------------
+ 1 file changed, 60 insertions(+), 66 deletions(-)
+
+diff --git a/tools/tpm2_pkcs11/utils.py b/tools/tpm2_pkcs11/utils.py
+index b803f4c2..91eab9ad 100644
+--- a/tools/tpm2_pkcs11/utils.py
++++ b/tools/tpm2_pkcs11/utils.py
+@@ -15,9 +15,7 @@
+ from cryptography.hazmat.primitives import hashes
+ 
+ from pyasn1_modules import pem, rfc2459
+-from pyasn1.codec.der import decoder
+-from pyasn1.codec.ber import encoder as berenc
+-from pyasn1.codec.der import encoder as derenc
++from pyasn1.codec.der import decoder as derdecoder, encoder as derencoder
+ from pyasn1.type import namedtype, tag, univ
+ 
+ from .pkcs11t import *  # noqa
+@@ -247,68 +245,64 @@ def asn1_format_ec_point_uncompressed(x, y):
+     return s
+ 
+ def pemcert_to_attrs(certpath):
+-            # rather than use pycryptography x509 parser, which gives native type access to certificate
+-        # fields use pyASN1 to get raw ASN1 encoded values for the fields as the spec requires them
+-        with open(certpath, "r") as f:
+-            substrate = pem.readPemFromFile(f)
+-            cert = decoder.decode(substrate, asn1Spec=rfc2459.Certificate())[0]
+-
+-        c = cert['tbsCertificate']
+-
+-        # print(cert.prettyPrint())
+-
+-        h = binascii.hexlify
+-        b = berenc.encode
+-        d = derenc.encode
+-
+-        bercert = b(cert)
+-        hexbercert = h(bercert).decode()
+-
+-        # the CKA_CHECKSUM attrs is the first 3 bytes of a sha1hash
+-        m = hashlib.sha1()
+-        m.update(bercert)
+-        bercertchecksum = m.digest()[0:3]
+-        hexbercertchecksum = h(bercertchecksum).decode()
+-
+-        subj = c['subject']
+-        hexsubj = h(d(str2bytes(subj))).decode()
+-
+-        issuer = c['issuer']
+-        hexissuer = h(d(str2bytes(issuer))).decode()
+-
+-        serial = c['serialNumber']
+-        hexserial = h(d(str2bytes(serial))).decode()
+-
+-        return {
+-            # The attrs of this attribute is derived by taking the first 3 bytes of the CKA_VALUE
+-            # field.
+-            CKA_CHECK_VALUE: hexbercertchecksum,
+-            # Start date for the certificate (default empty)
+-            CKA_START_DATE : "",
+-            # End date for the certificate (default empty)
+-            CKA_END_DATE : "",
+-            # DER-encoding of the SubjectPublicKeyInfo for the public key
+-            # contained in this certificate (default empty)
+-            CKA_PUBLIC_KEY_INFO : "",
+-            # DER encoded subject
+-            CKA_SUBJECT : hexsubj,
+-            # DER encoding of issuer
+-            CKA_ISSUER : hexissuer,
+-            # DER encoding of the cert serial
+-            CKA_SERIAL_NUMBER : hexserial,
+-            # BER encoding of the certificate
+-            CKA_VALUE : hexbercert,
+-            # RFC2279 string to URL where cert can be found, default empty
+-            CKA_URL : '',
+-            # hash of pub key subj, default empty
+-            CKA_HASH_OF_SUBJECT_PUBLIC_KEY : '',
+-            # Hash of pub key, default empty
+-            CKA_HASH_OF_ISSUER_PUBLIC_KEY : '',
+-            # Java security domain, default CK_SECURITY_DOMAIN_UNSPECIFIED
+-            CKA_JAVA_MIDP_SECURITY_DOMAIN : CK_SECURITY_DOMAIN_UNSPECIFIED,
+-            # Name hash algorithm, defaults to SHA1
+-            CKA_NAME_HASH_ALGORITHM : CKM_SHA_1
+-        }
++    # rather than using pycryptography x509 parser, which gives native type access to certificate
++    # fields use pyASN1 to get raw ASN1 encoded values for the fields as the spec requires them
++    with open(certpath, "r") as f:
++        bercert = pem.readPemFromFile(f)
++
++    cert = derdecoder.decode(bercert, asn1Spec=rfc2459.Certificate())[0]
++    c = cert['tbsCertificate']
++
++    # print(cert.prettyPrint())
++
++    h = binascii.hexlify
++    d = derencoder.encode
++
++    hexbercert = h(bercert).decode()
++
++    # the CKA_CHECKSUM attrs is the first 3 bytes of a sha1hash
++    bercertchecksum = hashlib.sha1(bercert).digest()[0:3]
++    hexbercertchecksum = h(bercertchecksum).decode()
++
++    subj = c['subject']
++    hexsubj = h(d(str2bytes(subj))).decode()
++
++    issuer = c['issuer']
++    hexissuer = h(d(str2bytes(issuer))).decode()
++
++    serial = c['serialNumber']
++    hexserial = h(d(str2bytes(serial))).decode()
++
++    return {
++        # The attrs of this attribute is derived by taking the first 3 bytes of the CKA_VALUE
++        # field.
++        CKA_CHECK_VALUE: hexbercertchecksum,
++        # Start date for the certificate (default empty)
++        CKA_START_DATE: "",
++        # End date for the certificate (default empty)
++        CKA_END_DATE: "",
++        # DER-encoding of the SubjectPublicKeyInfo for the public key
++        # contained in this certificate (default empty)
++        CKA_PUBLIC_KEY_INFO: "",
++        # DER encoded subject
++        CKA_SUBJECT: hexsubj,
++        # DER encoding of issuer
++        CKA_ISSUER: hexissuer,
++        # DER encoding of the cert serial
++        CKA_SERIAL_NUMBER: hexserial,
++        # BER encoding of the certificate
++        CKA_VALUE: hexbercert,
++        # RFC2279 string to URL where cert can be found, default empty
++        CKA_URL: '',
++        # hash of pub key subj, default empty
++        CKA_HASH_OF_SUBJECT_PUBLIC_KEY: '',
++        # Hash of pub key, default empty
++        CKA_HASH_OF_ISSUER_PUBLIC_KEY: '',
++        # Java security domain, default CK_SECURITY_DOMAIN_UNSPECIFIED
++        CKA_JAVA_MIDP_SECURITY_DOMAIN: CK_SECURITY_DOMAIN_UNSPECIFIED,
++        # Name hash algorithm, defaults to SHA1
++        CKA_NAME_HASH_ALGORITHM: CKM_SHA_1
++    }
+ 
+ def _pkcs11_to_str(value, prefix):
+ 
+@@ -407,7 +401,7 @@ def asn1parse_tss_key(keypath):
+         if len(substrate) == 0:
+             sys.exit('Did not find key in tss key file: {}'.format(keypath))
+ 
+-        tss2_privkey, _ = decoder.decode(substrate, asn1Spec=TSSPrivKey())
++        tss2_privkey, _ = derdecoder.decode(substrate, asn1Spec=TSSPrivKey())
+ 
+         return tss2_privkey
+ 
diff --git a/debian/patches/series b/debian/patches/series
index 27c0e51..89b38b2 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1,3 +1,4 @@
 remove-aclocal-git-command.patch
 set-version-of-library.patch
 no-werror.patch
+do-not-re-encode-the-signed-data-when-importing.patch