tpm2-software / tpm2-abrmd

TPM2 Access Broker & Resource Management Daemon implementing the TCG spec.
https://github.com/tpm2-software/tpm2-abrmd
BSD 2-Clause "Simplified" License
115 stars 98 forks source link

resource-manager: bypass command_special_processing when session is p… #819

Closed whooo closed 1 year ago

whooo commented 1 year ago

…rovided

TPM2_GetCapability accepts an audit session, when specified no special processing should be done. If a session without the audit flag set is passed let the TPM handle the error.

Fixes https://github.com/tpm2-software/tpm2-abrmd/issues/818

codecov[bot] commented 1 year ago

Codecov Report

Merging #819 (853d676) into master (1a5d326) will increase coverage by 1.53%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #819      +/-   ##
==========================================
+ Coverage   79.63%   81.17%   +1.53%     
==========================================
  Files          32       32              
  Lines        3722     3723       +1     
==========================================
+ Hits         2964     3022      +58     
+ Misses        758      701      -57     
Impacted Files Coverage Δ
src/resource-manager.c 81.57% <100.00%> (+3.84%) :arrow_up:
src/tpm2-command.c 69.23% <0.00%> (+4.85%) :arrow_up:
src/resource-manager-session.c 50.00% <0.00%> (+17.77%) :arrow_up:

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

williamcroberts commented 1 year ago

@whooo we should add a test that does that code snippet to ensure we don't break anything?

whooo commented 1 year ago

@whooo we should add a test that does that code snippet to ensure we don't break anything?

Do you have any flow in mind? Only thing I can think of is doing a unit test for command_special_processing and passing a Tpm2Command with a session

williamcroberts commented 1 year ago

@whooo we should add a test that does that code snippet to ensure we don't break anything?

Do you have any flow in mind? Only thing I can think of is doing a unit test for command_special_processing and passing a Tpm2Command with a session

I was thinking an integration test where we start a session and add the handle, but I am OK with a unit test as well.

whooo commented 1 year ago

@whooo we should add a test that does that code snippet to ensure we don't break anything?

Do you have any flow in mind? Only thing I can think of is doing a unit test for command_special_processing and passing a Tpm2Command with a session

I was thinking an integration test where we start a session and add the handle, but I am OK with a unit test as well.

I'll look into creating a integration test which test with an audit session and one which tests with a password session, just need to get familiar with SAPI again

williamcroberts commented 1 year ago

@whooo we should add a test that does that code snippet to ensure we don't break anything?

Do you have any flow in mind? Only thing I can think of is doing a unit test for command_special_processing and passing a Tpm2Command with a session

I was thinking an integration test where we start a session and add the handle, but I am OK with a unit test as well.

I'll look into creating a integration test which test with an audit session and one which tests with a password session, just need to get familiar with SAPI again

Their is start_auth_session in common code for testing. It sets up an encrypted session if you need some reference code.

whooo commented 1 year ago

Simple integration test added, I only added checks for failure with a password session and the success with an audit session, but don't verify the audit session