Skip to content

Conversation

@baloo
Copy link
Contributor

@baloo baloo commented Aug 2, 2024

This is useful for running tests without having to spawn a simulator in another process.

https://github.com/tpm2-software/tpm2-tss/blob/master/doc/tcti.md#tcti-libtpms

@baloo
Copy link
Contributor Author

baloo commented Aug 2, 2024

Sadly this isn't enough to get the tests going with libtpms. There seems to be an error initializing.

---- context_tests::tpm_commands::enhanced_authorization_ea_commands_tests::test_policy_nv_written::test_policy_nv_written stdout ----
thread 'context_tests::tpm_commands::enhanced_authorization_ea_commands_tests::test_policy_nv_written::test_policy_nv_written' panicked at tss-esapi/tests/integration_tests/context_tests/tpm_commands/enhanced_authorization_ea_commands_tests.rs:807:14:
Start auth session failed: TssError(Tpm(FormatZero(Error(TpmFormatZeroErrorResponseCode { error_number: Initialize }))))
stack backtrace:
   0: rust_begin_unwind
   1: core::panicking::panic_fmt
   2: core::result::unwrap_failed
   3: core::result::Result<T,E>::expect
             at /build/rustc-1.78.0-src/library/core/src/result.rs:1034:23
   4: integration_tests::context_tests::tpm_commands::enhanced_authorization_ea_commands_tests::test_policy_nv_written::test_policy_nv_written
             at ./tests/integration_tests/context_tests/tpm_commands/enhanced_authorization_ea_commands_tests.rs:798:41
   5: integration_tests::context_tests::tpm_commands::enhanced_authorization_ea_commands_tests::test_policy_nv_written::test_policy_nv_written::{{closure}}
             at ./tests/integration_tests/context_tests/tpm_commands/enhanced_authorization_ea_commands_tests.rs:796:32
   6: core::ops::function::FnOnce::call_once
             at /build/rustc-1.78.0-src/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

@baloo
Copy link
Contributor Author

baloo commented Aug 2, 2024

It doesn't work because the context needs to be StartupType::Clear before it can be used.

@ionut-arm
Copy link
Member

It doesn't work because the context needs to be StartupType::Clear before it can be used.

Oh, is this because there's no option for tpm2_startup -c -T <some libtpms option here>? Perhaps we can do something in the tests themselves to replace that.

@baloo baloo force-pushed the baloo/libtpms-backend branch from 2eae78b to 30deecf Compare August 2, 2024 20:20
@baloo
Copy link
Contributor Author

baloo commented Aug 2, 2024

This makes it possible to run almost all the tests here. Only 4 will fail to run:

error output of 4 integration tests
---- abstraction_tests::transient_key_context_tests::activate_credential stdout ----
thread 'abstraction_tests::transient_key_context_tests::activate_credential' panicked at tss-esapi/tests/integration_tests/abstraction_tests/transient_key_context_tests.rs:685:10:
called `Result::unwrap()` on an `Err` value: TssError(Tpm(FormatOne(TpmFormatOneResponseCode { error_number: Integrity, argument_number: Parameter(1) })))
stack backtrace:
   0: rust_begin_unwind
   1: core::panicking::panic_fmt
   2: core::result::unwrap_failed
   3: core::result::Result<T,E>::unwrap
             at /build/rustc-1.78.0-src/library/core/src/result.rs:1077:23
   4: integration_tests::abstraction_tests::transient_key_context_tests::activate_credential
             at ./tests/integration_tests/abstraction_tests/transient_key_context_tests.rs:678:21
   5: integration_tests::abstraction_tests::transient_key_context_tests::activate_credential::{{closure}}
             at ./tests/integration_tests/abstraction_tests/transient_key_context_tests.rs:605:25
   6: core::ops::function::FnOnce::call_once
             at /build/rustc-1.78.0-src/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

