Add Parameter Encryption support #129

tomoveu · 2020-10-09T11:50:59Z

Added AES CFB parameter support, salted unbound TPM session

Added AES CFB support
Added calculation of command hash and HMAC for sessions.
Added response HMAC validation.
Fixes and cleanups for KDFa
Added KDFa unit test (passes)
Inlined the param encryption buffers
Added "-aes" or "-xor" option to some examples to enable parameter encryption.
Refactor of the session authentication.
Fixup for the getPrimaryStoragekey to allow persisting if not WOLFTPM_WINAPI.
Fixes for nonce and auth count.
Fix for AES CFB decrypt set key (requires AES_ENCRYPTION on SetKey).
Added support for encrypted RSA salt and salted-unbounded session
Add innerWrap and outerWrap support for sensitive to private.

tomoveu · 2020-10-09T11:52:27Z

@dgarske I wanted to make this PR a draft, but I see the option did not apply. Could you please make the change, because I do not have write permission to the wolfTPM repo?

https://github.blog/changelog/2020-04-08-convert-pull-request-to-draft/

src/tpm2_param_enc.c

tomoveu · 2020-10-27T15:58:48Z

@dgarske now that I understand what do to you mean by something is missing here, it makes sense.

If the data is not digest size, it would be unaligned and then this would result in improper marshaling(appending), thus the TPM_RC_SENSITVE error that we are now getting would make sense.

Let's look at this together, because I really did get stuck with this KDFa issue.

tomoveu · 2020-10-27T16:36:24Z

Pushed my latest changes to the keygen example. There might be something of value for resemblance between the keygen examples with and without parameter encryption #131

tomoveu · 2020-10-30T19:59:20Z

Opting out from unsalted-unbounded session in favor of salted-bounded session that adds more entropy.

This also helps to simplify the KDFa.

tomoveu · 2020-11-13T16:55:03Z

@dgarske Hi David, after all the fixes and debugging, this should now be working.

There is one enhancement that I need to add in the tpm2.c for handling the different TPM2 commands. For the moment I have only created handling for TPM2_Create and TPM2_Load.

Please review, so we can confirm this stage and then I would just add the enhancement.

tomoveu · 2020-11-13T21:03:07Z

Found issue in the handling of authSessions when we have more than the default/minimum required by a command. Pushed DEBUG commit to demonstrate the issue.

Will add helper to check the set sessions and provide session count that will be used in all TPM2_{Command} helpers.

Once this is solved will check again if the keygen/keyload examples fail, as there could still be an issue with the inner/outer integrity check.

tomoveu · 2020-11-17T21:37:47Z

Intermediate status update - Today I added handling of the maximum possible number of TPM sessions. I tried using WOLFTPM2_SESSION and significantly change how auth sessions are handled in wolfTPM, but then I went with this much simpler and cleaner implementation.

This solved the issue with not appending the correct number of auth sessions for TPM2_Create and TPM2_Load. And after this fix another auth session issue emerged for parameter encryption. I have been working on solving that new auth session issue and is still ongoing.

Additionally, while working on this new issue I noticed that we use dev->session in wolfTPM2_CreateKey and wolfTPM2_LoadKey although everywhere else ctx->authCmd is used. For example, wolfTPM2_CreateKey manipulates dev->session while the later called TPM2_Create uses ctx->authCmd. Probably dev->ctx->authCmd is one way to go about this.

tomoveu · 2020-11-18T08:42:41Z

status update: cause for the RC_BAD_AUTH is that the calculated HMAC over the parameter encryption session differs from what the TPM expects. investigating why the difference occurs, is it the sessionKey/KDFa or later packet marshaling.

tomoveu · 2020-11-18T10:27:00Z

there is something strange happening with the HMAC calculation from the sessionKey on the TPM simulator side.

The authValue prepared for the AIK key generation is compared with the HMAC resulting from the sessionKey.
I am continuing to investigate.

tomoveu · 2020-11-18T12:06:01Z

OK, I think I found the issue.

