vmcoreinfo_item() copies a key's value string from the vmcoreinfo heap
blob into the caller's buffer using memcpy() without checking the source
length. The length is derived from the distance between the '=' separator
and the next '\n' (or '\0') in the blob, which is bounded only by the
total vmcoreinfo size. A crafted dump with a vmcoreinfo value >= 1024
bytes would overflow the buffer.
Additionally, the len parameter of vmcoreinfo_item() was declared UNUSED
and never checked.
Fix by computing val_len before the copy and returning -1 if val_len >= len.
Change len type from int to size_t, which is natural for a buffer size,
drops the need for a negativity guard, and makes the call site passing
sizeof(str) type-consistent.
Write directly into the caller's buf, instead of going through the
intermediate str[].
Replace two strchr() calls with a single strchrnul().
Signed-off-by: Mikhail Zaslonko <zaslonko@linux.ibm.com>
Reviewed-by: Alexander Egorenkov <egorenar@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
The 16-bit sec5_len field from the ADSR record was used directly as
the byte count for zg_read() and ebc_2_asc() into a pair of 1024-byte
stack buffers. A crafted VMDUMP file with sec5_len > 1024 would
overflow both buffers and write past the stack frame.
The overflow is only reachable when zgetdump is invoked with the -V
(verbose) flag, but that is a common diagnostic usage.
Section 5 contains a human-readable dump symptom string used only for
display purposes. Truncate sec5_len to sizeof(buf) - 1 so that
oversized values are silently clamped rather than causing an abort,
and the symptom string is still printed up to the buffer limit.
Signed-off-by: Mikhail Zaslonko <zaslonko@linux.ibm.com>
Reviewed-by: Alexander Egorenkov <egorenar@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Building with ASAN=1 produces the following warning:
utils.c: In function 'sysfs_get_mkvps':
utils.c:416:9: warning: writing 8 bytes into a region of size 0
[-Wstringop-overflow=]
416 | memcpy(mk_reg->mkvp, &mkvp, sizeof(mkvp));
| ^
In file included from utils.c:32:
utils.h:51:17: note: at offset 4 into destination object 'mk_state'
of size 4
51 | int mk_state;
This is a false positive, field mk_reg->mkvp is defined as an array of
u8 with a length of 16 elements, thus it is 16 bytes in size, and thus
more than long enough to copy 8 bytes into it.
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Reviewed-by: Finn Callies <fcallies@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
When the BER encoded BITRSTRING field has a zero length, then skipping
the unused-bits byte that a BITRSTING normally contains will cause a
underflow and thus may cause an out of bounds read by the caller of
sk_ep11_parse_spki().
Assisted-by: IBM Bob:2.0.0
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Reviewed-by: Finn Callies <fcallies@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Check for the size of struct cca_rsa_pub_key_section, instead of
struct cca_ec_pub_key_section.
Assisted-by: IBM Bob:2.0.0
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Reviewed-by: Finn Callies <fcallies@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
When EVP_PKEY_get0_EC_KEY(), EC_KEY_set_ex_data(), EVP_PKEY_get0_RSA()
or RSA_set_ex_data() fails, the function returns -EIO without freeing
the data struct or the data->key_blob that were allocated before.
Assisted-by: IBM Bob:2.0.0
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Reviewed-by: Finn Callies <fcallies@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Field spki_size is defined as size_t, so it can never be negative.
Before the check, sk_ep11_valid_ep11_blob() ensures that hdr->len is
never greater than key_token_length, so the subtraction can never
overflow the result, but it still can be zero.
Assisted-by: IBM Bob:2.0.0
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Reviewed-by: Finn Callies <fcallies@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
For long-length field encoding, check if the number of length bytes
exceed the data size.
Assisted-by: IBM Bob:2.0.0
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Reviewed-by: Finn Callies <fcallies@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
A malformed CCA key token with a zero size section can cause an endless
loop in sk_cca_get_pka_section().
Assisted-by: IBM Bob:2.0.0
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Reviewed-by: Finn Callies <fcallies@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Array element key_value_structure.public_exponent[0] is written twice,
but key_value_structure.public_exponent[1] is not set, leading to a
wrong public exponent value (i.e. 0x01) in the key value structure.
CCA key generation will fail is this case.
Assisted-by: IBM Bob:2.0.0
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Reviewed-by: Finn Callies <fcallies@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Functions SK_CCA_generate_ec_key_pair(), SK_CCA_generate_ec_key_pair(),
SK_EP11_generate_ec_key_pair() and SK_EP11_generate_rsa_key_pair() are
supposed to allow parameter key_token to be NULL. In this case the
required buffer size is returned in key_token_length only.
This size query does not work because the parameter checking rejects
the call with a NULL key_token returning -EINVAL.
Currently the size query is not used by any known caller, but lets
correct it anyway.
Assisted-by: IBM Bob:2.0.0
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Reviewed-by: Finn Callies <fcallies@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
The party info contained in the response might be of an arbitrary size.
Check for an overflow before allocating the party info buffer for the
combined request and response party info data. With a very large
response party info size, the combined size might overflow (i.e. wrap
around), causing a too small buffer being allocated and the following
memcpy's would cause an out of bounds write.
Assisted-by: IBM Bob:2.0.0
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Reviewed-by: Finn Callies <fcallies@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
On large data sizes the calculation may cause an overflow, if done with
int type. Do the calculation with size_t instead to avoid an overflow.
Assisted-by: IBM Bob:2.0.0
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Reviewed-by: Finn Callies <fcallies@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Fix the error checking to check the correct variables using the right
conditions.
Assisted-by: IBM Bob:2.0.0
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Reviewed-by: Finn Callies <fcallies@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Don't attempt to free the local function parameter 'signature', but the
memory where *signature points to.
Assisted-by: IBM Bob:2.0.0
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Reviewed-by: Finn Callies <fcallies@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Function strtoull() silently accepts a leading '-' and returns a large
positive number (two's complement wrap). Reject this by checking for
the minus character.
Assisted-by: IBM Bob:2.0.0
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Reviewed-by: Finn Callies <fcallies@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Function gmtime() might return NULL which would then be passed to
strftime(). Return an error in case gmtime() return NULL.
Assisted-by: IBM Bob:2.0.0
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Reviewed-by: Finn Callies <fcallies@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
A deeply nested or pathologically large KMIP node tree crafted by a
malicious server can wrap the length calculation around to a small
value. Detect this and return an error in this case.
Assisted-by: IBM Bob:2.0.0
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Reviewed-by: Finn Callies <fcallies@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Functions kmip_node_get_structure_element_count() and
kmip_node_get_structure_element_by_tag_count() return an unsigned int,
but the error case returns -1. This leads to a wrap around actually
returning 0xffffffff which the caller might interpret as a very large
number of elements.
Return 0 in case of an error instead.
Also fix some callers to not unconditionally subtract -1 from the
returned value, but return an error if the returned value is zero.
These callers check the preconditions upfront, so the error case
won't be hit anyway.
Assisted-by: IBM Bob:2.0.0
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Reviewed-by: Finn Callies <fcallies@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Limit the value_len to not be larger than INT_MAX, because later on
BIO_read() is called with value_len and it uses the int type for
length parameter and return value.
This check also prevents the 'value_len + 1' from overflow, because
value_len is a size_t and this accepts larger values than int (even
on 32 bit architectures).
Assisted-by: IBM Bob:2.0.0
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Reviewed-by: Finn Callies <fcallies@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
When decoding a value that is exactly at the boundary of the available size
(i.e., *size == value_len), and the value length is not a multiple of the
TTLV block length, then *size wraps to a huge value when the pad_len is
subtracted.
Assisted-by: IBM Bob:2.0.0
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Reviewed-by: Finn Callies <fcallies@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
If length is >= 0x80000000 (2 GB), length * 2 wraps around to a small
value, calloc allocates a too small buffer, then the loop writes
length * 2 bytes into it causing a heap buffer overflow.
Fix this by using a size_t for size calculation, and also checking
the length before multiplication (needed on 32 bit platforms).
Assisted-by: IBM Bob:2.0.0
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Reviewed-by: Finn Callies <fcallies@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
In C, operands of '<<' are subject to integer promotion. So the unsigned
char array elements are promoted to signed int and then shifted producing
signed overflow — undefined behaviour in C.
Fix this by first casting to uint32_t and then shift.
Assisted-by: IBM Bob:2.0.0
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Reviewed-by: Finn Callies <fcallies@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
KMIP STRUCTURE elements can be nested, which causes a recursion of
functions kmip_decode_ttlv(), kmip_decode_xml(), and kmip_decode_json().
A malformed KMIP response may thus cause stack exhaustion.
Limit the KMIP STRUCTURE nesting level to 32 levels. This is more than
enough for currently defined KMIP responses. The practically used
nesting level is 8 or 9, dependent on the type of KMIP response.
Assisted-by: IBM Bob:2.0.0
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Reviewed-by: Finn Callies <fcallies@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
When creating or writing files, make sure that the file is not a
sysmlink. Such files created by libkmipclient are typically stored inside
the zkey repository and the owner and mode of them are changed to.
allow read/write for the owner user and the 'zkeyadm' group. It
would allow a symlink-following attack if the file being created are
symlinks. Make sure to open such files with the 'O_NOFOLLOW' flag.
Assisted-by: IBM Bob:2.0.0
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Reviewed-by: Finn Callies <fcallies@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
When creating or writing files, make sure that the file is not a
sysmlink. Such files created by libekmfweb are typically stored inside
the zkey repository and the owner and mode of them are changed to.
allow read/write for the owner user and the 'zkeyadm' group. It
would allow a symlink-following attack if the file being created are
symlinks. Make sure to open such files with the 'O_NOFOLLOW' flag.
Assisted-by: IBM Bob:2.0.0
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Reviewed-by: Finn Callies <fcallies@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
When creating or writing files, make sure that the file is not a
sysmlink. Such files created by libseckey are typically stored inside
the zkey repository and the owner and mode of them are changed to.
allow read/write for the owner user and the 'zkeyadm' group. It
would allow a symlink-following attack if the file being created are
symlinks. Make sure to open such files with the 'O_NOFOLLOW' flag.
Assisted-by: IBM Bob:2.0.0
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Reviewed-by: Finn Callies <fcallies@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Files in the zkey repository can be created by any member of the
'zkeyadm' group as well as 'root'. Such files are owned by the creator
and the 'zkeyadm' group, and allow read and write for the owner user
and owner group.
When creating or writing files inside the zkey repository, make sure
that the file is not a sysmlink. That way, only files within the zkey
repository are set to be read/write for the owner user and members of
the 'zkeyadm' group. Make sure to open such files with the 'O_NOFOLLOW'
flag, and use 'lstat()' to check for files and directories.
Assisted-by: IBM Bob:2.0.0
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Reviewed-by: Finn Callies <fcallies@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Check the values that are incorporated into commands that might get
executed to not contain any characters that could be used for
shell-injection attacks.
Assisted-by: IBM Bob:2.0.0
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Reviewed-by: Finn Callies <fcallies@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Pin the supported filesystem type to ext4 to prevent the risk of auto fs
parsing bugs. Additionally mount the boot partition with more
restrictive options.
Reviewed-by: Holger Dengler <dengler@linux.ibm.com>
Signed-off-by: Finn Callies <fcallies@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Fix the sel-ebc-pvebc.service unit to execute the failure action when
the sics directory does not exist instead of getting skipped.
Reviewed-by: Holger Dengler <dengler@linux.ibm.com>
Signed-off-by: Finn Callies <fcallies@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Return an error when the provided request data is smaller than the
expected request length. The previous condition used the wrong
comparison, which could lead to an out-of-bounds slice and panic.
Add a regression test.
Fixes: 34bef977e8 ("rust/pv: User-data signing and verifying")
Assisted-by: IBM Bob:1.0.5
Signed-off-by: Marc Hartmayer <marc@linux.ibm.com>
Reviewed-by: Steffen Eiden <seiden@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Perform certificate verification in two stages. First, verify the
certificate chain without CRL checks. Once the chain has been validated,
download the referenced CRLs and repeat the verification with CRL
checking enabled.
Fixes: c6f621d0dc ("rust: Add library for pv tools")
Signed-off-by: Marc Hartmayer <marc@linux.ibm.com>
Reviewed-by: Steffen Eiden <seiden@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
3-second timeout might be too short for slow networks therefore increase
it to 10s. In addition move this constant to a more prominent position
and add documentation.
Assisted-by: IBM Bob:1.0.5
Signed-off-by: Marc Hartmayer <marc@linux.ibm.com>
Reviewed-by: Steffen Eiden <seiden@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Add a dedicated HKD verification error for CRL downloads that exceed the
maximum file size of 10 MiB and cover the max_filesize behavior in the
helper tests and add tests for it.
This commit adds a new development/test dependency as it verifies that
the correct messages are being logged.
Fixes: c6f621d0dc ("rust: Add library for pv tools")
Assisted-by: IBM Bob:1.0.5
Signed-off-by: Marc Hartmayer <marc@linux.ibm.com>
Reviewed-by: Steffen Eiden <seiden@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Adapt the script to generate the test certificates to use the prefix
'http://' for the CRL distribution points as otherwise the test would
fail with an upcoming change. Regenerate the certificates and adapt the
test case dist_points' accordingly as the distribution point has
changed.
Note: The files der.crt and der.crl are skipped as that would result in
binary changes which caues trouble with quilt. They are only used
by some basic tests read_crls and read_crts anyway
Assisted-by: IBM Bob:1.0.5
Signed-off-by: Marc Hartmayer <marc@linux.ibm.com>
Acked-by: Steffen Eiden <seiden@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Fix datetime.datetime.utcnow() deprecation and fix a TypeError.
File "create_certs.py", line 16, in createEcKeyPair
return ec.generate_private_key(curve=curve, backend=default_backend())
~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: curve must be an EllipticCurve instance
Signed-off-by: Marc Hartmayer <marc@linux.ibm.com>
Reviewed-by: Steffen Eiden <seiden@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Replace the existing mocks with a trait-based test infrastructure. This
allows testing download_first_crl_from_x509() functionally while
avoiding actual network access, resulting in more comprehensive and
realistic test coverage.
Assisted-by: IBM Bob:1.0.5
Signed-off-by: Marc Hartmayer <marc@linux.ibm.com>
Acked-by: Steffen Eiden <seiden@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Rename the module to the more common test module name tests and remove a
superfluous comment.
Signed-off-by: Marc Hartmayer <marc@linux.ibm.com>
Reviewed-by: Steffen Eiden <seiden@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Use a trait-based abstraction for CRL downloads so the download
implementation can be replaced more easily, for example in tests that
should avoid real network calls.
Assisted-by: IBM Bob:1.0.5
Signed-off-by: Marc Hartmayer <marc@linux.ibm.com>
Reviewed-by: Steffen Eiden <seiden@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
The comment mentions certificate but it's a certificate revocation
list (CRL). In addition, use 1200 bytes as vector capacity as the
comment says 1200.
Signed-off-by: Marc Hartmayer <marc@linux.ibm.com>
Reviewed-by: Steffen Eiden <seiden@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Although most errors are ignored and the next CRL distribution point is
tried, log the errors as debug messages.
Signed-off-by: Marc Hartmayer <marc@linux.ibm.com>
Reviewed-by: Steffen Eiden <seiden@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
