Ben Kaduk [Thu, 3 Jul 2014 14:42:21 +0000 (10:42 -0400)]
Fix build on systems without RTM_OLD*
For example, FreeBSD has removed RTM_OLDADD and RTM_OLDDEL from its API
in March 2014, with the message:
Garbage collect long time obsoleted (or never used) stuff from routing API
Only attempt to define behavior for these cases if they are defined.
Tom Yu [Wed, 2 Jul 2014 20:13:23 +0000 (16:13 -0400)]
Fix bugs in bindresvport_sa() changes
In svctcp_create() and svcudp_bufcreate(), set sa->sa_len on platforms
where that field exists, so that a subsequent call to socklen() will
return the correct result.
To make the code more self-evidently correct, zero the entire struct
sockaddr_storage object, using the memset(&ss, 0, sizeof(ss)) idiom.
Greg Hudson [Thu, 12 Jun 2014 18:34:26 +0000 (14:34 -0400)]
Remove indent workaround in man page RST sources
docutils 0.10 properly adds indentation to example blocks in man
pages, so we do not need to force an extra indentation level. Get rid
of the workaround wherever we use it.
Neng Xue [Mon, 30 Jun 2014 21:04:56 +0000 (14:04 -0700)]
Fix unlikely null dereference in TGS client code
If krb5_get_tgs_ktypes fails (due to an out-of-memory condition or an
error re-reading the profile), k5_make_tgs_req will dereference a null
pointer. Check the return value before dereferencing defenctypes.
Make clnttcp_create, clntudp_bufcreate, svctcp_create, and
svcudp_bufcreate work with unbound IPv6 sockets using bindresvport_sa
and other socket helpers. For caller-supplied sockets, call
getsockname to determine the address family we should attempt to bind.
[ghudson@mit.edu: clarified commit message, minimized code changes,
used socket-utils.h helpers, fixed fallback find on bindresvport
failure, restored getsockaddr call to get port after binding]
This functions allows you to pass IPv4 and IPv6 addresses. If no
address is given, t will determine the family by checking the socket
with getsockname.
[ghudson@mit.edu: clarified commit message, split out setport helper,
squashed with next commit, minimized code changes from old
bindresvport, used socket-utils.h helpers]
Greg Hudson [Wed, 18 Jun 2014 16:58:39 +0000 (12:58 -0400)]
Fix KDC worker process argument parsing
To create worker processes, the KDC shuts down realms, forks off the
worker processes, then reinitializes realms in each child.
Reinitializing realms requires making a second pass over the
command-line arguments. To do this with getopt, optind must be
reinitialized to 1 for each pass; otherwise, no options will be seen
the second time around.
Greg Hudson [Thu, 19 Jun 2014 17:49:16 +0000 (13:49 -0400)]
Handle invalid RFC 1964 tokens [CVE-2014-4341...]
Detect the following cases which would otherwise cause invalid memory
accesses and/or integer underflow:
* An RFC 1964 token being processed by an RFC 4121-only context
[CVE-2014-4342]
* A header with fewer than 22 bytes after the token ID or an
incomplete checksum [CVE-2014-4341 CVE-2014-4342]
* A ciphertext shorter than the confounder [CVE-2014-4341]
* A declared padding length longer than the plaintext [CVE-2014-4341]
If we detect a bad pad byte, continue on to compute the checksum to
avoid creating a padding oracle, but treat the checksum as invalid
even if it compares equal.
CVE-2014-4341:
In MIT krb5, an unauthenticated remote attacker with the ability to
inject packets into a legitimately established GSSAPI application
session can cause a program crash due to invalid memory references
when attempting to read beyond the end of a buffer.
In MIT krb5 releases krb5-1.7 and later, an unauthenticated remote
attacker with the ability to inject packets into a legitimately
established GSSAPI application session can cause a program crash due
to invalid memory references when reading beyond the end of a buffer
or by causing a null pointer dereference.
Greg Hudson [Wed, 25 Jun 2014 15:41:54 +0000 (11:41 -0400)]
Load plugins with RTLD_NODELETE if possible
On platforms which support RTLD_NODELETE, use it to load plugin
modules. While using this flag makes plugins stay in the process map
after libkrb5/libgssapi_krb5 are unloaded, it solves several problems:
1. It prevents plugin modules which link against OpenSSL (PKINIT and
k5tls) from repeatedly initializing instances of libssl or libcrypto,
leaking heap memory each time. This is only an issue because we
cannot safely uninitialize OpenSSL.
2. It prevents finalization ordering issues from causing a process
crash when unloading libgssapi_krb5 (issue #7135).
3. It makes memory leak tracing with valgrind easier.
Nalin Dahyabhai [Wed, 25 Jun 2014 16:56:42 +0000 (12:56 -0400)]
Fix unlikely null dereference in mk_cred()
If krb5_encrypt_keyhelper() returns an error, the ciphertext structure
may contain a non-zero length, but it will already have freed the
pointer to its data, making encrypt_credencpart()'s subsequent attempt
to clear and free the memory fail. Remove that logic.
Greg Hudson [Sat, 14 Jun 2014 15:23:08 +0000 (11:23 -0400)]
Fix error checking in PKINIT authdata creation
In create_identifiers_from_stack: check for allocation errors from
PKCS7_ISSUER_AND_SERIAL_new and M_ASN1_INTEGER_dup. Use
PKCS7_ISSUER_AND_SERIAL_free to more concisely clean up the OpenSSL
issuer variable, and make sure that any partially processed value is
cleaned up on error. Use calloc to allocate krb5_cas so that all of
its pointers are initially nulled, so that
free_krb5_external_principal_identifier can operate on it safely in
case of error. Eliminate the retval variable as it was not used
safely. Rename the error label from "cleanup" to "oom" and separate
it from the successful return path (which has nothing to clean up).
Greg Hudson [Mon, 16 Jun 2014 19:46:09 +0000 (15:46 -0400)]
Consolidate DB option documentation
Document DB options in the kadmin/kadmin.local man page, in their own
section. Refer to that section from the documentation of the -x
parameter of each other command which supports DB options. Add
documentation for the "dbname" DB2 option.
Ben Kaduk [Fri, 13 Jun 2014 18:59:39 +0000 (14:59 -0400)]
Update the kadm5.acl example
Make the example and documentation a closer match to reality.
In particular, the list permission is all-or-nothing; it is not
restricted in scope by the target_principal field. Change the
table entry to try and indicate this fact, and do not put list
permissions on any example line that is scoped by a target_principal
pattern.
While here, remove the nonsensical granting of global inquire
permissions to */* (inaccurately described as "all principals"),
and the granting of privileges to foreign-realm principals.
It is not possible to obtain an initial ticket (as required by
the kadmin service) for a principal in a different realm, and
the current kadmind implementation can serve only a single realm
at a time -- this permission literally has no effect. Replace
it with a (presumably automated) "Service Management System"
example, where it might make sense to limit the principals which
are automatically created.
Greg Hudson [Sat, 7 Jun 2014 03:24:00 +0000 (23:24 -0400)]
Remove pkinit_win2k_require_binding option
When constructing a draft9 PKINIT request, always include
KRB5_PADATA_AS_CHECKSUM padata to ask for an RFC 4556 ReplyKeyPack.
Do not accept a draft9 ReplyKeyPack in the KDC response.
For now, retain the krb5_reply_key_pack_draft9 ASN.1 codec and the KDC
support for generating a draft9 ReplyKeyPack when a draft9 PKINIT
request does not contain KRB5_PADATA_AS_CHECKSUM.
Greg Hudson [Sat, 7 Jun 2014 02:48:04 +0000 (22:48 -0400)]
Remove PKINIT longhorn compatibility option
Remove the PKINIT Windows Server 2008 beta compatibility code
conditionalized under the "longhorn" variable. It is not required to
interoperate with any released version of Windows.
Greg Hudson [Fri, 6 Jun 2014 21:41:51 +0000 (17:41 -0400)]
Improve PKINIT certificate documentation
Describe how to use a commercially-issued server certificate for
anonymous PKINIT. Separate the KDC and client configuration
instructions so that the steps necessary for anonymous PKINIT are not
combined with the additional steps necessary for regular PKINIT.
Describe kpServerAuth as the EKU used in commercially issued server
certificates, not as the value used by Microsoft (which does not
appear to be true according to [MS-PKCA]).
Greg Hudson [Tue, 3 Jun 2014 15:48:13 +0000 (11:48 -0400)]
Simplify and fix k5_check_cert_address
Get rid of the address union. Store the result of get_cert_cn in a
signed variable so we can meaningfully check for negative results.
Make get_cert_cn return int for consistency with
X509_NAME_get_text_by_NID and its two callers.
Also add an emacs mode line to the top of the file.
Greg Hudson [Fri, 6 Jun 2014 21:56:23 +0000 (17:56 -0400)]
Remove stub pkinit_win2k code
As contributed, the PKINIT module contained code to read the
pkinit_win2k variable, but never used it. Get rid of the structure
field and the code to populate it.
Greg Hudson [Wed, 4 Jun 2014 20:18:21 +0000 (16:18 -0400)]
Add missing profile functions to libkrb5 exports
profile_flush_to_buffer, profile_flush_to_file, profile_free_buffer,
profile_init_flags, and profile_init_vtable are all public profile
functions, but are inaccessible to libkrb5 applications on some
platforms because they were never added to the export list. Add them
now.
(libprofile functions have never been part of the Windows DLL export
list, so do not change krb5_32.def at this time.)
Greg Hudson [Thu, 5 Jun 2014 16:03:16 +0000 (12:03 -0400)]
Simplify ticket retrieval from AP-REQs
After krb5_rd_req_decoded or krb5_rd_req_decoded_anyflag, the ticket
(with enc_part2 if we could decrypt it) is accessible via
request->ticket; there is no need to copy it. Stop using the ticket
parameter of those functions. Where we need to save the ticket beyond
the lifetime of the krb5_ap_req, steal the pointer before freeing the
request.
rbasch [Tue, 3 Jun 2014 22:44:17 +0000 (18:44 -0400)]
In KDC, log client principal in bad header ticket
Fix KDC logging to include client principal in TGS_REQ logging even
during error conditions such as "Ticket expired". As long as the
TGS_REQ can be decrypted and the client principal is available, it
should be included in the log, regardless of other errors which might
be detected.
krb5_rd_req_decoded and krb5_rd_req_decoded_anyflag (not public
interfaces) now leave the decrypted ticket in req->ticket->enc_part2
on success or failure, if the ticket was successfully decrypted. This
does not affect the behavior of krb5_rd_req.
[ghudson@mit.edu: removed extraneous change, added commit message
summary and description of internal API change, fixed possible memory
leak, removed comment and #if 0 code block of purely historical
interest]
Sam Hartman [Wed, 4 Jun 2014 16:06:27 +0000 (12:06 -0400)]
Do not loop on add_cred_from and other new methods
Several new GSS-API methods were added but GSSAPI_ADD_METHOD was
called to add them rather than GSSAPI_ADD_METHOD_NOLOOP. This means
that the implementation from the GSS-API mechglue would be used if the
mechanism had no implementation. As a result, the mechglue will call
into itself exhausting the call stack in an endless loop when one of
these methods is called.
Greg Hudson [Fri, 23 May 2014 23:58:41 +0000 (19:58 -0400)]
Treat LDAP KrbKey salt field as optional
Per the ASN.1 definition, the KrbKey salt field is optional. Since
1.7, we have been treating it as mandatory in the encoder; since 1.11,
we have been treating it as mandatory in the decoder. Mostly by luck,
we have been encoding a salt type of 0 when key_data_ver is 1, but we
really should not be looking at key_data_type[1] or key_data_length[1]
in this situation. Treat the salt field as optional in the encoder
and decoder. Although the previous commit ensures that we continue to
always encode a salt (without any dangerous assumptions about
krb5_key_data constructors), this change will allow us to decode key
data encoded by 1.6 without salt fields.
This also fixes issue #7918, by properly setting key_data_ver to 2 if
a salt type but no salt value is present. It is difficult to get the
decoder to actually assign 2 to key_data_ver just because the salt
field is there, so take care of that in asn1_decode_sequence_of_keys.
Adjust kdbtest.c to match the new behavior by setting key_data_ver to
2 in both test keys.
Greg Hudson [Sun, 25 May 2014 02:58:26 +0000 (22:58 -0400)]
Always include salt in LDAP KrbKey encoding
In the LDAP KDB module, ensure that every krb5_key_data we pass to
asn1_encode_sequence_of_keys includes a salt type, for compatibility
with the decoder in unpatched krb5 1.11 and 1.12.
This is not a behavior change by itself; since 1.7 the encoder has
always included a KrbKey salt field because it erroneously treats that
field as non-optional. (Luckily, the encoded salt always happens to
have salt type 0 because krb5_key_data constructors start with zeroed
memory.) The next commit will fix the encoder and decoder to properly
treat the KrbKey salt field as optional, so we need this change to
ensure that our encodings remain compatible.
Also fix the ASN.1 tests to set key_data_ver correctly for the sample
test key data.
Greg Hudson [Thu, 29 May 2014 03:51:49 +0000 (23:51 -0400)]
Read /etc/gss/mech if no mech.d/*.conf found
Always read /etc/gss/mech, even if globbing /etc/gss/mech.d/*.conf
doesn't work. Doing this using GLOB_DOOFFS proved error-prone, so use
a simpler approach: factor out the per-pathname handling into a helper
function load_if_changed, call it with MECH_CONF before the glob, then
pass each glob result through the helper.
Greg Hudson [Sun, 1 Jun 2014 14:44:51 +0000 (10:44 -0400)]
Remove stub pkinit_mapping_file code
As contributed, the PKINIT code contained code to read a mapping
filename, but never used the resulting structure variable. Get rid of
the structure field and the code to populate it.
Nalin Dahyabhai [Fri, 7 Feb 2014 23:56:10 +0000 (18:56 -0500)]
Add tests for MS-KKDCP client support
Exercise the MS-KKDCP client support using the test proxy server, for
AS, TGS, and kpasswd requests while also checking the certificate
verification and name checks.
Nalin Dahyabhai [Fri, 7 Feb 2014 23:03:29 +0000 (18:03 -0500)]
Add a simple KDC proxy test server
This proxy server uses python-paste to run the kdcproxy from
https://pypi.python.org/pypi/kdcproxy. It should be used along
with the proxy.pem certificate in ../tests/dejagnu/proxy-certs.
Add some longer-form documentation for the new HTTPS support, walking a
prospective administrator through generating a bare minimal signing
setup, deploying a WSGI-based proxy server onto an Apache httpd server
using mod_ssl and mod_wsgi, and configuring clients to use it.
When we connect to a KDC using an HTTPS proxy, check that the naming
information in the certificate matches the name or address which we
extracted from the server URL in the configuration.
Add an http_anchors per-realm setting which we'll apply when using an
HTTPS proxy, more or less mimicking the syntax of its similarly-named
PKINIT counterpart. We only check the [realms] section, though.
Add an 'HTTPS' transport type which connects to an [MS-KKDCP] proxy
server using HTTPS to communicate with a KDC. The KDC's name should
take the form of an HTTPS URL (e.g. "https://proxybox/KdcProxy").
An HTTPS connection's encryption layer can be reading and writing when
the application layer is expecting to write and read, so the HTTPS
callbacks have to handle being called multiple times.
[nalin@redhat.com: use cleanup labels, make sure we always send the
realm name, keep a copy of the URI on-hand, move most of the
conditionally-compiled sections into their own conditionally-built
functions, break out HTTPS request formatting into a helper function,
handle the MS-KKDCP length bytes, update comments to mention specific
versions of the MS-KKDCP spec, differentiate TCP and HTTP trace
messages, trace unparseable responses]
Switch to using per-transport-type functions when a socket that we're
using to communicate with a server becomes readable or writable, and add
them as pointers to the connection state. The functions are passed the
name of the realm of the server being contacted, as we expect to need
this in the near future.
[nalin@redhat.com: replace macros with typedefs]
[nalin@redhat.com: compare transports with TCP_OR_UDP rather than with 0]
Handle encoding and decoding [MS-KKDCP] proxy messages, including
handling of the additional length bytes. Early versions of [MS-KKDCP]
incorrectly omit that the size of the proxied message is prepended to
the proxied message, as it is when we're using plain TCP, before
encoding the proxy-message structure. This is fixed at least as of
version 2.1 of the spec.
Add a --with-proxy-tls-impl option to configure, taking 'openssl',
'auto', or invocation as --without-proxy-tls-impl. Use related CFLAGS
when building lib/krb5/os, and LIBS when linking libkrb5. Call the
OpenSSL library startup functions during library initialization.
In k5_sendto and k5_locate_server, replace "socktype" parameters with
a new enumerator k5_transport, so that we can add new transports which
are not in the socket type namespace. Control the order in which we
make connections of different types using a new k5_transport_strategy
enumerator, to simplify the logic for adding new transports later.
Control the result of k5_locate_server with a no_udp boolean rather
than a socket type.
[ghudson@mit.edu: renamed type to k5_transport; k5_locate_server
no_udp change; clarified commit message; fix for Solaris getaddrinfo]
[kaduk@mit.edu: name variables of type k5_transport 'transport']
[nalin@redhat.com: use transport rather than sock_type in more places,
add and use k5_transport_strategy, update the test program]
* Get rid of the "x" member of conn_state, which used to be a union
but hasn't been since r14742.
* Define a structure type for the "out" member of conn_state.
* Rename incoming_krb5_message to incoming_message for brevity.
* Make the "pos" member of incoming_message an offset instead of a
pointer, simplifying several present and future computations.
* Use "in" and "out" aliases to the conn_state in and out members
where it improves brevity.
* Rename set_conn_state_msg_length to set_transport_message and give
it a descriptive comment.
* Call set_transport_message from start_connection only, instead of
once in add_connection and perhaps again in start_connection. To
make this possible, pass the original message argument to maybe_send
and start_connection.
* Use make_data and empty_data helpers where appropriate.
Greg Hudson [Thu, 29 May 2014 16:54:50 +0000 (12:54 -0400)]
Make x-deltat.y work with bison 3
Bison 3 removed support for YYPARSE_PARAM and YYLEX_PARAM, breaking
x-deltat.y. Use %parse-param and %lex-param instead. (In Bison 3 we
could use just %param, but that doesn't work in 2.x.) The parameter
added by %parse-param is also passed to yyerror, so adjust the macro
we use to suppress yyerror accordingly.
Also use "%define api.pure" instead of "%pure_parser", which was
deprecated in bison 2.3b. (The correct spelling was actually
"%pure-parser", and bison 3 generates a warning about the underscore
spelling.)
Regenerate deltat.c using the new x-deltat.y and bison 3.0.2.
Greg Hudson [Tue, 27 May 2014 17:01:49 +0000 (13:01 -0400)]
Change example module name in host_config.rst
Don't use "otp" as the example clpreauth module name in
host_config.rst, since we now ship an effectively built-in otp
clpreauth module. Instead use "mypreauth".
Greg Hudson [Wed, 28 May 2014 22:06:59 +0000 (18:06 -0400)]
Make tcl_kadm5.c work with Tcl 8.6
Directly accessing the result field of Tcl_Interp has been deprecated
for a long time, requires a special define in Tcl 8.6, and will be
impossible in Tcl 9. Use Tcl_SetResult instead. The new error
messages are less helpful than the old ones, but this is just support
infrastructure for old tests, so it isn't important.
Greg Hudson [Wed, 28 May 2014 15:36:24 +0000 (11:36 -0400)]
Fix uninitialized variable bug in KEYRING ccache
Commit 5f4a4d7d357fedac5feadc65c09ecf487ff98db8 removed the only
unconditional assignment of ret in get_time_offsets, causing the
function to return an uninitialized value if nothing goes wrong.
Initialize ret at declaration time to fix this.
Greg Hudson [Fri, 23 May 2014 02:31:26 +0000 (22:31 -0400)]
Properly handle PKCS11 label in PKINIT
The CK_TOKEN_INFO label field is defined to be zero-filled, but it may
not be zero-terminated if all bytes of the field are used. Use only
length-counted operations to process it. Also avoid underrunning the
buffer pointer if the label is empty or contains only whitespace.
Greg Hudson [Thu, 22 May 2014 23:18:34 +0000 (19:18 -0400)]
Don't blindly use PKCS11 slot IDs in PKINIT
Passing invalid slot IDs to C_OpenSession can cause some PKCS #11
implementations (such as the Solaris one) to crash. If a PKINIT
identity specifies a slotid, use it to filter the result of
C_GetSlotList, but don't try it if it does not appear in the list.
Greg Hudson [Wed, 21 May 2014 16:03:00 +0000 (12:03 -0400)]
Improve pointer hygiene around gss_display_name
GSSAPI functions are responsible for setting their output parameters
on failure. Take greater care to do so in krb5_gss_display_name.
The mechglue is generally defensive about initializing variables used
as outputs, and not assuming that mechs will set them on failure.
Make gssint_convert_name_to_union_name initialize
union_name->external_name before calling mech->gss_display_name, so
that if the mech's gss_display_name doesn't touch it, we don't free an
uninitialized pointer.
Either one of these changes prevents an unlikely memory bug which
could occur if krb5_gss_init_context fails within
krb5_gss_display_name when called from
gssint_convert_name_to_union_name.
In addition to loading /etc/gss/mech, glob for *.conf files in
/etc/gss/mech.d. Load only config files which have changed since the
highest mtime we saw in the previous scan. Scan at most once per
second to avoid excessive numbers of filesystem syscalls for busy
GSSAPI applications.
Ben Kaduk [Mon, 19 May 2014 20:23:45 +0000 (16:23 -0400)]
Do not default to host/ for client keytabs
When the normal (acceptor) keytab is being used to obtain initial
credentials, it is reasonable to use the default hostbased service
principal (host/fully.qualified.localhost.domain) when no client
principal is given. This behavior is not very reasonable when
the default client keytab is being used, as host/ credentials are
not normally client credentials.
Make kinit -i match up with the GSS-API behavior when client keytabs
are in use, using the name of the first entry in the keytab when
no name is explicitly given.
Greg Hudson [Tue, 20 May 2014 18:52:44 +0000 (14:52 -0400)]
Add tests for krb5int_c_combine_keys
krb5int_c_combine_keys is used only by SAM-2 preauth with neither the
send-encrypted-sad nor use-sad-as-key flags, so it isn't covered by
any automated function tests. Add some unit tests to make sure that
it works and returns the expected results for two randomly generated
test vectors.
Ken Hornstein [Tue, 20 May 2014 18:50:22 +0000 (14:50 -0400)]
Fix krb5int_c_combine_keys
When krb5int_c_combine_keys was updated to use the new crypto
interface, a small bug was introduced. It turns out the temporary
keyblock created needs to have its enctype set; otherwise, when
krb5int_derive_keyblock() is called later, it will fail with
KRB5_BAD_ENCTYPE.
Greg Hudson [Sun, 18 May 2014 21:57:25 +0000 (17:57 -0400)]
Fix invalid JSON handling in KDC OTP module
If the OTP configuration for a principal contains invalid JSON, the
KDC OTP module calls k5_json_get_tid on a null pointer, causing the
KDC process to crash. Fix this bug by checking the return value of
k5_json_decode in decode_config_json.
Greg Hudson [Sun, 18 May 2014 20:40:35 +0000 (16:40 -0400)]
Fix t_marshal on big-endian platforms
t_marshal.c attempts to skip the version 1 and 2 tests on big-endian
platforms, but didn't do so correctly. Correctly start at version 3
on big-endian platforms, and change the way we do it to avoid
preprocessor conditionals inside a function body.
Greg Hudson [Tue, 6 May 2014 20:56:52 +0000 (16:56 -0400)]
Use cred marshalling functions in cc_keyring.c
Replace the cc_keyring.c credential and principal marshalling
functions with calls to the ccmarshal.c functions. Simplify the
remaining parsing functions (for the index and time offsets) and fold
the resulting code into the calling functions where appropriate.
Greg Hudson [Mon, 5 May 2014 04:06:56 +0000 (00:06 -0400)]
Use cred marshalling functions in cc_file.c
Replace the cc_file.c credential and principal marshalling and
unmarshalling functions with calls to the ccmarshal.c functions. For
unmarshalling, we still need code to read the appropriate amount of
data into a memory buffer. Because there is no outer length header
for principals and credentials, this code needs to understand the
credential and principal representations, but is much simpler than the
old code as it only needs to store the data it reads, not assemble a
data structure.
Greg Hudson [Sun, 4 May 2014 15:57:16 +0000 (11:57 -0400)]
Modernize cc_file.c and cc_keyring.c
Rename functions not to use the krb5_ prefix, and to use the fcc_ or
krcc_ prefixes only for ccache method implementations. Use shorter
function comments and add missing comments in some cases. Remove
forward declarations where they aren't needed. Use native types
(uint16_t, void *, etc.) in preference to Kerberos type wrappers. Use
"ret" as the variable name for krb5_error_code return values. Use 0
instead of KRB5_OK. Make whitespace conform to current practice.
Remove old #if 0 blocks. Don't cast to and from void * or between
integer types when C guarantees an implicit cast. Use literal 2 and 4
for the size of fixed-width 16-bit and 32-bit types.
In cc_file.c, rewrite the header comment to specify the header format
as updated by version 4 and refer to ccmarshal.c for the principal and
credential format. Also add a helper function to return the cache
version as an integer from 1 to 4, allowing more concise version
checks.
Greg Hudson [Tue, 6 May 2014 18:05:53 +0000 (14:05 -0400)]
Add ccache marshalling tests
Add a new ccache test program t_marshal.c which verifies the new
marshalling functions and also the FILE ccache type against the
expected representations of the ccache header, default principal, and
credentials.
Greg Hudson [Mon, 5 May 2014 04:06:38 +0000 (00:06 -0400)]
Add cred marshalling functions
Add a new file ccmarshal.c containing functions to marshal and
unmarshal credentials in file formats version 1-4. These will replace
the functions in cc_file.c and cc_keyring.c, and can be used for KCM
in the future.
Greg Hudson [Mon, 5 May 2014 04:05:54 +0000 (00:05 -0400)]
Add k5-input.h
Add a header containing a declaration for struct k5input and static
inline functions to use it. A k5input structure can be used to safely
extract bytes and integers from a fixed-sized input buffer without
overrunning the buffer. It supports deferred error checking (similar
to k5buf) by returning dummy values and storing a status value in the
input structure.
Greg Hudson [Thu, 8 May 2014 16:14:42 +0000 (12:14 -0400)]
Modernize rd_cred.c
Adjust the internal abstractions so that decrypt_encpart is
responsible for the fallback from receiving subkey to session key, and
krb5_rd_cred is responsible for decoding and calling decrypt_encpart.
Rename krb5_rd_cred_basic to make_cred_list since it is now only
responsible for constructing the list.
Simo Sorce [Mon, 5 May 2014 21:59:08 +0000 (17:59 -0400)]
Allow GSS mechs to force mechlistMIC in SPNEGO
During a SPNEGO negotiation, if the NTLMSSP mechanism is used and a
MIC is produced within the final initiator mechanism token, Microsoft
servers require a mechlistMIC even if NTLMSSP was the most preferred
mechanism.
In spnego_mech.c, add a helper function mech_requires_mechlistMIC
which queries the mechanism to determine whether we might need to
produce a mechlistMIC for interoperability. Call it after each call
to the mechanism's gss_init_sec_context and set sc->mic_reqd if it
returns true. Although only the second call to NTLMSSP will actually
ever return true, the first call makes the mechanism aware that the
SPNEGO implementation supports this feature.
[ghudson@mit.edu: clarified commit message and code]
Neng Xue [Wed, 7 May 2014 21:11:14 +0000 (14:11 -0700)]
Fix unlikely memory error in krb5_rd_cred
If an error occurs in the for loop in krb5_rd_cred_basic (which should
only happen on an ENOMEM), do not leave the caller with a dangling
reference to the freed credential array.
Greg Hudson [Thu, 1 May 2014 22:11:32 +0000 (18:11 -0400)]
Don't remove ccache creds before storing them
Revert #6291, which attempts to suppress duplicate credentials by
calling krb5_cc_remove in krb5_cc_store_cred. Most of our ccache
types don't implement remove_cred, and avoiding duplicate credentials
is a responsibility better suited to the ccache implementation for
atomicity reasons.
Removing this call gets rid of the misleading "Removing ... from ..."
messages in trace logs.
When krb5_rd_req cannot decrypt a ticket, try to produce the most
helpful diagnostic we can, and return an error code which corresponds
to the most applicable Kerberos protocol error. Add a trace log
containing the error message for ticket decryption failures, in case
the application server does not log it.
Add new tests to cover krb5_rd_req error messages and adjust existing
tests to match the new messages. Also adjust svc_auth_gssapi.c to
look for KRB5KRB_AP_ERR_NOT_US instead of KRB5KRB_AP_WRONG_PRINC.
We have used awk scripts in preference to the C version of compile_et
since 1994 (r4426). Get rid of the sources for the C version and all
of the Makefile.in references to them.
Tom Yu [Thu, 24 Apr 2014 21:10:58 +0000 (17:10 -0400)]
Refactor KDC option/flag processing
A lot of KDC code was spent copying options to flags, and copying
header_ticket flags to the output ticket.
Behavior change: previous code didn't copy PROXY from the
header_ticket, but this seems to have been a minor bug rather than
intentional. This also seems to have been an omission from RFC 4120.
Tomas Kuthan [Thu, 10 Apr 2014 13:16:06 +0000 (15:16 +0200)]
Check for asprintf failure in kdb5_util create
In add_admin_princ, remove build_name_with_realm and call asprintf
directly instead. Check for asprintf failure to avoid passing an
undefined pointer to krb5_parse_name.
Tomas Kuthan [Fri, 11 Apr 2014 13:36:53 +0000 (15:36 +0200)]
In PKINIT, use library initializer for OpenSSL
Use a library initializer to prevent multiple threads using PKINIT
from concurently initializing OpenSSL functions. For cases where
MT-safety is not assured by registering OpenSSL locking callbacks,
this significantly lowers the odds of crashes caused by races in
OpenSSL initialization. (If OpenSSL initialization functions are
called by some other thread directly, crashes are still possible.)
[ghudson@mit.edu: simplify code changes and commit message]
Where we have ATHENA.MIT.EDU stanzas in sample or test krb5.conf files
which define kdc entries, also define a master_kdc entry. Remove
default_domain and v4_instance_convert entries in examples as they are
only needed for krb5/krb4 principal conversions. In the krb5_conf.rst
example, remove enctype specifications as we don't want to encourage
their use when they aren't necessary, and remove a redundant
domain_realm entry.
Tomas Kuthan [Thu, 3 Apr 2014 15:58:43 +0000 (17:58 +0200)]
Remove adb_policy_init and adb_policy_close
Since f72c3ffa the policy is initialized as part of database.
adb_policy_close is now a no-op, and adb_policy_init just makes sure
the database is initialized. adb_policy_init is only called from
kadm5_flush, and only if database initialization was successful
beforehand, rendering this call redundant.
Remove adb_policy_init and adb_policy_close and all their references
in the code and documentation.
Tomas Kuthan [Wed, 2 Apr 2014 15:48:04 +0000 (17:48 +0200)]
Fix leak in kadm5_flush with LDAP KDB
Due to an inverted test in adb_policy_init, kadm5_flush calls
krb5_db_open twice. With the DB2 KDB module, the second open is a
no-op, but with the LDAP module, a new DB handle is allocated and the
old one is leaked.
Greg Hudson [Wed, 26 Mar 2014 17:21:45 +0000 (13:21 -0400)]
Refactor cm functions in sendto_kdc.c
Move get_curtime_ms and the cm functions near the top of the file
right after structure definitions. Except for cm_select_or_poll,
define each cm function separately for poll and for select, since the
implementations don't share much in common. Instead of
cm_unset_write, define cm_read and cm_write functions to put an fd in
read-only or write-only state. Remove the ssflags argument from
cm_add_fd and just expect the caller to make a subsequent call to
cm_read or cm_write. Always select for exceptions when using select.
(Polling for exceptions is implicit with poll).
With these changes, we no longer select/poll for reading on a TCP
connection until we are done writing to it. So in service_tcp_fd,
remove the check for unexpected read events.
Greg Hudson [Fri, 28 Mar 2014 16:33:43 +0000 (12:33 -0400)]
Get getopt from unistd.h (not getopt.h) in tests
POSIX defines getopt to be declared in unistd.h, and HP-UX (as of
version 11.31) does not appear to have getopt.h. In test programs
which currently include getopt.h and aren't currently built on
Windows, include unistd.h or just assume we will get it via k5-int.h.
Greg Hudson [Tue, 25 Mar 2014 16:28:05 +0000 (12:28 -0400)]
Fix t_seqstate build with Solaris compiler
The Solaris native compiler (as of version 5.9) outputs code for
static inline functions even if they are not used. So the
k5buf_to_gss helper in gssapiP_generic.h causes t_seqstate to have a
dependency on libkrb5support.
Tomas Kuthan [Wed, 26 Mar 2014 16:04:30 +0000 (17:04 +0100)]
Don't free cred handle used in kadm5 server handle
At the end of setup_gss(), gss_client_creds is released, but an alias
to the credential handle is saved in kadm5_server_handle_t in
handle->clnt->cl_auth->(struct rpc_gss_data *)ah_private->sec.cred.
Accessing these credentials (by authgss_refresh) can result in use
after free.
This fix stores credential reference in server handle and releases
the credentials in kadm5_destroy.
[ghudson@mit.edu: initialize handle->cred to correct constant; get rid
of gss_client_creds variable; clarify commit message slightly]
Greg Hudson [Fri, 21 Mar 2014 22:07:41 +0000 (18:07 -0400)]
Improve salt type display in kadmin getprinc
In krb5_salttype_to_string, output the salt type name we would
recognize as input.
In the output of getprinc, display the enctype and salt type in a form
we would accept--either enctype:salttype if the salt type is not the
default, or just the enctype if it is.
Update t_mkey.py and t_salt.py to expect the new output format.
Update documentation examples to show the new format.
Greg Hudson [Tue, 25 Mar 2014 14:52:38 +0000 (10:52 -0400)]
Modernize default_state.c
Use alloc_data() and empty_data() where appropriate. Keep mainline
logic to the left where possible. Name the output parameter of
krb5int_des_init_state with an _out suffix. Use a professional tone
in comments. Partly based on a patch from Alok Menghrajani.