Our implementation does not use TPM2_PolicyAuthValue and does not include the authValue for the sessionKey calculation. But at least one of these is expected from the TPM (simulator) side. So, when there is no PolicyAuthValue set and no authValue used for setting up parameter encryption, the check on TPM side operates under conditions that must not be and results in an error.

Ken Goldman helped with pointing out this note in the spec:

TCG Spec, Part 1, Chapter 21 Session-based encryption

If a session is also being used for authorization, sessionValue (see 21.2 and 21.3) is sessionKey || authValue. The binding of the session is ignored. If the session is not being used for authorization, sessionValue is sessionKey.

NOTE 1 A policy session that is used for parameter encryption uses authValue to calculate sessionValue even if the policy does not include TPM2_PolicyAuthValue().

I have read Chapter 21 from the spec so many times and somehow glimpsed over Note 1 ...

tomoveu · 2020-11-18T12:48:47Z

While working the new issue, I came up with a simpler way of handling the maximum possible TPM sessions.

I think that using authSession from active TPM context offloads the TPM2 Command wrappers and minimizes the room for error Therefore, I changed all TPM2 wrappers to be using new way of appending the authSession.

Tested using the native_tests , all good for using the new TPM2_Packet_AppendAuthFromContext

…xamples * Added key generation example with parameter encryption * Fixes and cleanups for KDFa * Added KDFa unit test (passes) * Added AES CFB support * Fix for nonceTPM * Added support for encrypted RSA salt and salted-unbounded session * Removed unsalted-unbounded specific code for authValueKDF from KDFa * Add innerWrap support * Add missing wolfcrypt header for AES CFB * Fixes for casting when calling KDFa for AES CFB parameter encryption * Add outerWrap support Signed-off-by: Dimitar Tomov <dimi@wolfssl.com>

…uterWrap Signed-off-by: Dimitar Tomov <dimi@wolfssl.com>

Signed-off-by: Dimitar Tomov <dimi@wolfssl.com>

tomoveu · 2020-11-26T14:18:16Z

I have tested and found the following issues:

native tests must be made compatible with the new session handling (partially fixed) because I found issue in TPM2_LoadExternal (see 3.)
some of the issues are introduced due to default auth session set in wolfTPM2 Init wrapper (fixed)

The above completely fixes TPM2_GetTag does not return TPM_ST_SESSIONS when required #120 TPM2_GetTag issue that was also addressed with adding the TPM2_GetSessionAuthCount to the TPM2_GetTag (TPM2_GetTag does not return TPM_ST_SESSIONS when required #120 can be closed once this PR is merged)

keyload example fails when parameter encryption is not used (investigating, I am close to solving it)

once I solve (3.) I will fix the rest of the native tests

tomoveu · 2020-11-26T21:38:10Z

I pushed my latest changes and all but two native tests should be now working

I am investigating issues with (the native tests of) TPM2_LoadExternal and TPM2_ECDH_KeyGen

tomoveu · 2020-11-27T15:25:59Z

Fixed TPM2_LoadExternal and HMAC native tests.

I compared the command request of the previously working native tests with the current state and went down the rabbit whole debugging. The storage key auth is set even though it is not needed for TPM2_LoadExternal. Simply disabling the auth session for the TPM2_LoadExternal solved the problem.

Although the storage auth is not in a perfect place in the code around LoadExternal, it is later used for other native tests, when TPM_RS_PW is re-enabled in code. The length of the native tests make it difficult to read at times.

tomoveu · 2020-11-27T15:40:24Z

All native tests fixed, but I noticed this diff when comparing the tests output @dgarske I will take a closer look and let's sync

Now


TPM2_ECDH_KeyGen: zPt 2, pubPt 60533
TPM2_ECDH_ZGen: zPt 2
...
TPM2_RSA_Encrypt: 256
TPM2_RSA_Decrypt: 2

Before

TPM2_ECDH_KeyGen: zPt 68, pubPt 68
TPM2_ECDH_ZGen: zPt 68

