Feature: ECC Support in TPM2 #4357

atreiber94 · 2024-10-08T11:25:10Z

This PR extends #4337 to include ECC*, which checks another box of the ToDos in #3877.

Changes and Additions

Adjust copyrights for funding attribution (that's why so many files are changed...).
Analogously to RSA, an EC Adapter module is added which allows to manage, load and use* TPM2 EC Keys.
Redundant logic of the RSA Adapter also used by the EC Adapter is generalized to TPM2::Signature_Operation and TPM2::Verification_Operation in tpm2_pkops.h.
Augmentation of the TPM2 Crypto Backend to support ECDH key exchange in order to allow for authenticated sessions with TPM2 EC keys.
Disentanglement of the Crypto Backend from the Adapters: Before, the Backend required the TPM2 RSA Adapter to establish a session via a TPM2 RSA key. Now, the backend RSA/ECDH functionalities require only the corresponding Botan software RSA/ECDH modules. (These are not requirements of the backend module though - they are only required if one wants to establish a session with a key of the respective type).
More defensive input checks in Crypto Backend.

* Fine Print: Limitations

As PK Ops of EC keys this PR only supports ECDSA Signing/Verification. Other types such as ECSchnorr and SM2 are not supported.
Particularly, ECDH Key Agreement in Botan with a TPM2 EC key is not yet supported. This is more involved since a TPM EC key may be used for ECDSA/ECDH/..., which is more of an operation as opposed to a key (ECDSA or ECDH ) in Botan. Note that this is not to be confused with ECDH for the Crypto Backend used for the sessions with any EC TPM2 key, which is supported (see above).
Only NIST curves are supported since 25519/448 and Brainpool are not yet supported by tpm2-tss.

src/lib/prov/tpm2/tpm2_util.h

To add ECDH support, different TPM2-TSS methods need to be incorporated into Key Agreement PK operations. The class hierarchy of Botan's EC keys currently makes this hard since a TPM2 EC key may not be restricted to just ECDH or ECDSA.

This includes disentangling the TPM2 Crypto Backend from the availability of the TPM2 RSA Adapter module. Instead, it now only depends on the software RSA/EC implementation of Botan.

Co-Authored-By: Amos Treiber <[email protected]>

reneme

I was closely involved with the implementation, so this is really more of a self-review. The CI failures seem to be inherited from master.

randombit

Looks good. If possible I'd like to avoid introducing new public APIs that use EC_Point since ideally that type goes away in Botan4, and using EC_AffinePoint from the outset avoids API breakage.

randombit · 2024-10-08T15:46:34Z

src/lib/prov/tpm2/tpm2_rsa/tpm2_rsa.cpp

-            } else if(signature->sigAlg == TPM2_ALG_RSAPSS) {
-               return signature->signature.rsapss;
+      std::vector<uint8_t> marshal_signature(const TPMT_SIGNATURE& signature) const override {
+         const auto& sig = [&] {


The if/else if formulation originally used seems cleaner to me tbh

randombit · 2024-10-08T15:53:20Z

src/lib/prov/tpm2/tpm2_key.cpp

+   }
+
+   auto curve = Botan::EC_Group::from_name(curve_name.value());
+   auto point = curve.point(BigInt(as_span(public_blob->publicArea.unique.ecc.x)),


I'd like to move the EC interfaces away from BigInt in general I'd suggest instead concatenating x and y as bytestrings with 04 prefix and deserializing.

randombit · 2024-10-08T15:58:00Z

src/lib/prov/tpm2/tpm2_ecc/tpm2_ecc.h

+      friend class TPM2::PublicKey;
+
+      EC_PublicKey(Object handle, SessionBundle sessions, const TPM2B_PUBLIC* public_blob);
+      EC_PublicKey(Object handle, SessionBundle sessions, std::pair<EC_Group, EC_Point> public_key);


Is there a specific reason to use EC_Point here vs EC_AffinePoint? (Likewise elsewhere in this module.) I'd like to transition the entire library eventually to EC_AffinePoint only, where EC_Point basically becomes an implementation detail that isn't exposed. So if we can avoid introducing new uses that would be a plus from my view.

- sigAlg cases via if/else in tpm2_rsa.cpp - Avoid BigInt and EC_Point

atreiber94 · 2024-10-09T13:57:14Z

Thanks for the review!

I transitioned the public interface TPM2::ecc_pubkey_from_tss2_public from using EC_Point to EC_AffinePoint.

The Crypto Backend still uses EC_AffinePoint::to_legacy_point() because ECDH does not seem transitioned yet, but at least now EC_Point does not appear in the public API.

randombit

LGTM thanks

atreiber94 added the enhancement Enhancement or new feature label Oct 8, 2024

atreiber94 added this to the Botan 3.6.0 milestone Oct 8, 2024

atreiber94 requested a review from randombit October 8, 2024 11:25

reneme mentioned this pull request Oct 8, 2024

Extend TPM 2.0 support #3877

Open

reneme force-pushed the feature/tpm2_ecc branch 2 times, most recently from 4c963f6 to d86ffe9 Compare October 8, 2024 12:41

reneme reviewed Oct 8, 2024

View reviewed changes

src/lib/prov/tpm2/tpm2_util.h Outdated Show resolved Hide resolved

reneme force-pushed the feature/tpm2_ecc branch 2 times, most recently from 6286c1f to 9e1439e Compare October 8, 2024 13:58

Generalize TPM2 RSA Adapter in preparation of ECC support

8c3f9e1

reneme force-pushed the feature/tpm2_ecc branch from 9e1439e to 8c882c5 Compare October 8, 2024 14:06

atreiber94 and others added 6 commits October 8, 2024 16:54

Add TPM2 ECC Adapter to support ECDSA TPM2 keys

7899203

To add ECDH support, different TPM2-TSS methods need to be incorporated into Key Agreement PK operations. The class hierarchy of Botan's EC keys currently makes this hard since a TPM2 EC key may not be restricted to just ECDH or ECDSA.

Add ECDH Support to TPM2 Crypto Backend

b2a3e5b

This includes disentangling the TPM2 Crypto Backend from the availability of the TPM2 RSA Adapter module. Instead, it now only depends on the software RSA/EC implementation of Botan.

Be defensive with the input parameters in the crypto backend

933155c

Co-Authored-By: Amos Treiber <[email protected]>

CI and Test infrastructure for TPM2 ECC support

9526e89

Add TPM2 ECC Documentation

ebea820

Add attribution to funding provider LANCOM

485d1f4

reneme force-pushed the feature/tpm2_ecc branch from 8c882c5 to 485d1f4 Compare October 8, 2024 14:54

reneme approved these changes Oct 8, 2024

View reviewed changes

randombit reviewed Oct 8, 2024

View reviewed changes

Address review comments:

1653ce0

- sigAlg cases via if/else in tpm2_rsa.cpp - Avoid BigInt and EC_Point

randombit approved these changes Oct 10, 2024

View reviewed changes

atreiber94 merged commit 45aba2c into randombit:master Oct 10, 2024
38 checks passed

atreiber94 deleted the feature/tpm2_ecc branch October 10, 2024 07:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: ECC Support in TPM2 #4357

Feature: ECC Support in TPM2 #4357

atreiber94 commented Oct 8, 2024 •

edited

Loading

reneme left a comment

randombit left a comment

randombit Oct 8, 2024

randombit Oct 8, 2024

randombit Oct 8, 2024

atreiber94 commented Oct 9, 2024 •

edited

Loading

randombit left a comment

Feature: ECC Support in TPM2 #4357

Feature: ECC Support in TPM2 #4357

Conversation

atreiber94 commented Oct 8, 2024 • edited Loading

Changes and Additions

* Fine Print: Limitations

reneme left a comment

Choose a reason for hiding this comment

randombit left a comment

Choose a reason for hiding this comment

randombit Oct 8, 2024

Choose a reason for hiding this comment

randombit Oct 8, 2024

Choose a reason for hiding this comment

randombit Oct 8, 2024

Choose a reason for hiding this comment

atreiber94 commented Oct 9, 2024 • edited Loading

randombit left a comment

Choose a reason for hiding this comment

atreiber94 commented Oct 8, 2024 •

edited

Loading

atreiber94 commented Oct 9, 2024 •

edited

Loading