Tom Yu [Wed, 22 Jul 2009 18:55:18 +0000 (18:55 +0000)]
jumbo pullup for kfw-3.2.3-alpha1
This is a jumbo pullup of multiple KfW-related changes. The are
primarily build system fixes, including changes to enable building on
amd64. Included are some changes from branches/kpkoch-ccapi that have
not yet been merged to the trunk. Relevant RT ticket numbers include:
ticket: new
subject: fix cleanup code in allocating preauth info
target_version: 1.6.4
tags: pullup
After an allocation failure, free up the previously allocated array
elements by counting back down to zero, not continuing to count up
until we hit zero.
asn1buf_imbed() can perform pointer arithmetic that causes the "bound"
pointer of the subbuffer to be less than the "next" pointer. This can
lead to malloc() failure or crash.
In asn1buf_imbed(), check the length before doing arithmetic to set
subbuf->bound. In asn1buf_remove_octetstring() and
asn1buf_remove_charstring(), check for invalid buffer pointers before
executing an unsigned length check against a (casted to size_t)
negative number.
Tom Yu [Wed, 8 Apr 2009 01:22:51 +0000 (01:22 +0000)]
CVE-2009-0844 (1.6.x) SPNEGO can read beyond buffer end
pull up rxxxxx from trunk
SPNEGO can read beyond the end of a buffer if the claimed DER length
exceeds the number of bytes in the input buffer. This can lead to
crash or information disclosure.
Thanks to Apple for reporting this vulnerability and providing
patches.
Tom Yu [Tue, 17 Mar 2009 21:34:13 +0000 (21:34 +0000)]
CVE-2009-0845 (1.6.x) SPNEGO can dereference a null pointer
pull up r22084 from trunk
acc_ctx_new() can return an error condition without establishing a
SPNEGO context structure. This can cause a null pointer dereference
in cleanup code in spnego_gss_accept_sec_context().
The NIM error reporting functions (in src/windows/identity/kherr ) keep
track of the the error message with the highest severity level that was
reported for a specific error reporting context. However, if another
error message of the same severity is reported, the error message being
tracked will be updated to be the newly received error.
The user will often only be notified of the error message that was
tracked for a specific operation. Therefore, tracking the last message
with the highest priority has the unfortunate side-effect of not
reporting the cause of a failure.
This patch changes the condition for updating the tracked error message
to be the first message with the highest severity.
This patch modifies the NIM Kerberos v5 plug-in to use the
krb5_get_error_message() function to look up the error string
if the call to krb5_get_init_creds_password() fails. If the call
to krb5_get_error_message() fails, the caller will failover to
the previous method of looking up a suitable error message based
on the error code.
The /src/windows/identity/plugins/common/dynimport.{c,h} files are used
by the NIM Kerberos v5 plug-ins for run-time dynamic linking. They
currently do not declare or import the following functions:
This patch adds declarations and definitions required for locating these
functions. Relies on the addition of these functions to the prototype
list in the Pismere loadfuncs-krb5.h. See ticket 6045.
The behavior of the HDN_ENDTRACK notification has changed slightly on
Vista. HDM_GETITEMRECT, when used while handling HDN_ENDTRACK, returns
the item extents that were there prior to the user starting the resizing
operation. Earlier it would return the extents that resulted from the
resizing operation.
This resulted in a visual update problem on Windows Vista/2008
in the NIM Advanced View.
Tom Yu [Fri, 25 Jul 2008 21:47:03 +0000 (21:47 +0000)]
pull up r20553 from trunk
r20553@cathode-dark-space: jaltman | 2008-07-21 14:48:03 -0400
ticket: new
subject: Assign fixed ordinals to comerr32.dll exports
component: krb5-libs
tags: pullup
All of the other libraries on Windows have fixed assignments
of ordinals to the exported functions. Assign the ordinals
that were in use in the last public release, kfw 3.2.2, so
that they will remain constant into the future in case additional
exports are added to the library.
kadm5_decrypt_key(). This patch prevents the returned keyblock's
enctype from being coerced to the requested 'ktype' if the requested
'ktype' == -1. A ktype of -1 is documented as meaning "to be ignored".
This patch addresses the issues raised in this ticket and ticket 5936.
(a) In the case where 'cred_handle' != 'verifier_cred_handle'[1]
krb5_gss_accept_sec_context() leaks the 'cred_handle' in the success
case and the failure cases that result in returning from the function
prior to reaching the end of the function.
(b) The meaningful 'minor_status' return value is destroyed during the
cleanup operations.
The approach taken is to add a new 'exit:' label prior to the end of the
function through which all function returns after reaching the 'fail:'
label will goto. After 'exit:', the 'cred_handle' will be released and
if there is a krb5_context 'context' to be freed, the error info will be
saved and krb5_free_context() will be called.
In the success case, the krb5_context is saved in the gss context and we
now set 'context' to NULL to prevent it from being freed.
In order to preserve the minor_status return code, a 'tmp_minor_status'
variable is added that is used after the 'fail:' label in calls to
krb5_gss_delete_sec_context() and krb5_gss_release_cred().
[1] If 'verifier_cred_handle' is non-NULL, then 'cred_handle' is set to
the value of 'verifier_cred_handle'.
ccdefault.c:
krb5_cc_default_name() is permitted to return a NULL
pointer as a valid output. Passing a NULL pointer to
strcmp() will result in an exception as NULL is not
a valid input parameter to strcmp().
Save the output of krb5_cc_default_name() to a variable
and modify the conditional to set the new default ccache
name in the case where there is no existing default
ccache name.
There are two mutex locking issues that Roland Dowdeswell noticed in
the memory ccache. The first one is in cc_memory.c:krb5_mcc_initialize().
When it is free(3)ing the existing credentials it does not lock the
data structures and hence two separate threads can run into issues.
The same problem exists in cc_memory.c:krb5_mcc_destroy().
Tom Yu [Mon, 21 Jul 2008 16:08:33 +0000 (16:08 +0000)]
pull up r20527 from trunk
r20527@cathode-dark-space: tlyu | 2008-07-15 17:43:35 -0400
ticket: new
subject: krb5_get_cred_via_tkt() should null out_cred on errors
tags: pullup
target_version: 1.6.4
component: krb5-libs
Helper function krb5_kdcrep2creds(), called from
krb5_get_cred_via_tkt(), should null its output pointer after freeing
allocated memory, to avoid returning an invalid pointer.
Tom Yu [Mon, 14 Jul 2008 22:12:38 +0000 (22:12 +0000)]
pull up r20304 from trunk
r20304@cathode-dark-space: raeburn | 2008-04-18 15:31:47 -0400
ticket: new
subject: fix possible buffer overrun in handling generic-error return
target_version: 1.6.5
tags: pullup
Jeff Altman reported this, based on a crash seen in KfW in the wild.
The krb5_data handle used to describe the message field returned by the KDC is
not null-terminated, but we use a "%s" format to incorporate it into an error
message string. In the right circumstances, garbage bytes can be pulled into
the string, or a memory fault may result.
However, as this is in the error-reporting part of the client-side code for
fetching new credentials, it's a relatively minor DoS attack only, not a
serious security exposure. Should be fixed in the next releases, though.
This patch is derived from a patch originally submitted to RT
by: Nik Conwell <nik@bu.edu>
krb5_set_real_time() accepts as input the time of the KDC
or an application server as a combination of seconds and
microseconds. Often it is the case that the time source
does not provide the real time with less than one second
granularity. Up until this patch such a caller would fill
in the microseconds parameter as zero. krb5_set_real_time()
would treat the zero microseconds as the actual reported
time and compute a microsecond based offset.
During a one second window subsequent calls to
krb5_set_real_time() would have an ever increasing offset
size until the number of seconds is incremented. This
in turn produces a side effect in which the microseconds
value of the local clock is effectively erased.
If there are multiple processes or threads on the same
machine each requesting service tickets using the same
client principal for the same service principal where
the number of seconds reported by the KDC are equivalent,
then they will now all create authenticators with
exactly the same timestamp. As a result, the authenticating
service will detect a replay attack even though the
authenticators are actually unique. The replay cache
only maintains a tuple of client, server and timestamp.
This patch modifies the interpretation of the microseconds
parameter. If -1 is specified, the microseconds offset is
ignored.
Tom Yu [Mon, 14 Jul 2008 22:11:33 +0000 (22:11 +0000)]
pull up r20311 from trunk
r20311@cathode-dark-space: rra | 2008-04-28 19:05:27 -0400
Ticket: new
Subject: Properly escape - in kdb5_ldap_util man page
Component: krb5-doc
Version_Reported: 1.6.3
Target_Version: 1.6.4
Tags: pullup
The LDAP plugin introduced a new man page which has unescaped hyphens.
Unicode-aware groffs may convert those to real hyphens rather than
the intended ASCII hyphen. This patch adds backslashes in front of
all the bare hyphens that I plus Debian's lintian program could find
to force interpretation as ASCII hyphens.
Fix MITKRB5-SA-2008-002: array overrun in libgssrpc.
Don't update the internally-tracked maximum file descriptor value if
the new one is FD_SETSIZE (or NOFILE) or above. Reject TCP file
descriptors of FD_SETSIZE (NOFILE) or above.
Tom Yu [Sat, 23 Feb 2008 02:10:59 +0000 (02:10 +0000)]
pull up r20228 from trunk
r20228@cathode-dark-space: rra | 2008-02-18 23:49:11 -0500
ticket: new
subject: man page macro and hyphen fixes
component: krb5-doc
Version_Reported: 1.6.3
Target_Version: 1.6.4
Tags: pullup
Fix various unescaped hyphens, lines starting with . that shouldn't be
macros, undefined strings, and misspelled macros in the man pages.
Found via man --warnings on a current Debian unstable system.
Tom Yu [Tue, 19 Feb 2008 18:29:15 +0000 (18:29 +0000)]
pull up r20222 from trunk
r20222@cathode-dark-space: tlyu | 2008-02-07 02:07:06 -0500
ticket: new
target_version: 1.6.4
tags: pullup
subject: more tests for libdb btree page split on zero index
component: krb5-kdc
Enhance btree debugging output somewhat to limit key printout to the
key length if the key is not null-terminated.
Add additional test case for the zero-index page split bug; test case
can create a corrupted btree database with records unreachable by
random access but reachable by sequential access. Requires
recompiling with CPPFLAGS='-DDEBUG -DDEBUG_IDX0SPLIT' to correctly
model mpool page reuse that would be present in production conditions.
(CPPFLAGS=-DDEBUG would otherwise explicitly overwrite the contents of
reused pages.)
Tom Yu [Tue, 19 Feb 2008 18:28:59 +0000 (18:28 +0000)]
pull up r20211 from trunk
r20211@cathode-dark-space: jaltman | 2008-01-23 17:10:56 -0500
ticket: new
subject: Windows: avoid use of cygwin mkdir and rmdir commands
tags: pullup
Microsoft's nmake versions 8.x and 9.x prefer executables over
internal shell commands. This is a change from previous versions.
Cygwin's mkdir and rmdir commands do not have the same semantics
as the cmd.exe shell versions.
Change the definitions of MKDIR and RMDIR to use 'md' and 'rd'
in order to avoid the use of the cygwin versions.
ticket: 5875
target_version: 1.6.4
version_fixed: 1.6.4
component: windows
Tom Yu [Fri, 1 Feb 2008 01:23:12 +0000 (01:23 +0000)]
pull up r20214 from trunk
r20214@cathode-dark-space: tlyu | 2008-01-31 20:03:11 -0500
ticket: new
target_version: 1.6.4
tags: pullup
subject: libdb btree page split on zero index corrupts db
component: krb5-kdc
Splitting a btree page on index 0 can corrupt the database if the key
length plus data length is exactly a certain value. This certain size
causes the item to get the left page to itself, and causes the right
page to contain an erroneous additional index "hole" having an
uninitialized value. This bug may be one of the remaining causes of
unexplained database corruption reported over the years. Shawn Emery
provided useful data from actual instances of this corruption.
Add a test case for this bug. (Raw libdb test rather than kdb; the
latter would be much harder.)
Tom Yu [Wed, 2 Jan 2008 23:49:06 +0000 (23:49 +0000)]
pull up r20176 from trunk
r20176@cathode-dark-space: jaltman | 2007-12-12 17:32:19 -0500
ticket: new
subject: KFW: BUG: KRB5CRED: Set identity data before sending notification
component: windows
tags: pullup
Call tc_set_ident_data() before kcdb_credset_collect(). Make sure the
identity data is set before the credentials change notification is broadcast.
The khm_show_main_window() function is no longer called
at startup with khm_nCmdShow == SW_SHOWMINIMIZED in order to
hide the main application by calling khm_hide_main_window().
Instead, the main application window is simply never shown.
As a result, khm_show_main_window() needs to respond to
khm_nCmdShow == SW_SHOWMINIMIZED not by hiding the window
but by changing the khm_nCmdShow state to SW_SHOW and then
calling ShowWindow().
This change will address the problem whereby "Show NIM Window"
had to be triggered twice by the user when the process
was started in a minimized state.
kt_file.c: Support multiple iterators active simultaneously, using a
counter. In get_entry, if the file was already open, rewind it to
just after the version number, and don't close it when done. Don't
allow add or remove calls if any iterator is active.
t_keytab.c: Test mixing two iterators with get_entry calls.
Tom Yu [Fri, 19 Oct 2007 20:51:43 +0000 (20:51 +0000)]
pull up r20128 from trunk
r20128@cathode-dark-space: jaltman | 2007-10-18 11:22:43 -0400
ticket: new
subject: KFW: BUG: WIX: Beta value hard coded
component: windows
tags: pullup
target: 1.6.3
The beta variable value was inadvertantly committed as part of
ticket 5820 (Revision 20117). The build script needs to
export this value when appropriate.
Tom Yu [Fri, 19 Oct 2007 20:51:27 +0000 (20:51 +0000)]
pull up r19881 from trunk
r19881@cathode-dark-space: jaltman | 2007-08-27 03:08:24 -0400
ticket: new
subject: Windows 64-bit - avoid missing symbol errors
component: windows
Microsoft defaults stack checking (/Gs) to on. This requires
that bufferoverflowU.lib be included in the link step. The
macro SCLIB in the build system specifies this library on
versions of Windows that require it. Include SCLIB on the
link line of the makefile.
There appears to be a bug either in the WiX engine or the Windows Installer 3.1.
The "File" type on the Registry Search property is supposed to provide the full
path name. Instead, we are being given just the directory as if it were being
processed with the "Directory" type.
We can avoid this for a REG_SZ value by using the "Raw" type because we are
sure that the string is not going to begin with a '#' character.
Because the full path was not being obtained for the UPGRADENSIS property, the
Uninstall routine was unable to CreateProcess() the uninstall program.
This commit also includes addition debugging in the NSIS Uninstall custom
handler to report the path and the GetLastError() value when the uninstall
fails. This will be logged in the msiexec log file and displayed in a
MessageBox.
Tom Yu [Mon, 15 Oct 2007 22:07:08 +0000 (22:07 +0000)]
pull up r20117 from trunk
r20117@cathode-dark-space: jaltman | 2007-10-12 15:01:38 -0400
ticket: new
subject: KFW: BUG: WIX: Improve Usability of multiple architecture MSI installations, remove non-unique GUID component identifiers, and include Beta ID in the package name
component: windows
tags: pullup
target_version: 1.6.3
The WiX installation package suffered from several problems:
* The Beta ID was not being included in the package name.
Fixed this by swapping the priority of "Release" and "Beta".
"Beta" is an official release that has a beta value.
A non-release has a datestamp as part of the package name.
* There were duplicate GUID values being used for registry components.
This would prevent proper removal of the components on uninstall.
* 64-bit Installers were being constructed with the 32-bit installer
schema. This prevented side-by-side installation of the 64-bit and
32-bit versions. This also permitted 64-bit installers to be
installed on 32-bit systems.
* The 64-bit and 32-bit installers had the same package name.
64-bit and 32-bit are now identified in the package name.
* 64-bit files were being installed to the WOW64 environment.
Tom Yu [Fri, 5 Oct 2007 16:31:34 +0000 (16:31 +0000)]
pull up r20101 from trunk
r20101@cathode-dark-space: jaltman | 2007-10-05 11:23:53 -0400
ticket: new
subject: NIM: BUG: APP: New edit controls should be marked ES_AUTOHSCROLL
component: windows
The EDIT controls used to accept input from the user must be set to
support automatic horizontal scrolling. Otherwise, the number of input
characters is arbitrarily restricted based upon the font selected by
the user as part of the active Windows theme.
Horizontal scrolling is enabled with the ES_AUTOHSCROLL flag during
control construction.
Tom Yu [Fri, 5 Oct 2007 14:32:59 +0000 (14:32 +0000)]
pull up r20099 from trunk
r20099@cathode-dark-space: kpkoch | 2007-10-04 21:26:34 -0400
Ticket: new
Tags: pullup
Target_Version: 1.6.3
Subject: KfW Build: add new installer build files to copyfiles.xml.
Files were added to the installer build area. They also need to be added to the copylist, so that they are copied to the staging area, where the installer is built.
Tom Yu [Tue, 2 Oct 2007 02:42:17 +0000 (02:42 +0000)]
pull up r20040 from trunk
r20040@cathode-dark-space: jaltman | 2007-10-01 16:09:55 -0400
ticket: new
subject: remove error tables by pointer
tags: pullup
target_version: 1.6.3
On Windows, it is possible for the same DLL to be loaded
into a process multiple times as separate instances. Each
time a DLL is loaded it registers its error tables at different
locations in the process address space. Removing the tables
by base instead of pointer value can result in the error table
list pointing at invalid memory.
The Network Identity Manager notification icon can display a tooltip
when the user hovers the mouse cursor over it. It is currently used
to indicate the default identity (if one is found). However, when
retrieving the name of the default identity, the size of the buffer
was left unspecified. This patch specifies the correct buffer size.
The function called with the incorrect buffer size was
kcdb_identity_get_name(). That function does not write more than
KCDB_IDENT_MAXCCH_NAME characters regardless of the size of the buffer
specified, and the buffer that was passed in is allocated to be this
size. No buffer overrun was present in the existing code, although
the behavior was incorrect.