TPM2_RSA_Encrypt: 256
TPM2_RSA_Decrypt: 68

tomoveu · 2020-11-27T18:10:56Z

I finished testing the parameter encryption examples. They all seem to work flawlessly with just one detail.

I noticed that for keyimport the EncSz is 0 and yet param enc is set and our implementation is executed.

I digged into the TCG Spec and found that for TPM2_Import the first TPM2B parameter is optional. And we are not using it, therefore our keyimport example does not demonstrate in reality parameter encryption when option '-e' is enabled.

encryptionKey the optional symmetric encryption key used as the inner
wrapper for duplicate
If symmetricAlg is TPM_ALG_NULL, then this
parameter shall be the Empty Buffer.

Awesome work @dgarske I am glad you approve changing core design choices of wolfTPM to get param encryption. We also now have the handling for the maximum auth sessions that opens all doors to enhanced authorization! Just awesome 💯

…encrypt" or "decrypt" if command doesn't allow it. Updated documentation.

dgarske

Approved. Going to assign to @elms for review.

embhorn

Wow, this is a great addition! Couple minor changes. Also, seeing failure on make check (could be my config).

embhorn · 2020-11-30T22:05:26Z

examples/tls/tls_common.h

@@ -60,7 +60,7 @@
 /* force use of a TLS cipher suite */
 #if 0
    #ifndef TLS_CIPHER_SUITE
-        #define TLS_CIPHER_SUITE "ECDHE-RSA-AES128-SHA256"
+        #define TLS_CIPHER_SUITE "ECDHE-rsa-AES128-SHA256"


Was this intentional?

embhorn · 2020-11-30T22:08:35Z

examples/tls/tls_server.c

@@ -319,7 +374,7 @@ int TPM2_TLS_Server(void* userCtx)

 #if 0
    /* Optionally choose the cipher suite */
-    rc = wolfSSL_CTX_set_cipher_list(ctx, "ECDHE-RSA-AES128-GCM-SHA256");
+    rc = wolfSSL_CTX_set_cipher_list(ctx, "ECDHE-rsa-AES128-GCM-SHA256");


Was this intentional?

embhorn · 2020-11-30T22:31:31Z

tests/unit_tests.c

+    rc = TPM2_KDFa(TPM_ALG_SHA256, &keyIn, label, &contextU, &contextV, key, keyIn.size);
+    AssertIntEQ(sizeof(keyExp), rc);
+
+    AssertIntEQ(XMEMCMP(key, keyExp, sizeof(keyExp)), 0);


This assert is failing when I run make check.

./configure --enable-debug=verbose make && make check

embhorn · 2020-11-30T22:36:43Z

src/tpm2_param_enc.c

+ * 'keyStream' points to the buffer storing the generated session key, and
+ * 'keyStream' can not be NULL.


keystream param was removed

embhorn · 2020-11-30T22:39:11Z

src/tpm2_param_enc.c

-        return BAD_FUNC_ARG;
-
-    if (doOnce != 0 && (sizeInBits & 7) != 0)
+    if (key == NULL || keyStream == NULL)


Check either, not both

…M Simulator for make check (does not build out of the box on all platforms). Make dist was not including the new tpm2_socket.h.

embhorn

Excellent work, Dimi and David!

…t. Allow NULL key for the HmacSetKey (for unsalted / unbound).

elms · 2020-12-02T00:33:18Z

Testing with --winapi and seeing failures. Attempting to compare to head of tree, but will have to come back to that.

keygen/keyload, signed_timestamp, and quote work.

CSR

./examples/csr/csr.exe
TPM2 CSR Example
TPM2: Caps 0x00000000, Did 0x0000, Vid 0x0000, Rid 0x 0
TPM2_ReadPublic failed 139: TPM_RC_HANDLE: The handle is not correct for the use
TPM2_CreatePrimary: 0x80ffffff (282 bytes)
Loading SRK: Storage 0x80ffffff (282 bytes)
Reading 727 bytes from rsa_test_blob.raw
TPM2_Load Key Handle 0x80fffffe
Loaded key to 0x80fffffe
CryptoDevCb Hash: Type 4
CryptoDevCb Hash: Type 4
CryptoDevCb Hash: Type 6
CryptoDevCb Hash: Type 6
Failure 0xffffff7d: RSA buffer error, output too small or input too big
TPM2_FlushContext: Closed handle 0x80ffffff
TPM2_FlushContext: Closed handle 0x80fffffe

