use BIGNUM for parent handle

[whooo]

This will encode/decode the parent handle as an unsigned long on 32bit systems.

Fixes https://github.com/tpm2-software/tpm2-tss-engine/issues/221

[whooo]

I just need to find a good way to handle keys with an negative parent handle so nothing breaks

[whooo]

Will leave the decoding as is as it seems to work fine with negative parent handles, even on 64 bit systems

[AndreasFuchsTPM]

Just to make sure: Does this still work with PEM files generated by the old implementation ?

[whooo]

Just to make sure: Does this still work with PEM files generated by the old implementation ?

According to my testing, I was able to use a key with -7EFFFFFF as the parent handle with openssl pkeyutl -sign, but I was only able to test the openssl parts on a 64bit system

[williamcroberts]

Dont worry about LGTM fix over here: https://github.com/tpm2-software/tpm2-tss-engine/pull/223

[williamcroberts]

We could

Just to make sure: Does this still work with PEM files generated by the old implementation ?

According to my testing, I was able to use a key with -7EFFFFFF as the parent handle with openssl pkeyutl -sign, but I was only able to test the openssl parts on a 64bit system

We could set up the i386 container so we get 32bit coverage.

[AndreasFuchsTPM]

32Bit-Containers would be nice (for all tss projects I guess). But I'm mostly concerned about backwards-compatibility. Maybe we could add a test that explicitely loads one of those old PEM files with the negative parent handle ?

[williamcroberts]

32Bit-Containers would be nice (for all tss projects I guess).

It's available now and used by tpm2-abmrd. However it's arm32v7 not i386. It should just be as simple as adding it to your container list (done here https://github.com/tpm2-software/tpm2-tss-engine/pull/224)

But I'm mostly concerned about backwards-compatibility. Maybe we could add a test that explicitely loads one of those old PEM files with the negative parent handle ?

Good idea, I can take one and ensure that tpm2_ptool can handle it.

[williamcroberts]

32Bit-Containers would be nice (for all tss projects I guess). But I'm mostly concerned about backwards-compatibility. Maybe we could add a test that explicitely loads one of those old PEM files with the negative parent handle ?

Is that actually going to be possible because the priv/pub blobs will be for differing TPMs?

[whooo]

I could add a unit test for tpm2tss_tpm2data_read using an already generated key with a negative parent handle.

[AndreasFuchsTPM]

Perfect

[williamcroberts]

LGTM ill let @AndreasFuchsSIT give the final sign-off

Oh let me "approve" the CI run, so obviously that needs to pass.

[whooo]

I'm unsure on why the unit test fails, works well on my laptop, is there anyway to get access to test-suite.log?

[williamcroberts]

I'm unsure on why the unit test fails, works well on my laptop, is there anyway to get access to test-suite.log?

Just expand the failure tab, that has the output:

Run cat build/test-suite.log || true
=================================================
   tpm2-tss-engine 1.1.0-rc1: ./test-suite.log
=================================================

# TOTAL: 19
# PASS:  18
# SKIP:  0
# XFAIL: 0
# FAIL:  1
# XPASS: 0
# ERROR: 0

.. contents:: :depth: 2

FAIL: test/tpm2-tss-engine-common
=================================

[==========] Running 1 test(s).
[ RUN      ] check_tpm2tss_tpm2data_read
[  ERROR   ] --- 0 != 0x1
[   LINE   ] --- ../test/tpm2-tss-engine-common.c:19: error: Failure!
[  FAILED  ] check_tpm2tss_tpm2data_read
[==========] 1 test(s) run.
[  PASSED  ] 0 test(s).
[  FAILED  ] 1 test(s), listed below:
[  FAILED  ] check_tpm2tss_tpm2data_read

 1 FAILED TEST(S)
FAIL test/tpm2-tss-engine-common (exit status: 1)

[williamcroberts]

I don't understand why I don't see error messages, since every failure path seems to have an ERR message. However, I think its becuase we build in a variant directory ./bootstrap && mdir build && cd build && ../configure --enable-unit --enable-debug && make -j4. So it's not finding the file path. If im right, you need to add srcdir to it. I would pass a -D in CFLAGS that has the path to the PEM file.

[williamcroberts]

Oh you need to add that file to make dist so that file exists... Add to EXTRA_DIST.

[AndreasFuchsTPM]

@williamcroberts Do we all agree on merging this now ?

[AndreasFuchsTPM]

@whooo Sorry this took so long, and now I've even introduced a merge conflict, Could you rebase to master and resolve it ? Highly appreciated. Afterwards, I'll roll the next stable release...

[whooo]

@whooo Sorry this took so long, and now I've even introduced a merge conflict, Could you rebase to master and resolve it ? Highly appreciated. Afterwards, I'll roll the next stable release...

Done

[whooo]

I'm not able to reproduce the issue with the multi-arch test, they work in a 32 bit chroot on a x86_64 system and I don't have access to any ARM device that I could run Linux on Was just in the wrong branch.

[whooo]

I had to redo the loading part, so it will check for a negative number and in that case it will use ASN1_INTEGER_get instead of BN_get_word.