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.
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.
Tom Yu [Tue, 4 Oct 2016 22:14:51 +0000 (18:14 -0400)]
Set alg param correctly for PKCS1
When using a smart card and constructing a DigestInfo to pass to the
CKM_RSA_PKCS mechanism, make sure to set the AlgorithmIdentifier
parameters correctly. This is typically an ASN.1 NULL value.
In the previous code, when the remote peer performed an orderly shutdown
on the socket, libkrad would enter a state in which all future requests
timed out. Instead, if the peer shuts down its socket, we need to
attempt to reopen it.
Ben Kaduk [Mon, 26 Jan 2015 16:15:42 +0000 (11:15 -0500)]
Improve keytab documentation
In the k5srvutil man page, do not give the impression that arbitrary
new keys can be added to the keytab (requested by Dan Gillmor), since
only the new keys randomly generated by the KDC via 'k5srvutil change'
can be added to the keytab. Reiterate the importance of running
k5srvutil delold after running k5srvutil change in the description of
k5srvutil change, as well as in the description of k5srvutil delold
itself.
In install_kdc.rst, mention using a separate keytab file when
generating a keytab on a KDC for use on another host.
[ghudson@mit.edu: squashed two commits, condensed commit message]
Ben Kaduk [Wed, 11 Jun 2014 20:38:57 +0000 (16:38 -0400)]
Document krb5_kt_next_entry() requirement
Successful calls to krb5_kt_next_entry() return a krb5_keytab_entry
that the caller is responsible for freeing. Note this, and the
proper function to do so, in the doxygen comments.
In prepare_error_as(), if krb5_us_timeofday() fails and error pa-data
was supplied, the FAST cookie and a shallow copy of the error padata
can be leaked. Reported by Will Fiveash.
Tom Yu [Wed, 7 Sep 2016 21:28:34 +0000 (17:28 -0400)]
Fix unaligned accesses in bt_split.c
In the libdb2 btree back end, splitting a page at an overflow key
could result in an unaligned access, causing a crash (and data
corruption) on platforms with strict alignment. This probably occurs
only rarely in practice.
Ben Kaduk [Thu, 11 Aug 2016 04:25:47 +0000 (23:25 -0500)]
Fix build with -O3 on ppc64el
Ubuntu runs ppc64el builds with -O3, which elicited a few warnings
from gcc that were not generated elsewhere, as documented at
https://bugs.launchpad.net/ubuntu/+source/krb5/+bug/1592841 .
Initialize the output variable at the top of a couple of helper functions
to silence the uninitialized-variable warnings.
Commit e3d9f03a658e247dbb43cb345aa93a28782fd995 (ticket 8481) added
several checks for negative length values when decoding DB2 principal
entries, including two unnecessary checks on unsigned values. Remove
those checks as they can generate warnings.
We depend on the behavior of having a separate subshell for each line in
our Makefiles, so force it where make (observed FreeBSD 10.3) does not
create one.
[ghudson@mit.edu: also changed rules in config/post.in]
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.
To work correctly with older Samba clients, we should guess the mutual
flag based on the ap_options from the AP-REQ and not set it
unconditionally. Found by the Samba torture testsuite.
[ghudson@mit.edu: edited comments and commit message]
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.
In spnego_gss_import_cred(), use create_spnego_cred() to create the
SPNEGO credential structure. Prior to this change, an imported SPNEGO
cred did not initialize the no_ask_integ field (added by commit cf39ed349976908626cad3e05e17788f8334bce9, ticket #6938).
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 [Sun, 14 Aug 2016 16:08:16 +0000 (12:08 -0400)]
Work around glibc OFD lock bug on 32-bit Linux
A bug in Gnu libc causes OFD locking to fail unpredictably on 32-bit
Linux, typically leading to deadlocks. Work around this bug by using
the fcntl64 system call and struct flock64.
See also: https://sourceware.org/bugzilla/show_bug.cgi?id=20251
Ben Kaduk [Wed, 3 Aug 2016 15:23:56 +0000 (10:23 -0500)]
Properly escape quotes for otp set_string example
The libss parser will consume paired double quotes, but within
a double-quoted region, repeated double quotes will be treated
as an escape and passed through as a single double quote.
(The new kadmin(1) parser in 1.14 that lets commands be specified
on the command line without -q does not go through the libss parser,
so standard shell methods for escaping quotes function as usual.)
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 23:38:36 +0000 (19:38 -0400)]
Fix leak in k5_free_cammac()
free_vmac(), a helper function used by k5_free_cammac(), must free its
val pointer as well as the contents; otherwise the krb5_verifier_mac
container is leaked.
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.
Greg Hudson [Fri, 24 Jun 2016 16:33:05 +0000 (12:33 -0400)]
Fix memory leak in db2 policy DB initialization
osa_adb_init_db() maintains a static linked list mapping filenames to
lock structures. Entries are never removed from the list; when their
reference counts hit 0, the lockfile is closed but the filename
remains allocated. However, the filename is allocated each time the
lockfile is re-opened, leaking the old value. Fix this leak by moving
filename initialization to entry creation.
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.
Commit 632260bd1fccfb420f0827b59c85c329203eafc9 (ticket #7517) allows
better error reporting for some client pre-authentication failures.
However, it breaks an assumption in the S4U2Self code that such errors
can be recognized by the KRB5_PREAUTH_FAILED error code. Instead of
passing through the error code reported by the first real preauth
module, wrap that error and return KRB5_PREAUTH_FAILED.
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.
When the default realm name is unspecified, and none was set in the
krb5_context object, return KRB5_CONFIG_NODEFREALM from libkdb5
instead of the confusing KRB5_KDB_DBTYPE_NOTFOUND. To accomplish
this, make kdb_get_library_name() return a krb5_error_code.
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, 28 Mar 2016 17:48:52 +0000 (13:48 -0400)]
Improve errors when DB2 database cannot be opened
When we cannot open a DB2 database, set a useful error message.
Change the signature of open_db() to to allow it to return an error
code with a message set.
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.