Add Symmetric Encryption #77

GaelGuegan · 2019-02-18T09:36:26Z

Description

Adding Symmetric Encryption : tpm2-tss-engine-ciphers.c. (Issue #70)

This is done by handling structure EVP_CIPHER_meth_new.

How to use ?

Shell Command Line utility :

> Create a persistent sym key
> openssl enc -aes-256-cbc -e -engine tpm2tss -in data.txt -out enc_data -K 81000001 -iv 0123456789012345
> openssl enc -aes-256-cbc -d -engine tpm2tss -in enc_data -out dec_data.txt -K 81000001 -iv 0123456789012345

See the shell script : ciphers.sh.

Openssl API in C code :

strcpy(key , "81000001");
EVP_EncryptInit_ex(ctx, EVP_aes_256_cbc(), libtpm2tss, key, iv);
EVP_EncryptUpdate(ctx, pBufOut, &tmpLen, pBufIn, inLen);
EVP_EncryptFinal_ex(ctx, pBufOut + outLen, &tmpLen);

See official wiki.

Create Key :

tpm2tss-genkey -a aes -m cfb -s 128 sym_key

Problems

For now, it is only working with persistent handle. We cannot passed key file through the openssl app (Doc), only hex string.

Signed-off-by: Gael Guegan [email protected]

codecov-io · 2019-02-20T15:58:33Z

Codecov Report

Merging #77 into master will increase coverage by 1.75%.
The diff coverage is 71.96%.

@@            Coverage Diff             @@
##           master      #77      +/-   ##
==========================================
+ Coverage   66.13%   67.88%   +1.75%     
==========================================
  Files           9       10       +1     
  Lines        1134     1308     +174     
==========================================
+ Hits          750      888     +138     
- Misses        384      420      +36

Impacted Files	Coverage Δ
src/tpm2-tss-engine-err.c	`72% <ø> (+24%)`	⬆️
src/tpm2-tss-engine.c	`58.46% <50%> (+0.99%)`	⬆️
src/tpm2-tss-engine-ciphers.c	`72.65% <72.65%> (ø)`
src/tpm2-tss-engine-rand.c	`100% <0%> (ø)`	⬆️
src/tpm2-tss-engine-ecc.c	`62.38% <0%> (+0.19%)`	⬆️
src/tpm2-tss-engine-rsa.c	`69.73% <0%> (+0.27%)`	⬆️
src/tpm2tss-genkey.c	`58.1% <0%> (+0.31%)`	⬆️
src/tpm2-tss-engine-common.c	`79.57% <0%> (+0.83%)`	⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 568d700...6886093. Read the comment docs.

GaelGuegan · 2019-02-20T16:03:26Z

After having some trouble with sserver.sh, clang, openssl 1.0.2 ... I resolved the bugs

Finally, CI is passing.

@AndreasFuchsSIT Can it be merged ?

AndreasFuchsTPM · 2019-02-21T12:44:22Z

Glad you could make it work, I know the hassle... ;-)

I'll have a closer look over it after the 1.0 engine release. And queue it up for the 1.1 release...

From a first look, I think this still lacks tpm2tss-genkey support and loading of these, documentation in man-pages and bash-completion.

GaelGuegan · 2019-02-21T13:51:35Z

I didn't find a way to use tpm2tss-genkey. Because of the interface openssl enc which does not have options to read a key from a file.

From the man pages of openssl (also source code here) :

-k password : the password to derive the key from. This is for compatibility with previous versions of OpenSSL. Superseded by the -pass argument.
-kfile filename : read the password to derive the key from the first line of filename. This is for compatibility with previous versions of OpenSSL. Superseded by the -pass argument.
-K key : The actual key to use: this must be represented as a string comprised only of hex digits. If only the key is specified, the IV must additionally specified using the -iv option. When both a key and a password are specified, the key given with the -K option will be used and the IV generated from the password will be taken. It does not make much sense to specify both key and password.

All it can do is derived a key from a password from a file. I was only able to use the option -K to pass a persistent handle of the TPM. Still looking for solutions ...

AndreasFuchsTPM · 2019-02-21T14:39:40Z

Hmmmm..... If -K really requires the actual key, then the TPM handle does not suit this IMHO, because it's just the handle...

GaelGuegan · 2019-02-21T15:23:53Z

I did that because we were doing the same with RSA. See this test :
echo "abc" | openssl pkeyutl -engine tpm2tss -keyform engine -inkey ${HANDLE} -sign -in ${DIR}/mydata.txt -out ${DIR}/mysig -passin stdin

AndreasFuchsTPM · 2019-02-21T16:57:18Z

I know, and I only justify it due to -keyform engine. Still no clean, but somewhat arguable.
Give me some time to think about this. Doc and Man would still be good though...