---- abstraction_tests::transient_key_context_tests::ctx_migration_test stdout ----
thread 'abstraction_tests::transient_key_context_tests::ctx_migration_test' panicked at tss-esapi/tests/integration_tests/abstraction_tests/transient_key_context_tests.rs:557:10:
called `Result::unwrap()` on an `Err` value: TssError(Tpm(FormatOne(TpmFormatOneResponseCode { error_number: Integrity, argument_number: Parameter(1) })))
stack backtrace:
   0: rust_begin_unwind
   1: core::panicking::panic_fmt
   2: core::result::unwrap_failed
   3: core::result::Result<T,E>::unwrap
             at /build/rustc-1.78.0-src/library/core/src/result.rs:1077:23
   4: integration_tests::abstraction_tests::transient_key_context_tests::ctx_migration_test
             at ./tests/integration_tests/abstraction_tests/transient_key_context_tests.rs:555:15
   5: integration_tests::abstraction_tests::transient_key_context_tests::ctx_migration_test::{{closure}}
             at ./tests/integration_tests/abstraction_tests/transient_key_context_tests.rs:501:24
   6: core::ops::function::FnOnce::call_once
             at /build/rustc-1.78.0-src/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

---- context_tests::general_esys_tr_tests::test_tr_serialize_tr_deserialize::test_tr_serialize_tr_deserialize stdout ----
Error: TssError(Tpm(FormatOne(TpmFormatOneResponseCode { error_number: Handle, argument_number: Handle(1) })))

---- context_tests::tpm_commands::duplication_commands_tests::test_duplicate::test_duplicate_and_import stdout ----
thread 'context_tests::tpm_commands::duplication_commands_tests::test_duplicate::test_duplicate_and_import' panicked at tss-esapi/tests/integration_tests/context_tests/tpm_commands/duplication_commands_tests.rs:256:14:
called `Result::unwrap()` on an `Err` value: TssError(Tpm(FormatOne(TpmFormatOneResponseCode { error_number: PolicyFail, argument_number: Session(1) })))
stack backtrace:
   0: rust_begin_unwind
   1: core::panicking::panic_fmt
   2: core::result::unwrap_failed
   3: core::result::Result<T,E>::unwrap
             at /build/rustc-1.78.0-src/library/core/src/result.rs:1077:23
   4: integration_tests::context_tests::tpm_commands::duplication_commands_tests::test_duplicate::test_duplicate_and_import
             at ./tests/integration_tests/context_tests/tpm_commands/duplication_commands_tests.rs:249:41
   5: integration_tests::context_tests::tpm_commands::duplication_commands_tests::test_duplicate::test_duplicate_and_import::{{closure}}
             at ./tests/integration_tests/context_tests/tpm_commands/duplication_commands_tests.rs:23:35
   6: core::ops::function::FnOnce::call_once
             at /build/rustc-1.78.0-src/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.


failures:
    abstraction_tests::transient_key_context_tests::activate_credential
    abstraction_tests::transient_key_context_tests::ctx_migration_test
    context_tests::general_esys_tr_tests::test_tr_serialize_tr_deserialize::test_tr_serialize_tr_deserialize
    context_tests::tpm_commands::duplication_commands_tests::test_duplicate::test_duplicate_and_import

test result: FAILED. 535 passed; 4 failed; 1 ignored; 0 measured; 0 filtered out; finished in 8.01s

@baloo baloo force-pushed the baloo/libtpms-backend branch from 30deecf to b48a43a Compare August 2, 2024 21:05
@baloo
Copy link
Contributor Author

baloo commented Aug 3, 2024

related, the package changes required to get that working within nix: NixOS/nixpkgs#331676

in the mean time:

  tpm2-tss-libtpms = pkgs.tpm2-tss.overrideAttrs(old: {
    buildInputs = old.buildInputs ++ [ pkgs.libtpms ];

    postPatch = old.postPatch + ''
      for src in src/tss2-tcti/tcti-libtpms.c test/unit/tcti-libtpms.c; do
        substituteInPlace "$src" \
          --replace '"libtpms.so"' '"${  pkgs.libtpms.out}/lib/libtpms.so"' \
          --replace '"libtpms.so.0"' '"${pkgs.libtpms.out}/lib/libtpms.so.0"'
      done
    '';
  });

