NO_LONGJMP doesn't work - TpmFail() and _plat__Fail() do not use TPM_FAIL_RETURN as return attribute

[vsukhoml]

While GpMacros.h defines TPM_FAIL_RETURN as correct type to be used for NO_LONGJMP implementation (like embedded), both TpmFail() and _plat__Fail() aren't using this macro and define NORETURN void causing compilation errors.

[vsukhoml]

It also seems definition of TPM_FAIL_RETURN should be moved to TpmBuildSwitches.h to be shared with Platform.h and as a result consitent.

[bradlitterell]

Agree - NO_LONGJMP builds are currently broken in this version of the code. I suspect it is actually worse. An earlier (1.38) version of the code was not consistent about inspecting return values and correctly percolating failures up the stack, e.g. by pAssert. Presumably any failures turn up on the next ExecuteCommand that should detect the earlier call to TpmFail, but I know that (at least some variants of) 1.38 wasn't consistent about it. I don't know that 1.59+ is any better because I don't think the NO_LONGJMP configuration has been verified in a long time.

That's not to say it shouldn't work - I believe NO_LONGJMP should be supported.

[DemiMarie]

Why use NO_LONGJMP? setjmp and longjmp are C standard library functions that require no OS support.

[vsukhoml]

On embedded platforms you may not have C standard library fully implemented or prefer to avoid setjmp or longjmp for other reasons like safety, nuances of environment, etc.

[DemiMarie]

NO_LONGJMP is less safe. It means that execution will continue after the TPM is in an invalid state. The results of this are unpredictable and possibly insecure. Much better to longjmp() back to the command loop.

[vsukhoml]

I think TPM will benefit from avoiding functions returning void which can call TpmFail() or calling TpmFail() instead of returning TPM_RC_FAILURE , and do error escalation properly, so no need to longjmp(). longjmp() causes hidden control flow as you need to know what nested functions are doing while reviewing higher level.

Change can start with:

#ifndef NO_LONGJMP
#   define FAIL_RETURN(returnCode)
#   define TPM_FAIL_RETURN     NORETURN void
#if !FAIL_TRACE
#   define FAIL(errorCode) (TpmFail(errorCode))
#   define LOG_FAILURE(errorCode) (TpmLogFailure(errorCode))
#else
#   define FAIL(errorCode)        TpmFail(FUNCTION_NAME, __LINE__, errorCode)
#   define LOG_FAILURE(errorCode) TpmLogFailure(FUNCTION_NAME, __LINE__, errorCode)
#endif

#else /* NO_LONGJMP set */
#   define FAIL_RETURN(returnCode) return (returnCode)
#   define TPM_FAIL_RETURN     TPM_RC
#if !FAIL_TRACE
#   define FAIL(errorCode) return TpmFail(errorCode)
#   define LOG_FAILURE(errorCode)  return TpmLogFailure(errorCode)
#else
#   define FAIL(errorCode)        return TpmFail(FUNCTION_NAME, __LINE__, errorCode)
#   define LOG_FAILURE(errorCode)  return TpmLogFailure(FUNCTION_NAME, __LINE__, errorCode)
#endif

#endif

And adjusting prototypes and use of macros accordingly.

To be clear in the use case I have TPM2 is called from Rust code (whole platform and crypto adaptation is written in Rust), so I can't just use setjmp and longjmp without extra wrappers.

[DemiMarie]

To be clear in the use case I have TPM2 is called from Rust code (whole platform and crypto adaptation is written in Rust), so I can't just use setjmp and longjmp without extra wrappers.

Have you considered writing the whole thing in Rust? The TPM2 codebase has had at least one vulnerability due to memory unsafety.

[vsukhoml]

Have you considered writing the whole thing in Rust? The TPM2 codebase has had at least one vulnerability due to memory unsafety.

Yes, considered, but it is a heavy lifting for the whole functionality which was hard to justify for specific project - I'd rather prefer this to be done under umbrella of TCG. It also requires a comprehensive set of tests covering large set of corner cases, crypto, etc. TPM2 specification is not a formal specification (not written in formal language) and reference code is a kind of formal specification to test against. Another aspect - not many Rust developers, toolchain issues for embedded platforms.

[DemiMarie]

Personally, I would prefer to see an implementation proven correct in Coq, Isabelle, or another formal verification tool.

[vsukhoml]

Probably ChatGPT can be trained to serve as a translator from English to some formal verification tool to prove consistency, completeness and correctness 😇 But still, it is a request to TCG. So far this reference code as part of TPM spec is most formal specification of how TPM shall work even though there are known and unknown issues.