Greg Hudson [Fri, 20 Dec 2024 06:51:24 +0000 (01:51 -0500)]
Fix CI errors on Ubuntu 24.04
Supress a warning in the parser generated by Bison from x-deltat.y.
Convert krb5_ldap_get_age() to use a prototype. Fix memory leaks in
the ASN.1 tests. Check errflg after argument parsing in kdc5_hammer.
Pin the linux-clang job to Ubuntu 22.04 until we can work around an
llvm-symbolizer issue which causes test suite failures.
Ken Hornstein [Thu, 12 Dec 2024 17:56:52 +0000 (12:56 -0500)]
Remove use of -entry flag for DLLs on Windows
Remove the use of the -entry flag to specify a DLL entry point on
Windows, as it is no longer recommended. The default Windows entry
point will call a DllMain() function if one is provided; use that name
in kfwlogon instead of DllEntryPoint().
Greg Hudson [Mon, 2 Dec 2024 19:11:38 +0000 (14:11 -0500)]
Default kdc_tcp_listen to kdc_listen value
If kdc_tcp_listen is not specified in the realm or in [kdcdefaults],
use the same listeners as were given for UDP instead of separately
defaulting to port 88. This change makes the kdc_listen and
kpasswd_listen more consistent, while still allowing UDP and TCP
listening to be separately configured when required for the KDC.
Make the KDC and kadmind listen on UNIX domain sockets if any are
listed in kdc_listen, kadmind_listen, or kpasswd_listen. Send KDC and
kpasswd requests on UNIX domain sockets if any are listed in the kdc
and primary_kdc realm variables.
[ghudson@mit.edu: combined several commits; simplified client side by
treating UNIX domain socket entries like module-generated addresses;
edited commit message]
Add sa2sun() and ss2sun() helpers to socket-utils.h. Add UNIX domain
socket support to sa_socklen() and print_addr(). Expand buffers for
printing addresses to 128 bytes to accomodate the maximum UNIX domain
socket path length.
Add loop_add_unix_socket() to net-server.c, primarily using the
existing TCP support (renamed to "stream").
As there is no standard Kerberos address type for UNIX domain sockets,
add basic directional address support. Add a definition for
ADDRTYPE_DIRECTIONAL in krb5.h. Add private constant krb5_address
objects to libkrb5 for initiator and acceptor directional addresses.
Use directional addresses for the KRB-SAFE/KRB-PRIV source address in
the kprop and password change protocols when the transport is not IPv4
or IPv6.
krb5_address objects are used for auditing purposes in the KDC audit
and KDB pluggable interfaces. Add a local-use address type
ADDRTYPE_UNIXSOCK for use in these cases. Add a flag to
k5_sockaddr_to_address() to indicate whether this address type can be
used. Add UNIX domains socket conversion support to the test audit
plugin module.
[ghudson@mit.edu: combined several commits; used directional addresses
for KRB-SAFE/KRB-PRIV; reduced duplication in net-server.c support;
wrote commit message. Also based on work by Alexander Bokovoy.]
Greg Hudson [Thu, 14 Nov 2024 03:51:20 +0000 (22:51 -0500)]
Refactor libapputils handling of socket addresses
Add private libkrb5 APIs for sockaddr-to-krb5-address conversion and
for printing socket addresses. Use them in the KDC, kadmind,
kprop/kpropd, and trace logging.
In net-server.c, do not convert the local and remote socket addresses
to krb5_address; instead pass them directly to dispatch(). Convert
the addresses where needed in the KDC and kadmind.
Greg Hudson [Sun, 17 Nov 2024 18:54:12 +0000 (13:54 -0500)]
Fix minor logic errors
In k5_externalize_auth_context(), serialize the correct field when
remote_port is set. This is not a reachable bug because the function
is only accessible via gss_export_sec_context(), and the GSS library
does not set a remote port.
In generic_gss_oid_to_str(), remove an inconsistently-applied test for
a null minor_status. Also remove minor_status null checks from
generic_gss_release_oid() and generic_gss_str_to_oid(), but add output
initializations and pointer checks to the API functions in g_oid_ops.c
in a similar manner to other GSSAPI functions. Remove
gssint_copy_oid_set() and replace its one call with a call to
generic_gss_copy_oid_set().
In the checksum functions, avoid crashing if the caller passes a null
key and checksum type 0. An error will be returned instead when
find_cksumtype() can't find the checksum type.
(krb5_k_verify_checksum() already had this check.)
In pkinit_open_session(), remove an unnecessary null check for
ctx->p11_module_name, and add a check for p11name being null due to an
asprintf() failure.
In profile_add_node(), add a check for null ret_node in the duplicate
subsection check. This is not a reachable bug because the function is
currently never called with null ret_node and null value.
In ksu's main(), check for krb5_cc_default_name() returning NULL
(which only happens on allocation failure). Also clean up some
vestiges left behind by commit 9ebae7cb434b9b177c0af85c67a6d6267f46bc68.
In ksu's get_authorized_princ_names(), close login_fp if we fail to
open k5users_path.
In the KDC and kpropd write_pid_file(), avoid briefly leaking the file
handle on write failure.
Ken Hornstein [Wed, 4 Dec 2024 02:54:35 +0000 (21:54 -0500)]
Fix Windows regression in k5_make_uri_query()
Commit d035119c3b2b402f3ad49a4c7b6264826ea923bb introduced an extra
parameter to the function k5_make_uri_query(), but did not add that
parameter to the Windows stub version of this function, causing a null
pointer exception when this function was called. Add the parameter
now.
Ken Hornstein [Fri, 29 Nov 2024 21:32:06 +0000 (16:32 -0500)]
Check for sys/random.h
The function getentropy() is supported on newer versions of MacOS X,
but requires the include file sys/random.h. Check for that and include
it where getentropy() is used.
Greg Hudson [Mon, 18 Nov 2024 23:45:16 +0000 (18:45 -0500)]
Constify some socket-utils.h functions
Accept constant pointers in sa_getport(), sa_is_inet(),
sa_is_wildcard(), and sa_socklen().
Make sa2sin() and sa2sin6() accept and return constant pointers. For
the most part we write to sockaddr_storage and read from sockaddr.
The biggest exception is udppktinfo.c; adjust its internal APIs to use
sockaddr_storage for output and inferred lengths for input. All of
the other exceptions use sa_setport().
Currently setting kdc_listen or kdc_tcp_listen to the empty string
disables listening for UDP and TCP connections respectively, but
setting kadmind_listen or kpasswd_listen to the empty string listens
on the wildcard address. Make the behavior consistent by changing
loop_add_addresses() to add no listeners when the string contains no
tokens. Remove the conditionals from the KDC code.
Document the new behavior of kadmind_listen and kpasswd_listen, and
the existing behavior of kdc_listen.
[ghudson@mit.edu: simplified loop_add_addresses(); combined several
commits and rewrote commit message]
Greg Hudson [Wed, 6 Nov 2024 22:31:37 +0000 (17:31 -0500)]
Prevent undefined shift in decode_krb5_flags()
In the statement "f |= bits[i] << (8 * (3 - i))", bits[i] is
implicitly promoted from uint8_t to int according to the integer
promotion rules (C99 6.3.1.1). If i is 0 and bits[i] >= 128, the
result cannot be represented as an int and the behavior of the shift
is undefined (C99 6.5.7). To ensure that the shift operation is
defined, cast bits[i] to uint32_t.
(f and the function output are int32_t, but the conversion of uint32_t
to int32_t is implementation-defined when the value cannot be
represented, not undefined. We check in configure.ac that the
platform is two's complement.)
Greg Hudson [Sun, 27 Oct 2024 23:01:51 +0000 (19:01 -0400)]
Fix krb5_ldap_list_policy() filtering loop
The loop at the end of this function is intended to ignore ticket
policy DNs that can't be converted to names. But it instead leaves a
hole in the output list if that happens, effectively truncating the
list and leaking any subsequent entries. Use the correct index for
the output list.
Greg Hudson [Mon, 28 Oct 2024 15:51:54 +0000 (11:51 -0400)]
Fix type violation in libkrad
remote.c uses casts to cover up a signature difference between
iterator() and krad_packet_iter_cb. The difference is unimportant in
typical platform ABIs, but calling the function this way is undefined
behavior (C99 6.3.2.8). Fix iterator() to conform to
krad_packet_iter_cb and remove the casts.
Greg Hudson [Sun, 20 Oct 2024 06:09:26 +0000 (02:09 -0400)]
Allow null keyblocks in IOV checksum functions
Null keyblocks are allowed by the libk5crypto checksum functions when
the checksum type is not keyed. However, krb5_c_make_checksum_iov()
and krb5_c_verify_checksum_iov() crash on null keyblock inputs because
they do not check before converting to krb5_key as their non-IOV
variants do. Add the missing null checks.
Greg Hudson [Mon, 21 Oct 2024 23:04:08 +0000 (19:04 -0400)]
Prevent late initialization of GSS error map
Some of the peripheral libgssapi_krb5 utility functions, such as
gss_str_to_oid(), do not access the mechanism list and therefore do
not reach any of the calls to gssint_mechglue_initialize_library().
If one of these functions is called early and produces an error, its
call to map_error() will operate on the uninitialized error map. When
the library is later initialized, any entries added to the error map
this way will be leaked.
To ensure that the error map is initialized before it is operated on,
add library initialization calls to gssint_mecherrmap_map() and
gssint_mecherrmap_get().
In kpasswd_sendto_msg_callback(), if getsockname() does not reveal the
local address, a copy of the first local address's contents is made
and never freed. Instead of making an allocated copy of the address
contents, make a shallow copy of the whole address. Delay freeing the
address array until the end of the function so that alias pointer made
by the shallow copy remains valid.
[ghudson@mit.edu: further simplified code; rewrote commit message]
Commit aa91cb5dbbd4356c7a9069f4f52a10f70d91bc00 added kadmind_listen,
kpasswd_listen, and iprop_listen fields to kadm5_config_params, but
did not add them to the fields freed in kadm5_free_config_params().
Add them now.
Julien Rische [Thu, 22 Aug 2024 15:15:50 +0000 (17:15 +0200)]
Generate and verify message MACs in libkrad
Implement some of the measures specified in
draft-ietf-radext-deprecating-radius-03 for mitigating the BlastRADIUS
attack (CVE-2024-3596):
* Include a Message-Authenticator MAC as the first attribute when
generating a packet of type Access-Request, Access-Reject,
Access-Accept, or Access-Challenge (sections 5.2.1 and 5.2.4), if
the secret is non-empty. (An empty secret indicates the use of Unix
domain socket transport.)
* Validate the Message-Authenticator MAC in received packets, if
present.
FreeRADIUS enforces Message-Authenticator as of versions 3.2.5 and
3.0.27. libkrad must generate Message-Authenticator attributes in
order to remain compatible with these implementations.
[ghudson@mit.edu: adjusted style and naming; simplified some
functions; edited commit message]
Arjun [Fri, 11 Oct 2024 03:22:52 +0000 (08:52 +0530)]
Fix potential PAC processing crash
An input to krb5_pac_parse() with a zero-length buffer at the end of
the PAC can cause an assertion failure in k5_pac_locate_buffer() due
to an off-by-one error. Correct the assertion.
Arjun [Thu, 10 Oct 2024 19:07:52 +0000 (00:37 +0530)]
Disable include and includedir in fuzzing build
When building for fuzz teting, ignore "include" and "incluedir"
directives in the profile library's parse_line(), to prevent it from
trying to open non-existent paths generated by the fuzzing library.
Alexey Tikhonov [Fri, 4 Oct 2024 16:00:21 +0000 (18:00 +0200)]
Avoid mutex locking in krb5int_trace()
Trace logging doesn't need unique timestamps, so the locking within
krb5_crypto_us_timeofday() makes trace logging slower for no reason.
Add a new helper k5_us_timeofday(), which is merely a wrapper around
the existing get_time_now(), and use it in krb5int_trace().
Alexey Tikhonov [Thu, 3 Oct 2024 16:40:04 +0000 (18:40 +0200)]
Fix krb5_crypto_us_timeofday() microseconds check
Commit a60db180211a383bd382afe729e9309acb8dcf53 mistakenly reversed
the sense of the krb5_crypto_us_timeofday() conditional that enforces
fowards movement of the microseconds value within a second. Moreover,
the macros ts_after() and ts_incr() should not have been applied to
non-timestamp values. Revert the incorrect changes.
In klists's show_credential(), ensure that the column counter doesn't
decrease if printf() fails.
In process_k5beta7_princ(), bounds-check the e_length field.
In ndr_enc_delegation_info(), initialize b so it is always valid for
the cleanup handler.
In krb5_dbe_def_decrypt_key_data(), change the flow control so ret is
always set by the end of the function. Return KRB5_KDB_INVALIDKEYSIZE
if there isn't enough data in the first key_data_contents field or if
the serialized key length is invalid.
In svcauth_gss_validate(), expand rpchdr to accomodate the header plus
MAX_AUTH_BYTES.
In svcudp_reply(), change slen to unsigned to match the return type of
XDR_GETPOS() and eliminate an unnecessary check for slen >= 0.
In krb5int_pthread_loaded()(), remove pthread_equal() from the weak
symbol checks. It is implemented as an inline function in some glibc
versions, which makes the comparison "&pthread_equal == 0" always
false.
[ghudson@mit.edu: further modified krb5_dbe_def_decrypt_key_data() for
clarity; added detail to commit message]
Greg Hudson [Thu, 29 Aug 2024 19:25:54 +0000 (15:25 -0400)]
Block library unloading to avoid finalizer races
Library finalizers can run due to the library being unloaded or the
process exiting. If the library is being unloaded, global memory
resources must be released to avoid memory leaks. But if the process
is exiting, releasing memory resources can lead to race conditions if
another thread invokes functions from the library during or after
finalizer execution. Most commonly this manifests as an assertion
error about trying to lock a destroyed mutex.
We can block unloading of our library on ELF platforms by passing "-z
nodelete" to the linker. Add a shell variable "lib_unload_prevented"
to the shlib.conf outputs, set it on platforms where we can block
unloading, and suppress finalizers when it is set.
On Windows we can detect if the process is exiting by checking for a
non-null lpvReserved argument in DllMain(). Do not execute finalizers
when the process is executing.
Greg Hudson [Thu, 15 Aug 2024 15:30:07 +0000 (11:30 -0400)]
Replace Windows installer FilesInUse dialog text
Windows installers must provide a FilesInUse dialog, with the intent
of minimizing the need to restart the system after software
installations. For reasons related to the LSA cache, the KfW
installer always schedules a post-installation reboot (see commit 50a3c3cbeab32577fba2b21deb72a64015c48ec7). Reword the FilesInUse
dialog text to recommend clicking Ignore, causing the conflicting
files to replaced on the reboot. (The affected files are likely C
runtime DLLs, and the existing versions will almost certainly suffice
if KfW programs are run prior to a reboot.)
Greg Hudson [Mon, 24 Jun 2024 19:46:50 +0000 (15:46 -0400)]
Refactor GSS per-message token parsing
Replace kg_unseal_v1() and gss_krb5int_unseal_token_v3() with new
functions using current coding practices. Notable differences
include:
* The new functions use k5input for improved safety.
* The new functions do not modify the input buffer.
* The new functions will never try to allocate zero bytes of memory.
* There are separate functions for unwrap and verify_mic, which means
there is no message_buffer parameter acting conditionally as an
input or output.
Commit 1b57a4d134bbd0e7c52d5885a92eccc815726463 made the KDC stop
issuing des3 and rc4 session keys by default. To make the des3 and
arcfour passes of the test suite work, it added aes256-sha1 to the
permitted enctypes for those passes. Negotiating AES session keys
reduces coverage of pre-CFX GSSAPI code and other uses of the older
enctypes. Instead set the enable_des3 and enable_rc4 variables.
Greg Hudson [Wed, 19 Jun 2024 22:55:35 +0000 (18:55 -0400)]
Refactor GSS token header parsing
Rewrite g_verify_token_header() to use k5input and not to handle
two-byte token IDs (which are specific to the krb5 mechanism). Add a
more general g_get_token_header() which can handle detached wrappers
and cases where the mech is not known in advance. Adjust all current
callers, and get rid of the duplicate DER parsing code added in commit b0a2f8a5365f2eec3e27d78907de9f9d2c80505a.
In k5-input.h, split out tag and length parsing into
k5_der_get_taglen(), needed by g_get_token_header().
draft-ietf-kitten-iakerb-03 section 3.1 specifies a way for the
initiator to query the acceptor's realm. Implement this facility in
the IAKERB acceptor.
Add krb5_get_default_config_files() to the public API; it was already
in the library export list and the DLL export list. Also add
krb5_free_config_files().
Commit b0031448502561da31fb8c2543c8b01d7df9a872 removed the only
consumers of util_set.c. Also remove declarations for g_strdup() and
g_local_host_name(), which were unused as far back as krb5-1.0.
Greg Hudson [Thu, 27 Jun 2024 11:25:21 +0000 (07:25 -0400)]
Change krb5_get_credentials() endtime behavior
Historically, krb5_get_credentials() uses in_creds->times.endtime both
as the TGS request endtime and as a cache lookup criterion. These
uses are in conflict; setting a TGS request endtime can only serve to
limit the maximum lifetime of the issued ticket, while a cache lookup
endtime restricts the minimum lifetime of an acceptable cached ticket.
The likely outcome is to never use a cached ticket, leading to poor
performance as we add an entry to the cache for each request.
Change to the Heimdal behavior of using in_creds->times.endtime only
as the TGS request endtime.
Greg Hudson [Mon, 24 Jun 2024 00:10:44 +0000 (20:10 -0400)]
Adjust removed cred detection in FILE ccache
In the FILE ccache, consider a cred to be removed if it has endtime 0
and authtime non-zero, instead of specifically authtime -1. This
change will let us filter out normal credentials deleted by Heimdal,
although not synthetic credentials such as config entries.
Greg Hudson [Fri, 14 Jun 2024 14:56:12 +0000 (10:56 -0400)]
Fix vulnerabilities in GSS message token handling
In gss_krb5int_unseal_token_v3() and gss_krb5int_unseal_v3_iov(),
verify the Extra Count field of CFX wrap tokens against the encrypted
header. Reported by Jacob Champion.
In gss_krb5int_unseal_token_v3(), check for a decrypted plaintext
length too short to contain the encrypted header and extra count
bytes. Reported by Jacob Champion.
In kg_unseal_iov_token(), separately track the header IOV length and
complete token length when parsing the token's ASN.1 wrapper. This
fix contains modified versions of functions from k5-der.h and
util_token.c; this duplication will be cleaned up in a future commit.
CVE-2024-37370:
In MIT krb5 release 1.3 and later, an attacker can modify the
plaintext Extra Count field of a confidential GSS krb5 wrap token,
causing the unwrapped token to appear truncated to the application.
CVE-2024-37371:
In MIT krb5 release 1.3 and later, an attacker can cause invalid
memory reads by sending message tokens with invalid length fields.
The initial implementation of IAKERB in MIT krb5 mistakenly used
draft-zhu-ws-kerb instead of draft-kitten-ietf-iakerb, and
additionally used the wrong ASN.1 tag value for the target-realm field
of the IAKERB-HEADER sequence. Correct the following aspects of the
protocol implementation:
* Require and use framing on all messages, not just the initial
context token.
* Use extension value 2 for the finish message instead of 1.
* Use key usage value 41 instead of 42 for the finish message
checksum.
* Use UTF8String (12) for target-realm instead of OCTET STRING (4).
With these changes, the IAKERB implementation is interoperable with
other krb5 implementations, but not with the implementation before
these changes.
Section 5.19 of RFC 2744 (about gss_init_sec_context) states,
"Initially, the input_token parameter should be specified either as
GSS_C_NO_BUFFER, or as a pointer to a gss_buffer_desc object whose
length field contains the value zero." In iakerb_initiator_step(),
handle both cases when deciding whether to parse an acceptor message.
Greg Hudson [Tue, 21 May 2024 23:10:50 +0000 (19:10 -0400)]
Fix recently-introduced profile parsing bugs
When parsing a "}", do not ascend to the parent node if we are still
within a discarded section after decrementing group_level, as we did
not descend into a child node at the beginning of the subsection.
(Discovered by OSS-Fuzz.)
Also adjust the level check to take into account the shifted meaning
of state->group_level, so that we properly reject a "}" within a
top-level section.
Nicolas Williams [Thu, 13 May 2021 05:43:26 +0000 (00:43 -0500)]
Support site-local KDC discovery via DNS
Add the sitename realm variable. If set, service location via DNS
will be attempted using the site name as specified in [MS-ADTS]
6.3.2.3, falling back to regular discovery on failure.
[ghudson@mit.edu: made this strictly a realm variable; moved
k5_get_sitename() to locate_kdc.c and made it take a krb5_data input;
fixed a memory leak; corrected documentation changes; fleshed out
commit message]
The Microsoft KERB_AP_OPTIONS_CBT extension (defined in [MS-KILE]
3.2.5.8) allows the client to request strict enforcement of GSS
channel bindings. Client support for this extension was added in
commit 225e6ef7f021cd1a8ef2a054af0ca58b7288fd81 (ticket 8900) but it
requires a configuration variable to be set. The choice to include
the extension should be made by the client application code, as it is
a promise to include channel bindings when operating within TLS.
In libkrb5, add an option AP_OPTS_CBT_FLAG to make
krb5_mk_req[_extended]() include KERB_AP_OPTIONS_CBT. In the GSS
initiator code, set this flag when the GSS_C_CHANNEL_BOUND flag is
included in the request options. GSS_C_CHANNEL_BOUND was introduced
in commit 429a31146083fac21958631c2af572b08ec91022 (ticket 8899) as an
acceptor output flag.
The profile library has two deconstructors, profile_release() and
profile_abandon(). profile_release() flushes in-memory changes to
backing files, while profile_abandon() does not. If a krb5_context
profile contains in-memory changes, they were copied from a profile
supplied to krb5_init_context_profile(), and the caller can decide
whether to flush them.
As profile_copy() is now a public function, remove the include of
prof_int.h and the associated LOCALINCLUDES setting in Makefile.in.
Replace the current implementation of profile_copy() with one that
copies the in-memory tree structure of non-shared data objects. Make
profile_copy() a public function.
The profile library normally attempts to reload a profile data tree if
the backing file has changed. Reloading a dirty profile object
discards any modifications made by the caller. If we assume that the
modifications are destined to be flushed back out to the backing file,
then there is no good answer--one or the other set of changes will be
lost. But the caller may have a different intended use for the
modified tree (profile_flush_to_file(), profile_flush_to_buffer(),
krb5_init_context_profile()), for which the caller's modifications may
be critical. Avoid discarding in-memory edits to ensure the
correctness of these use cases.
When parsing a file, ignore sections appearing after a final-flagged
section of the same name. Adjust the meaning of group_level in the
parser state so that it is 1 inside of top-level sections instead of
0, and simplify the addition of top-level sections to the tree by
relying on profile_add_node()'s section merging.
Make the final flag work for relations as well as sections. Check it
while parsing via a new check_final parameter in profile_add_node(),
and during iteration.
Output final flags for relations in dump_profile(). Make the final
flag available to it via a new output parameter in
profile_find_node_relation().
Greg Hudson [Sun, 31 Mar 2024 16:30:18 +0000 (12:30 -0400)]
Allow modifications of empty profiles
Add the notion of a memory-only prf_data_t object, indicated by an
empty filespec field and appropriate flags (do not reload, always
dirty, not part of shared trees). Do nothing when flushing a
memory-only data object to its backing file. When setting up an empty
profile for read/write access, create a memory-only data object
instead of crashing.
Move prf_data_t mutex initialization into profile_make_prf_data(),
simplifying its callers.
Commit bdcd6075bd4593c8f67722ce075c9519faec58b7 uses
EVP_PKEY_get_base_id(), which was added in OpenSSL 3.0. Add a
compatibility macro to use the old name for OpenSSL 1.0 and 1.1.
Commit 0f870b1bcad960fd5319a3f97aafd7f4a289e2fb added ECDH support,
but did not change the OpenSSL 1.0 versions of encode_spki(),
decode_spki(), or generate_dh_pkey() to work with elliptic curve
public keys. In each function, check the key type and skip the
DH-specific handling for key types other than DH.
Greg Hudson [Wed, 20 Mar 2024 21:17:50 +0000 (17:17 -0400)]
Improve error message for DES kadmin/history key
If the kadmin/history entry contains an unsupported encryption type,
produce a better error message than "Bad encryption type". Reuse the
error code KADM5_BAD_HIST_KEY (unused since release 1.8). Non-updated
kadmin clients will report the message "Password history principal key
version mismatch", which at least points in the direction of password
history.
Greg Hudson [Tue, 12 Mar 2024 16:45:24 +0000 (12:45 -0400)]
Fix type mismatches detected by LTO
Building with link-time optimization reveals some type mismatches in
the interface between libkrb5 serialization and the profile library,
as well as in consumers of the SS library. Fix them. Reported by Eli
Schwartz.
Greg Hudson [Wed, 6 Mar 2024 00:53:07 +0000 (19:53 -0500)]
Fix two unlikely memory leaks
In gss_krb5int_make_seal_token_v3(), one of the bounds checks (which
could probably never be triggered) leaks plain.data. Fix this leak
and use current practices for cleanup throughout the function.
In xmt_rmtcallres() (unused within the tree and likely elsewhere),
store port_ptr into crp->port_ptr as soon as it is allocated;
otherwise it could leak if the subsequent xdr_u_int32() operation
fails.
Greg Hudson [Tue, 5 Mar 2024 22:38:49 +0000 (17:38 -0500)]
Fix leak in KDC NDR encoding
If the KDC tries to encode a principal containing encode invalid UTF-8
sequences for inclusion in a PAC delegation info buffer, it will leak
a small amount of memory in enc_wchar_pointer() before failing. Fix
the leak.
Greg Hudson [Wed, 21 Feb 2024 20:29:02 +0000 (15:29 -0500)]
Support PKCS11 EC client certs in PKINIT
Move the digest computation and DigestInfo encoding from
cms_signeddata_create() to pkinit_sign_data_pkcs11(), and
conditionalize the DigestInfo encoding on the key type. Use CKM_ECDSA
instead of CKM_RSA_PKCS for EC keys, and convert the resulting
signature from the PKS11 encoding to the ASN.1 encoding required by
CMS.
Regenerate the test certificates with an additional EC client cert.
Add test cases for EC client certs with and without PKCS11.
Greg Hudson [Fri, 9 Feb 2024 22:57:40 +0000 (17:57 -0500)]
Correct PKINIT EC cert signature metadata
When generating CMS SignedData in PKINIT, check the certificate's
public key type and set the signatureAlgorithm field appropriately.
(This field is currently ignored by OpenSSL when verifying CMS
SignedData.)
Greg Hudson [Fri, 9 Feb 2024 22:32:40 +0000 (17:32 -0500)]
Simplify PKINIT cert representation
In the _pkinit_identity_crypto_context structure, the my_certs field
is a stack which only ever contains one cert and is only ever used to
retrieve that one cert. The cert_index field is always 0. Replace
these fields with a my_cert field pointing directly to the X509
certificate.
Simplify crypto_cert_select_default() by making it call
crypto_cert_select() with index 0 after verifying the certificate
count.
Greg Hudson [Tue, 27 Feb 2024 00:03:38 +0000 (19:03 -0500)]
Use SoftHSMv2 for PKCS11 PKINIT tests
Instead of softpkcs11, use SoftHSMv2 to mock the PKCS11 token for
PKINIT tests. Use pkcs11-tool from OpenSC to initialize the token and
import a certificate and key. SoftHSM does not support PIN-less
tokens (see https://github.com/opendnssec/SoftHSMv2/issues/480) so
remove that test for now.
Steffen Kieß [Tue, 13 Feb 2024 17:39:27 +0000 (18:39 +0100)]
Avoid strict-prototype compiler errors
Commit 4b9d7f7c107f01a61600fddcd8cde3812d0366a2 added the
-Werror=strict-prototypes parameter to the build process, but left
behind 28 function definitions using "()" instead of "(void)". Most
of these definitions could not cause compiler errors for various
reasons (such as an accompanying prototype), but a few could cause
errors in gcc depending on the build configuration.
For consistency and safety, add "(void)" to all 28 definitions.
Greg Hudson [Wed, 24 Jan 2024 22:33:18 +0000 (17:33 -0500)]
Fix NOTICE generation and regenerate it
In conf.py, exclude the formats directory (added in commit 68ac7ac1f1a1d2939a2c99fa49cecd734614d16d) when building notice.txt, to
prevent a "document isn't included in any toctree" warning.
Julien Rische [Mon, 8 Jan 2024 15:52:27 +0000 (16:52 +0100)]
Remove klist's defname global variable
Addition of a "cleanup" section in kinit's show_ccache() function as
part of commit 6c5471176f5266564fbc8a7e02f03b4b042202f8 introduced a
double-free bug, because defname is a global variable. After the
first call, successive calls may take place with a dangling pointer in
defname, which will be freed if krb5_cc_get_principal() fails.
Convert "defname" to a local variable initialized at the beginning of
show_ccache().
Greg Hudson [Sat, 2 Dec 2023 00:40:02 +0000 (19:40 -0500)]
Refactor PKINIT KDF internal interfaces
Simplify the client and server PKINIT code by renaming
pkinit_alg_agility_kdf() to pkinit_kdf() and making it do RFC 4556
octet2string if alg_oid is null. Move responsibility for tracing
inside the new interface. Constify some parameters and remove some
unnecessary casts. Rename "key" to "secret" in several internal
functions to avoid confusion between the input DH secret and the
output key.
Greg Hudson [Sat, 25 Nov 2023 16:04:56 +0000 (11:04 -0500)]
In PKINIT, check for null PKCS7 enveloped fields
The PKCS7 ContentInfo content field and EncryptedContentInfo
encryptedContent field are optional. Check for null values in
cms_envelopeddata_verify() before calling pkcs7_decrypt(). Reported
by Bahaa Naamneh.
unknown [Tue, 24 Oct 2023 01:29:14 +0000 (18:29 -0700)]
Make def-check.pl work with Windows git-bash perl
The version of Perl included in git-bash does not translate line
endings or filter out the end-of-file marker when reading from files
in text mode. Adjust def-check.pl to work in this environment.
Greg Hudson [Fri, 27 Oct 2023 04:44:53 +0000 (00:44 -0400)]
End connection on KDC_ERR_SVC_UNAVAILABLE
In sendto_kdc.c:service_fds(), if a message handler indicates that a
message should be discarded, kill the connection so we don't continue
waiting on it for more data.
projects / thirdparty / krb5.git / log
