Fix the sole case in process_chpw_request() where a return could occur
without allocating the data pointer in the response. This prevents a
later free() of an invalid pointer in kill_tcp_or_rpc_connection().
Also initialize rep->data to NULL in process_chpw_request() and clean
up *response in dispatch() as an additional precaution.
Tom Yu [Mon, 6 Dec 2010 23:23:17 +0000 (23:23 +0000)]
SA-2010-007 Checksum vulnerabilities (CVE-2010-1324 and others)
Apply patch for MITKRB5-SA-2010-007.
Fix multiple checksum handling bugs, as described in:
CVE-2010-1324
CVE-2010-1323
CVE-2010-4020
CVE-2010-4021
* Return the correct (keyed) checksums as the mandatory checksum type
for DES enctypes.
* Restrict simplified-profile checksums to their corresponding etypes.
* Add internal checks to reduce the risk of stream ciphers being used
with simplified-profile key derivation or other algorithms relying
on the block encryption primitive.
* Use the mandatory checksum type for the PKINIT KDC signature,
instead of the first-listed keyed checksum.
* Use the mandatory checksum type when sending KRB-SAFE messages by
default, instead of the first-listed keyed checksum.
* Use the mandatory checksum type for the t_kperf test program.
* Use the mandatory checksum type (without additional logic) for the
FAST request checksum.
* Preserve the existing checksum choices (unkeyed checksums for DES
enctypes) for the authenticator checksum, using explicit logic.
* Ensure that SAM checksums received from the KDC are keyed.
* Ensure that PAC checksums are keyed.
ticket: 6650
subject: Handle migration from pre-1.7 databases with master key kvno != 1
target_version: 1.7.1
tags: pullup
krb5_dbe_lookup_mkvno assumes an mkvno of 1 for entries with no
explicit tl_data. We've seen at least one pre-1.7 KDB with a master
kvno of 0, violating this assumption. Fix this as follows:
* krb5_dbe_lookup_mkvno outputs 0 instead of 1 if no tl_data exists.
* A new function krb5_dbe_get_mkvno translates this 0 value to the
minimum version number in the mkey_list. (krb5_dbe_lookup_mkvno
cannot do this as it doesn't take the mkey_list as a parameter.)
* Call sites to krb5_dbe_lookup_mkvno are converted to
krb5_dbe_get_mkvno, except for an LDAP case where it is acceptable
to store 0 if the mkvno is unknown.
ticket: 6662
subject: MITKRB5-SA-2010-001 CVE-2010-0283 KDC denial of service
tags: pullup
target_version: 1.8
Code introduced in krb5-1.7 can cause an assertion failure if a
KDC-REQ is internally inconsistent, specifically if the ASN.1 tag
doesn't match the msg_type field. Thanks to Emmanuel Bouillon (NATO
C3 Agency) for discovering and reporting this vulnerability.
Add a set_cred_option handler for SPNEGO which forwards to the
underlying mechanism. Fixes SPNEGO credential delegation in 1.7 and
copying of SPNEGO initiator creds in both 1.7 and trunk. Patch
provided by nalin@redhat.com.
Subject: ad-initial-verified-cas logic broken
ticket: 6587
status: open
In the initial pkinit implementation, the server plugin generates an
incorrect encoding for ad-initial-verified-cas. In particular, it
assumes that ad-if-relevant takes a single authorization data element
not a sequence of authorization data elements. Nothing looked at the
authorization data in 1.6.3 so this was not noticed. However in 1.7,
the FAST implementation looks for authorization data. In 1.8 several
more parts of the KDC examine authorization data. The net result is
that the KDC fails to process the TGT it issues.
However on top of this bug, there is a spec problem. For many of its
intended uses, ad-initial-verified-cas needs to be integrity
protected by the KDC in order to prevent a client from injecting it.
So, it should be contained in kdc-issued not ad-if-relevant.
For now we're simply removing the generation of this AD element until
the spec is clarified.
Tom Yu [Tue, 12 Jan 2010 05:37:06 +0000 (05:37 +0000)]
Pull up r22782, r22784, r23610 from trunk, with additional test suite
changes to compensate for the existence of the api.0/ unit tests that
removed for 1.8. Don't pull up the kadmin CLI changes for now.
The arcfour string-to-key operation in krb5 1.7 (or later) disagrees
with the dummy password used by the addprinc -randkey operation in
krb5 1.6's kadmin client, because it's not valid UTF-8. Recognize the
1.6 dummy password and use a random password instead.
Improve the mechanism used for addprinc -randkey. In the kadmin
server, if the password is null when creating a principal, treat that
as a request for a random key. In the kadmin client, try using the
new method for random key creation and then fall back to the old one.
r22529@squish: raeburn | 2009-08-12 13:49:45 -0400
.
r22530@squish: raeburn | 2009-08-12 13:55:57 -0400
Change KRBCONF_KDC_MODIFIES_KDB to a mostly run-time option.
Change all code conditionals to test a new global variable, the
initial value of which is based on KRBCONF_KDC_MODIFIES_KDB. There is
currently no way to alter the value from the command line; that will
presumably be desired later.
Change initialize_realms to store db_args in a global variable. In
process_as_req, call db_open instead of the old set_name + init.
Don't reopen if an error is reported by krb5_db_fini.
Add a test of running kinit with an incorrect password, to trigger a
kdb update if enabled.
r22531@squish: raeburn | 2009-08-12 13:58:13 -0400
Fix trailing whitespace.
ticket: 6589
subject: Fix AES IOV decryption of small messages
tags: pullup
target_version: 1.7.1
AES messages never need to be padded because the confounder ensures
that the plaintext is at least one block long. Remove a check in
krb5int_dk_decrypt_iov which was rejecting short AES messages because
it didn't count the header length.
ticket: 6588
subject: Fix ivec chaining for DES iov encryption
tags: pullup
target_version: 1.7.1
krb5int_des_cbc_decrypt_iov was using a plaintext block to update the
ivec. Fix it to use the last cipher block, borrowing from the
corresponding des3 function. The impact of this bug is not serious
since ivec chaining is not typically used with IOV encryption in 1.7.
ticket: 6585
subject: KDC MUST NOT accept ap-request armor in FAST TGS
target_version: 1.7.1
tags: pullup
Per the latest preauth framework spec, the working group has decided
to forbid ap-request armor in the TGS request because of security
problems with that armor type.
This commit was tested against an implementation of FAST TGS client to
confirm that if explicit armor is sent, the request is rejected.
Pullup to 1.7-branch is only for the test case, as krb5-1.7 behaved
correctly for these checksums.
Fix regression in MD4-DES and MD5-DES keyed checksums. The original
key was being used for the DES encryption, not the "xorkey". (key
with each byte XORed with 0xf0)
Add a test case that will catch future regressions of this sort, by
including a verification of a "known-good" checksum (derived from a
known-to-be-interoperable version of the implementation).
Quoting problems in pattern matching on the OS name cause Solaris
versions up through 9 to not be properly recognized in the
thread-system configuration setup. This causes our libraries to make
the erroneous assumption that valid thread support routines are
available on all Solaris systems, rather than just assuming it for
Solaris 10 and later.
The result is assertion failures like this one reported by Meraj
Mohammed and others:
Assertion failed: k5int_i->did_run != 0, file krb5_libinit.c, line 63
Thanks to Tom Shaw for noticing the cause of the problem.
The bug may be present in the 1.6.x series as well.
In 1.7, krb5_get_init_creds will continue attempting the same built-in
preauth mechanism (e.g. encrypted timestamp) until the loop counter
maxes out. Until the preauth framework can remember not to retry
built-in mechanisms, only continue with preauth after a PREAUTH_FAILED
error resulting from optimistic preauth.
ticket: 6568
subject: Fix addprinc -randkey when policy requires multiple character classes
tags: pullup
target_version: 1.7.1
The fix for ticket #6074 (r20650) caused a partial regression of
ticket #115 (r9210) because the dummy password contained only one
character class. As a minimal 1.7 fix, use all five character classes
in the dummy password.
ticket: 6557
subject: Supply canonical name if present in LDAP iteration
target_version: 1.7.1
tags: pullup
In the presence of aliases, LDAP iteration was supplying the first
principal it found within the expected realm, which is not necessarily
the same as the canonical name. If the entry has a canonical name
field, use that in preference to any of the principal names.
Tom Yu [Tue, 12 Jan 2010 02:49:59 +0000 (02:49 +0000)]
pull up r22708 from trunk
------------------------------------------------------------------------
r22708 | ghudson | 2009-09-03 13:39:50 -0400 (Thu, 03 Sep 2009) | 9 lines
ticket: 6556
subject: Supply LDAP service principal aliases to non-referrals clients
target_version: 1.7
tags: pullup
In the LDAP back end, return aliases when the CLIENT_REFERRALS_ONLY
flag isn't set (abusing that flag to recognize a client name lookup).
Based on a patch from Luke Howard.
Disable the COPY_FIRST_CANONNAME workaround on Linux glibc 2.4 and
later, since it leaks memory on fixed glibc versions. We will still
leak memory on glibc 2.3.4 through 2.3.6 (e.g. RHEL 4) but that's
harder to detect.
On certain error conditions, prep_reprocess_req() calls kdc_err() with
a null pointer as the format string, causing a null dereference and
denial of service. Legitimate protocol requests can trigger this
problem.
If the underlying mechanism's accept_sec_context returns an error, the
spnego accept_sec_context was leaving allocated data in
*context_handle, which is incorrect for the first call according to
RFC 2744.
Fix this by mirroring some code from the spnego init_sec_context,
which always cleans up the half-constructed context in case of error.
This is allowed (though not encouraged) by RFC 2744 for second and
subsequent calls; since we were already doing it in init_sec_context,
it seems simpler to do that than keep track of whether this is a first
call or not.
user() was replying to the user command and then calling login(),
which could send a continuation reply if it fails to chdir to the
user's homedir. Continuation replies must come before the actual
reply; the mis-ordering was causing ftp and ftpd to deadlock. To fix
the bug, invoke login() before reply() so that the continuation reply
comes first.
Include <assert.h> in k5-platform.h, since we use assertions in some
of the macros defined there, as well as in many source files which do
not themselves include <assert.h>. Report and fix by Rainer Weikusat.
ticket: 6530
target_version: 1.7.1
tags: pullup
subject: check for slogin failure in setup_root_shell
Add a check for a slogin message that indicates an unknown public key
fingerprint, as rlogin looks like it points to slogin by default on
Debian Lenny.
Add a new '-W' option to kadmind and kdb5_util create to allow reading
weak random numbers on startup, to avoid long delays in testing
situations. Use only for testing.
Add test case omitted in last commit.
------------------------------------------------------------------------
r22422 | tlyu | 2009-06-25 22:43:21 -0400 (Thu, 25 Jun 2009) | 8 lines
ticket: 6515
subject: reduce some mutex performance problems in profile library
tags: pullup
target_version: 1.7.1
version_reported: 1.7
In profile_node_iterator we unlock a mutex in order to call
profile_update_file_data, which wants to lock that mutex itself, and
then when it returns we re-lock the mutex. (We don't use recursive
mutexes, and I would continue to argue that we shouldn't.) On the
Mac, when running multiple threads, it appears that this results in
very poor peformance, and much system and user CPU time is spent
working with the locks. (Linux doesn't seem to suffer as much.)
So: Split profile_update_file_data into a locking wrapper, and an
inner routine that does the real work but requires that the lock be
held on entry. Call the latter from profile_node_iterator *without*
unlocking first, and only unlock if there's an error. This doesn't
move any significant amount of work into the locking region; it pretty
much just joins locking regions that were disjoint for no good reason.
On my tests on an 8-core Mac, in a test program running
gss_init_sec_context in a loop in 6 threads, this brought CPU usage
per call down by 40%, and improved wall-clock time even more.
Single-threaded performance improved very slightly, probably in the
noise.
Linux showed modest improvement (5% or less) in CPU usage in a
3-thread test on a 4-core system.
Similar tests with gss_accept_sec_context showed similar contention
around the profile-library mutexes, but I haven't analyzed the
performance changes there from this patch.
ticket: 6514
subject: minor memory leak in 'none' replay cache type
tags: pullup
target_version: 1.7.1
version_reported: 1.7
The replay cache type implementations are responsible for freeing the
main rcache structure when the cache handle is closed. The 'none'
rcache type wasn't doing this, resulting in a small memory leak each
time such a cache was opened and closed. Not a big deal for a server
process servicing a single client, but it could accumulate (very very
slowly) for a long-running server.
In the previous patch - I neglected a potential NULL deref in the call
to krb5int_yarrow_cipher_final. Trivial fix.
------------------------------------------------------------------------
r22410 | epeisach | 2009-06-11 13:01:13 -0400 (Thu, 11 Jun 2009) | 7 lines
subject: krb5int_yarrow_final could deref NULL if out of memory
ticket: 6512
krb5int_yarrow_final tests if the Yarrow_CTX* is valid (not NULL) -
and if not - signals and error for return - but still invokes
mem_zero (memset) with it as an argument. This will only happen in
an out-of-memory situation.
ticket: 6511
subject: krb5int_rd_chpw_rep could call krb5_free_error with random value
clang picked up on a path in which krberror is not set and passed as
an argument to krb5_free_error(). Essentially if the clearresult
length < 2 but everything decodes - you can hit this path...
ticket: 6509
subject: kadmind is parsing acls good deref NULL pointer on error
In kadm5int_acl_parse_line, if you setup an acl w/ restrictions
(i.e. the four argument acl format) - but have an error parsing the
first few fields, acle is NULLed out, and is then derefed.
This adds a conditional and indents according to the krb5 c-style...
Tom Yu [Mon, 28 Sep 2009 20:27:13 +0000 (20:27 +0000)]
pull up r22402 from trunk
------------------------------------------------------------------------
r22402 | epeisach | 2009-06-05 23:55:44 -0400 (Fri, 05 Jun 2009) | 7 lines
ticket: 6508
subject: kadm5int_acl_parse_restrictions could ref uninitialized variable
The variable sp is never initialized. If the first argument to the
function is null, the code falls through to freeing sp if valid.
However, sp is never set.
ticket: 6506
subject: Make results of krb5_db_def_fetch_mkey more predictable
tags: pullup
target_version: 1.7
krb5_db_def_fetch_mkey tries the stash file as a keytab, then falls
back to the old stash file format. If the stash file was in keytab
format, but didn't contain the desired master key, we would try to
read a keytab file as a stash file. This could succeed or fail
depending on byte order and other unpredictable factors. The upshot
was that one of the libkadm5 unit tests (init 108) was getting a
different error code on different platforms.
To fix this, only try the stash file format if we get
KRB5_KEYTAB_BADVNO trying the keytab format. This requires reworking
the error handling logic.
Correction to patch in r22364: "i" was used in two places, one of
which required an int-sized value and the other of which required a
size_t. Instead of changing the type, split the two uses into
separate variables.
Check C++ compatibility for some internal headers that may (now or in
the future) be used in C++ code on Windows.
------------------------------------------------------------------------
r21918 | raeburn | 2009-02-09 11:35:01 -0500 (Mon, 09 Feb 2009) | 3 lines
More C++ compatibility: Don't use "typedef struct tag *tag"; rename
the tag and keep the same typedefname.
------------------------------------------------------------------------
r21917 | raeburn | 2009-02-09 11:28:29 -0500 (Mon, 09 Feb 2009) | 3 lines
C++ compatibility fix -- g++ says "types may not be defined in casts",
so do the gcc unaligned-struct trick only for C, not C++.
------------------------------------------------------------------------
r21902 | raeburn | 2009-02-05 16:56:21 -0500 (Thu, 05 Feb 2009) | 2 lines
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.
Tom Yu [Tue, 26 May 2009 07:58:28 +0000 (07:58 +0000)]
pull up r22381 from trunk
------------------------------------------------------------------------
r22381 | ghudson | 2009-05-25 18:40:00 +0200 (Mon, 25 May 2009) | 10 lines
ticket: 6501
subject: Temporarily disable FAST PKINIT for 1.7 release
tags: pullup
target_version: 1.7
There are protocol issues and implementation defects surrounding the
combination of FAST an PKINIT currently. To avoid impacting the 1.7
scheduled and to avoid creating interoperability problems later,
disable the combination until the problems are resolved.
In the KDC, get_preauth_hint_list had two bugs initializing the
preauth array. It was allocating 21 extra entries instead of two due
to a typo (harmless), and it was only zeroing up through one extra
entry (harmful). Adjust the code to use calloc to avoid further
disagreements of this nature.
ticket: 6495
subject: Fix test rules for non-gmake make versions
target_version: 1.7
tags: pullup
The build rules for the new t_ad_fx_armor and t_authdata test programs
used $<, which is only portable for implicit rules (but is valid in
gmake for all rules). Stop using $< in those rules so that "make
check" works with System V make.
In handle_authdata in the KDC, remove a spurious assertion (added in
r21566 on the mskrb-integ branch) that authdata starts out empty.
authdata can be legitimately added by check_padata, which precedes
handle_authdata, and this happens with pkinit.
When using keyed checksum types with TGS subkeys, Microsoft AD 2003
verifies the checksum using the subkey, whereas MIT and Heimdal verify
it using the TGS session key. (RFC 4120 is actually silent on which
is correct; RFC 4757 specifies the TGS session key.) To sidestep this
interop issue, don't use keyed checksum types with RC4 keys without
explicit configuration in krb5.conf. Using keyed checksum types with
AES is fine since, experimentally, AD 2008 accepts checksums keyed
with the TGS session key.
Copy the sequence key rather than the subkey for lucid contexts in RFC
1964 mode, so that we map to raw des enctypes rather than say
des-cbc-crc.
------------------------------------------------------------------------
r22351 | ghudson | 2009-05-14 18:50:52 +0200 (Thu, 14 May 2009) | 9 lines
ticket: 6488
status: open
tags: pullup
target_version: 1.7
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: 6489
subject: UCS2 support doesn't handle upper half of BMP
tags: pullup
target_version: 1.7
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.
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.
Tom Yu [Mon, 11 May 2009 20:56:53 +0000 (20:56 +0000)]
pull up r22325 from trunk
------------------------------------------------------------------------
r22325 | hartmans | 2009-05-07 16:35:28 -0400 (Thu, 07 May 2009) | 18 lines
Changed paths:
M /trunk/src/include/k5-int.h
M /trunk/src/lib/krb5/krb/decode_kdc.c
M /trunk/src/lib/krb5/krb/gc_via_tkt.c
M /trunk/src/lib/krb5/libkrb5.exports
Subject: Try decrypting using session key if subkey fails in tgs rep handling
ticket: 6484
Tags: pullup
Target_Version: 1.7
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.
Tom Yu [Mon, 11 May 2009 20:56:50 +0000 (20:56 +0000)]
pull up r22324 from trunk
------------------------------------------------------------------------
r22324 | hartmans | 2009-05-07 16:35:19 -0400 (Thu, 07 May 2009) | 8 lines
Changed paths:
M /trunk/src/kadmin/cli/k5srvutil.M
M /trunk/src/kadmin/cli/kadmin.M
M /trunk/src/kadmin/cli/kadmin.local.M
M /trunk/src/kadmin/ktutil/ktutil.M
ticket: 6483
Subject: man1 in title header for man1 manpages
Target_Version: 1.7
Tags: pullup
A previous ticket moved kadmin, kadmin.local, ktutil and k5srvutil man
pages to man1 from man8. This updates the section within the man
page.
Tom Yu [Mon, 11 May 2009 20:55:57 +0000 (20:55 +0000)]
pull up r22298 from trunk
------------------------------------------------------------------------
r22298 | hartmans | 2009-04-30 16:17:42 -0400 (Thu, 30 Apr 2009) | 10 lines
Changed paths:
M /trunk/src/lib/crypto/des/Makefile.in
M /trunk/src/lib/crypto/des/des_int.h
A /trunk/src/lib/crypto/des/des_prf.c (from /trunk/src/lib/crypto/dk/dk_prf.c:22295)
M /trunk/src/lib/crypto/etypes.c
M /trunk/src/lib/crypto/t_cf2.comments
M /trunk/src/lib/crypto/t_cf2.expected
M /trunk/src/lib/crypto/t_cf2.in
ticket: 5587
Tags: pullup
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.
ticket: 6480
Subject: Do not return PREAUTH_FAILED on unknown preauth
Target_Version: 1.7
Tags: pullup
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.
Tom Yu [Mon, 11 May 2009 20:55:51 +0000 (20:55 +0000)]
pull up r22291 from trunk
------------------------------------------------------------------------
r22291 | ghudson | 2009-04-29 19:21:21 -0400 (Wed, 29 Apr 2009) | 9 lines
Changed paths:
M /trunk/src/include/k5-err.h
M /trunk/src/include/k5-int.h
M /trunk/src/lib/krb5/krb/kerrs.c
M /trunk/src/lib/krb5/libkrb5.exports
M /trunk/src/util/support/errors.c
M /trunk/src/util/support/libkrb5support-fixed.exports
ticket: 6479
subject: Add DEBUG_ERROR_LOCATIONS support
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.
Tom Yu [Mon, 11 May 2009 20:55:45 +0000 (20:55 +0000)]
pull up r22283, r22288 from trunk. r22283 was not originally part of
this ticket but is a prereq for the mk_cred.c change.
------------------------------------------------------------------------
r22288 | ghudson | 2009-04-28 14:00:13 -0400 (Tue, 28 Apr 2009) | 14 lines
Changed paths:
M /trunk/src/lib/krb5/krb/mk_cred.c
M /trunk/src/lib/krb5/krb/mk_priv.c
M /trunk/src/lib/krb5/krb/mk_safe.c
ticket: 6478
subject: 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.
------------------------------------------------------------------------
r22283 | ghudson | 2009-04-27 19:48:22 -0400 (Mon, 27 Apr 2009) | 5 lines
Changed paths:
M /trunk/src/lib/krb5/krb/mk_cred.c
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.