Isaac Boukris [Sat, 15 Dec 2018 09:56:36 +0000 (11:56 +0200)]
Remove incorrect KDC assertion
The assertion in return_enc_padata() is reachable because
kdc_make_s4u2self_rep() may have previously added encrypted padata.
It is no longer necessary because the code uses add_pa_data_element()
instead of allocating a new list.
CVE-2018-20217:
In MIT krb5 1.8 or later, an authenticated user who can obtain a TGT
using an older encryption type (DES, DES3, or RC4) can cause an
assertion failure in the KDC by sending an S4U2Self request.
[ghudson@mit.edu: rewrote commit message with CVE description]
Greg Hudson [Wed, 1 Aug 2018 19:53:12 +0000 (15:53 -0400)]
Don't include all MEMORY ccaches in collection
In the MEMORY ccache implementation, only yield a cache in the
per-type cursor if it is the context default cache, matching the
behavior of FILE after commit 45360c9688ca963f75a2480f2cf818424fc3dc7b
(ticket 6955).
Greg Hudson [Thu, 25 Oct 2018 15:56:58 +0000 (11:56 -0400)]
Fix leak on error in kadm5 randkey handling
An attempt to change the kadmin/history key with the -keepold flag
would leak the KDB entry and keysalt tuple as it returned an error.
Use the cleanup handler instead of returning directly. Reported by
Bean Zhang.
Isaac Boukris [Sat, 15 Sep 2018 07:28:48 +0000 (10:28 +0300)]
Don't rely on default realm in S4U2Self client
When converting server principal to enterprise name (to be possibly
used for cross-realm), ignore the realm when reparsing, to avoid a
spurious error if a default realm isn't configured.
[ghudson@mit.edu: added rewritten test case; edited commit message]
If gss_add_cred() is called with both an input_cred_handle and an
output_cred_handle, it creates a new credential with the elements of
the input credential plus the requested element. Making a shallow
copy of mechs_array and cred_array from the old credential creates
aliased pointers which become invalid when one of the two credentials
is released, leading to use-after-free and double-free errors.
Instead, make a full copy of the input cred for this case. Make this
copy at the beginning so that union_cred can always be modified in
place (and freed on error using gss_release_cred() if we created it),
removing the need for new_union_cred, new_mechs_array, and
new_cred_array. Use a stack object for target_mechs to simplify
cleanup and reduce the number of failure cases.
GSSAPI provides no facility for copying a credential; since we mostly
use the GSSAPI as our SPI for mechanisms, we have no simple way to
copy mechanism creds when copying the union cred. Use
gss_export_cred() and gss_import_cred() if the mechanism provides
them; otherwise fall back to gss_inquire_cred() and
gss_acquire_cred().
If gss_add_cred() is called with no input_cred_handle, it creates a
new credential with one element. At the end of the function, use the
created credential as the output container, rather than creating a
second one and leaking the first.
If gss_inquire_cred_by_mech() is called with a mechanism and there is
no corresponding mechanism credential in the union cred, return
GSS_S_NO_CRED (as Heimdal does) instead of interrogating the mechanism
about the default credential.
Greg Hudson [Tue, 28 Aug 2018 01:10:53 +0000 (21:10 -0400)]
Check strdup return in kadm5_get_config_params()
When copying the realm string, if strdup() returns NULL, fail out with
ENOMEM instead of pretending the realm wasn't specified. When copying
KRB5_DEFAULT_SUPPORTED_ENCTYPES, if strdup() returns NULL, fail out
with ENOMEM instead of crashing. Reported by Bean Zhang.
Robbie Harwood [Tue, 3 Oct 2017 18:28:47 +0000 (14:28 -0400)]
Correctly handle fallback in KDC OTP callback
In otp_state.c:callback(), avoid invoking the failure callback when we
fall back to the next token. Since request_send() consumes the
request, don't try to free it.
[ghudson@mit.edu: added test case; edited commit message]
Greg Hudson [Tue, 26 Jun 2018 16:47:10 +0000 (12:47 -0400)]
Fix OTP secret file leak and whitespace removal
In read_secret_file() in the OTP kdcpreauth module, add a cleanup
label and free filename on exit. Also fix the whitespace stripping
code to correctly find the end offset, and use size_t rather than int
offsets. The leak was reported by Bean Zhang.
Greg Hudson [Wed, 16 May 2018 04:52:08 +0000 (21:52 -0700)]
Fix option parsing on Windows
Commit 8f9ade8ec50cde1176411085294f85ecfb2820a4 (ticket 8391) moved
the built-in getopt() and getopt_long() implementations from a static
library in util/windows to util/support, where (on Windows) it is
built into k5sprt32.dll or k5sprt64.dll. The getopt() interface uses
global variables opterr, optind, optopt, and optarg, each renamed via
macro to have a k5_ prefix when we use the built-in implementation.
Data objects exported from DLLs need special handling in Windows; they
must be marked as DATA in the DLL .def file, and they must be declared
with "__declspec(dllimport)" in calling code. Without this handling,
optind begins with a garbage value and getopt_long() returns -1
immediately, so client programs always behave as if they have no
arguments.
Stop unnecessarily declaring optind and optarg in client programs.
Declare the getopt() global variables with __declspec(dllimport) on
Windows, except when compiling getopt.c itself. When creating
libkrb5support.exports on Windows (this file is later used by
lib/Makefile.in to create k5sprt32.def), add a DATA tag to the data
objects.
DNS canonicalization can interfere with the fallback tests by changing
"localhost" to have multiple components, or (less likely) changing the
parent domain of foo.krbtest.com or foo.krbtest2.com.
Greg Hudson [Sat, 5 May 2018 17:40:37 +0000 (13:40 -0400)]
Escape curly braces in def-check.pl regexes
Recent versions of Perl issue a warning or error when an unescaped
open curly brace is used in a position where it might introduce a
quantifier in a regular expression. Escape all regexp literal curly
braces in def-check.pl.
A memory ccache iterator stores an alias into the cache object's
linked list of credentials. If the cache is reinitialized while the
iterator is active, the alias becomes invalid. Also, multiple handles
referencing the same memory ccache all use aliases to the same data
object; if one of the handles is destroyed, the other contains a
dangling pointer.
Fix the first issue by adding a generation counter to the cache and to
cursors, incremented each time the cache is initialized or destroyed.
Check the generation on each cursor step and end the iteration if the
list was invalidated. Fix the second issue by adding a reference
count to the cache object, counting one reference for the table slot
and one for each open handle. Empty the cache object on each destroy
operation, but only release the object when the last handle to it is
destroyed or closed.
Add regression tests for the two issues to t_cc.c.
Commit 779a335f4e2deb2d76caf7d0dd3de847a040c050 added the
fail_to_start() helper in ovsec_kadmd.c, accidentally sending the
program name to stderr twice. Remove one of them.
For TGS requests, dispatch() doesn't set state->active_realm, which
leads to a NULL dereference in finish_dispatch() if the reply is too
big for UDP. Prior to commit 0a2f14f752c32a24200363cc6b6ae64a92f81379
the active realm was a global and was set when process_tgs_req()
called setup_server_realm().
Move TGS decoding out of process_tgs_req() so that we can set
state->active_realm before any errors requiring response. Add a test
case.
[ghudson@mit.edu: edited commit message; added test case; reduced code
duplication; removed server handle from process_tgs_req() parameters]
Fabiano Fidêncio [Wed, 28 Mar 2018 16:27:06 +0000 (18:27 +0200)]
Continue after KRB5_CC_END in KCM cache iteration
The KCM server returns KRB5_CC_END in response to a GET_CACHE_BY_UUID
request to indicate that the specified ccache uuid no longer exists.
In krb5_ptcursor_next(), ignore this error and continue the iteration,
as the Heimdal KCM client code does.
In addition to addressing the case where a third party deletes a cache
between the GET_CACHE_UUID_LIST request and when we reach that uuid in
the iteration, this change also fixes a bug in kdestroy -A where the
caller deletes the primary cache and we later request it by uuid when
iterating over the list.
Greg Hudson [Sat, 3 Mar 2018 18:44:00 +0000 (13:44 -0500)]
Fix capaths "." values on client
Commit b72aef2c1cbcc76f7fba14ddc54a4e66e7a4e66c (ticket 6966)
introduced k5_client_realm_path() for use on the client in place of
krb5_walk_realm_tree(), but failed to handle the special case of a
capaths "." value as is done in the latter function. Correct that
omission and add a test case.
Greg Hudson [Tue, 27 Feb 2018 16:56:58 +0000 (11:56 -0500)]
Fix KDC encrypting key memory leak on some errors
Commit 0ba5ccd7bb3ea15e44a87f84ca6feed8890f657d separated the
allocation and destruction of encrypting_key, causing it to leak when
any of the intervening calls jump to the cleanup label. Currently the
leak manifests on transited or authdata failures. Move encrypting_key
destruction to the cleanup label so that it can't leak. Reported by
anedvedicky@gmail.com.
sashan [Tue, 20 Feb 2018 22:03:36 +0000 (23:03 +0100)]
Fix memory leak in KDC PKINIT code
Commit e5c77a11341a79e6af1e5aef7c587a5b75a9e378 introduced a memory
leak of the client public key in server_process_dh(). Free
client_pubkey on success as well as failure.
Greg Hudson [Fri, 12 Jan 2018 16:43:01 +0000 (11:43 -0500)]
Fix flaws in LDAP DN checking
KDB_TL_USER_INFO tl-data is intended to be internal to the LDAP KDB
module, and not used in disk or wire principal entries. Prevent
kadmin clients from sending KDB_TL_USER_INFO tl-data by giving it a
type number less than 256 and filtering out type numbers less than 256
in kadm5_create_principal_3(). (We already filter out low type
numbers in kadm5_modify_principal()).
In the LDAP KDB module, if containerdn and linkdn are both specified
in a put_principal operation, check both linkdn and the computed
standalone_principal_dn for container membership. To that end, factor
out the checks into helper functions and call them on all applicable
client-influenced DNs.
CVE-2018-5729:
In MIT krb5 1.6 or later, an authenticated kadmin user with permission
to add principals to an LDAP Kerberos database can cause a null
dereference in kadmind, or circumvent a DN container check, by
supplying tagged data intended to be internal to the database module.
Thanks to Sharwan Ram and Pooja Anil for discovering the potential
null dereference.
CVE-2018-5730:
In MIT krb5 1.6 or later, an authenticated kadmin user with permission
to add principals to an LDAP Kerberos database can circumvent a DN
containership check by supplying both a "linkdn" and "containerdn"
database argument, or by supplying a DN string which is a left
extension of a container DN string but is not hierarchically within
the container DN.
gcc 7 cannot determine that appdefault_get() always sets *ret_value
when it returns zero, so issues a "may be used uninitialized" warning
in its caller. Set *ret_value at the beginning of the function body
in accordance with current practices.
Modify profile_add_node() to return the existing node, rather than
making a new one, when adding subsection configuration.
This fixes an issue where the first instance of a subsection will hide
the second instance entirely. In particular, it was previously
impossible to split realm-specific configuration across multiple
config files.
[ghudson@mit.edu: adjusted style, added test case]
Greg Hudson [Sat, 11 Nov 2017 18:42:28 +0000 (13:42 -0500)]
Length check when parsing GSS token encapsulation
gssint_get_mech_type_oid() is used by gss_accept_sec_context() to
determine the mechanism of the token. Without length checking, it
might read a few bytes past the end of the input token buffer. Add
length checking as well as test cases for truncated encapsulations.
Reported by Bar Katz.
Greg Hudson [Wed, 11 Oct 2017 17:19:03 +0000 (13:19 -0400)]
Fix default enctype order in docs
Commit 4c234d8754c063177bc627c6298b85020d91c223 added the aes-sha2
enctypes to the documented default enctypes, but in the wrong order.
Reported by Weijun Wang.
If krb5_db_fetch_mkey() prompts for a master key and needs to
determine the kvno, check that the master entry contains any key data
before dereferencing the first element. Reported by Joshua Schaeffer.
Assign status values if S4U2Self padata fails to decode, if an
S4U2Proxy request uses invalid KDC options, or if an S4U2Proxy request
uses an evidence ticket which does not match the canonicalized request
server principal name. Reported by Samuel Cabrero.
If a status value is not assigned during KDC processing, default to
"UNKNOWN_REASON" rather than failing an assertion. This change will
prevent future denial of service bugs due to similar mistakes, and
will allow us to omit assigning status values for unlikely errors such
as small memory allocation failures.
CVE-2017-11368:
In MIT krb5 1.7 and later, an authenticated attacker can cause an
assertion failure in krb5kdc by sending an invalid S4U2Self or
S4U2Proxy request.
After gss_init_sec_context() or gss_accept_sec_context() has created a
context, don't delete the mechglue context on failures from subsequent
calls, even if the mechanism deletes the mech-specific context (which
is allowed by RFC 2744 but not preferred). Check for union contexts
with no mechanism context in each GSS function which accepts a
gss_ctx_id_t.
CVE-2017-11462:
RFC 2744 permits a GSS-API implementation to delete an existing
security context on a second or subsequent call to
gss_init_sec_context() or gss_accept_sec_context() if the call results
in an error. This API behavior has been found to be dangerous,
leading to the possibility of memory errors in some callers. For
safety, GSS-API implementations should instead preserve existing
security contexts on error until the caller deletes them.
All versions of MIT krb5 prior to this change may delete acceptor
contexts on error. Versions 1.13.4 through 1.13.7, 1.14.1 through
1.14.5, and 1.15 through 1.15.1 may also delete initiator contexts on
error.
Greg Hudson [Mon, 26 Jun 2017 21:31:37 +0000 (17:31 -0400)]
Fix kadm5 setkey operation with LDAP KDB
Add mask assignments to kadm5_setv4key_principal() and
kadm5_setkey_principal_4() so that their changes to the principal are
properly written to KDB modules which use the mask flag, such as the
LDAP KDB module. Reported by Frank Lonigro.
Greg Hudson [Mon, 19 Jun 2017 15:30:38 +0000 (11:30 -0400)]
Fix kadm5.acl error reporting
In kadm5int_acl_get_line(), increment *lnp after skipping a blank or
comment line, so that kadm5int_acl_load_acl_file() correctly reports
the line number if it fails to parse a line.
In acl_syn_err_msg, use %.10s to limit the amount of the line included
in the error message, not %10s to left-pad it with spaces if it is
shorter than ten characters.
Tomas Kuthan [Tue, 16 May 2017 09:24:40 +0000 (11:24 +0200)]
Free GSS checksum data deterministically
In the normal course of execution, md5.contents allocated by
kg_checksum_channel_bindings() in make_ap_req_v1() is freed in
make_gss_checksum(). But when there is a failure in
krb5_mk_req_extended() or in make_gss_checksum() before free is
called, the memory leaks.
This patch frees the memory unconditionally in make_ap_req_v1().
Commit b496ce4095133536e0ace36b74130e4b9ecb5e11 (ticket #8268) adds
the clock skew to krb5 acceptor context lifetimes for
gss_accept_sec_context() and gss_inquire_context(), but not for
gss_context_time(). Add the clock skew in gss_context_time() as well.
We apply (as of ticket #7604) a ten-second minimum delay after a TCP
connection is accepted before creating new connections or sending UDP
packets. Apply this timeout to HTTPS connections as well, by removing
the transport check in get_endtime(). As the endtime field is only
set by service_tcp_connect(), it will always have the value 0 for UDP
connection state objects, so there is no need to check the transport
type.
krb5int_utf8cs_to_ucs2les() can read slightly beyond the end of the
input buffer if the buffer ends with an invalid UTF-8 sequence. When
computing the RC4 string-to-key result, make a zero-terminated copy of
the input string and use krb5int_utf8s_to_ucs2les() instead.
If we reach the encrypted challenge clpreauth process method without
an armor key, error out instead of crashing. This can happen if (a)
the KDC offers encrypted challenge even though the request doesn't use
FAST (the Heimdal KDC apparently does this), and (b) we fall back to
that preauth method before generating a preauthenticated request,
typically because of a prompter failure in encrypted timestamp.
Reported by Nico Williams.
The flags field in krb5_lcc_data is not initialized in
krb5_lcc_resolve(), so krb5_lcc_next_cred() can sometimes fail to
include a ticket when retrieving a ccache entry. This results in a
"Request did not supply a ticket" error from k5_make_tgs_req() when
trying to use the credential.
Martin Kittel [Wed, 15 Mar 2017 16:21:28 +0000 (17:21 +0100)]
Fix krb5int_open_plugin_dirs() error handling
In krb5int_open_plugin_dirs(), if constructing filepath fails,
filepath is set to null but accessed a few lines later. Add an error
check before calling krb5int_open_plugin().
Greg Hudson [Fri, 24 Mar 2017 15:07:21 +0000 (11:07 -0400)]
Ignore dotfiles in profile includedir
Editors and filesystems may create artifacts related to .conf files
which don't change the file suffix; these artifacts generally begin
with "." so that they don't appear in normal directory listings
(e.g. ".#filename" for emacs interlock files). Make sure to ignore
any such artifacts when processing a profile includedir directive.
Greg Hudson [Tue, 14 Mar 2017 23:39:38 +0000 (19:39 -0400)]
Force autoconf rebuild in maintainer rules
autoconf normally avoids recreating files that it does not consider
obsolete. Since it knows nothing about patchlevel.h (which we read at
autoconf time using m4's esyscmd()), changes to patchlevel.h won't be
reflected in configure unless another input to configure has changed,
and the maintainer rule will re-run autoconf over and over again. Fix
this issue by passing the force flag to autoconf when we invoke it
from the maintainer rule.
Isaac Boukris [Sat, 4 Mar 2017 19:23:32 +0000 (21:23 +0200)]
Allow null outputs to gss_get_name_attribute()
In krb5_gss_get_name_attribute(), always ask for kvalue and
kdisplay_value when calling krb5_authdata_get_attribute(), as it
currently expect non-null arguments. This change allows applications
to pass GSS_C_NO_BUFFER for the value and display_value output
parameters. (Passing NULL for the authenticated and complete output
parameters already works.)
[ghudson@mit.edu: initialized kvalue and kdisplay_value for safety]
Greg Hudson [Tue, 28 Feb 2017 03:35:07 +0000 (22:35 -0500)]
Fix udp_preference_limit with SRV records
In sendto_kdc:resolve_server() when resolving a server entry with a
specified transport, defer the resulting addresses if the strategy
dictates that the specified transport is not preferred. Reported by
Jochen Hein.
Greg Hudson [Fri, 24 Feb 2017 18:41:53 +0000 (13:41 -0500)]
Fix PKINIT two-component matching rule parsing
In pkinit_matching.c:parse_rule_set(), apply the default relation when
parsing the second component of a rule, not the third. Otherwise we
apply no default relation to two-component matching rules, effectively
reducing such rules to their second components. Reported by Sumit
Bose.
Greg Hudson [Mon, 30 Jan 2017 17:30:51 +0000 (12:30 -0500)]
Document multi-component PKINIT client certs
In pkinit.rst, note that the extensions.client file only works for
single-component client principals, and describe how to modify it for
multi-component principals.
Add an optional method to kdb_vftabl to free e_data pointer in a
principal entry, in case it was populated by a module using a more
complex structure than a single memory region.
[ghudson@mit.edu: handled minor version bump; simplified code; rewrote
commit message]
Greg Hudson [Wed, 18 Jan 2017 16:40:49 +0000 (11:40 -0500)]
Explicitly copy KDB vtable fields
In preparation for bumping the kdb_vftabl minor version, use explicit
field assignments when copying the module vtable to the internal copy,
so that we can conditionalize assignments for minor versions greater
than 0.
Greg Hudson [Wed, 25 Jan 2017 18:07:42 +0000 (13:07 -0500)]
Document default realm and login authorization
Add documentation to host_config.rst describing what the default realm
does. Also add documentation discussing login authorization
configuration, and give an example showing how to give login access to
principals from a realm other than the default realm.
Greg Hudson [Fri, 17 Feb 2017 18:38:19 +0000 (13:38 -0500)]
Add GSSAPI S4U documentation
Describe how a GSS application can perform S4U2Self and S4U2Proxy
requests using the MIT krb5 GSS library. Also add a reference to RFC
7546 at the top, and fix a reference to gssapi_krb5.h.
Greg Hudson [Mon, 26 Dec 2016 20:18:05 +0000 (15:18 -0500)]
Use pktinfo for explicit UDP wildcard listeners
In net-server.c, use pktinfo on UDP server sockets if they are bound
to wildcard addresses, whether that is explicit or implicit in the
address specification.
Greg Hudson [Mon, 26 Dec 2016 20:09:24 +0000 (15:09 -0500)]
Fix KDC/kadmind startup on some IPv4-only systems
getaddrinfo(NULL, ...) may yield an IPv6 wildcard address on IPv4-only
systems, and creating a socket for that address may result in an
EAFNOSUPPORT error. Tolerate that error as long as we can bind at
least one socket for the address.
Tomas Kuthan [Fri, 2 Dec 2016 14:22:54 +0000 (15:22 +0100)]
Add krbPwdPolicy attributes to kerberos.ldif
When LDAP backend support for policy extensions was added by 5edafa0532 (ticket 7223), the kerberos.ldif change neglected to add
the new attributes to krbPwdPolicy.
Greg Hudson [Sun, 27 Nov 2016 23:37:12 +0000 (18:37 -0500)]
Allow slapd path configuration in t_kdb.py
The upstream OpenLDAP installs slapd in libexec, which is not
typically in the path. Also, copying the binary can sometimes cause
it to fail; for instance, in the OpenCSW package,
/opt/csw/libexec/slapd is a script which chooses a binary based on the
system architecture and the path to the script. Allow the test runner
to set the SLAPD environment variable to specify the slapd location
and avoid the copy.
Greg Hudson [Mon, 31 Oct 2016 15:48:54 +0000 (11:48 -0400)]
Make zap() more reliable
The gcc assembly version of zap() could still be optimized out under
gcc 5.1 or later, and the krb5int_zap() function could be optimized
out with link-time optimization. Based on work by Zhaomo Yang and
Brian Johannesmeyer, use the C11 memset_s() when available, then fall
back to a memory barrier with gcc or clang, and finally fall back to
using krb5int_zap(). Modify krb5int_zap() to use a volatile pointer
in case link-time optimization is used.
Greg Hudson [Fri, 28 Oct 2016 14:26:04 +0000 (10:26 -0400)]
Add doxygen comments for RFC 8009, RFC 4757
The aes-sha2 specification has been published as RFC 8009. Add
Doxygen comments to the #defines for its enctype and checksum type
comments. Also add comments for the RC4 enctype and checksum type
constants referring to RFC 4757.
Greg Hudson [Fri, 7 Oct 2016 15:23:02 +0000 (11:23 -0400)]
Clarify krb5_kt_resolve() API documentation
Explicitly say to use krb5_kt_close() like we do for most other
allocating API calls. Note the default type. Instead of saying "The
key table is not opened," say that the keytab file for FILE keytabs is
not opened by this call.
projects / thirdparty / krb5.git / log
