Now heim_audit_addkv() correctly supports multiple values, the other functions
that (by design) replace existing values with a single value should use the
heim_audit_setkv prefix.
A subsequent commit could add variants that support multiple values for
non-string types.
Previously, if heim_audit_vaddkv() was called multiple times, values would be
concatenated; since changing the request kv type to a dictionary, only the most
recently set value was preserved.
Fix this by promoting multi-valued values to an array of values, which is
visualized in the same manner as it was prior to b1dcc1a4.
The auth event details audit key (formerly, parameter to auth_status)
contained, variously, an encryption type name; a PKINIT client certificate
name; or, a GSS initiator name. Audit these instead using individual keys that
reflect the values' contents.
Preserve integer/boolean audit values as their native types; convert to strings
when logging only. This commit goes some way towards unifying the two auditing
APIs.
Make Samba-specific HDB auth status API a wrapper on the existing auditing API,
with a view towards unifying the two APIs in a future commit.
The term "auth status" is replaced with "auth event", and the HDB auth_status
method is replaced with a more general purpose audit method which has access to
the entire request structure.
We're logging SUCCESS even when the KDC sends error replies. That's
because we're returning success to process_request() even when we send
errors to clients. The error we want to send to the client, and that we
succeed or fail to send it, are different statuses.
To fix this we'll add a `ret` field to the common service request state
structure, HEIM_SVC_REQUEST_DESC_COMMON_ELEMENTS /
heim_svc_req_desc_common_s.
Although not required to address bad code generation in
some versions of gcc 9 and 10, a coding style that requires
explicit comparison of the result to zero before use is
both clearer and would have avoided the generation of bad
code.
This change converts all use of cmp function usage from
```
if (strcmp(a, b) || !strcmp(c, d)) ...
```
to
```
if (strcmp(a, b) != 0 || strcmp(c, d)) == 0
```
for all C library cmp functions and related:
- strcmp(), strncmp()
- strcasecmp(), strncasecmp()
- stricmp(), strnicmp()
- memcmp()
Change-Id: Ic60c15e1e3a07e4faaf10648eefe3adae2543188
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>
KRB5_USE_PATH_TOKENS cannot be used within lib/base as its value
is declared in lib/krb5/krb5.h. Declare HEIM_BASE_USE_PATH_TOKENS
in lib/base/baselocl.h and test for it in
heim_config_parse_file_multi().
By conditionalizing heim_config_parse_file_multi() behavior on
KRB5_USE_PATH_TOKENS heim_expand_path_tokens() is not executed
and open() is called on a path without token substitution. As a
result open() always fails with ENOENT.
Change-Id: I29dc018bc560519b76314232b2d51f53bde6313c
When a function is assigned to a function pointer that is declared
with a particular calling convention, then the assigned function
must be declared with that calling convention as well. Otherwise,
kaboom!!!
The following functions are fixed by this change:
kuser/kx509.c
validate1()
add1_2chain()
lib/base/log.c
log_syslog()
close_syslog()
log_file()
close_file()
lib/gssapi/mech/context.c
gss_set_log_function()
lib/krb5/kx509.c
certs_export_func()
Change-Id: Ib68abf739e3385e98136fa4e4f5a0240e9fce033
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/
This is the first 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.
This commit only renames files to enable git log/diff/blame to follow
the renames: to help future code archeology, and to make reviewing these
two commits easier.
The next commit in this series ensures that the moved files have the
correct content (i.e., defining heim APIs instead of krb5 APIs), and
will create files in lib/krb5 with the same names and krb5 API wrappers
around the new heim API functions.
The next commit also explains the motivation, which, briefly, is to:
- remove krb5 API usage from lib/gssapi/,
- enable the use of configuration and plugins in lib/hx509/
(as well as lib/gssapi/ and future projects),
and
- enable the further disentanglement of bx509d from kdc/.