ionut-arm
ionut-arm previously approved these changes Sep 11, 2024
Copy link
Member

@ionut-arm ionut-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the failures were fixed in other PRs, should be fixed on rebase

Copy link
Collaborator

@wiktor-k wiktor-k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it looks good in general. I'd like to try it out locally in a couple of hours but don't mind me if you think it's good to go :)

@baloo baloo force-pushed the baloo/libtpms-backend branch 2 times, most recently from e631c83 to bc89ecf Compare September 11, 2024 20:32
Superhepper
Superhepper previously approved these changes Sep 12, 2024
wiktor-k
wiktor-k previously approved these changes Sep 12, 2024
Copy link
Collaborator

@wiktor-k wiktor-k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay folks, I've modified my tpm-box crate locally to use libtpms and it works really well 🎉

(it took me a while since it seems tss in Arch had libtpms disabled for some reason... will resolve this asynchronously).

I've added just a couple of nitpicks... nothing big :)

Thanks for your dedicated work @baloo 🙇

)?));
}

let libtpms_pattern = Regex::new(r"^libtpms(:(.*))?$").unwrap(); //should not fail
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Too bad we've got parsing all these regexes here, instead of some static variable... but the same happens with others so it's 👍

@wiktor-k
Copy link
Collaborator

Btw, while my local tests of tpm-box went just fine running TEST_TCTI=libtpms cargo test --no-default-features --features generate-bindings generates quite a lot of failures:

...
test structures_tests::pcr_tests::pcr_select_size_tests::test_invalid_conversions ... [2024-09-12T10:04:22Z ERROR tss_esapi::structures::pcr::select_size] Error converting sizeofSelect to a SelectSize: Invalid value 253
ok
[2024-09-12T10:04:22Z ERROR tss_esapi::structures::pcr::select_size] Error converting sizeofSelect to a SelectSize: Invalid value 254
[2024-09-12T10:04:22Z ERROR tss_esapi::structures::pcr::select_size] Error converting sizeofSelect to a SelectSize: Invalid value 255
test structures_tests::pcr_tests::pcr_select_size_tests::test_try_parse_usize_with_invalid_values ... ok
ERROR:tcti:src/tss2-tcti/tcti-libtpms.c:846:Tss2_Tcti_Libtpms_Init() libtpms function TPMLIB_ChooseTPMVersion() failed with return code 0x9 
ERROR:tcti:src/tss2-tcti/tctildr-dl.c:149:tcti_from_file() Could not initialize TCTI file: libtpms 
[2024-09-12T10:04:22Z ERROR tss_esapi::tcti_ldr] Error when creating a TCTI context: 655361
test tcti_ldr_tests::tcti_context_tests::new_context ... FAILED
test tcti_ldr_tests::tcti_info_tests::new_info ... ok
ERROR:tcti:src/tss2-tcti/tcti-libtpms.c:846:Tss2_Tcti_Libtpms_Init() libtpms function TPMLIB_ChooseTPMVersion() failed with return code 0x9 
ERROR:tcti:src/tss2-tcti/tctildr-dl.c:149:tcti_from_file() Could not initialize TCTI file: libtpms 
[2024-09-12T10:04:22Z ERROR tss_esapi::tcti_ldr] Error when creating a TCTI context: 655361
test utils_tests::get_tpm_vendor_test::get_tpm_vendor ... FAILED
test abstraction_tests::ak_tests::test_create_and_use_ak ... ok

failures:

---- abstraction_tests::nv_tests::write stdout ----
thread 'abstraction_tests::nv_tests::write' panicked at tss-esapi/tests/integration_tests/common/mod.rs:202:24:
called `Result::unwrap()` on an `Err` value: TssError(Tcti(TctiReturnCode { base_error: GeneralFailure }))
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- abstraction_tests::ak_tests::test_create_custom_ak stdout ----
thread 'abstraction_tests::ak_tests::test_create_custom_ak' panicked at tss-esapi/tests/integration_tests/common/mod.rs:202:24:
called `Result::unwrap()` on an `Err` value: TssError(Tcti(TctiReturnCode { base_error: GeneralFailure }))
...

