Stefan Berger [Fri, 23 Jul 2021 17:29:00 +0000 (13:29 -0400)]
tpm2: NVMarshal: Handle index orderly RAM without 0-sized terminating node
The NVRAM entries in s_indexOrderlyRam array do not need to contain a
0-sized terminating node. Instead, the entries may fill up this 512
byte array so that no NV_RAM_HEADER structure fits anymore. The fact
that no more NV_RAM_HEADER structure fits is also an indicator for the
last entry. We need to account for this in the code marshalling and
unmarshalling the entries so that we stop marshalling the entries
then and similarly stop unmarshalling.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Stefan Berger [Fri, 23 Jul 2021 01:23:58 +0000 (21:23 -0400)]
tpm2: Initialize a whole OBJECT before using it
Initialize a while OBJECT before using it. This is necessary since
an OBJECT may also be used as a HASH_OBJECT via the ANY_OBJECT
union and that HASH_OBJECT can leave bad size inidicators in TPM2B
buffer in the OBJECT. To get rid of this problem we reset the whole
OBJECT to 0 before using it. This is as if the memory for the
OBJECT was just initialized.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Nick Chevsky [Tue, 20 Jul 2021 19:48:24 +0000 (14:48 -0500)]
Improvements to .gitignore
- Add auto-generated files *.gch, debian/autoreconf.*, tests/NVChip
- Add .pc/ (temporary directory used by Quilt for patch management)
- Remove *.patch (Quilt requires these under debian/patches/)
- Change incorrect debian/libtpms/ to debian/libtpms0/
Signed-off-by: Nick Chevsky <nchevsky@users.noreply.github.com>
Stefan Berger [Fri, 9 Jul 2021 19:08:36 +0000 (15:08 -0400)]
tpm2: Avoid compiler warning by using memcpy instead of MemoryCopy (gcc 10.3)
Fix the following compiler warning from gcc 10.3.0 by using memcpy
instead of MemoryCopy (fixes issue #229).
tpm2/NVDynamic.c: In function 'NvRamGetEnd':
tpm2/NVDynamic.c:378:12: warning: function may return address of local variable [-Wreturn-local-addr]
378 | return iter;
| ^
tpm2/NVDynamic.c:339:26: note: declared here
339 | NV_RAM_HEADER header;
| ^
tpm2/NVDynamic.c: In function 'NvRamGetIndex':
tpm2/NVDynamic.c:411:12: warning: function may return address of local variable [-Wreturn-local-addr]
411 | return currentAddr;
| ^
tpm2/NVDynamic.c:339:26: note: declared here
339 | NV_RAM_HEADER header;
| ^
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Stefan Berger [Thu, 24 Jun 2021 00:54:44 +0000 (20:54 -0400)]
tpm2: Restore original value if unmarsalled value was illegal
Restore the original value of the memory location where data from
a stream was unmarshalled and the unmarshalled value was found to
be illegal. The goal is to not keep illegal values in memory.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Stefan Berger [Mon, 21 Jun 2021 18:04:34 +0000 (14:04 -0400)]
tpm2: Reset TPM2B buffer sizes after test fails for valid buffer size
Reset the buffer size indicator in a TPM2B type of buffer after it failed
the test for the maximum buffer size it allows. This prevents having bad
buffer sizes in memory that can come to haunt us when writing the volatile
state for example.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Stefan Berger [Fri, 28 May 2021 22:32:25 +0000 (18:32 -0400)]
tpm2: Consume padding bytes in TPM2_ContextLoad() (Win2k19, issue #217)
Windows 2019 Server padds the TPM_ContextLoad() command with additional
bytes up to TPM_PT_MAX_OBJECT_CONTEXT for the TPMS_CONTEXT part. Since
libtpms does not use an OBJECT to serialize the keys (anymore) it now
uses less bytes than the MAXimum of TPM_PT_MAX_OBJECT_CONTEXT bytes and
the padding leaves some unconsumed bytes that end up failing the command
since no left-over bytes are allowed in any command.
When unconsumed bytes are left in TPMS_CONTEXT_Unmarshal() we check that
the original passed in size was that of TPM_PT_MAX_OBJECT_CONTEXT and
only then consume the additional padding bytes. Luckily only one command
calls TPMS_CONTEXT_Unmarshal() so that no unwanted side effects should
occur anywhere else, such as no bytes left for unmarshalling the next
structure.
The wisdom behind the padding is not quite clear but it feels like
ill-fixing the code to work around a Windows 2019 server bug...
This patch fixes issed #217
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Stefan Berger [Fri, 28 May 2021 17:37:31 +0000 (13:37 -0400)]
Travis: Install libjson-glib-dev and drop a few python dependencies
Since swtpm_setup has been rewritten in 'C' now we can drop a few
python dependencies but need libjson-glib-dev as a new dependency
for testing with swtpm's master branch.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
build-sys: leave CFLAGS/LDFLAGS for user to be defined
This allows user to set specific flags during compilation, without
overriding configure-time cflags necessary for compilation.
See also:
https://www.gnu.org/software/automake/manual/html_node/User-Variables.html
https://www.gnu.org/software/automake/manual/html_node/Flag-Variables-Ordering.html
Stefan Berger [Fri, 30 Apr 2021 20:44:03 +0000 (16:44 -0400)]
tpm2: Switch to UINT16 for CONTEXT_SLOT and 64k context gap
This patch addresses issue #209.
The context gap for libtpms is currently only 0xff due to the CONTEXT_SLOT
being a UINT8. To extend this to 0xffff, we need to define the CONTEXT_SLOT
as UINT16 and introduce a global variable s_ContextArrayMask that takes on
two valid values, 0xff for simulating the CONTEXT_SLOT when it was UINT8
and 0xffff for usage with the new CONTEXT_SLOT of type UINT16. All
occurrences of casts to CONTEXT_SLOT are replaced with a macro
CONTEXT_SLOT_MASKED that applies this mask to a value instead of using the
cast. We also use it for some calculations to avoid spilling over from
1 byte into 2 bytes for example. The cast with the new code is the same as
applying the mask 0xffff, and using the 0xff mask we can simulate the old
CONTEXT_SLOT (1 byte), which we need for seamlessly resuming old state. We
switch from the 0xff mask to the 0xffff mask when the TPM is reset.
There's one place where the s_ContextArrayMask is initialized to 0xff, and
this is when we resume 'old' STATE_RESET_DATA. The places where it is
intialized to 0xffff are in TPM_Manufacture() and
TPM_SessionStartup(SU_CLEAR), both of which are not called after resuming
state.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Stefan Berger [Fri, 7 May 2021 01:02:17 +0000 (21:02 -0400)]
debian: Fix build warnings by updating compat to '10' and dh functions
This patch fixes the following issue pointed out in issue #212:
dh clean --parallel --with autotools-dev --with autoreconf
dh: warning: Compatibility levels before 10 are deprecated (level 9 in use)
dh: warning: The autotools-dev sequence is deprecated and replaced by dh in debhelper (>= 9.20160115)
dh: warning: This feature will be removed in compat 12.
dh_auto_clean -O--parallel
dh_auto_clean: warning: Compatibility levels before 10 are deprecated (level 9 in use)
make -j4 distclean
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Stefan Berger [Thu, 25 Feb 2021 19:05:28 +0000 (14:05 -0500)]
tpm2: Implement a cache for the private exponent D and prime Q
Implement a cache for the private exponent 'D' and prime 'Q' so that we
do not have to recalculate 'Q' and 'D' every time an RSA key is used. For
a cache hit we now use ~34000 cycles and on a cache miss it needs around
130000 cycles. Previously it needed around 100000 cycles to calcuate 'Q'
and 'D'. Assuming that keys will be reused and the cache is big enough
for the number of keys being use (64 entries), it seems well worth it.
This solution is better than extending the OBJECT with 'D' since OBJECT is
kept in the TPM's NVRAM and we would then need more memory to store OBJECTs
there.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Nicolas Iooss [Tue, 2 Mar 2021 12:51:42 +0000 (13:51 +0100)]
Fix many misspellings
When testing downgrading from libtpms 0.8 to 0.7 (which is not
possible), the error message which is reported is:
libtpms/tpm2: Unexpect value for MAX_RSA_KEY_BITS; its value 3072 is
not = 2048; (version: 2).
codespell (https://github.com/codespell-project/codespell) reports a
misspelling for "Unexpect", which should be "Unexpected". As the project
contains many more misspellings in comments, error messages and
documentation, fix all misspellings reported by codespell.
Signed-off-by: Nicolas Iooss <nicolas.iooss@ledger.fr>
Stefan Berger [Mon, 1 Mar 2021 14:19:02 +0000 (09:19 -0500)]
tpm2: CryptSym: fix AES output IV
The TPM is supposed to provide the output IV in the ivInOut parameter in
CryptSymmetricEncrypt. In the case of using the openssl routines, the
output IV is missed, and the resulting output from the TPM is in the
input IV.
OpenSSL unfortunately does not export EVP_CIPHER_CTX_iv() until
tags/OpenSSL_1_1_0, so we have to fall back to the reference code for
previous OpenSSL versions.
Signed-off-by: William Roberts <william.c.roberts@intel.com> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Stefan Berger [Fri, 26 Feb 2021 14:24:22 +0000 (09:24 -0500)]
tpm2: Fix public key context save due to ANY_OBJECT_Marshal usage
This patch addresses the bug reported in issue #195 where the saving of
an externally loaded public key's context doesn't work due to the usage of
ANY_CONTEXT_SAVE for saving key contexts. This patch fixes the issue by
creating local versions of TPM_SENSITIVE_Marshal/_Unmarshal that deals
with the case where sensitiveType is not a type of private key but a
public key instead that basically doesn't have much information in
TPM_SENSITIVE but is all zeros instead.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Stefan Berger [Tue, 23 Feb 2021 20:39:12 +0000 (15:39 -0500)]
tpm2: Test RSA private keys when -DDO_CHECK_RSA_KEY=1 is used
For some peace-of-mind add a function that allows us to check the RSA keys
that are generated, especially the primary keys that are not generated by
OpenSSL.
Use the following configure line to compile libtpms:
Stefan Berger [Sun, 21 Feb 2021 13:44:38 +0000 (08:44 -0500)]
tpm2: Sanitize values read from TPM state stream (Coverity)
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>
Stefan Berger [Sun, 21 Feb 2021 13:24:35 +0000 (08:24 -0500)]
tpm2: Prevent a potential buffer overrun (Coverity)
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>
Stefan Berger [Sun, 21 Feb 2021 13:04:21 +0000 (08:04 -0500)]
tpm2: Initialize variable and check rc before accessing nrh.size (Coverity)
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>
Stefan Berger [Wed, 17 Feb 2021 18:29:00 +0000 (13:29 -0500)]
tpm2: Save key and hash contexts using ANY_OBJECT_Marshal
Save key and hash contexts using the ANY_OBJECT_Marshal function and try
to load it using ANY_OBJECT_Unmarshal(). Unfortunately older contexts were
written out as plain OBJECTs, so we have to accomodate this case as well
so that we can restore key contexts from libtpms-0.7.x. We do not support
resuming HASH contexts from libtpms-0.7.x.
Before this modification context files written out by the IBM TSS stack
were 2692 bytes independent of content. Now an RSA 2048 key is 1222 bytes
and a NIST p384 key is 982 bytes.
Several of the original TPM 2 function exporting Sequence state and
importing it can now be disabled.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Stefan Berger [Wed, 17 Feb 2021 18:19:06 +0000 (13:19 -0500)]
tpm2: Make ANY_OBJECT_Marshal/Unmarshal non-static
Make the functions ANY_OBJECT_Marshal/Unmarshal non-static so that we can
call it from other places. Also allow passing a parameter 'verbose' to the
ANY_OBJECT_Unmarshal function that allows us to call this function without
it logging errors. We need this when trying to load a context from an older
libtpms versions that did not use ANY_OBJECT_Marshal to write out the
OBJECT (but copied it right from memory).
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Stefan Berger [Mon, 15 Feb 2021 14:44:51 +0000 (09:44 -0500)]
tpm2: Return properly sized array for b parameter for NIST P521 (HLK)
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>
William Roberts [Mon, 8 Feb 2021 20:33:21 +0000 (14:33 -0600)]
configure: support --with-pkgconfigdir
Support setting different install paths for package config files
using the --with-pkgconfigdir option.
Drop the hardcoded pkgconfigdir variable in the Makefile.am as per the
manpage http://manpages.ubuntu.com/manpages/cosmic/man7/pkg.m4.7.html
the macro PKG_INSTALLDIR defaults to $libdir/pkgconfig.
Signed-off-by: William Roberts <william.c.roberts@intel.com>
Stefan Berger [Sat, 2 Jan 2021 15:25:28 +0000 (10:25 -0500)]
tpm2: Address issues detected by cppcheck (false positives)
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>
Stefan Berger [Mon, 23 Nov 2020 18:15:53 +0000 (13:15 -0500)]
tpm2: Fix issue with misaligned address when marshalling NVRAM (UBSAN)
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>
Stefan Berger [Wed, 28 Oct 2020 13:49:20 +0000 (09:49 -0400)]
tpm2: EcSM2: Enforce that the random bnK has no leading zeros
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>
Stefan Berger [Tue, 27 Oct 2020 21:35:39 +0000 (17:35 -0400)]
tpm2: EcSchnorr: Enforce that the OpenSSL-generated bnD has no leading zeros
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>
Stefan Berger [Mon, 12 Oct 2020 23:44:17 +0000 (19:44 -0400)]
build-sys: Make --with-tpm2 the default and if used choose openssl
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>
Stefan Berger [Wed, 22 Jul 2020 04:08:12 +0000 (00:08 -0400)]
build-sys: Use AC_LINK_IFELSE to check for understood linker flags
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>
Stefan Berger [Mon, 17 Aug 2020 00:25:12 +0000 (20:25 -0400)]
tpm2: Fix compilation error in TPM2B_CREATION_DATA_Marshal (Fedora 32/s390x)
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>
Stefan Berger [Sun, 16 Aug 2020 22:48:46 +0000 (18:48 -0400)]
tpm12: Fix compilation error for Fedora 32 / s390x
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>
Stefan Berger [Fri, 26 Jun 2020 20:40:27 +0000 (16:40 -0400)]
rev162: fix PCRBelongsTCBGroup for PCClient (bugfix)
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>