Greg Hudson [Tue, 21 Jun 2016 22:46:28 +0000 (18:46 -0400)]
Fix recent memory leak in locate_kdc.c
Commit ce112dec844e4650b5ad174bd40f21c32aebe1d1 introduced a memory
leak in locate_srv_conf_1() by moving the free(realmstr) call to the
cleanup handler, because there was an early return after realmstr is
allocated. Convert that early return to a goto.
Before this patch libkrad would always subtract the existing buffer
length from pktlen before passing it to recv(). In the case of stream
sockets, this is incorrect since krad_packet_bytes_needed() already
performs this calculation. Subtracting the buffer length twice could
cause integer underflow on the len parameter to recv().
Greg Hudson [Tue, 21 Jun 2016 22:29:00 +0000 (18:29 -0400)]
Fix and simplify auth-indicator authdata module
Remove the authind_context count field. The indicators list must be
null-terminated because it is freed with k5_free_data_ptr_list().
authind_internalize() didn't null-terminate the list, and the presence
of the count field made it appear that this wasn't a bug. Use a
different scheme for setting *more in authind_get_attribute() to avoid
requiring an element count.
Also check more thoroughly for errors in authind_externalize() and
authind_internalize(), and remove some unnecessary pointer casts.
Dmitry Kalinkin [Fri, 17 Jun 2016 17:52:23 +0000 (13:52 -0400)]
Fix Makefile for paths containing '+' character
include/Makefile uses a regex to perform variable substitution with '+'
as the sed delimiter. Paths containing " are already invalid in this
approach, so it is better to use " as the delimiter instead of any
other rare symbol.
Matt Rogers [Fri, 13 May 2016 00:36:41 +0000 (20:36 -0400)]
Fail on error when processing KDC-issued authdata
Have k5_get_kdc_issued_authdata() return 0 on a verification failure and
non-zero for other failures, rather than call assert(). Check the
return value when called in krb5int_authdata_verify().
Matt Rogers [Fri, 13 May 2016 00:06:42 +0000 (20:06 -0400)]
Add auth indicator authdata module
This authdata module makes the 'auth-indicator' attribute available to
the GSSAPI name extension functions. The auth indicator values are UTF8
strings imported during AP_REQ processing.
Matt Rogers [Thu, 12 May 2016 23:43:55 +0000 (19:43 -0400)]
Add libkrb5 CAMMAC and auth-indicator functions
Add k5_unwrap_cammac_svc() and k5_authind_decode() internal functions
to libkrb5, for use by test programs and the forthcoming
auth-indicator authdata module. Remove the unused
cammac_check_svcver() from the KDC code. Modify tests/adata.c to use
the new functions, and add a test case to t_authdata.py for multiple
indicator values.
[ghudson@mit.edu: squash three commits; make k5_cammac_check_svcver()
a static helper]
Matt Rogers [Fri, 22 Apr 2016 16:23:37 +0000 (12:23 -0400)]
Add kprop and iprop default_realm tests
Add tests to t_iprop.py and t_kprop.py that exercise cases where
default_realm and domain_realm maps differ, as well as overriding the
default realm with the -r argument. This includes the testing of -r
with kadmind, and an update of expected ulog numbers for tests following
the addition of an incremental test. Also refactor some common code in
t_kprop.py to use in the new tests.
Matt Rogers [Thu, 25 Feb 2016 15:38:07 +0000 (10:38 -0500)]
Fix kprop and kpropd realm handling
Add the sn2princ_with_realm() helper function (currently duplicated in
kprop.c and kpropd.c) to simplify principal realm substitution. Use
sn2princ_with_realm() in kprop.c and kpropd.c in place of
krb5_sname_to_principal(), with the default realm if -r is not provided.
If a realm is given to kpropd, set it as the default realm on the
kpropd_context, allowing a later call of ulog_replay() to open the
correct database.
Remove referral realm code in kprop.c and kpropd.c. Pass the realm
(default or provided) to the kdb5_util and kprop commands called by
kadmind.
Greg Hudson [Thu, 9 Jun 2016 17:23:48 +0000 (13:23 -0400)]
Fix use_master handling with KDC hook reply
A post-receive KDC hook may synthesize a reply if k5_sendto() returns
an error. If this happens, krb5_sendto_kdc() must not use server_used
to check if the reply came from a master KDC, as it does not have a
valid value. Preemptively set *use_master to 1 in this case to bypass
the check.
Greg Hudson [Wed, 8 Jun 2016 04:00:55 +0000 (00:00 -0400)]
Fix kadmin min_life check with nonexistent policy
In kadmind, self-service key changes require a check against the
policy's min_life field. If the policy does not exist, this check
should succeed according to the semantics introduced by ticket #7385.
Fix check_min_life() to return 0 if kadm5_get_policy() returns
KADM5_UNK_POLICY. Reported by John Devitofranceschi.
Greg Hudson [Thu, 2 Jun 2016 15:58:35 +0000 (11:58 -0400)]
Fix bugs in recent locate_kdc.c change
The most recent change to locate_srv_conf_1() introduced a possible
double-free bug (detected by Coverity), and also broke MS-KKDCP
support. Separate the three uses of the "host" variable: the C string
copy of the realm name (now "realmstr"), the pointer to the hostname
or hostname:port specification in the profile values array (now
"hostspec"), and the hostname result of k5_parse_host_string() (still
"host"). Pass the correct pointer to k5_parse_host_string() if the
profile value is a URI.
Sarah Day [Tue, 19 Jan 2016 14:47:10 +0000 (09:47 -0500)]
Add k5_parse_host_string()
Add a helper function k5_parse_host_string() containing the
hostname-and-port parsing logic currently inlined into
locate_srv_conf_1(). The new function will also accept a port number
without hostname, for parsing listener addresses.
[ghudson@mit.edu: simplified parsing code and better handle edge
cases; split into two commits]
Greg Hudson [Mon, 9 May 2016 17:45:06 +0000 (13:45 -0400)]
Fix unlikely pointer error in get_in_tkt.c
In add_padata(), reset the caller's pointer and ensure the list is
terminated as soon as realloc() succeeds; otherwise, the old pointer
could be left behind if a later allocation fails.
Tom Yu [Fri, 27 May 2016 19:19:43 +0000 (15:19 -0400)]
Relax t_sn2princ.py reverse resolution test
Relax t_sn2princ.py check of the reverse resolution of the test
hostname. The new requirement is that it be different from the
forward resolved hostname. (There is also an existing implicit
requirement that it be in the mit.edu domain.) This makes
t_sn2princ.py more robust against changes in the reverse resolution of
the test hostname.
Simo Sorce [Wed, 30 Mar 2016 17:00:19 +0000 (13:00 -0400)]
Add SPNEGO special case for NTLMSSP+MechListMIC
MS-SPNG section 3.3.5.1 documents an odd behavior the SPNEGO layer
needs to implement specifically for the NTLMSSP mechanism. This is
required for compatibility with Windows services.
In otp_client_process(), call cb->set_as_key() later in the function
after the OTP request has been created. The previous position of this
call caused the AS key to be replaced even when later code in the
function failed, preventing other preauth mechanisms from retrieving
the correct AS key.
Robbie Harwood [Fri, 20 May 2016 00:31:38 +0000 (20:31 -0400)]
Do not indicate deprecated GSS mechanisms
The mechanisms themeselves will continue to work if requested, but will
not be included in the gss_indicate_mech() list. This works around a
bug in some legacy applications that cannot cope with deprecated mechs
being returned.
Greg Hudson [Thu, 12 May 2016 20:03:06 +0000 (16:03 -0400)]
Check princ length in krb5_sname_match()
krb5_sname_match() can read past the end of princ's component array in
some circumstances (typically when a keytab contains both "x" and
"x/y" principals). Add a length check. Reported by Spencer Jackson.
Greg Hudson [Tue, 17 May 2016 23:28:25 +0000 (19:28 -0400)]
Simplify principal and policy manipulation code
Now that principal entry and policy fields are allocated using the
malloc visible to the krb5 libraries, we don't need to use
krb5_db_alloc() and krb5_db_free() when modifying them within our
code.
Greg Hudson [Tue, 17 May 2016 02:54:06 +0000 (22:54 -0400)]
Use library malloc for principal, policy entries
Alter the KDB module contract to require that KDB modules use an
allocator compatible with the malloc() seen by libkrb5 and libkdb5.
Change krb5_db_alloc() and krb5_db_free() to provide access to this
allocator. Remove free_principal, free_policy, alloc, and free from
the KDB interface and from all in-tree KDB modules.
Sarah Day [Fri, 29 Apr 2016 14:26:31 +0000 (10:26 -0400)]
Implement principal renaming in LDAP
The generic method of renaming principals (by adding a new entry and
deleting the old one) does not work in LDAP. Add an LDAP
implementation of rename that properly renames the DN and attributes
when necessary.
[ghudson@mit.edu: minor naming changes and code simplifications]
Sarah Day [Thu, 31 Mar 2016 21:49:55 +0000 (17:49 -0400)]
Add new DAL function for renaming principals
Previously libkadm5srv renamed principals by getting the principal
entry, renaming the entry, putting it in the DB, then deleting the old
one. This does not work in certain KDB modules such as LDAP. A new
DAL function is necessary to support all KDB modules. Add a new DAL
function to support custom renames in all KDB modules, with a default
implementation that performs the previous functionality of adding and
deleting the principal entry.
NOTE: if the default rename function isn't used and iprop logging is
enabled, iprop would fail since it doesn't formally support renaming.
In that case, the call to krb5_db_rename_principal() will fail with
the code KRB5_PLUGIN_OP_NOTSUPP.
Greg Hudson [Mon, 2 May 2016 16:51:03 +0000 (12:51 -0400)]
Fix cstyle-file.py when emacs is not installed
emacs_reindent() is intended to fail gracefully when emacs is not
installed, but instead subprocess.call() throws an OSError. Check for
this error and return normally.
For each file we check in cstyle-file.py, try to use emacs to
batch-reindent a copy of the file, and compare the resulting lines to
the input to detect indentation errors.
In logger.c, remove conditionals around uses of syslog.h, syslog(),
openlog(), closelog(), and syslog priority constants. This header and
these symbols are used unconditionally in other non-Windows parts of
the tree. Leave the conditionals around facility constants for now,
as not all of the facility constants are specified in POSIX.
Greg Hudson [Mon, 28 Mar 2016 17:48:52 +0000 (13:48 -0400)]
Improve errors when DB2 database cannot be opened
When we cannot open a DB2 database, set a useful error message.
Change the signature of open_db() to to allow it to return an error
code with a message set.
Tom Yu [Thu, 28 Apr 2016 20:44:21 +0000 (16:44 -0400)]
Unconstify some krb5 GSS OIDs
gssapi_krb5.h declared some well-known OID constants as pointers to
const gss_OID_desc, which can't be assigned to application-declared
gss_OID variables or passed to GSSAPI functions without causing
warnings.
Declare these OID constants without the const qualifier on
gss_OID_desc, at the expense of some type safety. (Fixing this
"correctly" probably requires some standards revision.)
Add a sign_authdata method to the test KDB module. Add tests to
t_authdata.py for KDB module authdata and the kinit --request-pac and
--no-request-pac options.
Add a new public function to set a PAC request option for an AS
request.
[ghudson@mit.edu: simplified code; made signature conform to Heimdal
function; expanded on doxygen comment; added new function to API
reference; changed code to send encoded KERB-PA-PAC-REQUEST instead
of a single octet]
Matt Rogers [Tue, 26 Apr 2016 18:36:55 +0000 (14:36 -0400)]
Skip password prompt when running ksu as root
A change introduced in 5fd5a67 resulted in root always being prompted for
the target user password when running ksu. Restore the previous behavior
which is to only prompt if the principal is provided with -n.
Use strdur() to output the maximum and minimum password life for
policies, and output "[never]" instead of "[none]" for a zero password
expiration date for consistency with other dates.
When parsing kadmin time intervals using getdate.y relative time
formats, make sure the same timestamp is added to and substracted from
the relative value. To accomplish this, rename get_date() to
get_date_rel() with a second parameter for the current time, and make
get_date() a wrapper with the current signature for the benefit of
kdb5_util and kdb5_ldap_util.
When parsing time intervals in kadmin commands, try
krb5_string_to_deltat() first, then fall back to subtracting the
current time from get_date(). If we do fall back, treat "never" as a
zero interval, and error out rather than yield a huge interval if
get_date() returns a time in the past.
Notable behavior differences: bare numbers of seconds and suffixed
numbers (e.g. "5m" or "6h") are now supported for all intervals;
HH:MM:SS and HH:MM are now treated as intervals rather than absolute
times with the current time subtracted.
Greg Hudson [Mon, 29 Feb 2016 21:51:22 +0000 (16:51 -0500)]
Skip unnecessary mech calls in gss_inquire_cred()
If the caller does not request a name, lifetime, or cred_usage when
calling gss_inquire_cred(), service the call by copying the mechanism
list (if requested) but do not call into the mech.
This change alleviates an issue (reported by Adam Bernstein) where
SPNEGO can fail in the presence of expired krb5 credentials rather
than proceeding with a different mechanism, or can resolve a krb5
credential without the benefit of the target name.
Greg Hudson [Wed, 23 Mar 2016 17:15:58 +0000 (13:15 -0400)]
Amend KDC hook documentation
In the Doxygen comments for the new APIs and types, include @version
tags indicating that they are new in 1.15, and put @param declarations
just after the brief message for consistency with other comments.
Greg Hudson [Wed, 16 Dec 2015 17:51:36 +0000 (12:51 -0500)]
Fix kdb5_ldap_util stashsrvpw password file logic
kdb5_ldap_util stashsrvpw has several inconsistencies with the
password file determination in libkdb_ldap, and could try to fopen() a
NULL filename in some cases. Factor out the determination of the
configured password file and make it consistent with libkdb_ldap.
DEF_SERVICE_PASSWD_FILE is no longer used after these changes, as it
is not respected by libkdb_ldap.
Greg Hudson [Tue, 29 Mar 2016 22:32:56 +0000 (18:32 -0400)]
Integrate with appveyor for Windows CI
appveyor.com is a hosted continuous integration service for Windows.
Add an appveyor.yml file containing build instructions. The appveyor
virtual machines do not include the MFC libraries, so change
util/wshelper/resource.rc to avoid including <afxres.h> (which it does
not need) and add a build conditional for leash.
Right now we do not build the installers; the appveyor VMs do not
appear to have the version of the WiX toolkit we need, and we would
also have problems with the missing leash executable.
Matt Rogers [Wed, 24 Feb 2016 21:06:53 +0000 (16:06 -0500)]
Move the util/windows getopt to libkrb5support
Relocate the internal getopt() and getopt_long() code to util/support,
and build conditionally. Put declarations in k5-platform.h. Adjust
Windows build directives for src/clients. Remove getopt-related #defines
from kinit.c, allowing kinit to use getopt_long() on all platforms.
Sarah Day [Thu, 18 Feb 2016 21:54:27 +0000 (16:54 -0500)]
Default to LSA when TGT in LSA is inaccessible
When UAC is enabled and a domain user with Administrator privileges
logs in, the TGT is inaccessible. Access to the TGT in a
UAC-restricted session may allow a non-elevated user to bypass the
UAC. In a UAC-restricted session, ms2mit copies the current tickets
from the LSA ccache to the API ccache except the TGT, effectively
preventing a user session from getting additional service tickets
while appearing, for some purposes, to have a usable ccache.
Another bug is that ms2mit always copies from the LSA ccache to the
default ccache, even if the default ccache is itself the LSA ccache.
New behavior:
* If the TGT is accessible in the LSA ccache, copy the LSA ccache to
the API ccache.
* Set the registry key for the default ccname to "API:" if the copy
occurred, or to "MSLSA:" if it didn't occur.
Tom Yu [Mon, 28 Mar 2016 18:57:10 +0000 (14:57 -0400)]
Fix calling conventions
Commit fb4d426ddeb9d4802a53dfbd74189ef8eacbe65e added two new APIs but
didn't make the KRB5_CALLCONV decorations consistent between
declarations and definitions. This broke the build on Windows.
Peter Jones [Fri, 26 Feb 2016 14:49:58 +0000 (09:49 -0500)]
Make profile includedir accept all *.conf files
Since the main config file is krb5.conf, it is intuitive to name
included files with a ".conf" extension; currently such files are
silently ignored. Accept filenames ending in ".conf" as well as files
with no special characters.
[ghudson@mit.edu: shorten commit message and comment; accept the
filename ".conf" itself for simplicity; add a test; adjust
documentation change to note that allowing .conf is new in 1.15]
Sarah Day [Thu, 14 Jan 2016 18:11:21 +0000 (13:11 -0500)]
Remove port 750 from the KDC default ports
The KDC was still listening on port 750 despite the fact that
this functionality was supposed to have been removed in the
past. Remove port 750 from the list of UDP ports that the KDC
listens on. Also remove port 750 from the default ports that
the client connects to, and from example config fragments.
Greg Hudson [Mon, 14 Mar 2016 21:26:34 +0000 (17:26 -0400)]
Fix LDAP null deref on empty arg [CVE-2016-3119]
In the LDAP KDB module's process_db_args(), strtok_r() may return NULL
if there is an empty string in the db_args array. Check for this case
and avoid dereferencing a null pointer.
CVE-2016-3119:
In MIT krb5 1.6 and later, an authenticated attacker with permission
to modify a principal entry can cause kadmind to dereference a null
pointer by supplying an empty DB argument to the modify_principal
command, if kadmind is configured to use the LDAP KDB module.
Greg Hudson [Tue, 15 Mar 2016 21:45:26 +0000 (17:45 -0400)]
Revisit inquire_attrs_for_mech on old mechs
In gss_inquire_attrs_for_mech(), if the mech does not implement RFC
5587, return success with empty mech_attrs and known_mech_attrs sets
to indicate a lack of knowledge for all attributes. The previous
behavior of returning an error caused gss_indicate_mechs_by_attr() to
fail out in the presence of an old mechanism, in turn causing
gss_acquire_cred() and SPNEGO to break.
Matt Rogers [Thu, 25 Feb 2016 19:55:44 +0000 (14:55 -0500)]
Add auth indicator handling to libkdb_ldap
Have krb5_ldap_put_principal() store individual auth indicator values
in the new krbPrincipalAuthInd attribute, in addition to krbExtraData.
krb5_ldap_get_principal() retrieves auth indicator values from
krbPrincipalAuthInd, which takes precedence over any krbExtraData
entries.
Greg Hudson [Thu, 25 Feb 2016 19:41:48 +0000 (14:41 -0500)]
Remove hdb KDB module
The hdb module has bitrotted, both against our internal interfaces and
against Heimdal's database layer. It is now possible to dump Heimdal
databases in the MIT krb5 dump format, which is a better option than
bridging between the database layers.
Greg Hudson [Fri, 4 Mar 2016 18:25:28 +0000 (13:25 -0500)]
Allow zero cksumtype in krb5_k_verify_checksum()
A checksum type of 0 means to use the mandatory checksum type in
krb5_k_make_checksum(), krb5_k_make_checksum_iov(), and
krb5_k_verify_checksum_iov(). Extend this meaning to
krb5_k_verify_checksum() for the checksum type in the krb5_checksum
argument. This change also applies to krb5_c_verify_checksum().
Add code to t_cksums.c to test checksum verification, including with
checksum type 0 for applicable test cases.
Greg Hudson [Fri, 4 Mar 2016 18:57:19 +0000 (13:57 -0500)]
Fix assert hygiene in crypto tests
assert() should not be used with expressions with side-effects, as it
can be compiled out with the NDEBUG flag. Fix all uses of
side-effecting asserts in lib/crypto test programs.
Tom Yu [Thu, 25 Feb 2016 23:01:36 +0000 (18:01 -0500)]
Fix Makefiles for VS2010 KfW build
The new Makefile conditionals in commit 4552159e97007a45370dd49fa6b9fb963bb7d160 don't behave properly if
VISUALSTUDIOVERSION isn't set, probably due to the way nmake orders
macro expansion and boolean short circuiting. Use nested conditionals
instead.
Simo Sorce [Thu, 25 Feb 2016 21:31:09 +0000 (16:31 -0500)]
Interoperate with incomplete SPNEGO responses
We have found at least one HTTP/Negotiate implementation in Java that
does not set anything but the responseToken field in the first SPNEGO
acceptor response token. This is technically a violation of RFC 4178
section 4.2.2, but it is harmless to support; we know the mechanism we
were trying to negotiate, and can use that mechanism to process the
token.
These implementations are probably not supporting any real
negotiation, as the missing negState precludes any mechanism
negotiation on failure. If a supportedMech is included that differs
from the opportunistic one but no negState is provided,
init_ctx_reselect() will fail with GSS_S_DEFECIVE_TOKEN as it should.
[ghudson@mit.edu: edit comments and commit message]
Greg Hudson [Wed, 10 Feb 2016 16:50:54 +0000 (11:50 -0500)]
Fix and adjust t_kprop.py
The listprincs check was at the wrong indentation level and had the
wrong argument grouping; fix it so we actually verify the propagation.
Stop using the -t (runonce) flag to kpropd, so that kpropd continues
to run until k5test.py terminates it. Quit out of the read loop when
we see that the load process is completed, instead of looking for end
of input. This change is needed in order to add hooks in k5test.py
for checking daemons for memory leaks before terminating them.
Isaac Boukris [Mon, 1 Feb 2016 16:08:24 +0000 (18:08 +0200)]
Use cached S4U2Proxy tickets in GSSAPI
Ticket #7047 allowed credentials obtain using S4U2Proxy through GSSAPI
to be cached, but doesn't actually use the cached credentials. Modify
get_credentials() to check the cache for the desired client name
first, then to make an S4U2Proxy request if we don't find it.
Test this change by adding code to t_s4u.c to repeat the constrained
delegation request and verify that only three tickets are present in
the cache.
[ghudson@mit.edu: squash commits; commit message rewrite; minor style
edits; changed test code to use gss_store_cred_into() to avoid the
need to pick a principal to initialize the ccache with]
Simo Sorce [Sun, 20 Dec 2015 22:01:50 +0000 (17:01 -0500)]
Improve libkadm5 server stubs
Change the server_stubs.c functions to use thread-safe signatures (as
would be output by rpcgen -M). This change has no immediate impact
since kadmind is single-threaded, but is cleaner because it avoids the
use of static variables. Factor out some of the common initialization
and cleanup code from the server functions.
[ghudson@mit.edu: rename stub helper functions and reorder/rename
output arguments; error out in stub setup if princ is unexpectedly
null]
Greg Hudson [Tue, 23 Feb 2016 22:15:18 +0000 (17:15 -0500)]
Use blocking lock when creating db2 KDB
In 1.11 we switched from non-blocking to blocking locks in the DB2
module, but we missed one call to krb5_lock_file() in ctx_create_db().
This non-blocking lock can cause krb5_db_promote() to fail if the
database is locked when we try to promote the DB, in turn causing
kdb5_util load to fail. Correct this call to make krb5_db_promote()
more robust.
Greg Hudson [Mon, 22 Feb 2016 21:33:07 +0000 (16:33 -0500)]
Increase initial DNS buffer size
In dnsglue.c (which is used to look up SRV and TXT records), increase
the initial buffer size guess from 2048 to 4096 to accomodate DNSSEC
signatures. Suggested by Daniel Colascione.
Simo Sorce [Fri, 18 Dec 2015 23:13:29 +0000 (18:13 -0500)]
Add the ability to lock down principal keys
A new attribute named KRB5_KDC_LOCKDOWN_KEYS can be set on principals.
This flag prevents keys for the principal from being extracted or set
to a known value by the kadmin protocol. Principals with this flag
cannot be deleted or renamed, and cannot have keys set by setkey or
chpass. chrand operations are allowed, but keys are not returned.
This attribute can be set via the modify operation but cannot be
reset; an authorization error is resturned if an attempt to reset it
is performed.
When creating a KDB, set the lockdown flag on the krbtgt and kadmin
principals.
[ghudson@mit.edu: squash with t_kadmin_acl.py commit; condense commit
message]
projects / thirdparty / krb5.git / log
