Sam Hartman [Mon, 18 May 2009 19:08:29 +0000 (19:08 +0000)]
Document use of key fields in GSS context
The addition of etype negotiation has made the meanings of the various
keys in the GSS-API context structure more complicated. Document them
to aid in code understanding.
Greg Hudson [Thu, 14 May 2009 16:50:52 +0000 (16:50 +0000)]
gss_krb5int_export_lucid_sec_context was erroneously copying the first
sizeof(void *) bytes of the context into data_set, instead of the
pointer to the context.
ticket: 6488
status: open
tags: pullup
target_version: 1.7
Greg Hudson [Thu, 14 May 2009 16:16:32 +0000 (16:16 +0000)]
UCS2 support doesn't handle upper half of BMP
Make krb5_ucs2 an unsigned type. Eliminate the need for distinguished
values for ucs2 and ucs4 characters by changing the API of the single-
character conversion routines.
Tom Yu [Wed, 13 May 2009 20:41:37 +0000 (20:41 +0000)]
In util/support/utf8_conv.c, the SWAP16 macro is invoked with an
argument that has side effects. On platforms where SWAP16 can
evaluate its argument twice (including platforms where utf8_conv.c
creates a fallback definition for the SWAP16 macro), this can cause a
read overrun by a factor of two.
Rearrange the data flow to avoid calling SWAP16 with an argument that
has side effects.
Greg Hudson [Mon, 11 May 2009 16:57:45 +0000 (16:57 +0000)]
Refactor rule_an_to_ln, creating a new helper function to handle the
selection string specifier. Eliminate two (safe) uses of sscanf in
the process. Add a test case including literal text in the selection
string specifier.
Sam Hartman [Thu, 7 May 2009 20:35:28 +0000 (20:35 +0000)]
Try decrypting using session key if subkey fails in tgs rep handling
Heimdal at least up through 1.2 incorrectly encrypts the TGS response
in the session key not the subkey when a subkey is supplied. See RFC
4120 page 35. Work around this by trying decryption using the session
key after the subkey fails.
* decode_kdc_rep.c: rename to krb5int_decode_tgs_rep; only used for
TGS and now needs to take keyusage
* gc_via_tkt: pass in session key and appropriate usage if subkey
fails.
Note that the dead code to process AS responses in decode_kdc_rep is
not removed by this commit. That will be removed as FAST TGS client
support is integrated post 1.7.
Greg Hudson [Tue, 5 May 2009 16:55:58 +0000 (16:55 +0000)]
In pa_sam_2, free scratch in a couple of error-handling blocks where
it was live and not freed. The function should be reorganized to use
a cleanup handler, but (I believe) is not covered by the test suite
and should not undergo such major surgery until it is.
Greg Hudson [Tue, 5 May 2009 16:00:40 +0000 (16:00 +0000)]
Fix a case in the krb5_rd_rep error handler (introduced in the last
commit) where scratch.data could be indirected through even if it
wasn't allocated successfully.
Greg Hudson [Mon, 4 May 2009 16:08:03 +0000 (16:08 +0000)]
krb5_rd_rep could leak memory through its output parameter on error.
Adjust the flow control so that *repl is NULL on error and the memory
allocated by decode_krb5_ap_rep_enc_part is freed.
Greg Hudson [Sat, 2 May 2009 04:58:19 +0000 (04:58 +0000)]
Fix a memory leak in krb5int_rd_chpw_rep in a block of code handling
buggy MS KDC behavior. It's not entirely clear what should happen in
the case where memory was leaked (error packet received containing
e_data) so pick a conservative option.
Greg Hudson [Fri, 1 May 2009 20:19:43 +0000 (20:19 +0000)]
Fix a memory leak by reorganizing krb5_principal_internalize to use
the recommended flow control for error handling. Also initialize the
output parameter so that it is set in case of error.
Sam Hartman [Thu, 30 Apr 2009 20:17:42 +0000 (20:17 +0000)]
Implement DES and 3DES PRF. Patch fromKAMADA Ken'ichi
Currently the DES and 3DES PRF output 16-byte results. This is
consistent with RFC 3961, but we need to confirm it is consistent with
Heimdal and WG decisions. See IETF 74 minutes for some discussion of
the concern as it applies to AES and thus possibly all simplified
profile enctypes.
In ktfile_common_resolve, set the output pointer to NULL on error, and
use a cleanup label instead of freeing the same resources in multiple
error handling blocks.
Fix a memory leak by reorganizing krb5_ktf_keytab_internalize to use
the recommended exception-handling flow control. Eliminate the check
for ktdata being null after resolution because that's not possible.
Add a check for the resolved keytab being of a different type, since
that would result in data structure corruption.
Sam Hartman [Thu, 30 Apr 2009 00:38:48 +0000 (00:38 +0000)]
Do not return PREAUTH_FAILED on unknown preauth
If the KDC receives unknown pre-authentication data then ignore it.
Do not get into a case where PREAUTH_FAILED is returned because of
unknown pre-authentication. The main AS loop will cause
PREAUTH_REQUIRED to be returned if the preauth_required flag is set
and no valid preauth is found.
If DEBUG_ERROR_LOCATIONS is defined, replace uses of
krb5_set_error_message and krb5int_set_error with calls to the new
_fl variants of those functions, and include filename and line number
information in the calls. Requires C99-style variadic macros if
defined.
Fix handling of RET_SEQUENCE flag in mk_priv/mk_ncred
Regularize the handling of KRB5_AUTH_CONTEXT_RET_SEQUENCE in
krb5_mk_safe, krb5_mk_priv, and krb5_mk_ncred, using krb5_mk_safe as
a baseline. RET_SEQUENCE now implies DO_SEQUENCE for all three
functions, the sequence number is always incremented if it is used,
and outdata->seq is always set if RET_SEQUENCE is passed.
Note that in the corresponding rd_ functions, RET_SEQUENCE and
DO_SEQUENCE are independent flags, which is not consistent with the
above. This compromise is intended to preserve compatibility with
any working code which might exist using the RET_SEQUENCE flag.
Fix a few memory leaks in krb5_mk_ncred. Also tighten up the error
handling of the sequence number, only decreasing it if it was
increased. The handling of DO_SEQUENCE and RET_SEQUENCE may still be
flawed in some cases.
Move KRB5_KDB_OK_AS_DELEGATE from kdb_ext.h to kdb.h. Add kadmin
support for the flag. In the KDC, remove the restriction on returning
the flag on cross-realm TGTs since there is now a defined meaning for
that (it allows ok-as-delegate to be honored on the foreign realm's
service tickets).
Ken Raeburn [Sat, 25 Apr 2009 09:36:11 +0000 (09:36 +0000)]
make installed headers C++-safe
Now that we're installing the kadm5 headers, they should be C++-safe
like the others. Wrap the content in 'extern "C"' if compiling as
C++. New test program to verify.
In krb5_ktfileint_find_slot, don't continue the loop when we find a
final zero-length buffer. This is a minimal fix intended to be pulled
up to the 1.7 branch; a code cleanup commit will follow.
ticket: 6475
status: open
tags: pullup
target_version: 1.7
In krb5_principal_compare_flags, improve clarity slightly by using a
boolean temporary instead of an ordering temporary in the loop over
the elements, since we only care about the boolean result.
In krb5_rcache_externalize, remove a pointless null check of a pointer
we just dereferenced.
Rewrite krb5_rcache_internalize to use the recommended cleanup flow
control, closing a memory leak in the process.
In the cross-realm setup example in the admin documentation, use
"addprinc" instead of "add_princ" since the latter is not a recognized
alias for add_principal.
Change the signature of _make_etype_info_entry to take the canonical
client principal instead of the request structure. Also fixes the salt
we compute for SALTTYPE_NOREALM keys.
Sending an explicit salt for SALTTYPE_NORMAL keys is believed to be
necessary for some preauth scenarios involving aliases.
Will Fiveash [Mon, 13 Apr 2009 22:15:05 +0000 (22:15 +0000)]
Update kdb5_util man page with missing purge_mkeys command
While previously updating the kdb5_util command man page to include
documentation on new subcommands added as a result of the Master Key
Migration project I missed the purge_mkeys command. I've added that
with this commit.
Very little is likely to go wrong inside krb5_mkt_resolve (it just
allocates memory and plays with mutexes), but if anything did, the
handling was almost always wrong. Reorganize the function to handle
errors properly, using a helper create_list_node function to simplify
the task.
In krb5_get_in_tkt, r7002 introduced an unused local variable
"padata". It was accidentally used in r18641 instead of
preauth_to_use when sorting the received padata sequence, causing the
sort to be a no-op. Sort the correct sequence and eliminate the
unused local variable.
doc/definitions.texinfo had, predictably, fallen out of date with
respect to the code. Update a few of the out of date comments and
defaults, particularly the default enctype lists.
mk_safe and mk_priv require the local address to be set in the auth
context; rd_safe and rd_priv require the remote address to be set.
Create error codes for both kinds of missing addresses and stop trying
futilely to handle the cases where they are not set.
When getting initial credentials with a password, try the master if
preauth fails on a slave, since preauth can fail due to an out-of-date
key. This removes a snippet added in r14939 which was considering
only hardware preauth.
Subject k5_utf8s_to_ucs2s could deref NULL pointer..
Based on usage of this static function, this will never happen as
results are always malloced (and checked) by caller. However, the
function is already coded to handle the first argument being null - so
be consistent throughout.
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 [Tue, 7 Apr 2009 21:22:17 +0000 (21:22 +0000)]
CVE-2009-0844 SPNEGO can read beyond buffer end
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.
Adjust the logic in rule_an_to_ln to avoid a "can't happen" case of
strchr returning null when searching for a character we know is there.
Also properly return ENOMEM if we fail to allocate selstring.
Sam Hartman [Fri, 3 Apr 2009 05:36:25 +0000 (05:36 +0000)]
Handle authdata encrypted in subkey
RFC 4120 requires that if a subkey is present in the TGS request that
authorization data be encrypted in the subkey. Our KDC did not handle
this correctly.