| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
Caught by `make check` on fedora-rawhide (GCC 12):
libstb/secvar/test/../storage/fakenv_ops.c: In function 'fakenv_readpublic':
libstb/secvar/test/../storage/fakenv_ops.c:155:17: error: 'memcpy' reading 134 bytes from a region of size 34 [-Werror=stringop-overread]
155 | memcpy(&nv_name->t.name, tpmnv_vars_name, sizeof(TPM2B_NAME));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from libstb/secvar/test/secvar-test-secboot-tpm.c:5:
libstb/secvar/test/../storage/secboot_tpm.c:35:15: note: source object 'tpmnv_vars_name' of size 34
35 | const uint8_t tpmnv_vars_name[] = {
| ^~~~~~~~~~~~~~~
libstb/secvar/test/../storage/fakenv_ops.c:158:17: error: 'memcpy' reading 134 bytes from a region of size 34 [-Werror=stringop-overread]
158 | memcpy(&nv_name->t.name, tpmnv_control_name, sizeof(TPM2B_NAME));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
libstb/secvar/test/../storage/secboot_tpm.c:41:15: note: source object 'tpmnv_control_name' of size 34
41 | const uint8_t tpmnv_control_name[] = {
| ^~~~~~~~~~~~~~~~~~
The source and destination of each memcpy have known sizes, and we are
copying the smaller buffer into the larger one, so change the memcpy
size to that of the smaller buffer.
Signed-off-by: Reza Arbab <arbab@linux.ibm.com>
|
|
Several OpenSSL APIs that libstb uses have been deprecated in OpenSSL
3.0. Commit 9a1f95f87004 ("libstb/create-container: avoid using
deprecated APIs when compiling with OpenSSL 3.0") enabled `make` to
succeed on an OpenSSL 3.0 system, but `make check` still fails:
libstb/print-container.c:405:9: error: 'EC_KEY_new' is deprecated: Since OpenSSL 3.0 [-Werror=deprecated-declarations]
libstb/print-container.c:413:9: error: 'EC_KEY_set_group' is deprecated: Since OpenSSL 3.0 [-Werror=deprecated-declarations]
libstb/print-container.c:425:9: error: 'EC_POINT_bn2point' is deprecated: Since OpenSSL 3.0 [-Werror=deprecated-declarations]
libstb/print-container.c:429:9: error: 'EC_KEY_set_public_key' is deprecated: Since OpenSSL 3.0 [-Werror=deprecated-declarations]
libstb/print-container.c:434:9: error: 'ECDSA_do_verify' is deprecated: Since OpenSSL 3.0 [-Werror=deprecated-declarations]
libstb/print-container.c:449:9: error: 'EC_KEY_free' is deprecated: Since OpenSSL 3.0 [-Werror=deprecated-declarations]
This print-container.c is derived from the one in the sb-signing-utils
project. Ideally, OpenSSL 3.0 support should be added there first and
then synced back into skiboot. Until that is complete[1], build with
-Wno-error=deprecated-declarations so these errors stop blocking our CI.
[1] https://github.com/open-power/sb-signing-utils/issues/35
Signed-off-by: Reza Arbab <arbab@linux.ibm.com>
|
|
OpenSSL 3.0
OpenSSL 3.0 has deprecated functions that operate on raw key data, however the
closest replacement function are not available in OpenSSL 1.x. This patch
attempts to maintain compatibility with both 3.0 and 1.x versions.
Avoids using the following deprecated functions when compiling with 3.0:
- EC_KEY_get0_group
- EC_KEY_get0_public_key
- EC_POINT_point2bn
- EC_KEY_free
Signed-off-by: Eric Richter <erichte@linux.ibm.com>
Signed-off-by: Reza Arbab <arbab@linux.ibm.com>
|
|
This code isn't directly used by skiboot, but it is wrong and potentially
insecure so I'm fixing it in case it's used in the future.
We pass sizeof(hash) into mbedtls_pk_verify(). However, hash is a pointer,
not an array, so rather than passing the length of the hash to verify we'll
pass in 8, and only compare the first 8 bytes of the hash rather than all 32.
Pass in 0 instead. That tells mbedtls to work out the length based on the
hash type. We allocated enough memory for whatever hash type the PKCS#7
message declared so this will be safe.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
|
|
Each signed variable update contains a timestamp -- this timestamp is checked
against the previous timestamp seen for that particular variable (if any), and
the update is rejected if the timestamp is not a later time than the previous.
This timestamp check is intended to prevent re-use of signed update files.
Currently, the code stores the timestamps in the TS variable, which is then
stored in regular variable storage (typically PNOR). This patch promotes the
variable to "protected storage" (typically TPM NV), so avoid this variable
being accidentally cleared.
This change should only come into effect when either:
- initializing secvar for the first time (i.e. first boot, or
after a key-clear-request)
- processing any variable update
Systems that already have a TS variable in PNOR will not be affected until
either of the above actions are taken.
Signed-off-by: Eric Richter <erichte@linux.ibm.com>
Tested-by: Nick Child <nick.child@ibm.com>
Reviewed-by: Daniel Axtens <dja@axtens.net>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
|
|
As the PNOR variable space cannot be locked, the data must be integrity
checked when loaded to ensure it has not beeen modified by an unauthorized
party. In the event that a modification has been detected (i.e. hash mismatch),
we must not load in data that could potentially be compromised.
However, the previous code was a bit overzealous with its reaction to detecting
a compromised SECBOOT partition, and also had some inconsistencies in behavior.
Case 1: SECBOOT partition cleared.
.init() checks the header for the magic number and version. As neither matches,
will reformat the entire partition. Now, .load_bank() will pass, as the data
was just freshly reformatted (note: this also could trigger the bug addressed
in the previous patch). Only variables in the TPM will be loaded by
.load_bank() as the data in SECBOOT is now empty.
Case 2: Bank hash mismatch.
.load_bank() panics and returns an error code, causing secvar_main() to jump
to the error scenario, which prevents the secvar API from being exposed.
os-secure-enforcing is set unconditionally, and the user will have no API to
manage or attempt to fix their system without issuing a key clear request.
This patch unifies the behavior of both of these cases. Now, .init() handles
checking the header AND comparing the bank hash. If either check fails, the
SECBOOT partition will be reformatted. Variables in the TPM will still be
loaded in the .load_bank() step, and provided the backend stores its
secure boot state in the TPM, secure boot state can be preserved.
Signed-off-by: Eric Richter <erichte@linux.ibm.com>
Tested-by: Nick Child <nick.child@ibm.com>
Reviewed-by: Daniel Axtens <dja@axtens.net>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
|
|
When the SECBOOT partition is formatted, the bank hash stored in the
control TPM NV index must be updated to match, or else we will immediately
fail to load the freshly formatted data at the .load_bank() step.
However, while the secboot_format() function does calculate and update the
bank hash, it only writes the new hash for bank 0. It does not update the
value for bank 1, or set the current active bank. This works as expected if
the active bank bit happens to be set to 0. On the other hand, if the active
bit is set to 1, the freshly formatted bank 1 will be compared against the
unchanged bank hash in bank 1 at the load step, therefore causing an error.
This patch fixes this issue by also setting the active bit to 0 to match
the freshly calculated hash.
Signed-off-by: Eric Richter <erichte@linux.ibm.com>
Tested-by: Nick Child <nick.child@ibm.com>
Reviewed-by: Daniel Axtens <dja@axtens.net>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
|
|
There were a few instances in `get_hash_to_verify` where NULL is
returned before unallocating the md context. This commit ensures that
this memory is properly freed before returning.
Signed-off-by: Nick Child <nick.child@ibm.com>
Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
|
|
get_hash_to_verify claims to return a negative error code in the
case of error. It actually returns NULL. Fix the comment.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Nick Child <nick.child@ibm.com>
Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
|
|
process_update() has tbhbuffer and tbhbuffersize. However, tbhbuffer
doesn't contain to-be-hashed data, but a hash:
/* Prepare the data to be verified */
tbhbuffer = get_hash_to_verify(update->key, *newesl, *new_data_size,
timestamp);
And tbhbuffersize is initalised to zero and then never filled with
the actual length, so 0 is passed through to verify_signature.
verify_signature will in turn pass that to mbedtls, which will interpret
it as "figure out the length yourself based on the hash type". So this
has always worked, but by accident.
Rename tbhbuffer to hash, as that's what it is.
Drop tbhbuffersize, and pass 32 directly to verify_signature. We only
support SHA-256, and SHA-256 is 32 bytes long.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Nick Child <nick.child@ibm.com>
Tested-by: Nick Child <nick.child@ibm.com>
Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
|
|
verify_signature() currently takes newcert and new_data_len. However,
these variables are used only as parameters to
mbedtls_pkcs7_signed_hash_verify() where they represent a hash value
and the length of the hash value.
verify_signature() is static, and the only caller of the function is
process_update(). process_update() passes in tbhbuffer and tbhbuffersize.
Those are unfortunate names too - because the data that process_update()
passes in is not a to-be-hashed buffer, but a hash. We'll fix that later.
Call the parameters hash and hash_len.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Nick Child <nick.child@ibm.com>
Tested-by: Nick Child <nick.child@ibm.com>
Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
|
|
Fuzzing revealed a crash where pkcs7_get_signed_data was accessing beyond
the bounds of the object, despite valid data being passed in to
mbedtls_pkcs7_parse_der.
Further investigation revealed that pkcs7_get_content_info_type will
reset *p to start if the second call to mbedtls_asn1_get_tag fails,
but not if the first call fails.
mbedtls_asn1_get_tag does indeed advance *p even in some failure
cases, so a reset is required.
Reset *p to start if the first call to mbedtls_asn1_get_tag fails.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Nayna Jain <nayna@linux.ibm.com>
Tested-by: Nayna Jain <nayna@linux.ibm.com>
Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
|
|
We need to actually free the pkcs7 structure, not just pass it to
mbedtls_pkcs7_free().
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Nayna Jain <nayna@linux.ibm.com>
Tested-by: Nayna Jain <nayna@linux.ibm.com>
Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
|
|
If a declared size is smaller than uuid size, we end up allocating
with an allocation of a 'negative' number, which is a huge 64 bit
number.
This will probably then fail with an OPAL_NO_MEM, but it will be
better to catch it and return OPAL_PARAMETER instead.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Nayna Jain <nayna@linux.ibm.com>
Tested-by: Nayna Jain <nayna@linux.ibm.com>
Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
|
|
Catch another OOB read picked up by the fuzzer.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Nayna Jain <nayna@linux.ibm.com>
Tested-by: Nayna Jain <nayna@linux.ibm.com>
Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
|
|
Currently, in `verify_signature`, the return code `rc` is initialized
as 0 (our success value). While looping through the ESL's in the given
secvar, the function will break if the remaining data in the secvar is
not enough to contain another ESL. This break from the loop was not
setting a return code, this means that the successful return code
can pass to the end of the function if the first iteration meets
this condition. In other words, if a current secvar has a size that
is less than minimum size for an ESL, than it will approve any update.
In response to this bug, this commit will return an error code if
the described condition is met. Additionally, a test case has been
added to ensure that this unlikely event is handled correctly.
Signed-off-by: Nick Child <nick.child@ibm.com>
Reviewed-by: Nayna Jain <nayna@linux.ibm.com>
Tested-by: Nayna Jain <nayna@linux.ibm.com>
Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
|
|
Currently, in `validate_esl_list`, the return code is initialized to zero
(our success value). While looping though the ESL's in the submitted ESL
chain, the loop will break if there is not enough data to meet minimum ESL
requirements. This condition was not setting a return code, meaning that the
successful return code can pass to the end of the function if there is extra
data at the end of the ESL. As a consequence, any properly signed update can
successfully commit any data (as long as it is less than the min size of an
ESL) to the secvars.
This commit will return an error if the described condition is met. This
means all data in the appended ESL of an auth file must be accounted for. No
extra bytes can be added to the end since, on success, this data will become
the updated secvar.
Additionally, a test case has been added to ensure that this commit
addresses the issue correctly.
Signed-off-by: Nick Child <nick.child@ibm.com>
Reviewed-by: Nayna Jain <nayna@linux.ibm.com>
Tested-by: Nayna Jain <nayna@linux.ibm.com>
Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
|
|
Recently we had an issue where we did the following:
uint16_t year = le32_to_cpu(timestamp->year);
This is wrong and will break on BE. However, we didn't catch this
with sparse because there was a whole slew of warnings.
The reason for the slew of warnings is that we didn't annotate the
types that store little-endian specific data in edk2.h.
Provide the appropriate annotations.
We now get a single sparse warning for the file, which correctly
identifies the issue:
edk2-compat-process.c:374:46: warning: incorrect type in argument 1 (different base types)
edk2-compat-process.c:374:46: expected restricted leint32_t [usertype] le_val
edk2-compat-process.c:374:46: got restricted leint16_t const [usertype] year
We do lose the signedness of efi_time->timezone, but that's probably OK:
we never use the timezone anyway and the comment above the data structure
makes the signedness pretty clear.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
|
|
Currently, the loop in validate_esl_list is not iterating through the ESL
entries. As a consequence, all of entries after the first are not being
validated and can contain any data. In order to iterate, the pointer to the
esl buffer must be incremented by the amount of already read bytes.
This commit also adds a new test case and file. The file is
`multipletrimmedKEK.h` the array is very similar to the one in `trimmedKEK.h`
except this one only has an invalid ESL as the second ESL in the chain. This
then tests the condition that this commit tests because only the second ESL
is invalid.
Signed-off-by: Nick Child <nick.child@ibm.com>
Reviewed-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Nayna Jain <nayna@linux.ibm.com>
Tested-by: Nayna Jain <nayna@linux.ibm.com>
Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
|
|
Currently, `get_esl_cert` receives a data buffer containing an ESL and its
length. It is to return a data buffer of the certificate that is contained
inside the ESL. The ESL has header info that contains the certificates
`size` and the size of the header (`sig_data_offset`). We use this
information to copy `size` bytes starting `sig_data_offset` bytes after the
given ESL buffer. Currently we are checking that the length of the ESL
buffer is at least `sig_data_offset` bytes but we are not checking that it
also has enough bytes to also contain `size` bytes of the certificate. This
becomes problematic if some data at the end of the ESL gets lost. Since the
ESL claims it has more than it actually does, this will lead to a buffer
over-read. What is even worse, is that this buffer over-read can go
unnoticed since the last 256 bytes of the ESL are usually the x509 2048 bit
signature so the extra garbage bytes that are copied will appear to be a
valid rsa signature.
To resolve this, this commit ensures that the ESL buffer length is large
enough to hold the data that it claims it contains.
Lastly, a new test case is added to test the described condition. It
includes a new test file `trimmedKEK.h` which contains a struct a valid KEK
auth file minus 5 bytes, therefore making it invalid.
Signed-off-by: Nick Child <nick.child@ibm.com>
Reviewed-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Nayna Jain <nayna@linux.ibm.com>
Tested-by: Nayna Jain <nayna@linux.ibm.com>
Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
|
|
We only handle sha256 hashes in auth structures.
In the process of verifying an auth structure, we extract the pkcs7
message and we calculate the hopefully-matching hash, which is
sha256(name || vendor guid || attributes || timestamp || newcontent)
We then verify that the PKCS#7 signature matches that calculated hash.
However, at no point do we check that the PKCS#7 hash algorithm is
sha256. So if the PKCS#7 message says that it is a signature on a sha512,
mbedtls will compare 64 bytes of hash from the signature with 64 bytes
from our hash, resulting in a 32 byte overread.
Verify that the hash algorithm in the PKCS#7 message is sha256.
Add a test.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Nayna Jain <nayna@linux.ibm.com>
Tested-by: Nayna Jain <nayna@linux.ibm.com>
Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
|
|
unpack_timestamp() calls le32_to_cpu() for endian conversion of
uint16_t "year" value. This patch fixes the code to use le16_to_cpu().
Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
Reviewed-by: Daniel Axtens <dja@axtens.net>
Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
|
|
This isn't currently used in skiboot but may be used by external
users of skiboot's secvar code.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
|
|
We use these types but haven't included the header: every file that
includes edk2.h has already included it.
This might not be true for other users of edk2.h and skiboot's secvar
code, so include it explictly.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
|
|
If allocating the secure variable name of a secure variable struct,
`secvar->key`, fails then the secvar struct should be freed before
returning NULL. Previously, if this allocation fails, then only the
`secvar->key` is freed (which is likely a typo) leaving the allocated
`secvar` struct allocated and returning NULL. This memory leak can be
seen with the static analysis tool `cppcheck`. After running valgrind
tests, this commit ensures that memory is properly freed if an error
occurs when allocating the `key` field of the `secvar` struct.
Signed-off-by: Nick Child <nick.child@ibm.com>
Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
|
|
Functions `get_esl_cert`, `validate_esl_list` and
`get_esl_signature_list_size` all contain the same debug print
statement. This statement prints the size of the ESL.
`validate_esl_list` calls `get_esl_cert` so the same debug information
prints twice when validating the newly submitted ESL. Additionally,
the same debug prints twice when validating the current ESL since
`get_esl_cert` and `get_esl_signature_list_size` are both called by
the function `verify_signature`. Since `get_esl_cert` is the common
factor, this commit removes the other two print statements (and adds
some information to an error message to maintain clarity, in case
`validate_esl_list` fails before calling `validate_esl_cert`). After
double checking that these functions are not being called anywhere
else, the only real change is to reduce the number of redundant print
statements for the secvar update process.
Signed-off-by: Nick Child <nick.child@ibm.com>
Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
|
|
The structes we import from EDK2 are expected to be packed.
The code we imported does this a #pragma pack, but it doesn't
restore the original non-packed state at the end of the header.
Rather than changing that, just explictly pack every structure.
The resulting skiboot.elf has the same disassembly (objdump -dr)
and readelf -a output, but I haven't been able to test this on
a real machine.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
|
|
Follow the inclusive terminology from the "Conscious Language in
your Open Source Projects" guidelines [*] and replace the word
"whitelist" appropriately.
[*] https://github.com/conscious-lang/conscious-lang-docs/blob/main/faq.md
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
|
|
Signed-off-by: Stewart Smith <stewart@flamingspork.com>
Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
|
|
The edk2 test file includes some mbedtls files directly, make sure that
those also include the correct mbedtls config file.
Without this, the default config file is used, which conflicts with the
version we build as part of skiboot.
As host libc includes a SIZE_MAX macro, this also changes the SIZE_MAX
macro defined in mbedtls_config.h (needed for some mbedtls functions) to
only be defined if it isn't already.
Signed-off-by: Eric Richter <erichte@linux.ibm.com>
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
|
|
Linking against the host mbedtls introduces problems if the host does not
have the library, or if the host has a different version installed.
This patch changes the tests to instead build mbedtls from the version
included in skiboot using the host compiler, removing the dependency on
external mbedtls.
Signed-off-by: Eric Richter <erichte@linux.ibm.com>
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
|
|
The secvar makefiles use $(SRC) in a few places they shouldn't and don't
use it in a few places they should. Also drop the _SRCS rules and the
pattern substuituion that turns them into _OBJS rules because chaining
dependent rules is infuriating at the best of times.
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
|
|
Some versions of GCC complain about this. That and since it's a static
global it goes in the BSS and is initialized to zero anyway.
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
|
|
This patch adds following more unit test cases and improve comments.
* Check for successful processing of queued updates
* Check for queued updates when one update fail, especially when PK is
added.
* Check for queued updates when one update fail, especially when PK is
deleted.
* Check hw-key-hash addition/deleting/verification.
* Update dbxcert file
* Update rc checks against specific failure error return codes.
Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
|
|
This patch fixes following bugs. Additionally, it improves logs.
* Failure in adding/deleting PK as part of failure of processing any
subsequential update in the queue didn't reset the global variable
setup_mode to the original value. This patch adds the fix to always
set the value of setup_mode as per final contents in variable_bank
before existing process().
* Deletion of HWKH as part of deleting PK was only updating the value of
the variable to be zero. However, this didn't deallocate the variable from
the bank and was getting exposed via sysfs.
* The mismatch in verification of hw-key-hash, was also clearing staging
bank, which isn't initialized in this case. Fix the cleanup tag to only
clear update_bank.
* Fixes a memory leak in validate_esl_list().
* Convert signature verification error code from mbedtls into
opal error code as OPAL_PERMISSION.
Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
|
|
The TPM NV index size for storing the PK was originally set to 1024,
which was determined to be a "smallest maximum" size that we
determined to be enough to store the PK. However with overhead, this
only allowed for about ~912 bytes, which is far too small to store a
certificate, as it only permits about ~10 characters in the x509
subject field.
This patch increases the TPM NV Vars index to 2048 bytes, which is the
largest size a single NV index can be on the Nuvoton npct650 chip.
Signed-off-by: Eric Richter <erichte@linux.ibm.com>
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
|
|
indices
The Nuvoton npct650 chip has a command buffer max size of 1024.
Attempting to read or write from an NV index larger than this value
would return an error.
This patch changes the tss_nv_read and tss_nv_write commands to chunk
their operations in 1024-byte batches to allow support for larger NV
indices.
Signed-off-by: Eric Richter <erichte@linux.ibm.com>
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
|
|
This patch contains a set of tests to exercise the edk2 driver using actual
properly (and in some cases, improperly) signed binary data.
Due to the excessive size of the binary data included in the header files,
this test was split into its own patch.
Co-developed-by: Nayna Jain <nayna@linux.ibm.com>
Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
Signed-off-by: Eric Richter <erichte@linux.ibm.com>
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
|
|
As part of secureboot key management, the scheme for handling key updates
is derived from tianocore reference implementation[1]. The wrappers for
holding the signed update is the Authentication Header and for holding
the public key certificate is ESL (EFI Signature List), both derived from
tianocore reference implementation[1].
This patch adds the support to process update queue. This involves:
1. Verification of the update signature using the key authorized as per the
key hierarchy
2. Handling addition/deletion of the keys
3. Support for dbx (blacklisting of hashes)
4. Validation checks for the updates
5. Supporting multiple ESLs for single variable both for update/verification
6. Timestamp check
7. Allowing only single PK
8. Failure Handling
9. Resetting keystore if the hardware key hash changes
[1] https://github.com/tianocore/edk2-staging.git
Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
Signed-off-by: Eric Richter <erichte@linux.ibm.com>
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
|
|
This patch adds a pkcs7 parser for mbedtls that hasn't yet
gone upstream. Once/if that implementation is accepted,
this patch can be removed.
Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
Signed-off-by: Eric Richter <erichte@linux.ibm.com>
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
|
|
This patch adds a small userspace utility to locally generate the
expected hash returned by a TSS_NV_ReadPublic command for the NV
indices as defined by the secboot_tpm storage driver. This removes the
need for manually copying in the hash from the ReadPublic output if for some
reason the set of attributes used when defining the NV indices changes in the
future.
As this is an auxiliary tool, it is not built by default and must be
manually built using `make gen_tpmnv_public_name`.
Signed-off-by: Eric Richter <erichte@linux.ibm.com>
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
|
|
This patch adds some simple unit cases to exercise the storage driver,
using the fake TPM NV implementation.
Signed-off-by: Eric Richter <erichte@linux.ibm.com>
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
|
|
The secboot_tpm storage driver heavily relies on the TPM to ensure data
integrity, which makes it difficult to test in userspace or on hardware
without a TPM.
This patch adds a bunch of functions that implement the tssskiboot
interface, and simulates the expected TPM behavior utilizing PNOR space
instead.
THIS IS NOT INTENDED FOR PRODUCTION USE.
Signed-off-by: Eric Richter <erichte@linux.ibm.com>
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
|
|
This patch implements the platform specific logic for persisting the
secure variable storage banks across reboots via the SECBOOT PNOR
partition.
For POWER 9, all secure variables and updates are stored in the
in the SECBOOT PNOR partition. The partition is split into three
sections: two variable bank sections, and a section for storing
updates. The driver alternates writes between the two variable
sections, so that the final switch from one set of variables to
the next can be as atomic as possible by flipping an "active bit"
stored in TPM NV.
PNOR space provides no lock protection, so prior to writing the
variable bank, a sha256 hash is calculated and stored in TPM NV.
This hash is compared against the hash of the variables loaded from
PNOR to ensure consistency -- otherwise a failure is reported, no keys
are loaded (which should cause skiroot to refuse to boot if secure boot
support is enabled).
Signed-off-by: Eric Richter <erichte@linux.ibm.com>
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
|
|
This patch adds a simple function to detect whether or not physical
presence has been asserted. In the current implementation, all physical
presence assertion modes are treated the same.
Signed-off-by: Eric Richter <erichte@linux.ibm.com>
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
|
|
This patch adjusts the API unit tests to use the secvar struct rather than
the old secvar_node. Where applicable, some manual allocations have also been
replaced with the util functions.
Signed-off-by: Eric Richter <erichte@linux.ibm.com>
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
|
|
Originally, the secvar struct was intended to contain all the variable
information seperate from the linked list/etc metadata, so that copying
and serialization/deserialization could be handled by a single memcpy().
This is fragile, potentially compiler dependent, and doesn't account for
endianness. Therefore, this patch removes the static allocation for key, now
allocates a buffer for data, and completely removes the now unnecessary
secvar_node struct.
As a side effect, some of the secvar_util functionality has been tweaked where
it makes sense. Most notably alloc_secvar now takes in an extra argument as it
now has to allocate the key
Signed-off-by: Eric Richter <erichte@linux.ibm.com>
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
|
|
This patch adds the following helper functions:
- dealloc_secvar()
- new_secvar()
- copy_bank_list()
dealloc_secvar() frees a whole secvar_node reference including
its children allocations. This also updates the clear_bank_list()
helper function to use this destructor.
new_secvar() allocates a secvar_node, and fills it with data
provided via arguments.
copy_bank_list() creates a deep copy of a secvar bank list
Signed-off-by: Eric Richter <erichte@linux.ibm.com>
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
|
|
Previously, backends were implicitly expected to operate on global
references to the variable and update banks. This patch changes the
interface for this driver to instead take the banks in as an argument.
This removes the implict dependency on these references, makes the
design consistent with the storage driver, and also will simplify unit
testing of these functions.
Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
Signed-off-by: Eric Richter <erichte@linux.ibm.com>
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
|
|
This patch adjusts the behavior of secvar_main to actually halt the boot
in some form if there is an issue initializing secure variables. The secvar
storage driver contains the secure boot state, and therefore if that fails
to initialize, we immediately need to halt the boot. For all other cases we
enforce secure boot in the bootloader by setting the secure mode flag, but
booting with an empty keyring (and thus, cannot verify a kexec image).
Previously, the storage driver was expected to handle any locking
procedures implicitly as part of the write operation. This patch uses the
new lockdown hook which makes locking explicit and part of the secvar_main
flow.
The storage driver is now locked unconditionally when exiting
secvar_main, and the lockdown() call should halt the boot if it encounters
any sign of struggle.
Signed-off-by: Eric Richter <erichte@linux.ibm.com>
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
|
