Repository navigation
Backport a few patches for v0.10.3 - #613
stefanberger wants to merge 9 commits into
Conversation
Cast size_t i to ssize_t i to check for underrun. An underrun could never occur since it is not possible that all commands are disabled. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
When rolling back after an unsuccessful switch to the new attributes/algorithms/commands profile, use the maximum possible StateFormatLevel value to set the old profile. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Rather than asserting, display an error log message in case an ANY_OBJECT could not be unmarshalled. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Check the untrusted array_size of AuditCommandList and PPList against a reasonable maximum value of 1024. Artificially setting BYTE buf[0xffff], which would be the maximum value array_size could get with the unmarshalling, does not cause any issues (crash due to large on-stack buffer) on my Linux system. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Limit the size of a profile to 32kb. There is no reason that a valid profile would have even close to 32kb. This also prevents issues in String_Marshal() that assumes a reasonably sized string below 64kb. Place the check for the size of the profile into GetParametersFromJSON since this function is used by RuntimeProfileSet and RuntimeProfileTest. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 44.44% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 7 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
-
Check name Status Explanation Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled. Title check ✅ Passed The title accurately states the main change: backporting multiple patches for v0.10.3. It is concise and relevant to the six changes described. Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request. Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
-
📝 Generate docstrings 💡
-
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
-
- Create PR with unit tests
- Commit unit tests in branch
stable-0.10.next
-
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Check for rc == TPM_RC_SUCCESS before calling function to check whether the EC algorithm is allowed by the profile. This is necessary since rc could already have been set to TPM_RC_CURVE before. In case the function call failed, it would again be set to TPM_RC_CURVE, which is unnecessary. Also do not call the function when TPM_ECC_NONE was unmarshalled since this would always fail and unmarshalling TPM_ECC_NONE would never work. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
1467875 to
5668fde
Compare
8d14511 to
5668fde
Compare
The attributes of an OBJECT were previously at a fixed offset from the beginning of an OBJECT in the NVRAM byte array. However, since ANY_OBJECT_Marshal is now being used to write an OBJECT into the NVRAM, the offset of the OBJECT attributes may be different depending on the current StateFormatLevel and the layout of the OBJECT. Therefore, ANY_OBJECT_Unmarshal has to be used to get the attributes of an OBJECT. Fixes: #626 Reported-by: Kun Qin <kuqin12@gmail.com> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
…rypt
TPM_SymmetricKeyData_Decrypt (AES path in src/tpm12/tpm_crypto.c,
OpenSSL backend; and the NSS/FreeBL backend in
src/tpm12/tpm_crypto_freebl.c) only checked
encrypt_length < TPM_AES_BLOCK_SIZE before calling
AES_cbc_encrypt(..., AES_DECRYPT) on a buffer allocated to exactly
encrypt_length bytes. CBC decryption always consumes a whole final
block, so a length that is not a multiple of 16 makes OpenSSL read up
to 15 bytes past the end of the input buffer.
That length is TPM_CONTEXT_BLOB.sensitiveData.size from the command
stream. TPM_Process_LoadKeyContext and TPM_Process_LoadAuthContext run
under TPM_CHECK_ALLOW_NO_OWNER and decrypt the blob with the permanent
contextKey before the integrity HMAC is checked, so the over-read is
reachable with a single unauthenticated command.
Reproduced with a TPM_LoadKeyContext command whose sensitiveData.size
is 17; AddressSanitizer reports a heap over-read:
READ of size 4 ...
#0 AES_decrypt aes_core.c
#1 CRYPTO_cbc128_decrypt cbc128.c
#2 TPM_SymmetricKeyData_Decrypt tpm_crypto.c
#3 TPM_Process_LoadKeyContext tpm_session.c
The fix rejects a length that is not a whole number of blocks, the same
requirement the DES backend already enforces. The command then returns
TPM_DECRYPT_ERROR. A zero length is still rejected by TPM_Malloc().
Signed-off-by: sameera <sameera@research.itecz.au>
Backport a few patches to v0.10.3.
Summary by CodeRabbit