The new [hdb] new_service_key_delay parameter should not apply to
principal entries when used as clients. Otherwise new passwords would
not take effect immediately, and that would be very confusing.
This is a large commit that adds several features:
- Revamps and moves virtual host-based service principal functionality
from kdc/ to lib/hdb/ so that it may be automatically visible to
lib/kadm5/, as well as kadmin(1)/kadmind(8) and ktutil(1).
The changes are backwards-incompatible.
- Completes support for documenting a service principal's supported
enctypes in its HDB entry independently of its long-term keys. This
will reduce HDB bloat by not requiring that service principals have
more long-term keys than they need just to document the service's
supported enctypes.
- Adds support for storing krb5.conf content in principals' HDB
entries. This may eventually be used for causing Heimdal KDC
services to reconfigure primary/secondary roles automatically by
discovering the configured primary in an HDB entry for the realm.
For now this will be used to help reduce the amount of configuration
needed by clients of an upcoming HTTP binding of the kadmin service.
This avoids these compiler warnings on Ubuntu 18.04
gcc version 7.5.0 (Ubuntu 7.5.0-3ubuntu1~18.04)
expand_path.c: In function ‘expand_token’:
expand_path.c:493:17: warning: ignoring return value of ‘asprintf’, declared with attribute warn_unused_result [-Wunused-result]
asprintf(&arg, "%.*s", (int)(token_end - colon - 1), colon + 1);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
log.c: In function ‘fmtkv’:
log.c:646:5: warning: ignoring return value of ‘vasprintf’, declared with attribute warn_unused_result [-Wunused-result]
vasprintf(&buf1, fmt, ap);
^~~~~~~~~~~~~~~~~~~~~~~~~
mech/context.c: In function ‘gss_mg_set_error_string’:
mech/context.c:212:5: warning: ignoring return value of ‘vasprintf’, declared with attribute warn_unused_result [-Wunused-result]
(void) vasprintf(&str, fmt, ap);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
mech/context.c: In function ‘_gss_mg_log_name’:
mech/context.c:319:6: warning: ignoring return value of ‘vasprintf’, declared with attribute warn_unused_result [-Wunused-result]
(void) vasprintf(&str, fmt, ap);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
mech/context.c: In function ‘_gss_mg_log_cred’:
mech/context.c:346:5: warning: ignoring return value of ‘vasprintf’, declared with attribute warn_unused_result [-Wunused-result]
(void) vasprintf(&str, fmt, ap);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
kerberos5.c: In function ‘_kdc_set_e_text’:
kerberos5.c:338:5: warning: ignoring return value of ‘vasprintf’, declared with attribute warn_unused_result [-Wunused-result]
vasprintf(&e_text, fmt, ap);
^~~~~~~~~~~~~~~~~~~~~~~~~~~
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
bcb5ed28fb ("kdc/kx509.c: clean up
krb5_get_error_message strings used for mk_error_resp") introduced a
build failure. 'context' is not a local variable but a field in the
kx509_req_context.
Change-Id: I38a8a9b1a19c3370dfc039d6e09445425936c32b
mk_error_response expects the strings it builds will need to be freed but
not those passed in; krb5_get_error_message generates a string which needs to
be freed. keep track of and free those strings.
on error from der_parse_heim_old we early exit via out, which frees
s and cprinc; we should not do so a second time unless we explicitly
NULL the pointers but there is no value in doing so.
This is the second of two commits in a series that must be picked together.
This series of two commits moves parts of lib/krb5/ infrastructure
functionality to lib/base/, leaving behind wrappers.
Some parts of libkrb5 are entirely generic or easily made so, and could
be useful in various parts of Heimdal that are not specific to the krb5
API, such as:
- lib/gssapi/ (especially since the integration of NegoEx)
- lib/hx509/
- bx509d (which should really move out of kdc/)
For the above we need to move these bits of lib/krb5/:
- lib/krb5/config_file.c (all of it, leaving forwardings behind)
- lib/krb5/config_reg.c (all of it)
- lib/krb5/plugin.c (all of it, leaving forwardings behind)
- lib/krb5/log.c (all of it, ditto)
- lib/krb5/heim_err.et (all of it)
And because of those two, these too must also move:
- lib/krb5/expand_path.c (all of it, leaving forwardings behind)
- lib/krb5/warn.c (just the warning functions, ditto)
The changes to the moved files are mostly quite straightforward and are
best reviewed with --word-diff=color.
We're also creating a heim_context and a heim API to go with it. But
it's as thin as possible, with as little state as necessary to enable
this move. Functions for dealing with error messages use callbacks.
Moving plugin.c does have one knock-on effect on all users of the old
krb5 plugin API (which remains), which is that a global search and
replace of struct krb5_plugin_data to struct heim_plugin_data was
needed, though the layout and size of that structure doesn't change, so
the ABI doesn't either.
As well, we now build lib/vers/ and lib/com_err/ before lib/base/ so as
to be able to move lib/krb5/heim_err.et to lib/base/ so that we can make
use of HEIM_ERR_* in lib/base/, specifically in the files that moved.
Once this is all done we'll be able to use config files and plugins in
lib/hx509/, we'll be able to move bx509d out of kdc/, and so on.
Most if not all of the new functions in lib/base/ are Heimdal-private,
thus calling conventions for them are not declared.
Status:
- builds and passes CIs (Travis, Appveyor)
- ran make check-valgrind and no new leaks or other memory errors
- ready for review
HOW TO REVIEW:
$ # Review file moves:
$ git log --stat -n1 HEAD^
$
$ # Review changes to moved files using --word-diff=color
$ git log -p -b -w --word-diff=color HEAD^..HEAD \
lib/base/config_file.c \
lib/base/config_reg.c \
lib/base/expand_path.c \
lib/base/warn.c \
lib/krb5/config_file.c \
lib/krb5/config_reg.c \
lib/krb5/expand_path.c \
lib/krb5/warn.c
$
$ # Review the whole thing, possibly adding -b and/or -w, and
$ # maybe --word-diff=color:
$ git log -p origin/master..HEAD
$ git log -p -b -w origin/master..HEAD
$ git log -p -b -w --word-diff=color origin/master..HEAD
TBD (future commits):
- make lib/gssapi use the new heimbase functions
- move kx509/bx509d common code to lib/hx509/ or other approp. location
- move bx509d out of kdc/
A HEAD or GET of / or /health will now produce a 200 instead of a 404.
Ideally we should add configuration arguments that would allow /health
to get a token, make a CSR, and test the /bx509 (and/or /bnegotiate)
functionality, that way we'd have a real health check. For now we defer
that work, as external health monitoring can be done using a simple
script anyways.
The Negotiate token validation plugin links against libgssapi, but on macOS the
dynamic linker cannot find it before Heimdal is installed. This causes tests to
fail. Although test_token_validator itself does not require libgsaspi, link
against it so that the test can proceed.
Now we'll put the "reason=..." last in the log lines and we won't escape
spaces -- just newlines and other control characters. This makes
reading log lines much easier without complicating parsing of log lines
because interior key=value pairs do get whitespace escaped or removed.
krb5_kdc_process_request() must return 0 when it produces a reply, and only
return non-zero when it could not construct any kind of reply (e.g., ENOMEM, or
-1 if no handler claimed responsibility for the request).
We take all of the kdc_log() and _kdc_r_log() calls in AS and TGS
and move their log levels down to debugging on the assumption that
our new log line subsumes the "informational" requirements. We
collect some additional information in the kv-pair "pe-text" which
is like e-text except it is not returned to the client.
We refactor the code a bit to extend kdc_request_t which until now
was only used for the AS. We make the structure extensible and
start using it for the TGS as well. We leave digest and kx509
alone for the time being.
We also define the concept of kv-pairs in our audit trail which
allows us to define a rigorous but extensible format:
type error from-addr client server key1=val1 key2=val2 ...
