ECB, XTS encrypt and decrypt as well as CBC decrypt check that the buffer
is at least as large as the input and return CKR_BUFFER_TOO_SMALL if not,
but they do not update *out_data_len with the required buffer size.
This applies in the same way to to AES, DES, and DES3.
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
In aes_key_wrap_decrypt() when the PKCS#7 unpadding fails, cleanse the
output buffer to not leak decrypted data in case of a failure.
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
As per PKCS#7, the pad bytes must all equal the padding value, i.e. the number
of bytes to pad. Every byte that could be padding is examined, so the time
taken does not depend on the padding. Also make sure that the padding
value is not larger than the block size.
Suggested-by: Alexandre Laroche <larochealexandre@hotmail.com>
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Fix the AES/DES/DES3-CBC-PAD decryption handling to not produce more output
than allowed as per output buffer length.
Also check for errors during strip_pkcs_padding() to detect padding errors
and fail the operation.
With the buffer size checking, the IV must be saved before the actual
operations and must be restored in case of a CKR_BUFFER_TOO_SMALL error.
The operation might update the running IV in the context. However, the
caller must be able to retry with the original IV intact using a large
enough output buffer.
Suggested-by: Alexandre Laroche <larochealexandre@hotmail.com>
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
For Edwards and Montgomery private keys RFC 8410 defines that the private
key is an OCTET-STRING, and not a SEQUENCE as for EC keys (RFC 5915).
Fix this by encoding and decoding the correct PKCS#8 format. For decoding,
Edwards and Montgomery private keys encoded using the wrong EC-key style are
still accepted for backward compatibility.
Fixes: 3fd5120a41
Reported-by: Alexandre Laroche <larochealexandre@hotmail.com>
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
secret_key_unwrap() and priv_key_unwrap() set CKA_SENSITIVE to FALSE and
CKA_EXTRACTABLE to TRUE on every unwrapped key, after the caller's
template, and the unwrapping key's CKA_UNWRAP_TEMPLATE, had already been
validated and merged into the new object. Both were silently overwritten:
- a template asking for CKA_SENSITIVE = TRUE and CKA_EXTRACTABLE = FALSE
gave a key whose value could be read in clear with
C_GetAttributeValue(CKA_VALUE) and wrapped out again;
- an unwrapping key whose CKA_UNWRAP_TEMPLATE requires CKA_SENSITIVE =
TRUE or CKA_EXTRACTABLE = FALSE did not get that policy applied to the
keys unwrapped with it.
C_UnwrapKey returned CKR_OK in both cases.
Stop overwriting these two attributes. object_create_skel() already
starts the object from the class defaults (CKA_SENSITIVE = FALSE,
CKA_EXTRACTABLE = TRUE), lets the token's t_set_attrs_for_new_object()
supply its own defaults, and merges the template over them. CKA_LOCAL,
CKA_ALWAYS_SENSITIVE and CKA_NEVER_EXTRACTABLE are still set to FALSE, as
PKCS#11 requires for an unwrapped key.
For a template that does not specify the two attributes, under an
unwrapping key without a CKA_UNWRAP_TEMPLATE, tokens that keep the class
defaults give the same key as before. Tokens that default CKA_SENSITIVE
to TRUE (EP11 and CCA for private keys, and for secret keys when
FORCE_SENSITIVE is enabled) now apply that default to unwrapped keys
too, as they already do for generated and derived keys; until now the
unwrap path overwrote it with FALSE.
Signed-off-by: Alexandre Laroche <larochealexandre@hotmail.com>
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
If the source has an empty context, then also set the context to empty on the
target before returning early. Any left over context pointers in the target
might otherwise be freed later when the copy is freed.
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
When the deep-copy if the EC-AGGREGATE mechanism parameter fails, cleanup
the operation before returning.
The EC-AGGREGATE deep-copy function must NULLify the pAggrElements field
before attempting to copy. The destination starts as a shallow memcpy of
the source, so it held the caller's pointers. If a malloc failed partway
through, it would have freed those caller-owned buffers.
While at it, move the deep-copy case into the already existing switch
above inside the mechanism parameter guard. Also check the size of the
mechanism parameter to be the expected length before deep-copying it.
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
In case deep-copying the ML-DSA mechanism parameter fails, don't continue,
but go to the done label.
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Saving the operation state via C_SetOperationState() only works for
mechanisms with mechanism parameters that can be serialized. Thus, mechanisms
that use a mechanism parameter that gets deep-copied into the context can
not be saved. Set the state_unsaveable flag for those mechanisms. This
will cause C_SetOperationState() to fail with with CKR_STATE_UNSAVEABLE.
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Some mechanisms keep a deep copy of its mechanism parameter in the operation
context. Normal operation cleanup deep-frees them properly, but session_free()
and session_mgr_close_session() do not deep-free them, but just free the outer
structure, leaking any inner allocations. This happens when a session is
closed that has a still active operation with such a mechanism.
Fix this by directing the cleanup to the regular operation cleanup helpers
that already do it right.
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Functions der_decode_ECPrivateKey() and der_decode_ECPublicKey() can be called
to decode a EC, Edwards, or Montgomery key. For EC, the OID of the curve is
the param in the AlgorithmIdentifier, and for Edwards and Montgomery the OID
in the AlgorithmIdentifier specifies the curve OID directly. Check that the
OID is one of the known curves for EC, and one of the expected Edwards or
Montgomery OIDs for Edwards and Montgomery respectively.
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Function ber_decode_SPKI() extracts the OID and the parameters from the
AlgorithmIdentifier. It assumes that the BER encoded OID has a single byte
length field. While this is the case for all used OIDs, an malformed
SPKI could have an AlgorithmIdentifier that has a multi-byte length
OID. Decode the field length and do not assume a single byte length.
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
The decoding functions for public and private keys compare the decoded
AlgorithmIdentifier from the key material with constants to check for valid
key types. This check does not check the length of the decoded
AlgorithmIdentifier and thus might overrun the buffer producing an
out-of-bounds read for too short AlgorithmIdentifiers.
Reported-by: Stuart Thomas <stuart.thomas@triageforge.co.uk>
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
ICSF only accepts uncompressed EC Points when creating an EC public key
object, as well as for ECDH derive as public data.
Convert compressed or raw EC points to the uncompressed form before passing
it to ICSF.
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Make sure that C_Login, C_SetPIN, C_InitPIN, C_InitToken do not read or
update any token files or any field in the token shared memory without
holding the XProcLock.
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Use a common SPKI encoding function for all public key encoding functions,
instead of having each public key encoding function manually encode the
SPKI.
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Refactor the code to encode and decode the BER primitives. There was lots of
duplicated code in these functions. This is now refactored out into separate
helper functions.
Also the TLV encoding and decoding in these helpers have been refactored to
use a more streamlined implementation without many similar looking if cases.
With this, encoding and decoding data with more than 3 length octets (16 MB)
is now supported.
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
The use of liblber has been eliminated for most of the cases already.
The only leftover cases are in RSA, DSA, and DH public key encoding
functions for encoding BIT STRINGs.
Update these leftover places and use the already existing
ber_encode_BIT_STRING() function instead.
With this, '-lber' can be removed from the make files for most of
the tokens and tools. The ICSF token still requires liblber for its
LDAP communication.
Also adjust the configure script and example RPM spec accordingly.
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Refactor the code to use file descriptor operations relative to the directory
(e.g. openat(), unlinkat(), fstatat(), renameat()) instead of path based
operations. This is safer against TOCTOU issues and symlink-following
attacks where the files or directories are replaced after the check, but
before the path based operation on it.
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
As per PKCS#11, CKA_EXTRACTABLE and CKA_SENSITIVE can be set to any
value during an unwrap operation. The current code does not allow this.
Also make sure to set CKA_NEVER_EXTRACTABLE to FALSE when CKA_EXTRACTABLE
is set to TRUE. This was already fixed for private keys in commit
0462717a8d but was missed for secret keys.
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Restrict the token name (tokname) and config name (confname) that is
specified in opencryptoki.conf to only contain certain characters.
They must not be '.' or '..', and must not contain '/'.
If the confname specified an absolute path, no restriction is applied.
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
The ICSF token name is retrieved from LPAP, and could theoretically contain
characters like '..' or '/' that could cause the config file name to be
created outside of the config directory. The token name must not be '.' or
'..', and must not contain '/'.
Create relative names for the confname field in the config entry.
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Prevent integer overflows when calculating the length of a buffer to allocate.
An overflow would cause a too short buffer to be allocated causing an
out of bounds access later.
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Check the range of the ulCounterBits field in the CK_AES_CTR_PARAMS
mechanism parameter structure to be 0 < ulCounterBits < 128.
Remove a bogus overflow check for ulCounterBits in the final functions.
This does not make any sense, because ulCounterBits is not modified,
and thus can not overflow, especially not when it is in range.
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
The old data format only allows to store encrypted object data up to
UINT32_MAX - 5 bytes. Refuse to save if the data is larger.
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
The pub_len is determined at the length-only first pass, but is then reused
for the second pass as well. Use the length from the freshly decoded BITSTRING
instead of reusing pub_len from above for the final ber_encode_SEQUENCE call.
It should result in the same value, but it is clearer to anywhere use the
same value.
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Cleanse protected keys or CPACF param blocks containing protected keys
on stack when no longer used.
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
When opening a directory for iterating over the files contained in it, use
open() with O_DIRECTORY and O_NOFOLLOW, followed by fdopendir() instead of
the plain opendir() to protect from symlink-following on the directory itself.
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Create the lock files using O_NOFOLLOW to prevent symlink-following.
Lock files are just opened but no data is written to it or read from it.
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Open the trace file with O_NOFOLLOW to prevent symlink-following attacks.
An attacker who can create a symlink at /var/log/opencryptoki/trace.<pid>
before the process opens it can redirect arbitrary writes.
However, trace is only written when it is enabled via environment variable
OPENCRYPTOKI_TRACE_LEVEL, and is disabled by default. Thus an attack is
not practically possible.
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
On some versions of OpenSSL, EVP_CIPHER_CTX_get_updated_iv() might push
an error like "000003FF8C3F3EA0:error:07800081:common libcrypto routines:
set_ptr_internal:param of incompatible type:crypto/params.c:1506:" to
the OpenSSL error stack, see https://github.com/openssl/openssl/issues/27117
This has been fixed in OpenSSL, but in case we are running with an OpenSSL
version that still has the bug, remove the error entry in case
EVP_CIPHER_CTX_get_updated_iv() returned success.
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
The public key material is a BIT STRING, so use the correct function to
determine its length. This avoids the opaque +1 to account for the
unused-bits byte that is part of a BIT STRING. Also check for overflows
when adding up the length.
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Check the user supplied ul*SizeInBits values to be within the allowed range.
This avoids overflow errors.
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Check the length of the to be derived key and the shared data to not
overflow when allocating a buffer.
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Check the length of the derived key and the shared data to not overflow when
building the hash context.
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
A very large data length in the encoding functions might cause an overflow.
Both INTEGER as well as BIT_STRING add padding as part of the encoding, so
the data length must be small enough that also the padding fits.
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
The calculation for the len2 might overflow, so check the input
length (len1) before the calculation to avoid such overflows.
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Check the length using subtraction instead of addition to avoid unsigned
overflow during addition.
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
