Use TPM_PrintFourLimit in those cases where less than 4 bytes of
valid data may exist. Hashes, nonces, encrypted data, and others
typically have more than 4 bytes but data read from NVRAM or
to be encrypted data may have less.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Sanitize some of the values read from the TPM state stream.
All Coverity discoveries seem to be false positives.
Coverity doesn't like to see array_size being used in the loop even
though it was compared against ARRAY_SIZE() before. We solve this by
using ARRAY_SIZE() as the loop limit now rather than array size.
Compare seed.b.size against PRIMARY_SEED_SIZE even though this is
already being done in TPM2B_Unmarshal().
The num_bytes parameter is sanitized via a comparison involving a
sum over a sum of values, but Coverity doesn't seem to detect this.
Then we have to use it as a loop limit. I don't see another way.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Prevent a potential buffer overrun by checking that EVP_DecryptUpdate()
has not overrun the buffer it was passed in, so this overrun should
never occurr unless EVP_DecryptUpdate() was wrong. Also the pAssert above
it should have taken care of it already.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Coverity complains that the *output* variable passed to
AES_set_encrypt_key contains uninitialized bytes, so we initialize
the variables now.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Coverity complains that nrh may not be initialize when copying nrh.size
from it into the buffer pointer to by nrhp. So resolve this by clearing
nrh at the beginning of the loop and checking 'rc' after the Unmarshal.
Previously we could have copied an uninitialized nrh.size but would have
propagated the rc error code from UINT32_Unmarshal(), so this fix doesn't
really change anything.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
This patch adds a description to the buggy RSAAdjustPrimeCandidate
implementation that can only be properly fixed after 0.7.x.
Reported-by: Nicolas Iooss <nicolas.iooss@ledger.fr>
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
This patch ensures that the leading zeros in the b parameter for NIST P521
are being kept so that HLK accepts the returned parameters from
TPM2_ECC_Parameters. Now 66 bytes are reported for 'b' rather than only 65.
Do the same for the 'a' parameter, though that one was properly reported
already because it didn't have any leading zeros.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
This patch addresses issue #177 by fixing some typos and error
reporting inconsistencies (how structures are spelled) in NVMarhsal.c.
Reported-by: Nicolas Iooss <nicolas.iooss@ledger.fr>
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
cppcheck has detected the following issues in 2 functions. However,
neither one of the out-of-bounds array access can happen with the
existing code (see comments in patch).
src/tpm2/Session.c:399:5: note: After for loop, slotIndex has value 3
for(slotIndex = 0; slotIndex < MAX_LOADED_SESSIONS; slotIndex++)
^
src/tpm2/Session.c:414:15: note: Assuming condition is false
if(result != TPM_RC_SUCCESS)
^
src/tpm2/Session.c:419:15: note: Array index out of bounds
s_sessions[slotIndex].occupied = TRUE;
^
src/tpm2/Session.c:591:27: error: Array 's_sessions[3]' accessed at index 3, which is out of bounds. [arrayIndexOutOfBounds]
MemoryCopy(&s_sessions[slotIndex].session, session, sizeof(SESSION));
^
src/tpm2/Session.c:571:5: note: After for loop, slotIndex has value 3
for(slotIndex = 0; slotIndex < MAX_LOADED_SESSIONS; slotIndex++)
^
src/tpm2/Session.c:581:8: note: Assuming condition is false
&& contextIndex != s_oldestSavedSession)
^
src/tpm2/Session.c:591:27: note: Array index out of bounds
MemoryCopy(&s_sessions[slotIndex].session, session, sizeof(SESSION));
^
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
UBSAN detects possibly misaligned address when reading out of the
TPM 2's NVRAM and when writing back into it. The NV_RAM_HEADER may
be unaligned like this:
tests/test_tpm2_save_load_state_3.log:tpm2/Marshal.c:117:29: \
runtime error: load of misaligned address 0x7ffcb53b3bca for type 'UINT32', which requires 4 byte alignment
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Make sure that the value of bnK is not short so that the subsequent
BnEccModMult() runs in constant time. We take the same approach as with
the modifications to BnEccGenerateKeyPair() where we request bnK to have
all bytes set (no leading zeros that will be cut away) in case the order
of the curve is as byte boundary. In the other cases we add the order
to bnK, which creates bnK1, which we then use for BnEccModMult's scalar
parameter.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Suggested-by: Charanjit Jutla <csjutla@us.ibm.com>
Reviewed-by: Charanjit Jutla <csjutla@us.ibm.com>
Tested-by: Stefan Berger <stefanb@linux.ibm.com>
To avoid a potential side channel in the EcSchnorr signing algorithm,
enforce that the OpenSSL-generated bnD does not have leading zeros
that may then cause a timing side channel in the BnEccModMult() operation.
We modified BnEccGenerateKeyPair() so it calls BnEccModMult with a scalar
of constant number of bytes (for a particular curve):
In this version of BnEccGenerateKeyPair we take a dual approach to constant
time requirements: For curves whose order is at the byte boundary, e.g.
NIST P224/P256/P384, we make sure that bnD has all bytes set (no leading zeros)
so that OpenSSL BIGNUM code will not reduce the number of bytes and the
subsequent BnEccModMult() would run faster for a shoter value. For all other
curves whose order is not at the byte boundary, e.g. NIST P521, we simply
always add the order to bnD and call BnEccModMult() with the result bnD1,
which leads to the same result.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Suggested-by: Charanjit Jutla <csjutla@us.ibm.com>
Reviewed-by: Charanjit Jutla <csjutla@us.ibm.com>
Tested-by: Stefan Berger <stefanb@linux.ibm.com>
Run some additional IBM TSS2 related tests for better code
coverage. We need to switch to Bionic to get the tss2 package.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
This patch fixes the following compilation error on Fedora 32 / s390x:
tpm2/Marshal.c: In function 'TPM2B_CREATION_DATA_Marshal':
tpm2/Marshal.c:95:19: error: 'sizePtr' may be used uninitialized in this function [-Werror=maybe-uninitialized]
95 | (*buffer)[0] = (BYTE)((*source >> 8) & 0xff);
| ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tpm2/Marshal.c:2201:11: note: 'sizePtr' was declared here
2201 | BYTE *sizePtr;
| ^~~~~~~
The error is a false positive since sizePtr will have been initialized if
UINT16_Marshal() is called.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
tpm12/tpm_nvram.c: In function 'TPM_Process_NVWriteValue':
tpm12/tpm_nvram.c:2313:45: error: 'd1NvdataSensitive' may be used uninitialized in this function [-Werror=maybe-uninitialized]
2313 | if ((d1NvdataSensitive->pubInfo.permission.attributes & TPM_NV_PER_WRITEALL) &&
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
This compiler error is a false positive since the above statement is inside
this if clause:
if ((returnCode == TPM_SUCCESS) && !done && !dir) {
However, if d1NvdataSensitive was not set then returnCode is
either != TPM_SUCCESS OR
- case index0 = FALSE : dir = TRUE per line 2106 OR
- case index0 = TRUE (nvIndex = 0): done = TRUE per line 2215.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Fix PCRBelongsTCBGroup by adjusting the set of PCRs that belong to the TCB
Group. The effect of this is that PCR changes to PCR 16 (for example) do
not change the pcrUpdateCounter anymore. The effect *should not* have any
negative side effects when using the TPM.
We also need to update the test cases that now show a different
pcrUpdateCounter in the responses. Also 'swtpm' test cases need
to be fixed to expect the changed result.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
In corner cases where the size of the salt and the size of the hash
to sign + 2 exceed the signature size we cannot use the salt length =
hash length but have to resort to using the maximum possible salt
length.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Call EVP_PKEY_CTX_set_rsa_pss_saltlen(ctx, -1) when creating an RSA
signature to set the PSS salt length to the digest length. Without
this call we previously set the salt length to the maximum
permissible value, but this is not how TPM 2 implements it.
Per interoperability testing between signatures created previously
with the max. permissible value and the new code, which does not
modify the signature verification code, old signatures still verify.
New signatures also verify.
This patch may solve interoperability with hardware TPMs that signatures
created following this patch now verify on hardware TPMs as well.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Always use a temporary buffer large enough to meet the requirements of the
EVP_DecryptUpdate() call.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Add missing call of EVP_CIPHER_CTX_set_padding(ctx, 0) in the symmetric
decryption case. This was missing and failed some decryption cases.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Add a missing input size check for CryptSymmetricDecrypt so that we return
the proper error code TPM_RC_SIZE in case the input size is not a multiple
of the block size. Before TPM_RC_FAILURE was returned.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
For the RSA decryption we have to use an output buffer of the size of the
(largest possible) RSA key for the decryption to always work.
This fixes a stack corruption bug that caused a SIGBUS and termination of
'swtpm'.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
