tpm2-software / tpm2-tss-engine

OpenSSL Engine for TPM2 devices

https://tpm2-software.github.io

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

150 stars 100 forks source link

added NO_DL build configuration. #125

Closed yufeng-wr closed 3 years ago

yufeng-wr commented 5 years ago

If defined NO_DL, the engine will not support using the tcti shared library. Some system such as VxWorks builds the tcti as static library and links with OS image together. In this situation, shared library is not supported and should be removed from engine.

codecov[bot] commented 5 years ago

Codecov Report

Merging #125 into master will increase coverage by 1.17%. The diff coverage is n/a.

@@            Coverage Diff            @@
##           master    #125      +/-   ##
=========================================
+ Coverage   67.43%   68.6%   +1.17%     
=========================================
  Files           9       9              
  Lines        1176    1239      +63     
=========================================
+ Hits          793     850      +57     
- Misses        383     389       +6

Impacted Files	Coverage Δ
src/tpm2-tss-engine-tcti.c	`56.19% <ø> (ø)`	:arrow_up:
src/tpm2-tss-engine-ecc.c	`68.68% <0%> (+6.29%)`	: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 0bd2b64...8571051. Read the comment docs.

yufeng-wr commented 5 years ago

Is there any update for this review?

AndreasFuchsTPM commented 5 years ago

IMO, this patch is too narrow. Instead the whole engine-control should be deactivated instead and I guess the complete -tcti.c file excluded from build. I mean, what purpose does the engine-crtl serve, if there is no backing code ?

yufeng-wr commented 5 years ago

Good suggestion. How about create a new file tpm2-tss-engine-static-tcti.c? It will use the tcti static library.

AndreasFuchsTPM commented 5 years ago

The uestion is, whether we want to statically set a TCTI for the engine here, or whether we just want to rely on ESYS to set a static TCTI correctly ?

yufeng-wr commented 5 years ago

The engine should rely on ESYS to set a static TCTI by default. Should we consider the different TCTI configuration or not? If yes, the dynamic and static libraries should be considered also.

AndreasFuchsTPM commented 5 years ago

Since you're the only user of NO_DL at this moment: You tell me, if you need a TCTI-ctrl for static.

In either case, the current TCTI-ctrl needs should be disabled alltogether for no-dynamic builds and I guess the complete file tpm2-tss-engine-tcti.c should be excluded from the build.

It should take a configure-flag as well, I guess.

yufeng-wr commented 5 years ago

The ESYS default configuration is enough for me. It can work with VxWorks. tpm2-tss-engine-tcti.c includes some public APIs, such as tcti_clear_opts() and tcti_set_opts(). Currently, it can't be removed for no-dynamic builds. In addition, some functions in other .c files need to be modified (i.e esys_auxctx_init(), esys_auxctx_free().) It seems the ESYS initializing and finalizing processes need to be redesigned.

AndreasFuchsTPM commented 5 years ago

Hmpf... sounds like it... :-(

AndreasFuchsTPM commented 5 years ago

We may run into a redesign of this anyways with the tss2-tctildr.so library appearing on tpm2-tss. Thus, I guess we can postpone this until that library is available upstream.

AndreasFuchsTPM commented 3 years ago

Seems stalled. So closing.

tctildr is a thing now though, so might make this whole PR much simpler.