Add support to store FDO Credentials in the TPM #261
Conversation
shrikant1407
force-pushed
the
tpm-nv-store
branch
4 times, most recently
from
b73a8c7 to
906307b
Compare
…NV storage Signed-off-by: Shrikant Temburwar <shrikant.temburwar@intel.com>
shrikant1407
force-pushed
the
tpm-nv-store
branch
from
906307b to
3002825
Compare
…orage Signed-off-by: Shrikant Temburwar <shrikant.temburwar@intel.com>
shrikant1407
force-pushed
the
tpm-nv-store
branch
from
c8deb8c to
c3ab532
Compare
Signed-off-by: Shrikant Temburwar <shrikant.temburwar@intel.com>
shrikant1407
changed the title
Signed-off-by: Shrikant Temburwar <shrikant.temburwar@intel.com>
shrikant1407
changed the title
shrikant1407
force-pushed
the
tpm-nv-store
branch
from
b0ff1ab to
1b92b86
Compare
Signed-off-by: Shrikant Temburwar <shrikant.temburwar@intel.com>
Signed-off-by: Shrikant Temburwar <shrikant.temburwar@intel.com>
Signed-off-by: Shrikant Temburwar <shrikant.temburwar@intel.com>
- Add TPM2_NV_WriteLock/TPM2_NV_ReadLock support - Update readme for FDO TPM usage Signed-off-by: Shrikant Temburwar <shrikant.temburwar@intel.com>
…r writes Signed-off-by: Shrikant Temburwar <shrikant.temburwar@intel.com>
| if (BN_bn2bin(s, sig_s) <= 0) { | ||
| LOG(LOG_ERROR, "Sig s conversion Failed\n"); | ||
| goto error; | ||
| } | ||
|
|
||
| *signature_length = sig_r_len + sig_s_len; |
There was a problem hiding this comment.
There is no check whether message_signature has enough space to store a total of sig_r_len + sig_s_len bytes - there is no validation of the buffer size . Maybe something like following check we can have
if (*signature_length > MAX_SIGNATURE_BUFFER_SIZE)
|
|
||
| sealed_data_len = | ||
| PLATFORM_HMAC_SIZE + BLOB_CONTENT_SIZE + n_bytes; | ||
|
|
There was a problem hiding this comment.
Possibility of integer overflow in sealed_data_len whenever you do any arithmetic operation please make sure total length is not crossing integer boundary otherwise integer wrap around can happen. Please all the places
| */ | ||
| write_context_len = | ||
| PLATFORM_HMAC_SIZE + BLOB_CONTENT_SIZE + n_bytes; | ||
|
|
There was a problem hiding this comment.
Possibility of integer overflow in write_context_len whenever you do any arithmetic operation please make sure total length is not crossing integer boundary otherwise integer wrap around can happen. Please all the places
There was a problem hiding this comment.
And also try to make this function more modular. Dont write very big function as it difficult to read/ maintain as well as cyclomatic matrix is very high. Check all other place also
| } | ||
|
|
||
| if (fdo_tpm_nvwrite(write_context, write_context_len, nv)) { | ||
| LOG(LOG_ERROR, "Failed to write in TPM NV!\n"); |
There was a problem hiding this comment.
fdo_tpm_nvwrite operation has no check on returned length and no guarantee that full write has been performed without error
| #else | ||
| size_t dev_cred_len = | ||
| fdo_blob_size((char *)FDO_CRED_NORMAL, FDO_SDK_NORMAL_DATA); | ||
| #endif | ||
| // Device has not yet been initialized. | ||
| // Since, Normal.blob is empty, the file size will be 0 | ||
| if (dev_cred_len == 0) { | ||
| LOG(LOG_DEBUG, | ||
| LOG(LOG_INFO, | ||
| "DeviceCredential is empty. Set state to run DI\n"); |
There was a problem hiding this comment.
Why this is change to LOG_INFO as this might reveal extra info.. Please make sure if you are dealing with sensitive info like key, password please put log at LOG_DEBUG level. Check other places also
Add support to store FDO Credentials in the TPM NV storage as per specs