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>
Make --with-tpm2 the implicit default now and choosen openssl.
When using --without-tpm2 one has to again choose the crypto-library
which defaults to freebl as before. This type of build seems rather
rare by now.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
A tpm12 function that is only needed with freebl library can
be conditionally enabled with '#if USE_FREEBL_CRYPTO_LIBRARY'.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
The freebl build (TPM 1.2 only) is currently broken:
configure: error: OpenSSL crypto function usage requires openssl as crypto library
Set 'enable_use_openssl_functions=no' in the freebl case to avoid probing
the OpenSSL crypto functions.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Use AC_LINK_IFELSE to check whether linker flags like 'now' and
'relro' are understood or remain unused (= useless), and only add them
to the set of HARDENING_LDFLAGS, if they are used by the linker. clang
for example does not seem to use them and Cygwin's linker does not
understand them.
Note: This patch merely improves on the handling of these flags but does
not solve a compilation issue when clang is used, unlike swtpm where
this created issues.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
The JSON returned by TPM2_GetInfo contains a leading zero in the level.
$> swtpm_ioctl --tcp :10000 --info 1
{"TPMSpecification":{"family":"2.0","level":00,"revision":162+0}}
This patch fixes this to:
$> swtpm_ioctl --tcp :10000 --info 1
{"TPMSpecification":{"family":"2.0","level":0,"revision":162+0}}
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>
This does not affect the proper functioning of the code since all
of the commands at the end of the array are currently disabled.
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>
Sync up on the #define's for HASH_ALIGNMENT, which does not have much
relevance for the OpenSSL implementation.
The affected 32 or 64 bit align field in the ANY_HASH_STATE doesn't carry
any significance. It can be commented without side effects.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Remove CURVE_NAME_DEF field from ECC_CURVE structure and add
#define CURVE_NAME(N)
so that nothing misses the removed field, which wasn't used
before, either.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>