Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

qualifying data as hex string or binary file and manpage fix #1822

Merged
merged 2 commits into from Oct 30, 2019

Conversation

williamcroberts
Copy link
Member

  • Unify qualifying data like options so the data argument can be a hex string or binary file.
  • Fix the tpm2_nvextend manpage.

Fixes: #1631
Fixes: #1820

}
}

TPM2B_NONCE policy_qualifier = { .size = (uint16_t) file_size };
Copy link
Member Author

Choose a reason for hiding this comment

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

@idesai this code is wrong, and broken. file_size is a long, which can hold a value larger than 16 bits. You could be reading only partial files as you cast it done without checking that it is not larger than UINT16_MAX.

Additionally, this code was over complicated... all you need to do is:

  1. set a size to the max size of the buffer
  2. call files_load_bytes_from_path(), which will read up to max bytes and error out if the file hasn't been fully consumed as well as update the size parameter to the actual size in bytes read.
  3. go on your way using the contents, validate size if your expecting something certain.

return tool_rc_general_error;
}

*policy_qualifier = malloc(file_size + sizeof(uint16_t));
Copy link
Member Author

Choose a reason for hiding this comment

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

@idesai this code is wild.

  1. if you need to malloc a TPM2B_* data type, just malloc based on the sizeof of the type. We're not counting the few bytes it wastes.
  2. Why did you even malloc it? There was no point in complicating the flow with needing to free and dealing with oom. Just stack allocate and pass a pointer, which is funny, as a lot of the other parameter structures worked like that.

@lgtm-com

This comment has been minimized.

@codecov
Copy link

codecov bot commented Oct 29, 2019

Codecov Report

Merging #1822 into master will increase coverage by 0.13%.
The diff coverage is 90%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1822      +/-   ##
==========================================
+ Coverage   74.39%   74.53%   +0.13%     
==========================================
  Files         115      115              
  Lines       11553    11501      -52     
==========================================
- Hits         8595     8572      -23     
+ Misses       2958     2929      -29
Impacted Files Coverage Δ
lib/tpm2_util.h 100% <ø> (ø) ⬆️
tools/tpm2_policyticket.c 65.3% <0%> (ø) ⬆️
tools/tpm2_gettime.c 57.69% <0%> (+2.13%) ⬆️
tools/tpm2_create.c 81.17% <100%> (+1.28%) ⬆️
tools/tpm2_policysigned.c 74.75% <100%> (ø) ⬆️
lib/files.c 69.47% <100%> (ø) ⬆️
tools/tpm2_createprimary.c 75.78% <100%> (+1.53%) ⬆️
tools/tpm2_policysecret.c 72.72% <100%> (ø) ⬆️
tools/tpm2_quote.c 74.6% <100%> (+0.95%) ⬆️
tools/tpm2_certifycreation.c 71.9% <100%> (+1.44%) ⬆️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cc54cb5...1f49ace. Read the comment docs.

William Roberts added 2 commits October 29, 2019 13:51
Unify the -q option by allowing them to be a file or hex array for
inputs.

Fixes: tpm2-software#1631

Signed-off-by: William Roberts <william.c.roberts@intel.com>
tpm2_nvextend was identical to tpm2_nvsetbits due to a copy + paste
error. Correct the manpage by making it reflect the tpm2_nvextend tool.

Fixes: tpm2-software#1820

Signed-off-by: William Roberts <william.c.roberts@intel.com>
@williamcroberts williamcroberts merged commit 9e22ef4 into tpm2-software:master Oct 30, 2019
@williamcroberts williamcroberts deleted the qualifying-data branch November 5, 2019 16:38
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.

nvextend man page is wrong qualifyingdata should be file or byte array
1 participant