Tom Yu [Thu, 25 Aug 2016 22:02:56 +0000 (18:02 -0400)]
Avoid byte-swap cache corruption in libdb2
Apply a patch from NetBSD to restore the cached copy of a page to the
machine byte order after a write operation swapped it to the file byte
order. As a regression test, modify test13 to sync the database file
after each put to exercise this bug.
Tom Yu [Fri, 26 Aug 2016 22:40:49 +0000 (18:40 -0400)]
Add known file test case for btree byte swap
Add a regression test for the preceding bugfix. This test uses btree
database files with known contents and byte orders with short keys and
overflow data items.
Tom Yu [Fri, 26 Aug 2016 19:24:52 +0000 (15:24 -0400)]
Fix btree byte swapping for overflow data
When operating on a btree database file of the opposite endianness,
libdb2 was swapping the wrong bytes if a record had a short key but
overflow data. Fix this bug by not incrementing p when swapping a
P_BIGKEY overflow pointer, and by always skipping the full key size
before swapping a P_BIGDATA overflow pointer (instead of assuming that
a P_BIGKEY pointer always precedes a P_BIGDATA pointer).
Greg Hudson [Tue, 23 Aug 2016 17:41:00 +0000 (13:41 -0400)]
Improve checking of decoded DB2 principal values
In krb5_decode_princ_entry(), verify the length of the principal name
before calling krb5_parse_name() or strlen(), to avoid a possible
buffer read overrun. Check all length fields for negative values.
Avoid performing arithmetic as part of bounds checks. If the value of
key_data_ver is unexpected, return KRB5_KDB_BAD_VERSION instead of
aborting.
Greg Hudson [Tue, 23 Aug 2016 16:35:50 +0000 (12:35 -0400)]
Fix GSSRPC server credential memory leak
In svc_auth_gss.c, stop using the global svcauth_gss_creds, and
instead keep a credential in struct svc_rpc_gss_data. This change
ensures that the same credential is used for each accept_sec_context
call for a particular context, and ensures that the credential is
freed when the authentication data is destroyed. Also, do not acquire
a credential when the default name is used (as it is in kadmind) as it
is not needed.
Leave the svcauth_gss_creds around for the backportable fix as it is
in the library export list. It will be removed in a subsequent
commit.
Greg Hudson [Fri, 5 Aug 2016 16:28:03 +0000 (12:28 -0400)]
Use responder for non-preauth AS requests
If no AS reply key is computed during pre-authentication (typically
because no pre-authentication was required by the KDC), ask for the
password using the responder before calling gak_fct for the key, and
supply any resulting responder items to gak_fct.
Sarah Day [Mon, 15 Aug 2016 20:11:31 +0000 (16:11 -0400)]
Fix KDC to drop repeated in-progress requests
When a KDC receives a repeated request while the original request is
still in progress, it is supposed to be to drop the request. Commit f07760088b72a11c54dd72efbc5739f231a4d4b0 introduced a bug in this
logic, causing the KDC to instead send an empty reply. In
kdc_check_lookaside(), return a NULL reply_packet for empty entries,
restoring the expected behavior.
[ghudson@mit.edu: edited commit message, added a comment]
Greg Hudson [Tue, 28 Jun 2016 16:28:11 +0000 (12:28 -0400)]
Fix leak in gss_display_name() for non-MN names
RFC 2744 states that the gss_display_name() output_name_type result is
"a pointer into static storage, and should be treated as read-only by
the caller (in particular, the application should not attempt to free
it)". For non-mechanism names, we were making a copy of the name type
from the union name structure, causing a memory leak; stop doing that.
Greg Hudson [Mon, 27 Jun 2016 21:49:57 +0000 (17:49 -0400)]
Fix leaks on error in krb5 gss_acquire_cred()
In acquire_cred_context(), when releasing the partially constructed
cred on error, make sure to free the password and impersonator fields,
and to destroy the ccache if we created it.
Tom Yu [Wed, 3 Aug 2016 21:00:05 +0000 (17:00 -0400)]
Warn about dump -recurse nonfunctionality
kdb5_util dump -recurse hasn't behaved as documented since krb5-1.5,
when the DAL was integrated. Restoring it is a nontrivial amount of
work, so just document it for now.
In validate_as_request(), when enforcing restrict_anonymous_to_tgt,
use client.princ instead of request->client; the latter is NULL when
validating S4U2Self requests.
CVE-2016-3120:
In MIT krb5 1.9 and later, an authenticated attacker can cause krb5kdc
to dereference a null pointer if the restrict_anonymous_to_tgt option
is set to true, by making an S4U2Self request.
The KDC now needs write access to the LDAP KDB, unless password
lockout and tracking of the last successful authentication time are
disabled. Update the example LDAP access control configuration in
conf_ldap.rst to reflect this, add a note that only read access is
required if lockout is disabled, and add a section to lockout.rst
calling out the need for write access. Reported by Will Fiveash.
Greg Hudson [Thu, 23 Jun 2016 16:01:56 +0000 (12:01 -0400)]
Fix profile_flush_to_file() state corruption
In write_data_to_file(), do not clear the profile data object's flags.
If the call to this function resulted from profile_flush_to_file(), we
do not want to clear the DIRTY flag, and we especially do not want to
clear the SHARED flag for a data object which is part of
g_shared_trees. Instead, clear the DIRTY flag in
profile_flush_file_data().
Add a test case to prof_test1 to exercise the bug in unfixed code.
Also modify test1 to abandon the altered profile after flushing it to
a file, to preserve the external behavior of the script before this
fix.
Before this patch libkrad would always subtract the existing buffer
length from pktlen before passing it to recv(). In the case of stream
sockets, this is incorrect since krad_packet_bytes_needed() already
performs this calculation. Subtracting the buffer length twice could
cause integer underflow on the len parameter to recv().
Greg Hudson [Wed, 8 Jun 2016 04:00:55 +0000 (00:00 -0400)]
Fix kadmin min_life check with nonexistent policy
In kadmind, self-service key changes require a check against the
policy's min_life field. If the policy does not exist, this check
should succeed according to the semantics introduced by ticket #7385.
Fix check_min_life() to return 0 if kadm5_get_policy() returns
KADM5_UNK_POLICY. Reported by John Devitofranceschi.
Greg Hudson [Mon, 9 May 2016 17:45:06 +0000 (13:45 -0400)]
Fix unlikely pointer error in get_in_tkt.c
In add_padata(), reset the caller's pointer and ensure the list is
terminated as soon as realloc() succeeds; otherwise, the old pointer
could be left behind if a later allocation fails.
Tom Yu [Fri, 27 May 2016 19:19:43 +0000 (15:19 -0400)]
Relax t_sn2princ.py reverse resolution test
Relax t_sn2princ.py check of the reverse resolution of the test
hostname. The new requirement is that it be different from the
forward resolved hostname. (There is also an existing implicit
requirement that it be in the mit.edu domain.) This makes
t_sn2princ.py more robust against changes in the reverse resolution of
the test hostname.
In otp_client_process(), call cb->set_as_key() later in the function
after the OTP request has been created. The previous position of this
call caused the AS key to be replaced even when later code in the
function failed, preventing other preauth mechanisms from retrieving
the correct AS key.
Greg Hudson [Thu, 12 May 2016 20:03:06 +0000 (16:03 -0400)]
Check princ length in krb5_sname_match()
krb5_sname_match() can read past the end of princ's component array in
some circumstances (typically when a keytab contains both "x" and
"x/y" principals). Add a length check. Reported by Spencer Jackson.
Greg Hudson [Mon, 29 Feb 2016 21:51:22 +0000 (16:51 -0500)]
Skip unnecessary mech calls in gss_inquire_cred()
If the caller does not request a name, lifetime, or cred_usage when
calling gss_inquire_cred(), service the call by copying the mechanism
list (if requested) but do not call into the mech.
This change alleviates an issue (reported by Adam Bernstein) where
SPNEGO can fail in the presence of expired krb5 credentials rather
than proceeding with a different mechanism, or can resolve a krb5
credential without the benefit of the target name.
Sarah Day [Thu, 18 Feb 2016 21:54:27 +0000 (16:54 -0500)]
Default to LSA when TGT in LSA is inaccessible
When UAC is enabled and a domain user with Administrator privileges
logs in, the TGT is inaccessible. Access to the TGT in a
UAC-restricted session may allow a non-elevated user to bypass the
UAC. In a UAC-restricted session, ms2mit copies the current tickets
from the LSA ccache to the API ccache except the TGT, effectively
preventing a user session from getting additional service tickets
while appearing, for some purposes, to have a usable ccache.
Another bug is that ms2mit always copies from the LSA ccache to the
default ccache, even if the default ccache is itself the LSA ccache.
New behavior:
* If the TGT is accessible in the LSA ccache, copy the LSA ccache to
the API ccache.
* Set the registry key for the default ccname to "API:" if the copy
occurred, or to "MSLSA:" if it didn't occur.
Greg Hudson [Mon, 14 Mar 2016 21:26:34 +0000 (17:26 -0400)]
Fix LDAP null deref on empty arg [CVE-2016-3119]
In the LDAP KDB module's process_db_args(), strtok_r() may return NULL
if there is an empty string in the db_args array. Check for this case
and avoid dereferencing a null pointer.
CVE-2016-3119:
In MIT krb5 1.6 and later, an authenticated attacker with permission
to modify a principal entry can cause kadmind to dereference a null
pointer by supplying an empty DB argument to the modify_principal
command, if kadmind is configured to use the LDAP KDB module.
Greg Hudson [Tue, 23 Feb 2016 22:15:18 +0000 (17:15 -0500)]
Use blocking lock when creating db2 KDB
In 1.11 we switched from non-blocking to blocking locks in the DB2
module, but we missed one call to krb5_lock_file() in ctx_create_db().
This non-blocking lock can cause krb5_db_promote() to fail if the
database is locked when we try to promote the DB, in turn causing
kdb5_util load to fail. Correct this call to make krb5_db_promote()
more robust.
Greg Hudson [Fri, 8 Jan 2016 18:16:54 +0000 (13:16 -0500)]
Fix leaks in kadmin server stubs [CVE-2015-8631]
In each kadmind server stub, initialize the client_name and
server_name variables, and release them in the cleanup handler. Many
of the stubs will otherwise leak the client and server name if
krb5_unparse_name() fails. Also make sure to free the prime_arg
variables in rename_principal_2_svc(), or we can leak the first one if
unparsing the second one fails. Discovered by Simo Sorce.
CVE-2015-8631:
In all versions of MIT krb5, an authenticated attacker can cause
kadmind to leak memory by supplying a null principal name in a request
which uses one. Repeating these requests will eventually cause
kadmind to exhaust all available memory.
Greg Hudson [Fri, 8 Jan 2016 17:52:28 +0000 (12:52 -0500)]
Check for null kadm5 policy name [CVE-2015-8630]
In kadm5_create_principal_3() and kadm5_modify_principal(), check for
entry->policy being null when KADM5_POLICY is included in the mask.
CVE-2015-8630:
In MIT krb5 1.12 and later, an authenticated attacker with permission
to modify a principal entry can cause kadmind to dereference a null
pointer by supplying a null policy value but including KADM5_POLICY in
the mask.
Greg Hudson [Fri, 8 Jan 2016 17:45:25 +0000 (12:45 -0500)]
Verify decoded kadmin C strings [CVE-2015-8629]
In xdr_nullstring(), check that the decoded string is terminated with
a zero byte and does not contain any internal zero bytes.
CVE-2015-8629:
In all versions of MIT krb5, an authenticated attacker can cause
kadmind to read beyond the end of allocated memory by sending a string
without a terminating zero byte. Information leakage may be possible
for an attacker with permission to modify the database.
Greg Hudson [Thu, 14 Jan 2016 22:51:53 +0000 (17:51 -0500)]
Fix iprop server stub error management
The ipropd stubs free client_name and server_name in the cleanup
handler, so should not free them in out-of-memory conditions.
Reported by Will Fiveash.
Robbie Harwood [Wed, 13 Jan 2016 23:17:09 +0000 (18:17 -0500)]
Fix EOF check in kadm5.acl line processing
On platforms where the char type is unsigned, the check for EOF (which
is negative) will always fail, leaving a 255 byte at the end of the
line. This can cause a syntax error, in turn causing the contents of
kadm5.acl to be ignored. Fix this bug by removing the cast on EOF.
[ghudson@mit.edu: more precisely describe consequences of bug in
commit message]
Greg Hudson [Wed, 25 Nov 2015 19:43:35 +0000 (14:43 -0500)]
Fix memory leak in SPNEGO gss_init_sec_context()
After the initial call to spnego_gss_init_sec_context(), the context
handle can leak if init_ctx_cont() returns an error, because the
cleanup handler assumes that spnego_ctx contains the value of
*context_handle. Fix this leak by setting spnego_ctx before the if
block which contains that call. Reported by Adam Bernstein.
Greg Hudson [Fri, 8 Jan 2016 16:54:55 +0000 (11:54 -0500)]
Make ksu work with prompting clpreauth modules
Commit 5fd5a67c5a93514e7d0a64425baa007ad91f57de switched ksu from
using krb5_get_in_tkt_with_password() to
krb5_get_init_creds_password(), but did not supply a prompter
argument. Pass krb5_prompter_posix so that clpreauth modules can
prompt for additional information during authentication.
Tom Yu [Wed, 30 Dec 2015 20:26:54 +0000 (15:26 -0500)]
Add .travis.yml
Do Travis CI testing with clang and gcc, on 64-bit Ubuntu Trusty.
Performance would probably be better using the container-based Travis
infrastructure, but that is currently limited to Precise, and we would
need some important apt packages whitelisted, e.g., dejagnu.
Tom Yu [Wed, 30 Dec 2015 22:17:02 +0000 (17:17 -0500)]
Don't canonicalize hostname in sim_client.c
krb5_mk_req() already canonicalizes the target hostname, so don't try
to use a buffer of size MAXHOSTNAMELEN to canonicalize the hostname
beforehand. This buffer will be too short for some unusually long
FQDNs.
Tom Yu [Wed, 6 Jan 2016 20:24:16 +0000 (15:24 -0500)]
Work around uninitialized warning in cc_kcm.c
Some versions of clang erroneously detect use of an uninitialized
variable reply_len in kcmio_call() when building on non-Mac platforms.
Initialize it to work around this warning.
Simo Sorce [Tue, 5 Jan 2016 17:11:59 +0000 (12:11 -0500)]
Check internal context on init context errors
If the mechanism deletes the internal context handle on error, the
mechglue must do the same with the union context, to avoid crashes if
the application calls other functions with this invalid union context.
[ghudson@mit.edu: edit commit message and code comment]
Tomas Kuthan [Tue, 29 Dec 2015 10:47:49 +0000 (11:47 +0100)]
Check context handle in gss_export_sec_context()
After commit 4f35b27a9ee38ca0b557ce8e6d059924a63d4eff, the
context_handle parameter in gss_export_sec_context() is dereferenced
before arguments are validated by val_exp_sec_ctx_args(). With a null
context_handle, the new code segfaults instead of failing gracefully.
Revert this part of the commit and only dereference context_handle if
it is non-null.
Simo Sorce [Wed, 9 Dec 2015 23:09:18 +0000 (18:09 -0500)]
Set TL_DATA mask flag for master key operations
When kdb5_util adds or removes master keys, it modifies tl-data but
doesn't set the KADM5_TL_DATA mask flag, causing KDB modules that rely
on this signaling (such as the LDAP module) not to store the tl-data
changes. Fix this issue by setting the mask bit in add_new_mkey() and
kdb5_purge_mkeys().
Greg Hudson [Fri, 11 Dec 2015 16:05:32 +0000 (11:05 -0500)]
Add libkrb5support dependencies to test plugins
In some build environments, dependencies on libkrb5support can be
generated just from static inline functions in our header files, even
if those functions aren't used. In two test plugin modules, use
$(KRB5_BASE_DEPLIBS) and $(KRB5_BASE_LIBS) to depend on libkrb5support
as well as libkrb5. (This also pulls in libk5crypto, which is
unnecessary for these modules, but is inconsequential for a test
module.) Reported by Will Fiveash.
Tomas Kuthan [Wed, 30 Sep 2015 13:18:05 +0000 (15:18 +0200)]
Check output params on GSS OID set functions
Add sanity checks for the output parameters of
generic_gss_create_empty_oid_set() and
generic_gss_add_oid_set_member(), which are used directly by the API
functions gss_create_empty_oid_set() and gss_add_oid_set_member().
k5_utf8s_to_ucs2s() reads and ignores one extra byte from the input
string before terminating its loop, possibly overrunning the input
buffer of its caller. This overrun is typically without consequence,
but can show up in tools like asan or valgrind during RC4
string-to-key operations. Fix the bug by swapping the order of the
loop conditions.
Tomas Kuthan [Wed, 16 Sep 2015 10:13:26 +0000 (12:13 +0200)]
Fix error mappings for IOV MIC mechglue funcs
The mechglue functions gss_get_mic_iov(), gss_get_mic_iov_length(),
and gss_verify_mic_iov() don't call map_error() to map
mechanism-specific error codes. As a result, a subsequent call to
gss_display_status() fails with GSS_S_BAD_MECH, because no translation
for the error code is found in the error table.
This patch adds the missing map_error call.
[ghudson@mit.edu: correct a whitespace issue, edit commit message]
Ticket #7223 added new policy fields and a new dump format version to
marshal them, but did not add a new iprop dump format version. As a
result, slave KDCs running 1.11 or later cannot receive full resyncs
from master KDCs running 1.10 or earlier. (Reported by John
Devitofranceschi.)
Retroactively add support for pre-1.11 policy entries by making
process_r1_11_policy() read the first ten fields, check whether the
next whitespace character is a newline, and then read the rest if it
is not.
Greg Hudson [Mon, 2 Nov 2015 03:46:56 +0000 (22:46 -0500)]
Fix SPNEGO context import
The patches for CVE-2015-2695 did not implement a SPNEGO
gss_import_sec_context() function, under the erroneous belief that an
exported SPNEGO context would be tagged with the underlying context
mechanism. Implement it now to allow SPNEGO contexts to be
successfully exported and imported after establishment.
Greg Hudson [Mon, 2 Nov 2015 03:45:21 +0000 (22:45 -0500)]
Fix IAKERB context export/import [CVE-2015-2698]
The patches for CVE-2015-2696 contained a regression in the newly
added IAKERB iakerb_gss_export_sec_context() function, which could
cause it to corrupt memory. Fix the regression by properly
dereferencing the context_handle pointer before casting it.
Also, the patches did not implement an IAKERB gss_import_sec_context()
function, under the erroneous belief that an exported IAKERB context
would be tagged as a krb5 context. Implement it now to allow IAKERB
contexts to be successfully exported and imported after establishment.
CVE-2015-2698:
In any MIT krb5 release with the patches for CVE-2015-2696 applied, an
application which calls gss_export_sec_context() may experience memory
corruption if the context was established using the IAKERB mechanism.
Historically, some vulnerabilities of this nature can be translated
into remote code execution, though the necessary exploits must be
tailored to the individual application and are usually quite
complicated.
Greg Hudson [Tue, 27 Oct 2015 04:44:24 +0000 (00:44 -0400)]
Fix two IAKERB comments
The comment explaining why there is no iakerb_gss_import_sec_context()
erroneously referenced SPNEGO instead of IAKERB (noticed by Ben
Kaduk). The comment above iakerb_gss_delete_sec_context() is out of
date after the last commit.
In build_principal_va(), use k5memdup0() instead of strdup() to make a
copy of the realm, to ensure that we allocate the correct number of
bytes and do not read past the end of the input string. This bug
affects krb5_build_principal(), krb5_build_principal_va(), and
krb5_build_principal_alloc_va(). krb5_build_principal_ext() is not
affected.
CVE-2015-2697:
In MIT krb5 1.7 and later, an authenticated attacker may be able to
cause a KDC to crash using a TGS request with a large realm field
beginning with a null byte. If the KDC attempts to find a referral to
answer the request, it constructs a principal name for lookup using
krb5_build_principal() with the requested realm. Due to a bug in this
function, the null byte causes only one byte be allocated for the
realm field of the constructed principal, far less than its length.
Subsequent operations on the lookup principal may cause a read beyond
the end of the mapped memory region, causing the KDC process to crash.
Nicolas Williams [Mon, 14 Sep 2015 16:28:36 +0000 (12:28 -0400)]
Fix IAKERB context aliasing bugs [CVE-2015-2696]
The IAKERB mechanism currently replaces its context handle with the
krb5 mechanism handle upon establishment, under the assumption that
most GSS functions are only called after context establishment. This
assumption is incorrect, and can lead to aliasing violations for some
programs. Maintain the IAKERB context structure after context
establishment and add new IAKERB entry points to refer to it with that
type. Add initiate and established flags to the IAKERB context
structure for use in gss_inquire_context() prior to context
establishment.
CVE-2015-2696:
In MIT krb5 1.9 and later, applications which call
gss_inquire_context() on a partially-established IAKERB context can
cause the GSS-API library to read from a pointer using the wrong type,
generally causing a process crash. Java server applications using the
native JGSS provider are vulnerable to this bug. A carefully crafted
IAKERB packet might allow the gss_inquire_context() call to succeed
with attacker-determined results, but applications should not make
access control decisions based on gss_inquire_context() results prior
to context establishment.