RFC: Fapi preview fw and ima logging cel

[JuergenReppSIT]

FAPI: Add event logging for firmware and IMA events.

Add serialization and deserialization for firmware events:

• Serialization and deserialization of firmware events specified in the TCG PC Client Platform Firmware Profile was added.
• Also for the firmare legacy format (sha1) the serialization and deserialization was added.

Add serialization and deserialization for IMA events:

• Serialization of IMA events as described on: https://sourceforge.net/p/linux-ima/wiki/Home/ was added. The templates ima, img-sig, ima-ng, and ima-sig are supported.
• Tests in fapi-jons.c were adapted.

Integrate firmware and IMA eventlog:

• The IMA event logging was integrated.
• The PC-Client firmware logging was integrated.

CEL Events:

• The cel_version event was added as first event for the event log of PCR 0.
• The firmware_end event was added behind the last firmware event.
• Serialization and deserialization of CEL events was added.

Event logging:

• For invalid digests an error will only produced for quote verification. In other cases only a warning will be displayed.
• rename "type" to "content_type".
• rename "sub_event" to "content".

Tests for event logging for firmware and IMA events:

• An Unit test for several binary examples was added.
• Tests in fapi-jons.c were adapted.
• Copied functions were removed from unit test fapi-json.c
• An Integration test for The PC-Client firmware logging was added.
• An Integration test for The PC-Client firmware logging in the legacy format was added.

Signed-off-by: Juergen Repp [email protected]

[lgtm-com[bot]]

This pull request introduces 2 alerts when merging e77b7f9c3f94b6153b5b8a4f72a91d81c710bc7b into 5902fbabe9dc5f24473756baec08377ebe52e765 - view on LGTM.com

new alerts:

• 1 for Suspicious add with sizeof
• 1 for Comparison result is always the same

[lgtm-com[bot]]

This pull request introduces 2 alerts when merging 9e13b2f1da70ad9e5890cfc5cecba4ac5a19bc44 into 5902fbabe9dc5f24473756baec08377ebe52e765 - view on LGTM.com

new alerts:

• 1 for Suspicious add with sizeof
• 1 for Comparison result is always the same

[lgtm-com[bot]]

This pull request introduces 2 alerts when merging 9ef82b6170e797dce6f3d3bf37d9e5e4b1f21fa8 into 5902fbabe9dc5f24473756baec08377ebe52e765 - view on LGTM.com

new alerts:

• 1 for Suspicious add with sizeof
• 1 for Comparison result is always the same

[lgtm-com[bot]]

This pull request introduces 2 alerts when merging ddb1768c6e16a79a330a245ca73db868d04d7ed5 into 5902fbabe9dc5f24473756baec08377ebe52e765 - view on LGTM.com

new alerts:

• 1 for Suspicious add with sizeof
• 1 for Comparison result is always the same

[lgtm-com[bot]]

This pull request introduces 2 alerts when merging 476bd6e4e0343580a2773dc2aacbe6815fe953d1 into 5902fbabe9dc5f24473756baec08377ebe52e765 - view on LGTM.com

new alerts:

• 1 for Suspicious add with sizeof
• 1 for Comparison result is always the same

[lgtm-com[bot]]

This pull request introduces 2 alerts when merging dc12cb67cf7c547eb21c1c5ca9a5dd46f3ea2396 into 5902fbabe9dc5f24473756baec08377ebe52e765 - view on LGTM.com

new alerts:

• 1 for Suspicious add with sizeof
• 1 for Comparison result is always the same

[lgtm-com[bot]]

This pull request introduces 2 alerts when merging 1bb8dec4e9be4a7330a1ad419452a85d05d44f7b into 5902fbabe9dc5f24473756baec08377ebe52e765 - view on LGTM.com

new alerts:

• 1 for Suspicious add with sizeof
• 1 for Comparison result is always the same

[lgtm-com[bot]]

This pull request introduces 2 alerts when merging 2fa0d3f5ab55ec657765e181b66488e27b0e16e1 into 5902fbabe9dc5f24473756baec08377ebe52e765 - view on LGTM.com

