Greg Hudson [Fri, 20 Nov 2015 00:32:11 +0000 (19:32 -0500)]
Use krb5_expand_hostname() to get admin service
In libkadm5's kadm5_get_admin_service_name(), use
krb5_expand_hostname() instead of custom canonicalization code to
canonicalize the hostname. There are some minor behavior differences;
in addition to the changes listed in the previous commit, the old code
did not downcase the result of the getaddrinfo() lookup, while the new
code does.
Greg Hudson [Tue, 17 Nov 2015 18:33:21 +0000 (13:33 -0500)]
Use krb5_expand_hostname() when creating KDB
In kdb5_util's add_admin_princs(), use krb5_expand_hostname() instead
of custom canonicalization code to canonicalize the hostname. There
are some minor behavior differences:
* Canonicalization will no longer use AI_ADDRCONFIG.
* Canonicalization will use reverse DNS if configuration permits.
* Canonicalization will be affected by the dns_canonicalize_hostname
and rdns profile variables.
* If name lookup fails, the original hostname will be used.
* A trailing dot will be removed from the name lookup result, if
present.
Greg Hudson [Tue, 17 Nov 2015 18:06:31 +0000 (13:06 -0500)]
Add krb5_expand_hostname() API
Add a new public libkrb5 function expand_hostname(). It follows the
same contract as the Heimdal function, except that the caller should
use krb5_free_string() instead of krb5_xfree() to free the result.
As a small side effect, we no longer remove trailing dots from the
hostname in krb5_sname_to_principal() when invoked with type
KRB5_NT_UNKNOWN. Adjust a test case in t_sn2princ.py accordingly.
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 [Mon, 8 Aug 2016 18:39:24 +0000 (14:39 -0400)]
Add another AD-SIGNTICKET corner case test
Prior to the fix for #8139, forwarded TGTs obtained across a krbtgt
re-key could fail if the preferred krbtgt enctype changed, because
krb5_c_verify_checksum() returns an bad-enctype error due to the
mismatched checksum. Add a test case for this scenario, using a new
test harness program which obtains a forwarded TGT.
Greg Hudson [Wed, 17 Aug 2016 15:50:31 +0000 (11:50 -0400)]
Output last command info on k5test failures
When a k5test failure occurs, display the last executed command, its
command index, and its output. This will make it easier to understand
"make check" failures when it is not easy to run subsequent commands
or investigate the filesystem of the build host.
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.
Tom Yu [Tue, 9 Aug 2016 01:46:16 +0000 (21:46 -0400)]
Recursive btree traversal test case
Add an unlink page command to the dbtest program. This dbtest command
finds a page that has both a left and a right neighbor and unlinks it,
making it inaccessible to conventional sequential traversal. This
simulates some btree corruption that has been seen in the field.
Unlike the bttest command, the dbtest unlink command always searches
for a leaf page with both a left and a right sibling, and doesn't
allow the user to specify internal pages or a specific page number.
Add a new dbtest command to recursively dump a btree database.
Add a new test case to run.test that uses these new commands to verify
the correct functioning of the recursive btree traversal options.
Tom Yu [Mon, 8 Aug 2016 12:50:40 +0000 (08:50 -0400)]
Add bttest unlink page command
To enable testing of recursive btree traversal, add an unlink page
command to the bttest program (used for debugging the libdb2 btree
back end). This new bttest command can unlink a specified page
number, or it can search for and unlink a page that has both a left
and a right sibling. (The user can specify whether to find an
internal page or a leaf page.)
This unlinking makes the page inaccessible to conventional sequential
traversal, simulating some btree corruption that has been seen in the
field.
Tom Yu [Mon, 8 Aug 2016 13:15:42 +0000 (09:15 -0400)]
Refactor btree recursive traversal code
Previous releases had a nonstandard entry point (bt_rseq) into libdb2
to perform recursive traversal of a btree database that might be
corrupt so that an operator could attempt data recovery. This entry
point became inaccessible to user commands after krb5-1.5 due to
integration of the DAL.
Refactor the recursive traversal code into the existing btree
sequential traversal code, accepting new movement flags R_RNEXT
(recursively advance to next item) and R_RPREV.
Add commands to the libdb2 btree test program bttest to exercise this
functionality. Fix up the existing "rlist" command of bttest to use
the updated interface.
Tom Yu [Mon, 8 Aug 2016 13:06:16 +0000 (09:06 -0400)]
Fix MPOOL_IGNOREPIN to ease btree debugging
Various libdb2 test programs use the MPOOL_IGNOREPIN flag to examine
arbitrary mpool pages that may or may not be pinned. This flag is
apparently intended to allow fetching pages that are already pinned,
and to avoid setting the MPOOL_PINNED flag. When there was a cache
hit, mpool_get was setting MPOOL_PINNED anyway, causing aborts when
using debugging programs such as dbtest and bttest.
Fix this inconsistency by not setting MPOOL_PINNED when returning a
cached page when the caller requested MPOOL_IGNOREPIN. In bttest, add
MPOOL_IGNOREPIN to allow dumping of pages while they are pinned
without disrupting their pinned status.
Tom Yu [Mon, 8 Aug 2016 13:13:59 +0000 (09:13 -0400)]
Make bttest build with restricted lib exports
On platforms that use strict library export lists, the libdb2 btree
debugging program bttest won't build with -DDEBUG or -DSTATISTICS
because some needed functions aren't in the export list.
Add the missing bt_debug.c and mpool.c functions to libdb.exports.
Stub out these functions when built without -DDEBUG or -DSTATISTICS to
avoid unreferenced symbols, because conditionalizing library export
lists isn't easy.
Tom Yu [Mon, 8 Aug 2016 13:12:53 +0000 (09:12 -0400)]
Fix bttest printing of unterminated strings
The libdb2 btree debugging program bttest can attempt to print keys or
data that aren't null-terminated, reading past the end of the
length-counted byte array. Use the "%.*s" format specifier to provide
an explicit length when printing keys or data.
Tom Yu [Mon, 8 Aug 2016 13:12:16 +0000 (09:12 -0400)]
Improve EINTR handling in bttest
The libdb2 btree debugging program bttest doesn't handle EINTR in its
input loop. This causes difficulties with debugging using lldb,
because bttest gets signals like SIGSTOP when being attached, and lldb
doesn't seem to share the terminal well with a program being debugged.
ticket: 8478 (new)
subject: usability improvements for bttest
Greg Hudson [Sun, 14 Aug 2016 23:00:12 +0000 (19:00 -0400)]
Allow libev to use pthreads on old platforms
The upgrade to libev 4.22 introduced the use of "memory fences" for
more reliable signal processing. Memory fences are usually
implemented using assembly or compiler primitives, but may be
implemented using pthreads as a last resort. The unmodified libev
errors out at compile time if pthreads is used, but notes that this
error can be removed if relying on pthreads is okay. Because the
project's nightly build infrastructure includes an old Solaris machine
whose toolchain is too old for any of the non-pthreads memory fence
implementations, remove the error to allow the build to succeed. (A
dependency on pthreads functions on Solaris does not require linking
with libpthread.)
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 [Thu, 30 Jun 2016 16:32:20 +0000 (12:32 -0400)]
Add asan build support
Add the --enable-asan configure option. This option suppresses the
undefined symbol check when building shared libraries on Linux, and
adds -fsanitize=address to the compiler and linker options.
Greg Hudson [Wed, 29 Jun 2016 21:13:33 +0000 (17:13 -0400)]
Fix a variety of one-time leaks
Eliminate some memory leaks which should not affect normal operation,
but which make it harder to detect more serious memory leaks.
In kdb5_util, start using the already existing quit() function and
remove redundant DB and master key cleanup performed by individual
commands. In kdb5_destroy(), use util_context instead of creating a
new one. Add an mkey_fullname global variable and use it to make
a bunch of krb5_db_setup_mkey_name() calls unnecessary.
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 [Mon, 27 Jun 2016 04:21:30 +0000 (00:21 -0400)]
Fix memory leak in old gssrpc authentication
auth_gssapi_create(), which is now only used to connect to ancient
servers, can leak memory on error or when multiple GSSAPI_INIT calls
are required. Ensure that call_res is freed along all exit paths and
before each repeat clnt_call() invocation.
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.
Greg Hudson [Mon, 27 Jun 2016 17:13:47 +0000 (13:13 -0400)]
Update libev sources to 4.22
This update fixes a memory leak when freeing null pointers using
ev_realloc_emul(). In 4.04, that function assumed that realloc(x, 0)
is equivalent to free(x) under glibc, but in at least some versions of
glibc, realloc(NULL, 0) behaves like malloc(0) rather than free(NULL)
and allocates memory.
Sarah Day [Tue, 19 Jan 2016 14:50:33 +0000 (09:50 -0500)]
Add unit tests for k5_parse_host_string
Make is_string_numeric() visible outside of parse_host_string.c as
k5_is_string_numeric() so it can be tested. Make
k5_parse_host_string() return an error when address begins with ':',
for consistency with APR's apr_parse_addr_port().
[ghudson@mit.edu: squashed three commits; added t_parse_host_string to
.gitignore and clean rule; clarified commit message]
When encrypted timestamp pre-authentication fails, respond with error
code KDC_ERR_PREAUTH_FAILED, rather than KRB_AP_ERR_BAD_INTEGRITY, for
consistency with other Kerberos implementations.
[ghudson@mit.edu: clarified commit message and comment]
Greg Hudson [Sun, 7 Aug 2016 04:02:00 +0000 (00:02 -0400)]
Fix and simplify locate_kdc.c port byte order
Commit c89587b4476139f05f34aa2323bd7c7db348c44c introduced a bug in
the handling of KDC specifications using the default port, by passing
it to k5_parse_host_string() in network byte order. Fix this by
removing all of the port byte order transformations in locate_kdc.c,
and storing the struct serverlist port field in host byte order.
Adjust other consumers of struct serverlist to expect the port to be
in host field to be in host byte order.
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.
Change the build system to descend into every directory where we
create a Makefile, but not to build or run anything during "make all"
and "make check" in directories we previously didn't visit. Document
specific build targets that can be used in those directories.
Do not generate a Makefile for the securid_sam2 module unless we are
building it, for consistency with other conditionally built
directories.
Double-colon rules allow the flexibility to specify commands in
multiple places, but they also make the order of commands and
dependencies dependent on the order of declarations in the Makefile.
Convert all of our double-colon rules to single-colon rules, with the
exception of "clean-unix" and "clean-windows" which have commands
defined in both post.in and in numerous Makefile.in files.
Sarah Day [Wed, 27 Jul 2016 16:44:49 +0000 (12:44 -0400)]
Move CFLAGS and CPPFLAGS after local includes
The gss-kernel-lib Makefile overrides ALL_CFLAGS. It was setting
the CFLAGS and CPPFLAGS to occur before local includes, which
causes some compilers to include system header files before the
local header files. Moving the CPPFLAGS and CFLAGS to the end of
ALL_CFLAGS corrects this behavior.
Tom Yu [Wed, 27 Jul 2016 17:19:51 +0000 (13:19 -0400)]
Rename k5-queue.h macros
Some BSD-derived systems (e.g., FreeBSD and Mac OS X) inappropriately
include sys/queue.h from some non-kernel network-related headers that
we include (net/if.h is one example). Because our k5-queue.h is a
copy from a BSD sys/queue.h, many compilers will warn about macro
redefinitions on those systems. Rename the queue macros to have a K5_
prefix.
Also delete the QUEUEDEBUG macros because they are only useful for
kernel use on the BSD systems where this header originated.
kinit currently outputs "Password incorrect" if it sees a
bad-integrity error code, which results if the KDC reply couldn't be
decrypted, or when encrypted timestamp preauth fails against an MIT
krb5 1.14 or earlier KDC. Expand this check to include general
preauth failures reported by the KDC, but only if a password was
prompted for.
Will Fiveash [Wed, 20 Jul 2016 00:20:51 +0000 (19:20 -0500)]
Better handle failures to resolve client keytab
In krb5_gss_acquire_cred(), treat failure to resolve the client keytab
similarly to a client keytab which resolves but does not exist or has
no entries. The client keytab could fail to resolve if its name
contains %{username} and the current process is acting on behalf of
the NSS system.
[ghudson@mit.edu: rewrote commit message; changed tracing call to use
a macro; cleared error message when ignoring krb5_kt_client_default()
error; added test case]
Greg Hudson [Thu, 16 Jun 2016 20:38:07 +0000 (16:38 -0400)]
Minimize timing leaks in PKINIT decryption
pkcs7_dataDecode() is derived from OpenSSL's PKCS7_datadecode() and is
used by PKINIT clients to decrypt ReplyKeyPack values in RSA mode.
The upstream function was changed for CVE-2012-0884 to minimize the
timing difference when RSA decryption results in the wrong padding.
Although the impact on Kerberos is negligible (because clients do not
ordinarily choose to use RSA mode, and cannot easily be induced to
make many thousands of requests with the same key), change
pkcs7_dataDecode() to match the upstream change, generating a random
symmetric key and using it when RSA decryption fails. Also rename
"tmp" and "tmp_len" to "ek" and "eklen" to match the more descriptive
upstream variable names.
Greg Hudson [Thu, 16 Jun 2016 17:54:01 +0000 (13:54 -0400)]
Simplify pkcs7_dataDecode() in PKINIT
RFC 4556 requires that the EnvelopedData in the encKeyPack contain
only one RecipientInfo. Take advantage of this constraint to simplify
pkcs7_dataDecode().
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.
Matt Rogers [Fri, 15 Jul 2016 14:17:45 +0000 (10:17 -0400)]
Add the kprop-port option to kadmind
The -k option for kadmind sets the port number that kprop is spawned
with during an iprop full resync. Fall back to checking the
KPROP_PORT environment variable if the option is not set.
Sarah Day [Wed, 23 Dec 2015 20:01:44 +0000 (15:01 -0500)]
Allow user to restrict KDC to specific addresses
krb5kdc has always only supported binding to the wildcard addresses.
Add two configuration options to allow specifying the address/port
that krb5kdc listens on for UDP and TCP connections.
[ghudson@mit.edu: edited documentation; preserved kdc_ports = ""
behavior; made kdc_ports and kdc_tcp_ports continue to work in
kdcdefaults section]
Sarah Day [Wed, 23 Dec 2015 17:11:34 +0000 (12:11 -0500)]
Allow user to restrict kadmind bind addresses
kadmind has always only supported binding to the wildcard addresses.
Add three configuration options to allow specifying the address/port
that kadmind listens on for kpasswd, kadmin, and iprop connections.
[ghudson@mit.edu: edited documentation; minimized changes to
setup_loop(); added iprop_listen]
Sarah Day [Mon, 21 Dec 2015 19:07:49 +0000 (14:07 -0500)]
Add ability to bind addresses to the net-server
The net-server.c logic can accept individual addresses to bind to
using the standard host:port string format, in a list with a comma
delimiter.
Since pktinfo support was removed, users with systems lacking
pktinfo that have multiple NICs may specify each of the local
addresses directly that kadmind or krb5kdc should listen on in
kdc.conf.
[ghudson@mit.edu: edited comments and variable names; simplified
setup_socket()]
Sarah Day [Mon, 21 Mar 2016 20:35:15 +0000 (16:35 -0400)]
Remove workaround when pktinfo is unsupported
Currently if the system doesn't support pktinfo and kadmind or
krb5kdc are binding to a UDP address, then the net server binds to
all the local addresses. Currently most systems support pktinfo,
so the workaround isn't really required anymore. Removing the
workaround will only affect systems that don't have pktinfo
support, have multiple NICs, and are listening on a wildcard
address.
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.
Matt Rogers [Tue, 26 Jan 2016 19:59:43 +0000 (14:59 -0500)]
Add hints for -A flag to kdestroy
When using a collection ccache, a user accustomed to the FILE ccache
behavior may not be aware of all active caches, and the default
kdestroy command could make it seem like there is no active cache
left. Print a warning to use -A after kdestroy if there are other
caches.
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.
Greg Hudson [Tue, 21 Jun 2016 22:46:28 +0000 (18:46 -0400)]
Fix recent memory leak in locate_kdc.c
Commit ce112dec844e4650b5ad174bd40f21c32aebe1d1 introduced a memory
leak in locate_srv_conf_1() by moving the free(realmstr) call to the
cleanup handler, because there was an early return after realmstr is
allocated. Convert that early return to a goto.
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 [Tue, 21 Jun 2016 22:29:00 +0000 (18:29 -0400)]
Fix and simplify auth-indicator authdata module
Remove the authind_context count field. The indicators list must be
null-terminated because it is freed with k5_free_data_ptr_list().
authind_internalize() didn't null-terminate the list, and the presence
of the count field made it appear that this wasn't a bug. Use a
different scheme for setting *more in authind_get_attribute() to avoid
requiring an element count.
Also check more thoroughly for errors in authind_externalize() and
authind_internalize(), and remove some unnecessary pointer casts.
Dmitry Kalinkin [Fri, 17 Jun 2016 17:52:23 +0000 (13:52 -0400)]
Fix Makefile for paths containing '+' character
include/Makefile uses a regex to perform variable substitution with '+'
as the sed delimiter. Paths containing " are already invalid in this
approach, so it is better to use " as the delimiter instead of any
other rare symbol.
Matt Rogers [Fri, 13 May 2016 00:36:41 +0000 (20:36 -0400)]
Fail on error when processing KDC-issued authdata
Have k5_get_kdc_issued_authdata() return 0 on a verification failure and
non-zero for other failures, rather than call assert(). Check the
return value when called in krb5int_authdata_verify().
Matt Rogers [Fri, 13 May 2016 00:06:42 +0000 (20:06 -0400)]
Add auth indicator authdata module
This authdata module makes the 'auth-indicator' attribute available to
the GSSAPI name extension functions. The auth indicator values are UTF8
strings imported during AP_REQ processing.