wrap

 ./examples/wrap/wrap_test.exe
TPM2 Demo for Wrapper API's
TPM2: Caps 0x00000000, Did 0x0000, Vid 0x0000, Rid 0x 0
Mfg STM  (2), Vendor , Fw 73.20 (1151343132), FIPS 140-2 1, CC-EAL4 0
TPM2_FlushContext failed 139: TPM_RC_HANDLE: The handle is not correct for the use
TPM2_FlushContext failed 139: TPM_RC_HANDLE: The handle is not correct for the use
TPM2_FlushContext failed 139: TPM_RC_HANDLE: The handle is not correct for the use
TPM2_CreatePrimary: 0x80ffffff (314 bytes)
TPM2_FlushContext: Closed handle 0x80ffffff
TPM2_ReadPublic failed 139: TPM_RC_HANDLE: The handle is not correct for the use
TPM2_CreatePrimary: 0x80fffffe (282 bytes)
TPM2_EvictControl failed -2144861184: unknown error number
Failure 0x80280400: unknown error number
TPM2_Shutdown failed 0x80280400: unknown error number
wolfTPM2_Shutdown complete

dgarske · 2020-12-02T15:47:37Z

@elms : On the winapi test of the param enc PR. The wrapper is trying to use TPM2_EvictControl, which is not allowed on winapi... That is an easy fix. For the csr one I would need verbose logs to debug cause (--enable-debug=verbose).

elms · 2020-12-02T18:08:48Z

@elms : On the winapi test of the param enc PR. The wrapper is trying to use TPM2_EvictControl, which is not allowed on winapi... That is an easy fix. For the csr one I would need verbose logs to debug cause (--enable-debug=verbose).

oops I had used --enable-debug=VERBOSE

./examples/csr/csr.exe
TPM2 CSR Example
TPM2: Caps 0x00000000, Did 0x0000, Vid 0x0000, Rid 0x 0
TPM2_ReadPublic failed 139: TPM_RC_HANDLE: The handle is not correct for the use
Found 1 auth sessions
CommandProcess: Handles (Auth 1, In 1), CmdSz 355, AuthSz 9, ParamSz 328, EncSz 36
ResponseProcess: Handles (Out 1), RespSz 474, ParamSz 451, DecSz 282, AuthSz 5
TPM2_CreatePrimary: 0x80ffffff (282 bytes)
Loading SRK: Storage 0x80ffffff (282 bytes)
Reading 825 bytes from rsa_test_blob.raw
Found 1 auth sessions
CommandProcess: Handles (Auth 1, In 1), CmdSz 550, AuthSz 41, ParamSz 491, EncSz 207
ResponseProcess: Handles (Out 1), RespSz 59, ParamSz 36, DecSz 34, AuthSz 5
TPM2_Load Key Handle 0x80fffffe
Loaded key to 0x80fffffe
CryptoDevCb Hash: Type 4
CryptoDevCb Hash: Type 4
CryptoDevCb Hash: Type 6
CryptoDevCb Hash: Type 6
CryptoDevCb Pk: Type 1
Found 1 auth sessions
CommandProcess: Handles (Auth 1, In 1), CmdSz 304, AuthSz 24, ParamSz 262, EncSz 256
TPM2_RSA_Decrypt failed 162: TPM_RC_BAD_AUTH: Authorization failure without DA implications
wolfTPM2_CryptoDevCb failed rc = 162
Failure 0xffffff08: Error with hardware crypto use
TPM2_FlushContext: Closed handle 0x80ffffff
TPM2_FlushContext: Closed handle 0x80fffffe

