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

Add basic implementation for ECDH. #207

Closed abeltrano closed 3 years ago

abeltrano commented 3 years ago

Fixes #206.

Add support for ECDH

The implementation uses TPM2_ECDH_ZGen as provided by tss2-esys (Esys_ECDH_ZGen()). A persistent handle value referencing the TPM private key is specified as the key context. Like other engine function implementations, a password session (ESYS_TR_PASSWORD) is used for authorization of the session handle. The public point is expected in uncompressed form.

ECDH Validation Use newly added integration test under tests/ecdh.sh.

abeltrano commented 3 years ago

Could you sQuash the first two and the last two commits ? The first for consistency and the last to so we're always runnable.

Also the tests are actually just bash scripts, so the example from the PR text could directly be used there. I don't want to merge without a test and need some time before writing one myself.

Sure, no problem.

abeltrano commented 3 years ago

Could you sQuash the first two and the last two commits ? The first for consistency and the last to so we're always runnable.

Also the tests are actually just bash scripts, so the example from the PR text could directly be used there. I don't want to merge without a test and need some time before writing one myself.

Could you sQuash the first two and the last two commits ? The first for consistency and the last to so we're always runnable. Also the tests are actually just bash scripts, so the example from the PR text could directly be used there. I don't want to merge without a test and need some time before writing one myself.

Sure, no problem.

I've added an integration test for ecdh but not yet enabled it. Since this feature requires openssl >= 1.1.x, is there a facility in place to skip tests for certain ossl versions?

abeltrano commented 3 years ago

Could you sQuash the first two and the last two commits ? The first for consistency and the last to so we're always runnable. Also the tests are actually just bash scripts, so the example from the PR text could directly be used there. I don't want to merge without a test and need some time before writing one myself.

Could you sQuash the first two and the last two commits ? The first for consistency and the last to so we're always runnable. Also the tests are actually just bash scripts, so the example from the PR text could directly be used there. I don't want to merge without a test and need some time before writing one myself.

Sure, no problem.

I've added an integration test for ecdh but not yet enabled it. Since this feature requires openssl >= 1.1.x, is there a facility in place to skip tests for certain ossl versions?

Tests are now conditionally run based on presence of the OpenSSL API required. My autoconf-fu isn't the best, but it works. @AndreasFuchsSIT, should be ready for your final review now.

abeltrano commented 3 years ago

Could you sQuash the first two and the last two commits ? The first for consistency and the last to so we're always runnable. Also the tests are actually just bash scripts, so the example from the PR text could directly be used there. I don't want to merge without a test and need some time before writing one myself.

Could you sQuash the first two and the last two commits ? The first for consistency and the last to so we're always runnable. Also the tests are actually just bash scripts, so the example from the PR text could directly be used there. I don't want to merge without a test and need some time before writing one myself.

Sure, no problem.

I've added an integration test for ecdh but not yet enabled it. Since this feature requires openssl >= 1.1.x, is there a facility in place to skip tests for certain ossl versions?

Tests are now conditionally run based on presence of the OpenSSL API required. My autoconf-fu isn't the best, but it works. @AndreasFuchsSIT, should be ready for your final review now.

@AndreasFuchsSIT please let me know if any additional changes are needed to complete this PR. Thanks.