Rull Deef [Thu, 27 Mar 2025 07:54:46 +0000 (10:54 +0300)]
Fix getsockname() call in Windows localaddr
When querying local IP addresses, local_addr_fallback_kludge() is
called on Windows if gethostname() or gethostbyname() fails. This
function's call to getsockname() erroneously passes a small integer as
the third argument instead of a pointer, likely causing a process
crash. This bug has been present since 1.1, but is rarely encountered
due to the rarity of requests for addressful tickets. Fix it.
Greg Hudson [Mon, 31 Mar 2025 23:01:54 +0000 (19:01 -0400)]
Avoid gss_inquire_attrs_for_mech() null outputs
gss_inquire_attrs_for_mech() can return successfully with *mech_attrs
or *known_mech_attrs set to GSS_C_NO_OID_SET, which is at best
inconvenient as gss_test_oid_set_member() does not allow
GSS_C_NO_OID_SET as an input. Create empty sets instead.
When iakerb_gss_accept_sec_context() processes an initial token which
is not an IAKERB token (because the client already has a service
ticket), set *context_handle. Otherwise subsequent GSS calls using
this context will dereference a null pointer and crash.
[ghudson@mit.edu: moved fix to cleanup handler to avoid code
duplication; added tests; rewrote commit message]
When importing a name to IAKERB, don't add the default realm when we
parse strings. Host-based name imports will continue to use
krb5_sname_to_principal(), which may add a realm from [domain_realm]
but won't add the default realm.
In the IAKERB state machine, query for the service's realm if the
client name doesn't have a realm. To reduce code duplication, make
iakerb_make_token() responsible for saving the token and incrementing
the message count.
[ghudson@mit.edu: added tests; added a discovery state to the machine;
expanded import; adjusted iakerb_make_token() contract; rewrote commit
message]
Julien Rische [Tue, 14 Jan 2025 12:31:11 +0000 (13:31 +0100)]
Add PKINIT paChecksum2 from MS-PKCA v20230920
In 2023, Microsoft updated MS-PKCA to add the optional paChecksum2
element in the PKAuthenticator sequence. This checksum accepts SHA-1,
SHA-256, SHA-384, and SHA-512 digests.
In Windows Server 2025, this checksum becomes mandatory when using
PKINIT with FFDH (but strangely not with ECDH if SHA-1 is configured as
allowed).
[ghudson@mit.edu: refactored crypto interfaces to reduce complexity of
calling code]
Ken Hornstein [Sun, 2 Mar 2025 04:02:58 +0000 (23:02 -0500)]
Use k5_path_join() in krb5int_open_plugin_dirs()
Simplify and improve the portability of krb5int_open_plugin_dirs()
using k5_path_join(). (There is no immediate practical benefit as
this function is only used to find kdb5, authdata, and locate plugin
modules.)
[ghudson@mit.edu: further simplified code; edited commit message]
Ken Hornstein [Fri, 6 Dec 2024 20:29:33 +0000 (15:29 -0500)]
Build PKINIT on Windows
Include PKINIT in the Windows build if OPENSSL_DIR is set.
In the installer, require PKINIT to be built, and require
OPENSSL_VERSION to be set in the environment. Include the PKINIT
module and a copy of the OpenSSL libcrypto DLL.
Document the OpenSSL dependency in windows/README. Also clarify the
Visual Studio dependencies, and restore the prep-windows build step
which was accidentally removed by commit 5f33fb2cf19562175c8271cb9c54a67212f63b93.
[ghudson@mit.edu: added CI and installer integration; edited README
and commit message]
Greg Hudson [Wed, 15 Jan 2025 02:25:51 +0000 (21:25 -0500)]
Add alias support
Add a new kadmin command add_alias. Implement it for DB2 and LMDB by
writing stub principal entries with a tl-data entry giving the target
name. Add libkdb5 functions to create and interpret alias entries.
Handle these stub entries in krb5_db_get_principal(), iteratively
resolving aliases up to a depth of 10.
To allow kadm5_delete_principal() to work on aliases, remove the code
that fetches the entry prior to deletion; it was needed before commit 0780e46fc13dbafa177525164997cd204cc50b51 to decrement the policy
reference count, but now serves no purpose. Adjust kdb_delete_entry()
to translate KRB5_KDB_NOENTRY instead of ignoring it, as we still want
to return KADM5_UNK_PRINC when deleting a nonexistent principal name.
Modify the LDAP KDB module to work with alias entries. In
krb5_ldap_put_principal(), recognize stub alias entries and add an
alias to the object for the target principal. In
krb5_ldap_delete_principal(), don't delete the LDAP object when
deleting an alias name. In krb5_ldap_iterate(), generate stub entries
for each alias name in addition to the populated entry for the
canonical name. A small amount of refactoring was done as part of
this work: the LDAP-specific principal name parsing and unparsing
functions were simplified, and a helper function search_princ() was
added to find the LDAP object for a principal name.
In kdb5_util tabdump, add a dump type "alias" to display a list of
aliases in the database.
Greg Hudson [Wed, 18 Dec 2024 19:52:28 +0000 (14:52 -0500)]
Remove 32-bit build from KfW installer
In src/windows/README, document only the steps for a 64-bit build. In
the installer, expect only 64-bit binaries. In the CI, perform only a
64-bit Windows build, and split up the build into three steps so that
build failures are easier to understand.
Greg Hudson [Wed, 29 Jan 2025 05:22:57 +0000 (00:22 -0500)]
Allow only one salt type per enctype in key data
In the default libkdb5 password change method, omit requested key/salt
combinations that duplicate an earlier encryption type, even if they
have a different salt type. Any use cases for multiple salts for the
same enctype disappeared with single-DES support. (We already have
this behavior for chrand requests.)
If LISTEN_PID and LISTEN_FDS are set in the environment, expect
listener sockets to be present at fds starting at 3. If any match a
configured listener address and port, use it instead of creating a new
socket.
[ghudson@mit.edu: combined two commits; changed sa_compare() to
sa_equal(); restructured changes to fetch listener sockets into a list
once and to match on socket type as well as address; added tests]
Zoltan Borbely [Tue, 28 Jan 2025 21:39:25 +0000 (16:39 -0500)]
Prevent overflow when calculating ulog block size
In kdb_log.c:resize(), log an error and fail if the update size is
larger than the largest possible block size (2^16-1).
CVE-2025-24528:
In MIT krb5 release 1.7 and later with incremental propagation
enabled, an authenticated attacker can cause kadmind to write beyond
the end of the mapped region for the iprop log file, likely causing a
process crash.
[ghudson@mit.edu: edited commit message and added CVE description]
Julien Rische [Thu, 1 Aug 2024 08:56:07 +0000 (10:56 +0200)]
Set missing mask flags for kdb5_util operations
Set KADM5_TL_DATA for the use_mkey and update_princ_encryption
commands. (Commit c877f13c8985d820583b0d7ac1bb4c5dc36e677e did this
for the add_new_mkey and purge_mkeys commands.) Set appropriate flags
for the add_random_key command.
[ghudson@mit.edu: combined two commits; pruned out proposed mask flag
additions for values represented within key data or tl-data (like
KADM5_MKVNO), as those flags are currently only used in the kadm5
protocol, not to communicate with the KDB module]
Greg Hudson [Mon, 30 Dec 2024 01:30:56 +0000 (20:30 -0500)]
Work around Debian cyrus-sasl2 leak in t_kdb.py
A Debian patch to cyrus-sasl2 causes a memory leak in the DIGEST-MD5
plugin. To avoid CI failures in the asan build, suppress leak
detection during the DIGEST-MD5 tests for now.
Greg Hudson [Mon, 23 Dec 2024 22:16:00 +0000 (17:16 -0500)]
Work around llvm-symbolizer LD_LIBRARY_PATH issue
On some platforms llvm-symbolizer is linked against libcurl, which
depends on the krb5 libraries. When the test suite runs programs with
LD_LIBRARY_PATH pointed at asan-compiled krb5 libraries,
llvm-symbolizer will encounter a dynamic linker error at startup,
interfering with the display of stack traces.
In k5test.py, when the build enables asan, wrap the platform's
symbolizer in a script that unsets LD_LIBRARY_PATH (or similar) and
then runs the real llvm-symbolizer.
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.