Works on head of tree although the keyblobs didn't seem compatible, failed integrity check. Is that expected? Maybe an issue with unexpected auth change?

dgarske · 2020-12-02T22:09:39Z

@dgarske : Feedback from @elms, which I am investigating.

The CSR example required re-running the keygen. Did the .blob format change in this PR?
The TLS client / server examples with -aes are not working.

…h/rpHash calculation). Fix for sessionAttributes when command / response doesn't support it. Fixes for the TLS client / server examples. Added back the useful param enc / hmac debugging enabled with `--enable-debug=verbose`.

elms

I did tests using --enable-winapi (far from exhaustive) and they behave the same as before. Read over code (again not exhaustive) and looks good to me.

Awesome work @tomoveu and @dgarske.

tomoveu mentioned this pull request Oct 9, 2020

Add TPM 2.0 Parameter Encryption support #110

Closed

dgarske marked this pull request as draft October 9, 2020 13:41

dgarske assigned tomoveu Oct 9, 2020

dgarske requested changes Oct 23, 2020

View reviewed changes

src/tpm2_param_enc.c Outdated Show resolved Hide resolved

dgarske force-pushed the add-keygen-example branch from fa84692 to d85e172 Compare October 23, 2020 22:01

dgarske reviewed Oct 23, 2020

View reviewed changes

src/tpm2_param_enc.c Show resolved Hide resolved

dgarske force-pushed the add-keygen-example branch from e856988 to 4267b1e Compare October 26, 2020 22:54

tomoveu force-pushed the add-keygen-example branch 2 times, most recently from c35fff4 to 49ac89b Compare November 4, 2020 17:06

tomoveu force-pushed the add-keygen-example branch from 6b7e4ed to 19d54f6 Compare November 13, 2020 14:59

tomoveu marked this pull request as ready for review November 13, 2020 16:52

tomoveu requested a review from dgarske November 13, 2020 16:53

dgarske force-pushed the add-keygen-example branch from 7f6abff to dff540e Compare November 14, 2020 00:37

tomoveu mentioned this pull request Nov 18, 2020

New auth session handling for TPM2 command handlers #133

Merged

Dimitar Tomov added 5 commits November 19, 2020 15:08

Encrypt innerWrap and TPM2B_Private together for TPM2_Load, without o…

cc812e5

…uterWrap Signed-off-by: Dimitar Tomov <dimi@wolfssl.com>

Modify keygen_paramenc to use new wolfTPM2 wrappers for Create and Load

324bdea

Signed-off-by: Dimitar Tomov <dimi@wolfssl.com>

Added keyload with parameter encryption example

23e5b62

Signed-off-by: Dimitar Tomov <dimi@wolfssl.com>

Minor change, remove obsolete code from keygen_paramenc example

191412f

Signed-off-by: Dimitar Tomov <dimi@wolfssl.com>

Added parameter encryption support to more examples. Fix to not set "…

4b0b708

…encrypt" or "decrypt" if command doesn't allow it. Updated documentation.

dgarske force-pushed the add-keygen-example branch from b3d511f to 4b0b708 Compare November 30, 2020 20:01

dgarske approved these changes Nov 30, 2020

View reviewed changes

dgarske assigned elms and embhorn and unassigned tomoveu and elms Nov 30, 2020

embhorn requested changes Nov 30, 2020

View reviewed changes

dgarske assigned dgarske and unassigned embhorn Nov 30, 2020

Fixes from peer review. Resolve issue with make check. Removed the TP…

8087afa

…M Simulator for make check (does not build out of the box on all platforms). Make dist was not including the new tpm2_socket.h.

dgarske requested a review from elms December 1, 2020 17:17

Fixes for scan-build and g++ warnings.

d044360

dgarske requested a review from embhorn December 1, 2020 18:42

embhorn approved these changes Dec 1, 2020

View reviewed changes

Updates to demonstrate using parameter encryption with the native tes…

75db91c

…t. Allow NULL key for the HmacSetKey (for unsalted / unbound).