I wonder if that's the limitation of libtpms (which claims: "Libtpms provides a very narrow
public API for this purpose so that integration is possible. Only the minimum of necessary APIs are made publicly available." source) or maybe I'm holding it wrong somehow.

Do tests of this crate pass for your @baloo?

@baloo
Copy link
Contributor Author

baloo commented Sep 12, 2024

I have a different set of tests that fail here:
#535 (comment)

This is useful for running tests without having to spawn a simulator in
another process.

Signed-off-by: Arthur Gautier <[email protected]>
@baloo baloo dismissed stale reviews from wiktor-k and Superhepper via bc7c440 September 12, 2024 21:15
@baloo baloo force-pushed the baloo/libtpms-backend branch from bc89ecf to bc7c440 Compare September 12, 2024 21:15
@wiktor-k
Copy link
Collaborator

I have a different set of tests that fail here: #535 (comment)

Interesting. I'm using version 0.9.6 of libtpms which seems to be the latest stable. Just out of curiosity what version are you using?

(Just to be clear it seems libtpms integration will be quite nice for testing and I'm planning to use it for my projects. I'm just trying to gauge the feature-completeness of it :) ).

Thanks for your work on this!

@Superhepper
Copy link
Collaborator

Can you really run the tests in parallel like that? Is there some kind of built in resource manager in that lib? Otherwise you need to force to just run the tests sequentially.

@baloo
Copy link
Contributor Author

baloo commented Sep 14, 2024

Interesting. I'm using version 0.9.6 of libtpms which seems to be the latest stable. Just out of curiosity what version are you using?

Same libtpms 0.9.6 and tpm2-tss 4.1.3

(Just to be clear it seems libtpms integration will be quite nice for testing and I'm planning to use it for my projects. I'm just trying to gauge the feature-completeness of it :) ).

Yeah, my only intent is to simplify my CI and remove the need for a full simulator.

@baloo
Copy link
Contributor Author

baloo commented Sep 14, 2024

Can you really run the tests in parallel like that? Is there some kind of built in resource manager in that lib? Otherwise you need to force to just run the tests sequentially.

I have to force it to run on a single thread.

Copy link
Collaborator

@wiktor-k wiktor-k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you really run the tests in parallel like that? Is there some kind of built in resource manager in that lib? Otherwise you need to force to just run the tests sequentially.

D'oh, right! 🤦 with -- --test-threads=1 I get the same exact failures as @baloo.

So, I'm happy to see this merged.

The failures indicate which parameter, handle or session looks bad but I guess digging further into that would consume too much time while this change is already providing benefits. Let's get this in and if we have someone interested in libtpms and/or correctness of these failing functions we can always revisit.

@Superhepper Superhepper merged commit 1d8337c into parallaxsecond:main Sep 16, 2024
12 checks passed
@baloo baloo deleted the baloo/libtpms-backend branch September 16, 2024 16:26
@wiktor-k
Copy link
Collaborator

D'oh, right! 🤦 with -- --test-threads=1 I get the same exact failures as @baloo.

Just for completeness, I've got in touch with libtpms' @stefanberger and he ran the tests to completion which altered me to the remark that @Superhepper mentioned previously.

Running the tests with tpm2-abrmd --logger=stdout --tcti=libtpms: --session --flush-all in one window and command suite done like that: TEST_TCTI=tabrmd:bus_type=session RUST_BACKTRACE=1 RUST_LOG=info cargo test --features "generate-bindings integration-tests serde" -- --test-threads=1 --nocapture gives me a 100% success rate.

Maybe it'd be a good idea to run the test suite with libtmps too, since that seems to be relatively easy now :)

@stefanberger
Copy link

FYI: TPMLIB_Process is not thread-safe since the TPM itself is never accessed by multiple threads. libtpms assumes that there's a TPM interface, like either one of the TPM CRB, TIS, or SPAPR interfaces, on top of it that strictly send one request , then wait for the response, and then only send the next request. Any locking to prevent concurrent accesses would have to be done on higher layers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants