tpm2-software / tpm2-tss-engine

OpenSSL Engine for TPM2 devices

https://tpm2-software.github.io

BSD 3-Clause "New" or "Revised" License

151 stars 100 forks source link

TPM key import #194

Closed gotthardp closed 3 years ago

gotthardp commented 4 years ago

May I help finalizing the import feature (#39), please? I took over the content of the #140 and fixed (most of the) review comments, except the following.

Pending questions: "Can't we just import a public key only?" (@williamcroberts) -- Right now the tpm2tss-genkey generates both public and private portion. Consistently with this, the import function lets the user to import public&private portion of a pre-existing key.

"Shouldn't we make the arguments make consistent with tpm2-tools?" (@gotthardp) The tpm2-tools consistently use -u for the public portion and -r for the private portion. I'd suggest to stick to this convention and use -u/-r instead of -i/-k. Would you agree?

Is there anything else that should be addressed?

codecov[bot] commented 4 years ago

Codecov Report

Merging #194 into master will increase coverage by 0.12%. The diff coverage is 72.00%.

@@            Coverage Diff             @@
##           master     #194      +/-   ##
==========================================
+ Coverage   67.71%   67.84%   +0.12%     
==========================================
  Files           8        8              
  Lines        1112     1160      +48     
==========================================
+ Hits          753      787      +34     
- Misses        359      373      +14

Impacted Files	Coverage Δ
src/tpm2-tss-engine-common.c	`76.40% <65.62%> (-1.37%)`	:arrow_down:
src/tpm2tss-genkey.c	`59.11% <83.33%> (+2.14%)`	:arrow_up:

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 ae070ca...55c5350. Read the comment docs.

AndreasFuchsTPM commented 4 years ago

Awesome ! Thanks for taking up this task.

I'd be fine with using -u and -r for the keyfiles.

Please also sQuash both commits into one with you as the author. You can add the other authors using a Co-authored-by tag. And also please add some explanatory commit message as well.

I'll be VERY happy to merge this then. :-)

williamcroberts commented 4 years ago

LGTM, we'll see what CI has to say and if Andreas has anymore feedback.

williamcroberts commented 3 years ago

I used this to test GH Actions since it was an approved PR. Its been moved over to here and merged:

https://github.com/tpm2-software/tpm2-tss-engine/pull/202