elms approved these changes Dec 3, 2020

View reviewed changes

elms merged commit c7e9326 into wolfSSL:master Dec 3, 2020

tomoveu mentioned this pull request Dec 3, 2020

TPM2_GetTag does not return TPM_ST_SESSIONS when required #120

Closed

tomoveu deleted the add-keygen-example branch January 28, 2021 13:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Parameter Encryption support #129

Add Parameter Encryption support #129

tomoveu commented Oct 9, 2020 •

edited by dgarske

Loading

tomoveu commented Oct 9, 2020

tomoveu commented Oct 27, 2020

tomoveu commented Oct 27, 2020

tomoveu commented Oct 30, 2020

tomoveu commented Nov 13, 2020

tomoveu commented Nov 13, 2020

tomoveu commented Nov 17, 2020 •

edited

Loading

tomoveu commented Nov 18, 2020 •

edited

Loading

tomoveu commented Nov 18, 2020

tomoveu commented Nov 18, 2020

tomoveu commented Nov 18, 2020 •

edited

Loading

tomoveu commented Nov 26, 2020 •

edited

Loading

tomoveu commented Nov 26, 2020 •

edited

Loading

tomoveu commented Nov 27, 2020

tomoveu commented Nov 27, 2020

tomoveu commented Nov 27, 2020

dgarske left a comment

embhorn left a comment

embhorn Nov 30, 2020

embhorn Nov 30, 2020

embhorn Nov 30, 2020

embhorn Nov 30, 2020

embhorn Nov 30, 2020

embhorn left a comment

elms commented Dec 2, 2020 •

edited by dgarske

Loading

dgarske commented Dec 2, 2020

elms commented Dec 2, 2020

dgarske commented Dec 2, 2020

elms left a comment

		* 'keyStream' points to the buffer storing the generated session key, and
		* 'keyStream' can not be NULL.

Add Parameter Encryption support #129

Add Parameter Encryption support #129

Conversation

tomoveu commented Oct 9, 2020 • edited by dgarske Loading

tomoveu commented Oct 9, 2020

tomoveu commented Oct 27, 2020

tomoveu commented Oct 27, 2020

tomoveu commented Oct 30, 2020

tomoveu commented Nov 13, 2020

tomoveu commented Nov 13, 2020

tomoveu commented Nov 17, 2020 • edited Loading

tomoveu commented Nov 18, 2020 • edited Loading

tomoveu commented Nov 18, 2020

tomoveu commented Nov 18, 2020

tomoveu commented Nov 18, 2020 • edited Loading

tomoveu commented Nov 26, 2020 • edited Loading

tomoveu commented Nov 26, 2020 • edited Loading

tomoveu commented Nov 27, 2020

tomoveu commented Nov 27, 2020

tomoveu commented Nov 27, 2020

dgarske left a comment

Choose a reason for hiding this comment

embhorn left a comment

Choose a reason for hiding this comment

embhorn Nov 30, 2020

Choose a reason for hiding this comment

embhorn Nov 30, 2020

Choose a reason for hiding this comment

embhorn Nov 30, 2020

Choose a reason for hiding this comment

embhorn Nov 30, 2020

Choose a reason for hiding this comment

embhorn Nov 30, 2020

Choose a reason for hiding this comment

embhorn left a comment

Choose a reason for hiding this comment

elms commented Dec 2, 2020 • edited by dgarske Loading

dgarske commented Dec 2, 2020

elms commented Dec 2, 2020

dgarske commented Dec 2, 2020

elms left a comment

Choose a reason for hiding this comment

tomoveu commented Oct 9, 2020 •

edited by dgarske

Loading

tomoveu commented Nov 17, 2020 •

edited

Loading

tomoveu commented Nov 18, 2020 •

edited

Loading

tomoveu commented Nov 18, 2020 •

edited

Loading

tomoveu commented Nov 26, 2020 •

edited

Loading

tomoveu commented Nov 26, 2020 •

edited

Loading

elms commented Dec 2, 2020 •

edited by dgarske

Loading