GaelGuegan · 2019-02-22T14:07:26Z

I have made a working function tpm2tss_sym_genkey(), and write manual.

GaelGuegan · 2019-04-10T14:52:56Z

@AndreasFuchsSIT I have rebased and squashed, is that ok for you ?

AndreasFuchsTPM · 2019-04-10T18:57:30Z

Yes, that's great...
Now let's figure out how to get the TPM keyids in there...
I'll ask openssl upstream developers if they have something in mind...

GaelGuegan · 2019-04-11T08:23:55Z

Currently, I don't think that the openssl app can pass file. Maybe that in the meantime that openssl developpers find a solution, if there is one, shouldn't we merge this PR ?

However, I think it would be possible to use our key file through openssl API : EVP_Encrypt(...).
see wiki and man.

Maybe, one solution would be to get our filename, open and read it in our engine. But the filename need to be a hexadecimal string because of that function.

AndreasFuchsTPM

I'd be fine with merging this feature for persistent keys only then, until upstream openssl picked up on the new structure.

Since we cannot use file-based keys now, all references to that should be removed.
I've made some comments in this regard and also 2 more.

With that, I'd be happy to merge...

P.S. But keep the current code around somewhere, so we can recycle it once upstream is ready.

src/tpm2-tss-engine-ciphers.c

src/tpm2tss-genkey.c

test/ciphers.sh

src/tpm2-tss-engine-ciphers.c

man/tpm2tss_sym_genkey.3.md

man/tpm2tss-genkey.1.md

GaelGuegan · 2019-05-02T13:31:55Z

CFB and OFB are now the two symmetric modes available. CBC was apparently causing troubles with openssl sserver & sclient.

However if a mode is already defined within the TPM persistent key, it will use it.

GaelGuegan · 2019-05-06T14:29:18Z

😉

AndreasFuchsTPM

Thanks a lot for the rework.

Sorry for taking so long.
I've posted a few more things that I encountered when looking at the code.

AndreasFuchsTPM · 2019-05-06T14:40:54Z

man/tpm2tss-genkey.1.md

@@ -93,4 +93,3 @@ Technologies AG. License BSD 3-clause.
 ## SEE ALSO

 openssl(1)
-


That's a non-change, please revert it

src/tpm2-tss-engine-ciphers.c

AndreasFuchsTPM · 2019-05-06T15:08:00Z

src/tpm2-tss-engine-ciphers.c

+    /* Get App Data */
+    tpm2Data = EVP_CIPHER_CTX_get_app_data(ctx);
+    if (tpm2Data == NULL || tpm2Data->pub.size == 0 || inl == 0) {
+        return 0;


If tpm2Data is NULL, I think we want to call openSSL's standard cipher engine, right ?

mmmh don't know how to do that here.

And if tpm2Data is NULL, tpm2_cipher_init_key() function should have failed before.

src/tpm2-tss-engine-ciphers.c

AndreasFuchsTPM · 2019-05-06T16:25:18Z

src/tpm2-tss-engine-ciphers.c

+    TSS2_RC ret;
+    ESYS_AUXCONTEXT eactx = (ESYS_AUXCONTEXT){ NULL, NULL};
+    ESYS_TR keyHandle = ESYS_TR_NONE;
+    TPM2B_MAX_BUFFER *out_data, *in_data;


in_data is never freed.
Also you don't have to malloc it, you could just declare TPM2_MAX_BUFFER in_data; on stack instead of a pointer.

src/tpm2-tss-engine-ciphers.c

AndreasFuchsTPM · 2019-05-06T16:31:15Z

src/tpm2-tss-engine-ciphers.c

+}
+
+#if OPENSSL_VERSION_NUMBER < 0x10100000
+static EVP_CIPHER tpm2_aes_256_ofb =


Sorry for being so ignorant, but could you explain the 256 and ofb128 to me ?
I don't know what they mean.
Does that mean that aes 128 is not supported ?

I guess a comment in code would also be good...

So this is about the selector function and NIDS, I guess. So will try to explain my understanding of it.

So at first, you can try to type a openssl enc -ciphers on your machine. You will see there the different openssl symmetric encryption available. We have to define which algos of this list , our engine will implement. Currently, I have defined these two :

static int tpm2_cipher_nids[] = { NID_aes_256_cfb128, NID_aes_256_ofb128, 0 };

Which are defined by AES algorithm, 256 bits key size and cfb/ofb encryption mode.
When someone call openssl enc -aes-256-cfb -e -engine tpm2tss ..., our engine implementation will be used. However by using a persistent handle, the key already exist so in case of creating it using AES and a 256 bits size, we are currently retrieving it from the tpm in the tpm2_cipher_init_key function, then we choose either the encryption mode asked or either the one already existing in the key.

Moreover, IDs are defined here in the openssl repo.