new alerts:

• 1 for Suspicious add with sizeof
• 1 for Comparison result is always the same

[lgtm-com[bot]]

This pull request introduces 2 alerts when merging 81fe356e94a2d75a678be93e1d0dbebe5d1cd42e into 5902fbabe9dc5f24473756baec08377ebe52e765 - view on LGTM.com

new alerts:

• 1 for Suspicious add with sizeof
• 1 for Comparison result is always the same

[lgtm-com[bot]]

This pull request introduces 2 alerts when merging ba755a897fa48b2bfc5103e46af4a0dde8f8c974 into 5902fbabe9dc5f24473756baec08377ebe52e765 - view on LGTM.com

new alerts:

• 1 for Suspicious add with sizeof
• 1 for Comparison result is always the same

[JuergenReppSIT]

i'd like to see fuzz tests to catch issues with malicious log data as inputs. @williamcroberts The afl fuzzer revealed some bugs in the IMA part. I will test also the system event part and try to add the afl fuzzing to the ci.

[AndreasFuchsTPM]

@williamcroberts @JuergenReppSIT Let's have the discussion here: Should we roll a new major release (4.0) because we change the CEL format or do we consider the old format was still beta anyways and not much used and just roll 3.3 after this one ?

[JuergenReppSIT]

@williamcroberts @JuergenReppSIT Let's have the discussion here: Should we roll a new major release (4.0) because we change the CEL format or do we consider the old format was still beta anyways and not much used and just roll 3.3 after this one ?

@williamcroberts @AndreasFuchsTPM I think the new CEL format can be part of 3.3 because the old format was beta without IMA and without system events. An old log file could be converted to the new format with a simple sed script if really needed.

[williamcroberts]

@williamcroberts @JuergenReppSIT Let's have the discussion here: Should we roll a new major release (4.0) because we change the CEL format or do we consider the old format was still beta anyways and not much used and just roll 3.3 after this one ?

@williamcroberts @AndreasFuchsTPM I think the new CEL format can be part of 3.3 because the old format was beta without IMA and without system events. An old log file could be converted to the new format with a simple sed script if really needed.

Yeah this was what @JuergenReppSIT and I discussed before and we landed on this approach. If we can prevent a major release I am for it.

[JuergenReppSIT]

@williamcroberts @AndreasFuchsTPM AFL fuzzing has now been running for more than 5 days on 20 CPUs without finding more errors. So could the PR now be merged for 3.3? @williamcroberts I could not get into the maintainer sync today. Was it canceled?

[codecov[bot]]

Codecov Report

Merging #2170 (aeccd71) into master (da05fa4) will decrease coverage by 0.26%. The diff coverage is 75.74%.

@@            Coverage Diff             @@
##           master    #2170      +/-   ##
==========================================
- Coverage   83.68%   83.41%   -0.27%     
==========================================
  Files         351      354       +3     
  Lines       37975    39287    +1312     
==========================================
+ Hits        31779    32771     +992     
- Misses       6196     6516     +320     

Impacted Files	Coverage Δ
src/tss2-fapi/tpm_json_deserialize.c	85.92% <ø> (ø)
src/tss2-fapi/tpm_json_serialize.c	89.13% <ø> (+0.14%)	:arrow_up:
src/tss2-fapi/ifapi_json_serialize.c	87.05% <67.12%> (-4.60%)	:arrow_down:
src/tss2-fapi/ifapi_json_eventlog_serialize.c	68.84% <68.84%> (ø)
src/tss2-fapi/ifapi_json_deserialize.c	81.54% <73.72%> (-1.79%)	:arrow_down:
src/tss2-fapi/ifapi_eventlog_system.c	74.22% <74.22%> (ø)
src/tss2-fapi/api/Fapi_NvExtend.c	89.47% <83.33%> (ø)
src/tss2-fapi/ifapi_ima_eventlog.c	84.33% <84.33%> (ø)
src/tss2-fapi/ifapi_helpers.c	85.06% <85.00%> (+0.06%)	:arrow_up:
src/tss2-fapi/ifapi_eventlog.c	86.15% <86.76%> (+0.22%)	:arrow_up:
... and 9 more

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[williamcroberts]

