[PATCH] tests: add test_response_start_auth_session_no_sessions_2

[PATCH] tests: add test_response_start_auth_session_no_sessions_2

[Jarkko Sakkinen]

Add a test taken from tpm2sh trace output, which reveals a bug in the
response parser not catched by previous tests.

Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
This is a embarrassing critical bug and damage from refactorization
where `tpm_response!` macro was removed which was a compilicated task.
The bright side is that the quality of test suite also leveled up
as consequence and missing something as critical as this will become
factors more difficult.

Luckily response parsing it not too complicated, and this can be fixed
quite easily. Dispatcher must be internally updated to pass the runtime
parsed tag forward (just like it does for cc). I.e. a few macro updates
should do.

I should have a fix for this very soon!
 tests/runner.rs | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/tests/runner.rs b/tests/runner.rs
index b63c801..3d2a1dc 100644
--- a/tests/runner.rs
+++ b/tests/runner.rs
@@ -26,7 +26,7 @@ use tpm2_protocol::{
         TpmCreatePrimaryCommand, TpmEvictControlCommand, TpmFlushContextCommand,
         TpmFlushContextResponse, TpmGetCapabilityCommand, TpmHashCommand, TpmNvWriteCommand,
         TpmPcrEventResponse, TpmPcrReadCommand, TpmPcrReadResponse, TpmPolicyGetDigestResponse,
-        TpmStartAuthSessionCommand, TpmStartAuthSessionResponse,
+        TpmResponseBody, TpmStartAuthSessionCommand, TpmStartAuthSessionResponse,
     },
     TpmBuffer, TpmBuild, TpmErrorKind, TpmParse, TpmPersistent, TpmSession, TpmSized, TpmWriter,
     TPM_MAX_COMMAND_SIZE,
@@ -947,6 +947,26 @@ fn test_response_start_auth_session_no_sessions() {
     assert_eq!(resp, original_resp);
 }
 
+fn test_response_start_auth_session_no_sessions_2() {
+    let response_hex = "8001000000300000000002000000002000647915de6106c955b26456b8b8a3b10546fa446405d4eb2e1fb0247fb52080";
+    let response_bytes = hex_to_bytes(response_hex).unwrap();
+    let (rc, body, sessions) = tpm_parse_response(TpmCc::StartAuthSession, &response_bytes)
+        .unwrap()
+        .unwrap();
+    let mut built_bytes = [0; TPM_MAX_COMMAND_SIZE];
+    let len = {
+        let mut writer = TpmWriter::new(&mut built_bytes);
+        match body {
+            TpmResponseBody::StartAuthSession(ref resp_struct) => {
+                tpm_build_response(resp_struct, &sessions, rc, &mut writer).unwrap();
+            }
+            _ => panic!("Parsed the wrong response type!"),
+        }
+        writer.len()
+    };
+    assert_eq!(&built_bytes[..len], &response_bytes);
+}
+
 fn test_tpm2b_build_length_too_large() {
     let large_slice: &[u8] = unsafe {
         std::slice::from_raw_parts(
@@ -1156,6 +1176,7 @@ test_suite!(
     test_response_parse_policy_get_digest,
     test_response_start_auth_session,
     test_response_start_auth_session_no_sessions,
+    test_response_start_auth_session_no_sessions_2,
     test_tpm2b_build_length_too_large,
     test_tpmbuffer_try_from_slice_too_large,
     test_tpm_rc_base_from_raw,
-- 
2.39.5

Re: [PATCH] tests: add test_response_start_auth_session_no_sessions_2

[Jarkko Sakkinen]

On Tue, Sep 02, 2025 at 04:26:20AM +0300, Jarkko Sakkinen wrote:
> Add a test taken from tpm2sh trace output, which reveals a bug in the
> response parser not catched by previous tests.
> 
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
> This is a embarrassing critical bug and damage from refactorization
> where `tpm_response!` macro was removed which was a compilicated task.
> The bright side is that the quality of test suite also leveled up
> as consequence and missing something as critical as this will become
> factors more difficult.
> 
> Luckily response parsing it not too complicated, and this can be fixed
> quite easily. Dispatcher must be internally updated to pass the runtime
> parsed tag forward (just like it does for cc). I.e. a few macro updates
> should do.

What happened here basically was that I was so focused on command
parsing, which felt more complicated (given e.g. how authorization
area is located) that I made some really unconsidered change on
response parser.

It's easy to fix, and in the end better than having this other way
around.

BR, Jarkko