As i said before I took the gost engine as example, we can see here the implementation of its cipher selector. Also another really simple cipher engine exits here.

If you have other question, don't hesistate. I will try to be the clearest possible and explain my understanding.

AndreasFuchsTPM · 2019-05-06T16:38:43Z

src/tpm2-tss-engine-ciphers.c

+    tpm2_cipher_init_key,               // Init key
+    tpm2_do_cipher,                     // Encrypt/Decrypt
+    tpm2_cipher_cleanup,                // Cleanup
+    sizeof(TPM2_DATA),                  // Context size


I don't think this is correct. The man-page says EVP_CIPHER_meth_set_impl_ctx_size() sets the size of the EVP_CIPHER's implementation context so that it can be automatically allocated. but we are malloc'ing it ourselves. But I also don't know what this is really for and how we should use it.

To be honest, I don't fully understand this stuff neither.

I based my code by taking the gost engine as example. In there code, we can see that there are doing a EVP_CIPHER_meth_set_impl_ctx_size(_hidden_magma_cbc, sizeof(struct ossl_gost_cipher_ctx)) here and then struct ossl_gost_cipher_ctx *c = EVP_CIPHER_CTX_get_cipher_data(ctx); here.

So maybe I should put a 0 value here, I tried and it does not seem to change anything. I can change it if you want ?

test/ciphers.sh

AndreasFuchsTPM · 2019-05-06T16:46:01Z

CFB and OFB are now the two symmetric modes available. CBC was apparently causing troubles with openssl sserver & sclient.

However if a mode is already defined within the TPM persistent key, it will use it.

I guess that had to do with the comment I posted here: #77 (diff)

Please see that we enable CBC mode for the TPM and still have sserver running. It's basically the counter test that the engine does not interfere with any other operations; especially if the engine shall only be used for asymmetric operations.
Also I don't know about the switch-case statement in the selector function. Maybe the default there also needs to be a standard engine.

AndreasFuchsTPM · 2019-05-06T16:46:58Z

As a final comment: Since only very few TPMs support symmetric operations, I'd like to wrap support for it in a configure parameter --enable-symmetric; just to minimize interference possibilities with the asymmetric operations.

GaelGuegan · 2019-05-09T10:12:16Z

I have done some changes that you requested. However can you relaunch my CI build ? a weird error appeared, CI was not able to clone openssl.

AndreasFuchsTPM · 2019-05-09T11:51:57Z

You can't but I did

AndreasFuchsTPM · 2019-05-09T15:43:52Z

Since tpm2-software/tpm2-tools#1476 I'll retrigger the CI...

Signed-off-by: Gael Guegan <[email protected]>

AndreasFuchsTPM · 2021-03-01T15:50:06Z

Looks very much stalled. So closing

GaelGuegan force-pushed the master branch 7 times, most recently from 5f4b457 to 125dd34 Compare February 18, 2019 16:42

GaelGuegan changed the title ~~Added Symmetric Encryption~~ Add Symmetric Encryption Feb 20, 2019

GaelGuegan force-pushed the master branch 10 times, most recently from f4354ae to e6dc877 Compare February 20, 2019 15:42

GaelGuegan force-pushed the master branch from e6dc877 to 515cbaf Compare February 22, 2019 13:28

GaelGuegan closed this Feb 22, 2019

GaelGuegan reopened this Feb 26, 2019

GaelGuegan force-pushed the master branch from d6d9cac to a70548f Compare February 26, 2019 12:59

GaelGuegan force-pushed the master branch 4 times, most recently from 91618eb to f5a4ef9 Compare April 10, 2019 14:33

AndreasFuchsTPM mentioned this pull request Apr 10, 2019

HSM (engine) support for symmetric crypto openssl/openssl#8719

Closed

AndreasFuchsTPM requested changes Apr 12, 2019

View reviewed changes

GaelGuegan force-pushed the master branch 2 times, most recently from 6bed34d to 2121588 Compare April 15, 2019 15:39

GaelGuegan force-pushed the master branch from 2121588 to 94d9454 Compare May 2, 2019 13:12

AndreasFuchsTPM requested changes May 6, 2019

View reviewed changes

GaelGuegan force-pushed the master branch from 94d9454 to a213d35 Compare May 7, 2019 15:12

GaelGuegan force-pushed the master branch from a213d35 to 57ec2c5 Compare May 15, 2019 11:29

GaelGuegan force-pushed the master branch 3 times, most recently from 32efe28 to 33f681e Compare July 26, 2019 16:31

Added Symmetric Encryption

6886093

Signed-off-by: Gael Guegan <[email protected]>

GaelGuegan force-pushed the master branch from 33f681e to 6886093 Compare July 28, 2019 20:32

AndreasFuchsTPM removed this from the v1.1_feature-freeze milestone Mar 1, 2021

AndreasFuchsTPM closed this Mar 1, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Symmetric Encryption #77

Add Symmetric Encryption #77

GaelGuegan commented Feb 18, 2019 •

edited

Loading

codecov-io commented Feb 20, 2019 •

edited by codecov bot

Loading

GaelGuegan commented Feb 20, 2019 •

edited

Loading

AndreasFuchsTPM commented Feb 21, 2019

GaelGuegan commented Feb 21, 2019 •

edited

Loading

AndreasFuchsTPM commented Feb 21, 2019

GaelGuegan commented Feb 21, 2019

AndreasFuchsTPM commented Feb 21, 2019

GaelGuegan commented Feb 22, 2019 •

edited

Loading

GaelGuegan commented Apr 10, 2019

AndreasFuchsTPM commented Apr 10, 2019 •

edited

Loading

GaelGuegan commented Apr 11, 2019 •

edited

Loading

AndreasFuchsTPM left a comment

GaelGuegan commented May 2, 2019

GaelGuegan commented May 6, 2019

AndreasFuchsTPM left a comment

AndreasFuchsTPM May 6, 2019

AndreasFuchsTPM May 6, 2019

GaelGuegan May 7, 2019 •

edited

Loading

AndreasFuchsTPM May 6, 2019

AndreasFuchsTPM May 6, 2019

GaelGuegan May 7, 2019 •

edited

Loading

GaelGuegan May 7, 2019

AndreasFuchsTPM May 6, 2019

GaelGuegan May 7, 2019

AndreasFuchsTPM commented May 6, 2019

AndreasFuchsTPM commented May 6, 2019

GaelGuegan commented May 9, 2019

AndreasFuchsTPM commented May 9, 2019

AndreasFuchsTPM commented May 9, 2019

AndreasFuchsTPM commented Mar 1, 2021

		@@ -93,4 +93,3 @@ Technologies AG. License BSD 3-clause.
		## SEE ALSO

		openssl(1)

Add Symmetric Encryption #77

Add Symmetric Encryption #77

Conversation

GaelGuegan commented Feb 18, 2019 • edited Loading

Description

How to use ?

Problems

codecov-io commented Feb 20, 2019 • edited by codecov bot Loading

Codecov Report

GaelGuegan commented Feb 20, 2019 • edited Loading

AndreasFuchsTPM commented Feb 21, 2019

GaelGuegan commented Feb 21, 2019 • edited Loading

AndreasFuchsTPM commented Feb 21, 2019

GaelGuegan commented Feb 21, 2019

AndreasFuchsTPM commented Feb 21, 2019

GaelGuegan commented Feb 22, 2019 • edited Loading

GaelGuegan commented Apr 10, 2019

AndreasFuchsTPM commented Apr 10, 2019 • edited Loading

GaelGuegan commented Apr 11, 2019 • edited Loading

AndreasFuchsTPM left a comment

Choose a reason for hiding this comment

GaelGuegan commented May 2, 2019

GaelGuegan commented May 6, 2019

AndreasFuchsTPM left a comment

Choose a reason for hiding this comment

AndreasFuchsTPM May 6, 2019

Choose a reason for hiding this comment

AndreasFuchsTPM May 6, 2019

Choose a reason for hiding this comment

GaelGuegan May 7, 2019 • edited Loading

Choose a reason for hiding this comment

AndreasFuchsTPM May 6, 2019

Choose a reason for hiding this comment

AndreasFuchsTPM May 6, 2019

Choose a reason for hiding this comment

GaelGuegan May 7, 2019 • edited Loading

Choose a reason for hiding this comment

GaelGuegan May 7, 2019

Choose a reason for hiding this comment

AndreasFuchsTPM May 6, 2019

Choose a reason for hiding this comment

GaelGuegan May 7, 2019

Choose a reason for hiding this comment

AndreasFuchsTPM commented May 6, 2019

AndreasFuchsTPM commented May 6, 2019

GaelGuegan commented May 9, 2019

AndreasFuchsTPM commented May 9, 2019

AndreasFuchsTPM commented May 9, 2019

AndreasFuchsTPM commented Mar 1, 2021

GaelGuegan commented Feb 18, 2019 •

edited

Loading

codecov-io commented Feb 20, 2019 •

edited by codecov bot

Loading

GaelGuegan commented Feb 20, 2019 •

edited

Loading

GaelGuegan commented Feb 21, 2019 •

edited

Loading

GaelGuegan commented Feb 22, 2019 •

edited

Loading

AndreasFuchsTPM commented Apr 10, 2019 •

edited

Loading

GaelGuegan commented Apr 11, 2019 •

edited

Loading

GaelGuegan May 7, 2019 •

edited

Loading

GaelGuegan May 7, 2019 •

edited

Loading