@JuergenReppSIT

• [x] Make sure all added, non-generated, files have the BSD-2-Clause SPDX identifier
• [x] for the bin files, could we make a build rule that generates them from b64 encoded to avoid sticking a bunch of binary blobs in the repo?
• [x] Fix the conflict in Makefile.am

Then I think we can merge this, right?

[JuergenReppSIT]

@JuergenReppSIT

* [x]  Make sure all added, non-generated, files have the BSD-2-Clause SPDX identifier

I have added a new commit to the PR which adds the SPDX identifier to all files without one or a wrong identifier.

* [ ]  for the bin files, could we make a build rule that generates them from b64 encoded to avoid sticking a bunch of binary blobs in the repo?

Is this really necessary? The tpm-tools also store binaries under test/integration/fixtures

* [x]  Fix the conflict in Makefile.am

Then I think we can merge this, right? I would say yes.

[williamcroberts]

* [ ]  for the bin files, could we make a build rule that generates them from b64 encoded to avoid sticking a bunch of binary blobs in the repo?

Is this really necessary? The tpm-tools also store binaries under test/integration/fixtures

We should never be stuffing binary files into git. If we have to frequently update the files it can/may consume a lot of space in the .git db. I guess in this scenario it's unlikely, but I think its best to avoid any issue. We can then just have the Makefile-test.am include them as a dependency and build them using b64decode.

[JuergenReppSIT]

We should never be stuffing binary files into git. If we have to frequently update the files it can/may consume a lot of space in the .git db. I guess in this scenario it's unlikely, but I think its best to avoid any issue. We can then just have the Makefile-test.am include them as a dependency and build them using b64decode.

OK I will add it.

[williamcroberts]

We should never be stuffing binary files into git. If we have to frequently update the files it can/may consume a lot of space in the .git db. I guess in this scenario it's unlikely, but I think its best to avoid any issue. We can then just have the Makefile-test.am include them as a dependency and build them using b64decode.

OK I will add it.

Thanks, I know it's a pain.

[JuergenReppSIT]

Thanks, I know it's a pain.

I think next Friday will be the day of pain for me ;-)

[JuergenReppSIT]

@williamcroberts I have added a commit where the fapi binary test files are created from base64 encoded files.

[codecov[bot]]

Codecov Report

Merging #2170 (7382004) into master (6c88eea) will decrease coverage by 0.51%. The diff coverage is 70.23%.

:exclamation: Current head 7382004 differs from pull request most recent head 88bcdbe. Consider uploading reports for the commit 88bcdbe to get more accurate results

@@            Coverage Diff             @@
##           master    #2170      +/-   ##
==========================================
- Coverage   83.62%   83.11%   -0.52%     
==========================================
  Files         351      355       +4     
  Lines       37959    39643    +1684     
==========================================
+ Hits        31745    32949    +1204     
- Misses       6214     6694     +480     

Impacted Files	Coverage Δ
src/tss2-esys/api/Esys_Commit.c	93.93% <ø> (ø)
src/tss2-esys/api/Esys_EC_Ephemeral.c	93.58% <ø> (ø)
src/tss2-esys/api/Esys_GetCapability.c	96.15% <ø> (ø)
src/tss2-esys/api/Esys_GetTestResult.c	93.58% <ø> (ø)
src/tss2-esys/api/Esys_PCR_Allocate.c	96.10% <ø> (ø)
src/tss2-esys/api/Esys_PCR_Read.c	92.85% <ø> (ø)
src/tss2-esys/esys_iutil.c	91.33% <ø> (ø)
src/tss2-esys/esys_mu.c	91.53% <0.00%> (+2.88%)	:arrow_up:
src/tss2-fapi/fapi_crypto.c	78.80% <ø> (+1.16%)	:arrow_up:
src/tss2-fapi/tpm_json_deserialize.c	85.92% <ø> (ø)
... and 43 more

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more