Greg Hudson [Fri, 24 Oct 2014 20:56:47 +0000 (16:56 -0400)]
Add ASN.1 encoder and decoder for CAMMAC
Add internal type declarations for krb5_verifier_mac and krb5_cammac.
Add ASN.1 encoder and decoder functions and an internal free function
for krb5_cammac. Add ASN.1 tests for krb5_cammac as well as asn1c
test vectors for Verifier and AD-CAMMAC.
In kdc_preauth.c, commit be20a5f5cee8d6c4072d1b81712520dbf9f6eefd made
load_preauth_plugins() handle negative preauth type numbers.
get_plugin_vtables() also needs to handle negative preauth type
numbers, or it can return the wrong count and load_preauth_plugins()
can overflow the table.
Tom Yu [Thu, 2 Jul 2015 20:16:07 +0000 (16:16 -0400)]
Unify KDB principal flag conversion functions
These changes unify the KDB principal flag specifiers used by kadmin,
kdc.conf default_principal_flags, and kadm5.acl. Each of those
interfaces will now accept any of the historically accepted input
forms of any of those interfaces. Additionally, accept flag
specifiers in the forms that kadmin prints, as well as hexadecimal
numbers.
Replace krb5_string_to_flags() with krb5_flagspec_to_mask(). The
latter has a pseudo-ternary output, allowing different pointers for
flags to set versus flags to clear. Additional functionality includes
parsing hexadecimal numbers for flag settings.
Remove krb5_input_flag_to_string(), which nothing in the tree used,
and probably hasn't ever worked properly due to long-standing gaps in
the flag number sequence.
Remove krb5_flags_to_string(), which nothing in the tree used.
Verbose flag output can be added back through another interface if
there is demand.
Add krb5_flagnum_to_string(), which produces a string representation
of a flag number. Additional functionality includes output of
hexadecimal numbers for unknown flags.
Add krb5_flags_to_strings(), which produces an array of strings
describing the flags, using the output from krb5_flagnum_to_string().
Tom Yu [Wed, 1 Jul 2015 20:28:45 +0000 (16:28 -0400)]
Add test suite for KDB principal flags
Test kadmin.local reading of principal flag specifiers, kdc.conf
setting of default_principal_flags, and kadm5.acl restrictions. Only
really tests one flag at a time.
Also start requiring Python 2.5 for the test suite. It's been around
for long enough, and some syntax features such as conditional
expressions are useful.
ticket: 8215 (new)
subject: Unify KDB principal flag specifiers
target_version: 1.14
Greg Hudson [Sun, 15 Mar 2015 19:56:34 +0000 (15:56 -0400)]
Test client_keyblock kdcpreauth callback
Add internal clpreauth and kdcpreauth modules named "test" which can
exercise the client_keyblock callback (as well as get_string and
get_as_key on the client side). Add tests to t_etype_info.py to
verify that the callback matches the etype info sent by the KDC.
In the KDC's load_preauth_plugins(), correct a test for the end of
pa_type_list so that we can use a negative preauth type number for the
test module. (RFC 4120 reserves negative preauth type values for
unregistered use.)
Greg Hudson [Thu, 4 Jun 2015 18:08:06 +0000 (14:08 -0400)]
Add client_keyblock kdcpreauth callback
Add a new kdcpreauth callback which gets the selected client key.
This callback can be used by preauth mechs which need to use the
singular reply key in a challenge sent by the KDC, now that we send
only one etype-info entry in PREAUTH_REQUIRED errors.
Greg Hudson [Sat, 6 Jun 2015 01:19:15 +0000 (21:19 -0400)]
Add tests for KDC etype-info behavior
Create a new test harness etinfo.c which can display etype-info2
information in KDC responses. Use it to test the etype-info results
in preauth_required error e-data and AS-REP padata.
Greg Hudson [Sat, 6 Jun 2015 19:45:39 +0000 (15:45 -0400)]
Only include one key in etype-info
As described in RFC 6113 section 2.1, the KDC can choose a single
long-term key at the beginning of the preauth conversation based on
the request enctype list. Implement this change for the PA-ETYPE-INFO
and PA-ETYPE-INFO2 padata included in preauth hint lists, by selecting
the client key before checking padata, making the client keyblock
available in the preauth rock, and unifying the etype-info handlers to
use a single helper function for edata and AS-REP padata.
crypto_retrieve_cert_sans() is allowed to set its princs output to
NULL, although the OpenSSL implementation rarely does. Fix the
TRACE_PKINIT_CLIENT_SAN_KDCCERT_PRINC for loop to allow this like other
parts of the function do, and also get rid of the unnecessary princptr
variable by using an integer index like other parts of the function.
Bump the minor version of the kadm5_hook interface to 2 and add a
rename method. Invoke the rename method in kadm5_rename_principal()
like we do for other libkadm5srv operations.
Use memory cache in gss_acquire_cred_with_password
gss_acquire_cred_with_password() was originally introduced in Solaris.
When we introduced it in 1.9, we unfortunately gave it different and
less useful semantics. Restore this function to the Solaris
semantics, which are to always get credentials and store them in a
private memory ccache. The caller can use gss_store_cred() to make
the resulting creds visible to other processes if desired.
Tom Yu [Thu, 25 Jun 2015 23:31:53 +0000 (19:31 -0400)]
Deindent krb5_string_to_keysalts
Remove a level of indentation for the list-appending part of the
krb5_string_to_keysalts() loop body by consolidating the strtok_r()
calls into the controlling expreession of the loop.
In the SPNEGO mechanism, if we see the GSS_KRB5_CRED_NO_CI_FLAGS_X
option, do not explicitly ask for integrity flag from underlying
mechanisms. Adjust t_ciflags.c to match the new behavior, and add a
SPNEGO test using a normal initiator cred.
[ghudson@mit.edu: adjust style; fix tests here instead of in a
subsequent commit; clarify commit message]
Use a helper function to allocate SPNEGO cred handles, to make sure
that all members of the cred structure are zeroed or initialized.
[ghudson@mit.edu: avoid gss_ prefix on helper function and give it a
name similar to create_spnego_ctx(); unbrace some single-line if
bodies; clarify commit message]
Solly Ross [Mon, 8 Jun 2015 18:06:18 +0000 (14:06 -0400)]
Allow gss_inquire_context on partial krb5 contexts
RFC 2743 states that gss_inquire_context() must always return flags,
locally_initiated, and open even if a context is not yet fully
established. Additionally, a partially established context may also
return mech_type.
Previously, the krb5 mech raised an error for inquire_context on
partially completed contexts. It now follows the rules layed out in
RFC 2743.
Add a new test program to verify that gss_inquire_context() works
correctly on both in-progress and established contexts.
[ghudson@mit.edu: minor style changes and commit message edits]
Ben Kaduk [Wed, 24 Jun 2015 17:34:32 +0000 (13:34 -0400)]
Make the Principal column wider by default
The current 100-pixel default is too small to fit most principal
names (particularly the realm). There is no reason why all the
columns must be the same width, so promote this one as needing
more space.
ticket: 8211 (new)
subject: Leash's column for principal name truncates most principals
queue: kfw
tags: pullup
target_version: 1.13.3
Ben Kaduk [Tue, 23 Jun 2015 14:38:19 +0000 (10:38 -0400)]
Make registry hostrealm module highest precedence
Testing reveals that there are a number of machines in the wild
which retain old krb5.ini files across domain configuration changes,
and it is difficult to determine which machines are potentially
affected by incorrect stale configuration data.
To enable domain administrators to easily ensure that the correct
default realm is set, allow the registry hostrealm module to take
precedence over the profile.
Note that the registry hostrealm module can still be disabled
in the hostrealm interface configuration statment in the
[plugins] section of the profile.
ticket: 8209 (new)
subject: stale krb5.ini files still cause default realm WIN.MIT.EDU
tags: pullup
target_version: 1.13.3
Ben Kaduk [Fri, 29 May 2015 18:46:58 +0000 (14:46 -0400)]
Stop using the WiX src attribute
The src attribute has been deprecated for a while in favor of
different (more descriptive) attributes for the different XML
elements involved.
For the File element, use the Source attribute.
For the Directory element, use the FileSource attribute.
For the Merge, Binary, and Text elements, use the SourceFile attribute.
This makes the installer build much quieter, with the warnings from
the light.exe invocation all fitting into the default history buffer.
Ben Kaduk [Wed, 27 May 2015 19:36:51 +0000 (15:36 -0400)]
Stop using the WiX Registry element
It is deprecated in favor of more specific XML elements such
as RegistryKey, RegistryValue, and RemoveRegistryKey, so as to
stop overloading a single element for what are fundamentally
different types and operations.
RegistryValue elements can be children of RegistryKey elements,
allowing the Key attribute to be inherited, or bare within the
containing Component. We do not take advantage of the inheritance
at this time, since that would be a more disruptive change.
WiX would prefer for us to not use the createAndRemoveOnUninstall
attribute of <RegistryKey>, in favor of ForceCreateOnInstall
and/or ForceRemoveOnUninstall, but that can wait for a follow-up
commit.
Some instances of <Registry> were commented-out and can simply be
removed.
Some of the <Registry> elements used to create keys were also
setting the KeyPath attribute, which is not permitted in the
<RegistryKey> element. According to
http://sourceforge.net/p/wix/bugs/3197/ , this should never have
been allowed, and non-value registry keys should not be used as
MSI KeyPaths. In all affected cases, there are child
RegistryValue elements that are suitable for use as KeyPath
elements instead.
Some of the <Registry> elements were present with a duplicate
element with different Id that added no new attributes; those
duplicate elements can safely be removed.
<RemoveRegistryKey> is used to replace <Registry> elements
with Action=removeKeyOnInstall.
Ben Kaduk [Mon, 22 Jun 2015 23:11:28 +0000 (19:11 -0400)]
Put focus on password field when principal is set
In the Leash "Get Ticket" window, under some user-customized
configurations, the principal field can be pre-populated. In this case,
it makes sense for the default focus to be on the password field,
since most users will only be using a single principal and should
go directly to typing their password.
The focus was already set to the password field in the case when
the principal was not modifiable (such as when the "Get Ticket" dialog
was opened by an application requesting a specific ticket).
Ben Kaduk [Fri, 22 May 2015 18:38:27 +0000 (14:38 -0400)]
Add leash column for the ccache name
Previously, it was fairly easy to set the default cache to MSLSA:,
which is not collection-enabled (as opposed to the normal default
of API:, which is), and the non-collection behavior proved confusing
to many users.
Ideally there would be an option to choose the output ccache in
the "Get Ticket" window, but that is more complicated to implement
than just a display of what cache a given ticket is in. This
extra display column should still help to alleviate user confusion.
Ben Kaduk [Fri, 22 May 2015 18:42:02 +0000 (14:42 -0400)]
Make ribbon home tab more pretty
In the absence of a label title for a given command group, the
text "MS Shell Dlg" was inserted, which is somewhat confusing to
the user. Give the ticket-maniuplation group a title consisting
of a space, which removes the confusing text from the display.
Microsoft implements GSS-SPNEGO, a non-standard SASL mechanism which
omits the usual wrap exchange after the GSS context is established.
As a result, it does not support authzids, does not negotiate a
maximum message size, and implicitly negotiates a security layer based
on the GSS flags asserted by the client. If the client asserts GSS
flags corresponding to a security layer the server can't support, the
server has no recourse except to reject the connection.
Implement Heimdal's GSS_KRB5_CRED_NO_CI_FLAGS_X cred option. When set
on an initiator cred, do not assert the confidentiality and integrity
flags in initiator tokens unless they were requested by the caller.
Our SPNEGO mechanism always requests integrity from the underlying
mechanism, which limits the utility of this option. That issue will
be addressed in the future; even if it isn't, Samba currently uses its
own SPNEGO implementation, so can benefit from the cred option in
krb5.
[ghudson@mit.edu: expand GSS_KRB5_CRED_NO_CI_FLAGS_X comment, edit
commit message, use a boolean cred field]
Greg Hudson [Mon, 15 Jun 2015 21:34:23 +0000 (17:34 -0400)]
Fix make_signedpath_checksum() initialization bug
data needs to be initialized since it is freed in the cleanup handler.
The bug was introduced by 0c6498b2b9f4f4ad8b9f224714c84714425f2ca3 and
is not part of any release.
Greg Hudson [Tue, 3 Mar 2015 00:19:19 +0000 (19:19 -0500)]
Add tests for AD-SIGNTICKET corner cases
Test situations where the previous AD-SIGNTICKET logic would not use
the same key to create and verify the AD-SIGNTICKET data. Also test a
case which forces the new verification logic to try multiple krbtgt
versions.
Greg Hudson [Mon, 2 Mar 2015 21:27:47 +0000 (16:27 -0500)]
Use local TGT for AD-SIGNTICKET processing
Always use the first key of the local TGT to create the AD-SIGNTICKET
checksum, and try the first key of the last three kvnos of the local
TGT to verify the checksum.
Greg Hudson [Wed, 11 Feb 2015 18:40:44 +0000 (13:40 -0500)]
Load local TGT in KDC requests
Ensure that we have the server realm TGT principal entry at hand for
AS and TGS requests. In the common case, this is the same as the AS
server or TGS header server principal, but in less common cases
(direct AS requests for service tickets, cross-realm TGS requests) we
will need to explicitly load it.
The local TGT entry is not used in this commit. In the short term, it
will be used to verify and sign CAMMACs and to shore up some edge
cases in AD-SIGNTICKET. In the longer term, we might allow realm
configuration variables to be stored in local TGT tl-data.
Greg Hudson [Mon, 9 Feb 2015 20:23:05 +0000 (15:23 -0500)]
Rename krbtgt variable in KDC code
In a TGS request, the header ticket server is usually a local or
cross-realm TGS principal, but for ticket modification requests it
doesn't have to be. Similarly, the server for an AS request is
usually a krbtgt principal, but in some cases it is not. Since the
KDC code must consider all possibilities, avoid using the name
"krbtgt" for entries which aren't necessarily TGTs.
In process_tgs_req(), rename krbtgt to header_server and tgskey to
header_key. In handle_authdata(), rename the parameters similarly and
pass NULL from process_as_req() for the header_server and header_key
parameters; the code which uses those parameters is adjusted to match.
In validate_transit_path(), rename krbtgt to header_srv.
Do not change the semantics of the sign_authdata DAL method at this
time, but more accurately document the krbtgt and krbtgt_key
parameters.
Remove plugins/authdata/greet, which was a v0 KDC module. Modify
plugins/authdata/greet_server to use the new interface. Within
greet_auth.c, remove the unused function greet_kdc_verify. Build the
greet_client and greet_server modules by default, but do not install
them.
Remove the server authdata interface declarations from authdata.h and
the code to load and invoke old KDC authdata modules. Add code to
load and invoke authdata modules using the new kdcauthdata pluggable
interface.
Add a pluggable interface for KDC authdata using the current plugin
infrastructure, mirroring the KDC functionality of authdata_plugin.h.
Like the old interface, this one isn't yet public.
Remove the static_authdata_systems table from kdc_authdata.c and
instead call the relevant functions explicitly in handle_authdata.
Eliminate the flags field from krb5_authdata_systems as it is no
longer used. Rename the functions to be more descriptive. Move
simple conditionals on authdata processing to handle_authdata for
clarity. Move handle_authdata to the end of the file to avoid the
need for static function declarations.
Greg Hudson [Wed, 10 Jun 2015 23:48:51 +0000 (19:48 -0400)]
Tolerate null oid pointer in gss_release_oid()
Under some circumstances, gss_inquire_name() can call
gss_release_oid() with a null oid pointer, which currently causes a
null dereference. The least invasive fix is for gss_release_oid() to
check for the invalid null pointer and return an error, like other
GSS-API functions do.
Christian Heimes [Fri, 15 May 2015 07:37:31 +0000 (09:37 +0200)]
Support SNI in MS-KKDCP client
In the k5tls plugin module, call SSL_set_tlsext_host_name() to allow
the server to use SNI support. SSL_set_tlsext_host_name() is a macro
which uses SSL_CTRL_SET_TLSEXT_HOSTNAME and is not available in all
versions of OpenSSL, so conditionalize on that constant.
Ben Kaduk [Mon, 11 May 2015 17:08:42 +0000 (13:08 -0400)]
Reboot after KfW installs to help the LSA cache
It seems that we need to restart in order to be able to query the
contents of the the LSA cache, even if the only contents of the LSA
cache are what we put there, and even if the Microsoft klist.exe
correctly reports the presence of tickets in the LSA cache.
Ben Kaduk [Tue, 5 May 2015 20:55:41 +0000 (16:55 -0400)]
Supply a hostrealm module to query the registry
Implement a default_realm function that checks the
{HKLM,HKCU}\Software\MIT\Kerberos5\default_realm registry values
on Windows, and just returns KRB5_PLUGIN_NO_HANDLE on Unix.
Ben Kaduk [Fri, 6 Mar 2015 20:42:10 +0000 (15:42 -0500)]
Fix loop to determine MSLSA principal name
When looping over principals, check the i-th entry instead of
looking at the 0-th entry each time through the loop. This would
only affect cases when multiple ticket entries were returned from
the LSA, the first one did not have a valid principal name, but
some other one did. It is expected that all of the returned
ticket entries will always have a valid client principal name, so
this is unlikely to cause any functional difference.
Ben Kaduk [Wed, 5 Dec 2012 17:15:07 +0000 (12:15 -0500)]
KfW shortcuts for make default, change password
Shortcut keys such as these (in the ACCELERATORS entry in the resource
file) are what let users type, e.g., ctrl-t to get to the "get tickets"
dialog directly from the main frame. We had shortcut keys for all the
other buttons already, so add these to complete the set.
The make default and change password functionality were already available
using keyboard-only interfaces via the ribbon access keys (tap alt,
then letters to walk through the tree of controls), but the two forms
of keyboard access are implemented differently.
Ben Kaduk [Fri, 10 Apr 2015 21:33:40 +0000 (17:33 -0400)]
Switch to Windows SDK Ribbon from MFC Ribbon
The MFC Ribbon implementation is not very accessible (e.g., to
screen reading software), whereas the windows ribbon provides
essentially the same functionality and good integration with
screen reading software, including the built-in Windows Narrator.
Remove the RT_RIBBON_XML resource from the resource file and
replace it with an inclusion of the generated kfwribbon.rc file.
Also remove the ribbon1.mfcribbon-ms ribbon description from the
res/ directory. Add the appropriate dependency relation in the
Makefile.
LeashUIApplication implements the IUIUApplication interfaces. It
appears to be difficult to cleanly tear down the underlying
IUIFramework and ribbon, since the WM_DESTROY event is handled by the
parent MFC window, which will not call IUIFramework::Destroy().
Manually inserting a call to IUIFramework::Destroy() in the shutdown
handling of the MFC classes is difficult, since the WM_DESTROY message
is handled by a different window than where the ribbon is initialized,
and the MFC framework will attempt to access window objects
corresponding to the UI Ribbon resources after they are destroyed,
which raises exceptions. It seems best to just go without destroying
the IUIFramework, since its lifecycle matches that of the application
and there will be no leaks during the application lifecycle.
LeashUICommandHandler implements the IUICommandHandler interfaces,
passing messages through to the existing MFC handlers, though the
default values for the various checkbox controls must be duplicated.
The (MFC) CMainFrame creates and maintains a handle to the
LeashUIApplication associated with the ribbon it creates, so that
it can query the height of the ribbon and redraw when the
LeashUIApplication signals that the ribbon size has changed.
Record that the added object files depend on kfwribbon.h, so that
the XML markup is compiled sufficiently early in the build.
Ben Kaduk [Thu, 19 Mar 2015 18:14:02 +0000 (14:14 -0400)]
Mention Visual Studio 2010 SP1 in windows README
The service pack is needed to avoid a linker error due to an
issue with the cvtres.exe utility, which manifests as
LINK: fatal error LNK1123: failure during conversion to COFF: file
invalid or corrupt.
Benjamin Kaduk [Fri, 17 Jan 2014 21:02:13 +0000 (16:02 -0500)]
XML Ribbon markup file
The standard windows library ribbon interface is either constructed
at runtime or specified in an XML file. Since we have a static
set of functionality in our ribbon, it is simplest to just use the
XML file.
This should duplicate the interfaces currently provided by the
MFC ribbon, though the menu items in the file menu are slightly
taller than they used to be.
Use uicc.exe to compile the XML to the binary format and produce
a kfwribon.rc resource file and kfwribbon.h header.
Benjamin Kaduk [Mon, 27 Jan 2014 18:07:38 +0000 (13:07 -0500)]
Import separate large ribbon bitmaps
This is the content from homelarge.bmp split up into the
separate component images, since the windows ribbon has the
(more sane) interface of using a separate resource for each
graphic, instead of expecting them all in a single bitmap which
is sliced up at runtime.
The bitmaps are required to have alpha channels, and it seems that
the easiest way to generate bitmaps with alpha channels is to use
Microsoft Paint, since the normal Unix open-source graphics tools
do not want to output this format.
Benjamin Kaduk [Wed, 22 Jan 2014 05:05:57 +0000 (00:05 -0500)]
Remove MBCS from leash's DEFINES
This is just enabling the use of multi-byte character set in the
MFC library, but we do not appear to make use of this feature.
Visual Studio 2013 gives ominous warnings that support for it may
be removed in future versions, so quiet the build and do not
enable the deprecated feature we are not using.
Ben Kaduk [Tue, 14 Apr 2015 19:33:20 +0000 (15:33 -0400)]
Fix leash crash found in some build environments
When freeing a credentials cache name obtained from
krb5_cc_get_full_name(), the code was using plain free()
instead of the matching krb5_free_string(). If these routines
are picked from different modules at runtime, the mismatch
will cause a crash in free(), so change to using the matched
deallocation function.
In order to use it in leash, it must be declared in Lglobals.h and
the function pointer symbol defined in Leash.cpp.
This file is out of date, and we now use the wiki for the kind of
material it covers. Most of the information here is covered
http://k5wiki.kerberos.org/wiki/Committer_resources
Greg Hudson [Tue, 24 Mar 2015 16:02:37 +0000 (12:02 -0400)]
Prevent requires_preauth bypass [CVE-2015-2694]
In the OTP kdcpreauth module, don't set the TKT_FLG_PRE_AUTH bit until
the request is successfully verified. In the PKINIT kdcpreauth
module, don't respond with code 0 on empty input or an unconfigured
realm. Together these bugs could cause the KDC preauth framework to
erroneously treat a request as pre-authenticated.
CVE-2015-2694:
In MIT krb5 1.12 and later, when the KDC is configured with PKINIT
support, an unauthenticated remote attacker can bypass the
requires_preauth flag on a client principal and obtain a ciphertext
encrypted in the principal's long-term key. This ciphertext could be
used to conduct an off-line dictionary attack against the user's
password.
Greg Hudson [Wed, 4 Mar 2015 19:43:20 +0000 (14:43 -0500)]
Add tests for key rotation and 32-bit keytab kvnos
In t_keytab.py, test that kvnos no longer wrap after 255 or 32767, that
they do wrap from 65535 to 1, and that kadmin ktrem preserves the more
recent key after a wraparound.
Also test edge cases of the 32-bit keytab kvno extension using
hand-crafted keytab entries.
Greg Hudson [Sun, 8 Mar 2015 20:52:11 +0000 (16:52 -0400)]
Adjust keytab kvno workarounds
In krb5_ktfile_get_entry(), change the pivot and fuzzy match
workarounds for kvnos to work with the 32-bit kvno extension. For the
pivot logic, try to recognize kvno wraparound at boundary by looking
at the relative timestamps and the size of the version difference.
For the fuzzy match logic, remember the first match against the low 8
bits of the desired kvno, but keep searching for an exact match.
Greg Hudson [Wed, 4 Mar 2015 19:43:00 +0000 (14:43 -0500)]
Implement 32-bit keytab kvno extension
Heimdal and Shishi support a 32-bit kvno at the end of a keytab entry,
overriding the 8-bit version if present. Implement this in the FILE
keytab type and document it in keytab_file_format.rst.
Greg Hudson [Wed, 4 Mar 2015 22:19:56 +0000 (17:19 -0500)]
Expand kadmin protocol kvno range
Make xdr_krb5_kvno() use xdr_u_int() instead of xdr_u_char(), allowing
it to marshal kvno values up to 32 bits. This change is
backwards-compatible because XDR uses four bytes to marshal char
values and does no bounds checking of char values on decode.
Greg Hudson [Sun, 8 Mar 2015 20:20:07 +0000 (16:20 -0400)]
Use unsigned 16-bit type for key data kvno
Change key_data_kvno from a signed 16-bit field to an unsigned 16-bit
field, since negative values are never meaningful. When adding new
keys, wrap from 65535 to 1 to avoid using the special value 0.
Don't bump the KDB binary version since this change is unlikely to
affect callers.
krb5_ldap_get_value() takes a pointer to int, and should not be passed
a pointer to any integral type which might have a different width.
Use an intermediate variable for each call.
The erroneous calls in ldap_misc.c were passing pointers to int32_t,
which is harmless on all common platforms. The calls in
ldap_tkt_policy.c were passing pointers to long; on big-endian LP64
platforms, the result would be written to the high 32 bits of the long
value.
Tom Yu [Mon, 13 Apr 2015 20:45:35 +0000 (16:45 -0400)]
Remove STRING_BUFFER() macro in gssapi_generic.c
In gssapi_generic.c, struct mech_attr_info_desc included some
gss_buffer_desc members whose length fields were never used.
Additionally, the STRING_BUFFER() macro's computation of the (unused)
length fields was incorrect, causing warnings in some versions of
clang. Remove the problematic STRING_BUFFER() macro and adjust the
array and generic_gss_display_mech_attr() accordingly.
When resetting the ulog header or initializing it from a dump file
kdb_last_t value, instead of setting kdb_num to 0, create a dummy
entry for the last_sno value so that we can remember its timestamp.
With this change, a slave no longer needs to perform two full resyncs
after an upstream header initialization. Dummy entries are never
transmitted to downstream slaves because the iprop protocol never
transmits the kdb_first_sno update; if one is somehow transmitted, the
slave will ignore it because it doesn't have the kdb_commit flag set.
reset_header() is renamed to reset_ulog(), takes a kdb_log_context
parameter, and is responsible for syncing the header. sync_update()
now returns void and aborts if msync() fails, just like sync_header().
A new helper set_dummy() writes a dummy entry and sets the ulog to
point to it.
Adjust kproplog to recognize and display dummy entries. Adjust
t_ulog.c and t_iprop.py for the new behavior. In t_iprop.py, remove a
kpropd -t test which became redundant with the previous test.
Add a run_kpropd_once() method to K5Realm(), and add tests to
t_iprop.py for the cases where no updates are needed, where
incremental updates are needed, and where a full resync is needed
followed by a poll for updates.
If kpropd is asked to run just once, don't exit after starting a full
resync; we want to wait for the fullprop child to process the request,
and then request incremental updates afterwards. Also don't exit from
do_standalone() in the fullprop child, in case multiple full resyncs
are required to get the database up to date.
When kpropd operates in iprop mode, full resyncs are handled by a
child process. After a full resync, we want to poll for incremental
updates, as the dump we received may have come from a pre-existing
dump file which was not current. To make this polling happen
promptly, signal the parent process from the child process after a
dump is received.
With this change, t_iprop.py no longer has to prod kpropd after a full
resync occurs, so remove that logic.
Augment the LDAP KDB module tests to include client principal aliases
as well as server principal aliases. Also revise the server principal
alias tests to include an AS-REQ case. (This requires adjusting the
subsequent test not to assume a ccache containing a TGT.)
For the convenience of developers manually running Python test
scripts, set the executable bits on all of them, and make sure the
first line is always "#!/usr/bin/python".
Greg Hudson [Thu, 26 Mar 2015 16:47:06 +0000 (12:47 -0400)]
Disable principal renames for LDAP
The current principal rename procedure does not work with the LDAP KDB
module, instead having the effect of deleting the principal. The fix
is not easy and requires amending the DAL (see issue #8065). For now,
detect LDAP and error out when a rename operation is attempted.
Greg Hudson [Thu, 19 Mar 2015 17:42:56 +0000 (13:42 -0400)]
Process TGS authdata after transited in KDC
The CAMMAC authorization data container requires a checksum over the
encrypted part of the issued ticket, with the CAMMAC contents
substituted for the authdata field. For this to work, we must
finalize the non-authdata fields of the encrypted ticket part before
adding authdata. Call handle_authdata() after checking and modifying
the transited field and potentially setting the
transited-policy-checked flag.
Also remove a redundant and inoperative conditional change to
enc_tkt_reply.times.starttime which happens after the ticket is
encrypted. We do the same thing right after setting up the ticket
times.
Greg Hudson [Tue, 17 Mar 2015 18:07:38 +0000 (14:07 -0400)]
Fix renewable ticket lifetimes
Commit b0661f9176f5eb2644ba459e1b1e87d3dd502174 removed the starttime
hack in the EncTicketPart decoder. Take this into account when
computing the old lifetime of a ticket we are renewing. Without this
fix, we compute an old lifetime equal to the ticket end time, add that
to the current KDC time, and issue a ticket with a negative end time
due to wraparound. Add a simple test to t_renew.py to detect this by
making sure that a renewed ticket is usable.
This bug appeared only on master and not as part of any release, so
there is no associated ticket.
In clnt_create(), do not set a timeout after creating the handle;
doing so defeats the purpose of keeping track of whether the caller
has set a handle timeout.
Greg Hudson [Sat, 14 Mar 2015 18:21:06 +0000 (14:21 -0400)]
Extend kadmin client timeout to one hour
Retrieving the list of principals can take a long time for some
databases. Extend the libkadm5 client timeout from two minutes to one
hour. (We can't easily remove the timeout entirely.)
Greg Hudson [Fri, 13 Mar 2015 17:30:49 +0000 (13:30 -0400)]
Log invalid restrictions strings
In kadm5int_acl_parse_restrictions(), output a log message if we break
out of the parsing loop with an error. The current structure of the
loop makes it difficult to pinpoint the bad restrictions field, so
just output the whole string.
Greg Hudson [Fri, 13 Mar 2015 16:45:27 +0000 (12:45 -0400)]
Document correct flag names for kadm5.acl
kadm5.acl entries can include restrictions which can force flag values
on or off. These flag values are parsed with krb5_string_to_flags(),
which means the flag names are the ones for default_principal_flags,
not the ones for kadmin addprinc/modprinc.
Greg Hudson [Thu, 12 Mar 2015 20:36:33 +0000 (16:36 -0400)]
Fix scope of kadmind ACL wildcard back-references
In kadm5int_acl_find_entry(), clear the wildcard back-references list
for each acl entry. Otherwise the wildcards we process can affect
back-references for later entries.
Solly Ross [Thu, 5 Mar 2015 18:22:58 +0000 (13:22 -0500)]
Import names immediately with COMPOSITE_EXPORT
RFC 6680 specifies that GSS_Export_name_composite() "outputs a token that
"can be imported with GSS_Import_name(), using GSS_C_NT_COMPOSITE_EXPORT
as the name type...". Therefore, in the gss_import_name mechglue, we
should perform the import process imediately when either
GSS_C_NT_COMPOSITE_EXPORT or GSS_C_NT_EXPORT_NAME are used (not just
for the later, as is the current functionality).
The naming extension test was also updated to display the result
of importing with GSS_C_NT_COMPOSITE_EXPORT in addition to
GSS_C_NT_EXPORT_NAME.
Greg Hudson [Mon, 23 Feb 2015 20:47:21 +0000 (15:47 -0500)]
Add test KDB module
Add a simple read-only KDB module which can be used to exercise KDB
behavior which the DB2 module cannot reach. Right now it supports
very basic get_principal functionality, aliases, and delegation
policy; in the future it could issue referrals or sign authdata.
Greg Hudson [Thu, 26 Feb 2015 20:02:37 +0000 (15:02 -0500)]
Fix kadmin script mode command-not-found error
In ss_wrapper.c, if ss_execute_command() returns an error, we should
call ss_perror() with *args as the third argument and not request
(which is NULL). Expand out the conditional into three commented
branches for greater clarity, since the error-handling is no longer
identical for the ss_execute_command() and ss_execute_line() cases.
Greg Hudson [Mon, 9 Feb 2015 17:38:06 +0000 (12:38 -0500)]
Use preauth timestamp in PKINIT clpreauth module
Use the timestamp from the KDC's preauth-required error when
generating a PKAuthenticator in pa_pkinit_gen_req(), to allow PKINIT
authentication to succeed despite client clock skew if kdc_timesync is
set.
Because this timestamp is unauthenticated (unless FAST is used), an
attacker could induce a legitimate client to generate a
PKAuthenticator for a future timestamp. But replaying this request in
the future would only cause the KDC to issue a ticket which the
attacker cannot decrypt.
Thomas Calderon [Fri, 6 Feb 2015 14:55:34 +0000 (15:55 +0100)]
Check timestamp in PKINIT kdcpreauth module
RFC 4556 requires the KDC to check the PKAuthenticator timestamp in
order to prevent replays after the five-minute clock skew window. (A
replay attack has minimal value; it only causes the KDC to issue a
ticket which an attacker cannot decrypt.)
[ghudson@mit.edu: rewrote commit message; squashed with typo fix;
style fixes]
Greg Hudson [Sat, 31 Jan 2015 05:29:59 +0000 (00:29 -0500)]
Use kadmin script mode in Python tests
In k5test, rename kadmin_local to kadminl and remove the run_kadminl()
K5Realm method. Update all scripts to use realm.run([kadminl, 'cmd',
...]). run_kadmin() still exists but takes an argument array instead
of a query string.
Where we touch test code, rename "output" to "out" (since "output" is
a function name exported by k5test.py), elide ":normal" from salt
strings, and use expressions like realm.krbtgt_princ instead of
manually composed principal names where appropriate. In
t_kadmin_acl.py, get rid of the delprinc() helper since the equivalent
is now concise enough to be written out each time. In t_policy.py,
remove some inoperative getprinc invocations and reorder some tests
which didn't correspond to their comment headers.
Greg Hudson [Fri, 30 Jan 2015 17:48:15 +0000 (12:48 -0500)]
Support kadmin script mode
Add support for a command and argments to be specified on the kadmin
command line, with script-friendly behavior. kadmin_startup() now
yields either a request string or a request argv array, and sets
script_mode in the argv array case. Informational messages now go
through info() and are suppressed if script_mode is set. Prompts and
warning messages are also suppressed in script mode. Error messages
indicating a failure now go through error() and set exit_status if
script_mode is set. The extended com_err() hook is always installed
so that com_err messages go through error() and set exit_status.
getopt() is now invoked with a leading '+' to suppress Gnu getopt
argument reordering behavior, so that invokers don't need to pass "--"
to prevent query options from being treated as kadmin options.
Non-Gnu getopt implementations should harmlessly treat '+' as a valid
flag option, which has no effect as it will reach the same default
label in the switch statement.
Greg Hudson [Tue, 9 Dec 2014 17:37:44 +0000 (12:37 -0500)]
Fix krb5_read_message handling [CVE-2014-5355]
In recvauth_common, do not use strcmp against the data fields of
krb5_data objects populated by krb5_read_message(), as there is no
guarantee that they are C strings. Instead, create an expected
krb5_data value and use data_eq().
In the sample user-to-user server application, check that the received
client principal name is null-terminated before using it with printf
and krb5_parse_name.
CVE-2014-5355:
In MIT krb5, when a server process uses the krb5_recvauth function, an
unauthenticated remote attacker can cause a NULL dereference by
sending a zero-byte version string, or a read beyond the end of
allocated storage by sending a non-null-terminated version string.
The example user-to-user server application (uuserver) is similarly
vulnerable to a zero-length or non-null-terminated principal name
string.
The krb5_recvauth function reads two version strings from the client
using krb5_read_message(), which produces a krb5_data structure
containing a length and a pointer to an octet sequence. krb5_recvauth
assumes that the data pointer is a valid C string and passes it to
strcmp() to verify the versions. If the client sends an empty octet
sequence, the data pointer will be NULL and strcmp() will dereference
a NULL pointer, causing the process to crash. If the client sends a
non-null-terminated octet sequence, strcmp() will read beyond the end
of the allocated storage, possibly causing the process to crash.
uuserver similarly uses krb5_read_message() to read a client principal
name, and then passes it to printf() and krb5_parse_name() without
verifying that it is a valid C string.
The krb5_recvauth function is used by kpropd and the Kerberized
versions of the BSD rlogin and rsh daemons. These daemons are usually
run out of inetd or in a mode which forks before processing incoming
connections, so a process crash will generally not result in a
complete denial of service.
Tom Yu [Mon, 9 Feb 2015 20:02:20 +0000 (15:02 -0500)]
Add configure checks for portability assumptions
Check a few portability assumptions:
* Integers are two's complement. Testing that the bitwise complement
of the representation of -1 is zero is sufficient, because C99
6.2.6.2 only allows sign and magnitude, one's complement, and two's
complement as representations.
* Integer values with sign bit one and value bits zero are valid and
not trap representations. C99 6.2.6.2 allows such a value to be a
trap representation. Testing that the declared integer value bounds
are asymmetric in magnitude is sufficient.
* Conversion of an unsigned integer value that is not representable in
the corresponding signed type preserves the bit pattern. C99
6.3.1.3 says this is implementation-defined, or raises an
implementation-defined signal. Exhaustively checking for the
desired behavior is prohibitive, so this spot check will have to do.
Tom Yu [Wed, 4 Feb 2015 22:01:14 +0000 (17:01 -0500)]
Avoid uninitialized data in t_prf.c
In t_prf.c, make sure that the partially initialized, faked-up
structures gss_union_ctx_id_desc and krb5_gss_ctx_id_rec are zeroed.
This avoids uninitialized reads in gss_pseudo_random(), which can
cause intermittent test failures on some platforms.
Greg Hudson [Wed, 4 Feb 2015 18:03:20 +0000 (13:03 -0500)]
Bump DAL major version for iterate change
Commit ab009b8568d9b64b7e992ecdb98114e895b4a7ff for issue #7977
changed the signature of krb5_db_iterate() and properly bumped
KRB5_KDB_API_VERSION from 7 to 8. It also changed the signature of
the DAL iterate() function, but did not bump
KRB5_KDB_DAL_MAJOR_VERSION. Bump that version from 4 to 5 now.
Greg Hudson [Mon, 29 Dec 2014 18:17:56 +0000 (13:17 -0500)]
Fix gssrpc data leakage [CVE-2014-9423]
[MITKRB5-SA-2015-001] In svcauth_gss_accept_sec_context(), do not copy
bytes from the union context into the handle field we send to the
client. We do not use this handle field, so just supply a fixed
string of "xxxx".
In gss_union_ctx_id_struct, remove the unused "interposer" field which
was causing part of the union context to remain uninitialized.
Greg Hudson [Mon, 29 Dec 2014 18:27:42 +0000 (13:27 -0500)]
Fix kadmind server validation [CVE-2014-9422]
[MITKRB5-SA-2015-001] In kadmind's check_rpcsec_auth(), use
data_eq_string() instead of strncmp() to check components of the
server principal, so that we don't erroneously match left substrings
of "kadmin", "history", or the realm.
Greg Hudson [Sat, 27 Dec 2014 19:16:13 +0000 (14:16 -0500)]
Fix kadm5/gssrpc XDR double free [CVE-2014-9421]
[MITKRB5-SA-2015-001] In auth_gssapi_unwrap_data(), do not free
partial deserialization results upon failure to deserialize. This
responsibility belongs to the callers, svctcp_getargs() and
svcudp_getargs(); doing it in the unwrap function results in freeing
the results twice.
In xdr_krb5_tl_data() and xdr_krb5_principal(), null out the pointers
we are freeing, as other XDR functions such as xdr_bytes() and
xdr_string().
Greg Hudson [Wed, 5 Nov 2014 16:58:04 +0000 (11:58 -0500)]
Fix gss_process_context_token() [CVE-2014-5352]
[MITKRB5-SA-2015-001] The krb5 gss_process_context_token() should not
actually delete the context; that leaves the caller with a dangling
pointer and no way to know that it is invalid. Instead, mark the
context as terminated, and check for terminated contexts in the GSS
functions which expect established contexts. Also add checks in
export_sec_context and pseudo_random, and adjust t_prf.c for the
pseudo_random check.
projects / thirdparty / krb5.git / log
