Greg Hudson [Wed, 21 Jun 2017 04:54:32 +0000 (00:54 -0400)]
Parse all kadm5.acl fields at startup
Parse the client principal name, target principal name, and
restrictions field of kadm5.acl entries when the file is loaded, not
later on when an attempt is made to match the entry.
This change affects the error-handling behavior of kadm5.acl files.
Previously, a syntax error in the line structure (such as having only
one field) would cause the whole file to be rejected, but an error
within a principal name or restrictions string would cause only that
entry to be discarded. After this change, any parsing failure will
cause the whole file to be rejected.
Greg Hudson [Wed, 21 Jun 2017 14:38:41 +0000 (10:38 -0400)]
Modernize auth_acl.c
Change auth_acl.c to match current coding conventions. Use more
consistent identifier names, and drop the kadm5int_ prefix as the code
is now part of kadmind. Remove the acl_inited, acl_debug_level, and
acl_catchall_entry variables. Move global state into a structure, to
make it easier to migrate to a module handle later. Move
parse_restrictions() above parse_line() so it can later be used from
parse_line() without a forward declaration. Rewrite get_line() and
parse_line() to avoid the use of fixed-sized static buffers and
sscanf(). Add a parse_entry() helper to make memory management in
parse_line() easier. Add a free_acl_entry() helper (split out from
free_acl_entries()) to make error handling in parse_entry() easier.
Add a match_princ() helper to simplify find_entry().
Remove the GSS name translation wrapper in auth_acl.c. In the server
stubs, use handle->current_caller for the client principal. In the
iprop RPCs(), add a wrapper to parse the client display name before
calling acl_check().
Greg Hudson [Mon, 26 Jun 2017 21:31:37 +0000 (17:31 -0400)]
Fix kadm5 setkey operation with LDAP KDB
Add mask assignments to kadm5_setv4key_principal() and
kadm5_setkey_principal_4() so that their changes to the principal are
properly written to KDB modules which use the mask flag, such as the
LDAP KDB module. Reported by Frank Lonigro.
Greg Hudson [Mon, 19 Jun 2017 15:30:38 +0000 (11:30 -0400)]
Fix kadm5.acl error reporting
In kadm5int_acl_get_line(), increment *lnp after skipping a blank or
comment line, so that kadm5int_acl_load_acl_file() correctly reports
the line number if it fails to parse a line.
In acl_syn_err_msg, use %.10s to limit the amount of the line included
in the error message, not %10s to left-pad it with spaces if it is
shorter than ten characters.
Greg Hudson [Thu, 15 Jun 2017 15:59:18 +0000 (11:59 -0400)]
Suppress y2038 GSS tests when time_t is 32-bit
The GSSAPI time_t tests do not run correctly on 32-bit Solaris because
time_t conversions are involved in the "kinit -l 8500d" step.
Suppress the GSS y2038 tests when time_t is 32-bit.
Greg Hudson [Thu, 15 Jun 2017 00:45:15 +0000 (20:45 -0400)]
Turn off -Wmaybe-uninitialized
In gcc, maybe-uninitialized gives different warnings depending on the
optimization level, and in our experience usually gives false
positives. We don't ask for it (except implicitly through -Wall), but
gcc bundles it into the error behavior of -Werror=uninitialized.
Explicitly turn it off so that builds with -Og and -Os don't error
out.
In the KDC, pass the local address from dispatch() to
process_as_req(), then to log_as_req(), then to
krb5_db_audit_as_req(), and finally to the KDB modules.
[ghudson@mit.edu: squashed commits and rewrote commit message]
In net-server.c, pass a krb5_fulladdr representation of the local
address to dispatch. This representation is more convenient for
kadmind, and will make it more convenient for the KDC to pass the
local address to the DAL audit_as_req.
In libkdb5, libapputils, the KDC, kadmind, and both KDB modules, use
the name "remote_addr" for the variable containing the remote address.
In schpw.c:process_chpw_request(), use the name "local_addr" for the
parameter containing the local address. Make the remote_addr
parameter const in libkdb5 and the DAL.
[ghudson@mit.edu: combined commits and rewrote commit message]
In net-server.c:process_tcp_connection_read(), we don't expect
getsockname() to fail under ordinary circumstances, so instead of
passing a null local address to dispatch(), just error out. Simplify
schpw.c:dispatch() by assuming a non-null local_saddr.
Greg Hudson [Fri, 26 May 2017 20:20:11 +0000 (16:20 -0400)]
Fix gmt_mktime for y2038
gmt_mktime() is used as a fallback when the platform does not have
timegm(). Make it work for dates in the unsigned 32-bit range, not
the signed 32-bit range.
Tomas Kuthan [Tue, 16 May 2017 09:24:40 +0000 (11:24 +0200)]
Free GSS checksum data deterministically
In the normal course of execution, md5.contents allocated by
kg_checksum_channel_bindings() in make_ap_req_v1() is freed in
make_gss_checksum(). But when there is a failure in
krb5_mk_req_extended() or in make_gss_checksum() before free is
called, the memory leaks.
This patch frees the memory unconditionally in make_ap_req_v1().
Greg Hudson [Mon, 22 May 2017 19:12:58 +0000 (15:12 -0400)]
Remove ksetpwd
ksetpwd was added in commit ec50322c3076ab4517fb4fb5cc3a931f6adb4f20
but is not installed as it was "not of release quality yet." It has
not materially improved since then, and under current policy we do not
include unfinished code in the tree, so remove it.
Greg Hudson [Wed, 17 May 2017 19:21:34 +0000 (15:21 -0400)]
Remove vestigial svr_principal.c code
In kadm5_chpass_principal_3(), kadm5_randkey_principal_3(), and
kadm5_setv4key_principal(), remove the disabled code to enforce
pw_min_life (which is enforced in kadmind as noted in the comments),
as well as the unnecessary last_pwd lookups beforehand.
Add a test program for krb5int_validate_times() covering cases before
and across the y2038 boundary. Add a GSSAPI test program to exercise
lifetime queries, and tests using it in t_gssapi.py for ticket end
times after y2038. Add a new test script t_y2038.py which only runs
on platforms with 64-bit time_t to exercise end-user operations across
and after y2038. Add an LDAP test case to test storage of post-y2038
timestamps.
Wherever we manipulate krb5_timestamp values using arithmetic,
comparison operations, or conversion to time_t, use the new helper
functions in k5-int.h to ensure that the operations work after y2038
and do not exhibit undefined behavior. (Relying on
implementation-defined conversion to signed values is okay as we test
that in configure.in.)
In printf format strings, use %u instead of signed types. When
exporting creds with k5_json_array_fmt(), use a long long so that
timestamps after y2038 aren't marshalled as negative numbers. When
parsing timestamps in test programs, use atoll() instead of atol() so
that positive timestamps after y2038 can be used as input.
In ksu and klist, make printtime() take a krb5_timestamp parameter to
avoid an unnecessary conversion to time_t and back.
As Leash does not use k5-int.h, use time_t values internally and
safely convert from libkrb5 timestamp values.
Add k5-int.h helper functions to manipulate krb5_timestamp values,
avoiding undefined behavior and treating negative timestamp values as
times between 2038 and 2106. Add a doxygen comment for krb5_timestamp
indicating how third-party code should use it safely.
Commit ae0fee058ad883b2e82fa2b34f4e5f059e827a1b (ticket #5425) changed
the AS client code to use a random nonce, but left the TGS client code
using the current timestamp. Use a random nonce for TGS requests as
well.
During a TGS request, if we get a TGT response that we didn't directly
ask for (a referral TGT or an alternate TGT), don't cache it. It
would have limited value in the cache as similar operations won't look
for that TGT. If the overall TGS operation fails and is repeated, we
could wind up caching the same entry multiple times, which doesn't
work well with our current ccache implementations.
The flags field in krb5_lcc_data is not initialized in
krb5_lcc_resolve(), so krb5_lcc_next_cred() can sometimes fail to
include a ticket when retrieving a ccache entry. This results in a
"Request did not supply a ticket" error from k5_make_tgs_req() when
trying to use the credential.
Add a context parameter to the in_clock_skew() macro so that it isn't
implicitly relying on a local variable. Use it in
get_in_tkt.c:verify_as_reply().
Commit b496ce4095133536e0ace36b74130e4b9ecb5e11 (ticket #8268) adds
the clock skew to krb5 acceptor context lifetimes for
gss_accept_sec_context() and gss_inquire_context(), but not for
gss_context_time(). Add the clock skew in gss_context_time() as well.
We apply (as of ticket #7604) a ten-second minimum delay after a TCP
connection is accepted before creating new connections or sending UDP
packets. Apply this timeout to HTTPS connections as well, by removing
the transport check in get_endtime(). As the endtime field is only
set by service_tcp_connect(), it will always have the value 0 for UDP
connection state objects, so there is no need to check the transport
type.
Where we convert between UTF-8 and UCS-2 (RC4 string-to-key and PAC
client info), use UTF-16 instead of UCS-2. Add a test program for
the conversion functions.
Martin Kittel [Thu, 6 Apr 2017 19:03:23 +0000 (21:03 +0200)]
Add various bound checks
Add bounds checks where Coverity otherwise reports a defect. Most of
these checks are unlikely to be triggered in practice (Unicode regexps
are unused, and the caller of gss_krb5int_make_seal_token_v3 won't
have a plaintext object larger than half of the address space). The
checks in dump.c could prevent memory access errors resulting from a
malformed dump file.
Avoid using the krb5_error_code type (using int32_t instead), and
include k5-platform.h instead of k5-int.h, so that we can use
k5-input.h in libkrb5support.
Create a custom build matrix which passes -Werror to the clang build
via a make variable. (Using a configure variable does not currently
work, as some of our configure test programs generate warnings.)
Also set the language to C++ (so we use clang++ for the C++ test
programs and not g++ when compiling with clang), and turn on the
maintainer-mode checks for the Travis build.
Remove unused entry points as we only need to convert between
little-endian UCS-2 byte buffers and UTF-8. Rename and simplify the
remaining two function contracts. Avoid pointer alignment and
endianness issues by operating on byte buffers and using store_16_le()
and load_16_le(). Avoid two-pass operation using k5buf.
[ghudson@mit.edu: simplified code using k5buf; simplified function
names and contracts; rewrote commit message]
krb5int_utf8cs_to_ucs2les() can read slightly beyond the end of the
input buffer if the buffer ends with an invalid UTF-8 sequence. When
computing the RC4 string-to-key result, make a zero-terminated copy of
the input string and use krb5int_utf8s_to_ucs2les() instead.
Matt Rogers [Fri, 31 Mar 2017 02:18:24 +0000 (22:18 -0400)]
Add FAST encrypted challenge auth indicator
During ec_verify(), look up an authentication indicator string by the
profile realm option "encrypted_challenge_indicator". If found, add
an indicator to the reply upon succesful creation of the challenge
key. Add a test to t_authind.py. Document the option in
kdc_conf.rst.
We have had code since at least 1.6 in changepw.c and sendto_kdc.c
which assumes that we can pass a struct sockaddr * as the second
argument to getsockname() and getpeername(), so we can safely get rid
of that configure logic. Also fix potential alignment issues in
krb5_sendauth() by using a struct sockaddr_storage instead of a
1024-byte character buffer to hold the local and peer addresses.
[ghudson@mit.edu: adjusted style of new code slightly; rewrote commit
message]
When we are building a static object containing a finalizer function
(e.g. for the profile library tests), mark the finalizer as unused to
avoid warnings in gcc and clang.
[ghudson@mit.edu: commented UNUSED definition and moved it so we can
use it elsewhere later; rewrote commit message]
Robbie Harwood [Wed, 29 Mar 2017 22:34:37 +0000 (18:34 -0400)]
Avoid using tmpnam(3) in db2's hash.c
As we do not rely on anonymous db2 databases, get rid of the code
using tmpnam() for hash databases and reporting EINVAL if a filename
is not specified.
In kdb_log.h, cast through void * after computing the address in the
INDEX macro.
In ipropd_svc.c, use a void * instead of a char * as the generic
handler return value.
In rc4.c, cast through void * when using the cipher state data pointer
as a structure pointer.
In sha256.c and sha512.c, cast through void * when using the save
buffer as a structure pointer. (This code may not be conformant, but
it should work in practice given the offsets of the save field in the
sha256state and sha512state structures.)
If we reach the encrypted challenge clpreauth process method without
an armor key, error out instead of crashing. This can happen if (a)
the KDC offers encrypted challenge even though the request doesn't use
FAST (the Heimdal KDC apparently does this), and (b) we fall back to
that preauth method before generating a preauthenticated request,
typically because of a prompter failure in encrypted timestamp.
Reported by Nico Williams.
Simo Sorce [Thu, 30 Mar 2017 15:27:09 +0000 (11:27 -0400)]
Add support to query the SSF of a GSS context
Cyrus SASL provides a Security Strength Factor number to assess the
relative "strength" of the negotiated mechanism, and applications
sometimes make access control decisions based on it.
Add a call that allows us to query the mechanism that established the
GSS security context to ask what is the current SSF, based on the
enctype of the session key.
The kprop protocol uses cipher state via this call, perhaps along with
other. As there is no replacement, the call should not be deprecated
in the API.
Matt Rogers [Wed, 5 Apr 2017 20:48:55 +0000 (16:48 -0400)]
Use the canonical client principal name for OTP
In the OTP module, when constructing the RADIUS request, use the
canonicalized client principal (using the new client_name kdcpreauth
callback) instead of the request client principal.
Martin Kittel [Wed, 15 Mar 2017 16:21:28 +0000 (17:21 +0100)]
Fix krb5int_open_plugin_dirs() error handling
In krb5int_open_plugin_dirs(), if constructing filepath fails,
filepath is set to null but accessed a few lines later. Add an error
check before calling krb5int_open_plugin().
Martin Kittel [Wed, 29 Mar 2017 07:22:18 +0000 (09:22 +0200)]
Add null check to placate static analysis tools
In trace_format() when processing lenstr, if p is NULL and len is 0,
skip the call to buf_add_printable_len() as Coverity considers it an
unconditional dereference of p.
[ghudson@mit.edu: added explanation to commit message]
Greg Hudson [Mon, 27 Mar 2017 19:40:08 +0000 (15:40 -0400)]
Simplify null salt handling in string-to-key
The per-enctype string_to_key implementations are inconsistent about
whether a null salt is treated as empty or results in a null
dereference. Since the original DES string-to-key allowed a null
salt, substitute an empty salt in krb5_c_string_to_key_with_params().
Eliminate conditionals on accessing salt in the per-enctype
implementations as they are no longer needed. Based on a patch by
Martin Kittel.
Greg Hudson [Fri, 24 Mar 2017 15:07:21 +0000 (11:07 -0400)]
Ignore dotfiles in profile includedir
Editors and filesystems may create artifacts related to .conf files
which don't change the file suffix; these artifacts generally begin
with "." so that they don't appear in normal directory listings
(e.g. ".#filename" for emacs interlock files). Make sure to ignore
any such artifacts when processing a profile includedir directive.
Greg Hudson [Thu, 23 Mar 2017 18:26:50 +0000 (14:26 -0400)]
Remove some unnecessary PKINIT code
In cms_signeddata_create(), alg_buf and digest_buf are allocated but
never used. (Instead, a combined buffer is allocated and the alg and
digest objects are marshalled into it.) Remove them.
Matt Rogers [Wed, 15 Mar 2017 23:57:15 +0000 (19:57 -0400)]
Add the certauth dbmatch module
Add and enable the "dbmatch" builtin module. Add the
pkinit_client_cert_match() and crypto_req_cert_matching_data() helper
functions. Add dbmatch tests to t_pkinit.py. Add documentation to
krb5_conf.rst, pkinit.rst, and kadmin_local.rst.
Matt Rogers [Wed, 22 Mar 2017 01:24:14 +0000 (21:24 -0400)]
Simplify PKINIT cert iteration and selection
Remove the pkinit_cert_handle structures and iteration functions used
during certificate matching. Instead, make pkinit_matching.c obtain a
list of matching data objects from the crypto code, and then select a
cert based on the index into that list.
Also fix a typo in the name of crypto_retrieve_X509_key_usage().
Matt Rogers [Tue, 28 Feb 2017 20:55:24 +0000 (15:55 -0500)]
Add certauth pluggable interface
Add the header include/krb5/certauth_plugin.h, defining a pluggable
interface to control authorization of PKINIT client certificates.
Add the "pkinit_san" and "pkinit_eku" builtin certauth modules and
related PKINIT crypto X.509 helper functions. Add authorize_cert() as
the entry function for certauth plugin module checks called in
pkinit_server_verify_padata(). Modify kdcpreauth_moddata to hold the
list of certauth module handles, and load the modules when the PKINIT
kdcpreauth server plugin is initialized. Change
crypto_retrieve_X509_sans() to return ENOENT when no SAN is found.
Add test modules in plugins/certauth/test. Create t_certauth.py with
basic certauth tests. Add plugin interface documentation in
doc/plugindev/certauth.rst and doc/admin/krb5_conf.rst.
Greg Hudson [Tue, 14 Mar 2017 23:39:38 +0000 (19:39 -0400)]
Force autoconf rebuild in maintainer rules
autoconf normally avoids recreating files that it does not consider
obsolete. Since it knows nothing about patchlevel.h (which we read at
autoconf time using m4's esyscmd()), changes to patchlevel.h won't be
reflected in configure unless another input to configure has changed,
and the maintainer rule will re-run autoconf over and over again. Fix
this issue by passing the force flag to autoconf when we invoke it
from the maintainer rule.
Greg Hudson [Sun, 12 Mar 2017 16:42:37 +0000 (12:42 -0400)]
Fix minor memory leaks in kvno
In do_k5_kvno(), free allocated values on success as well as failure.
In t_kdb.py, run kvno with multiple arguments to manifest this leak in
asan and valgrind. Reported by Cel Skeggs.
Greg Hudson [Sun, 12 Mar 2017 16:29:50 +0000 (12:29 -0400)]
Fix memory leaks in test programs
Eliminate memory leaks detected by asan in test programs (and
introduced since commit 4947c270032691d556140b290e1b10846b692968), to
make it easier to find more serious leaks.
Isaac Boukris [Sat, 4 Mar 2017 19:23:32 +0000 (21:23 +0200)]
Allow null outputs to gss_get_name_attribute()
In krb5_gss_get_name_attribute(), always ask for kvalue and
kdisplay_value when calling krb5_authdata_get_attribute(), as it
currently expect non-null arguments. This change allows applications
to pass GSS_C_NO_BUFFER for the value and display_value output
parameters. (Passing NULL for the authenticated and complete output
parameters already works.)
[ghudson@mit.edu: initialized kvalue and kdisplay_value for safety]
Greg Hudson [Tue, 28 Feb 2017 03:35:07 +0000 (22:35 -0500)]
Fix udp_preference_limit with SRV records
In sendto_kdc:resolve_server() when resolving a server entry with a
specified transport, defer the resulting addresses if the strategy
dictates that the specified transport is not preferred. Reported by
Jochen Hein.