Commit Graph

19 Commits

Author SHA1 Message Date
Luke Howard
e15e711b13 kdc: remove auth_event_details audit key
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.
2022-01-02 15:25:52 +11:00
Luke Howard
cc0874d410 kdc: preserve value types in auditing
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.
2022-01-02 15:25:52 +11:00
Luke Howard
b1dcc1a474 kdc: refactor Samba-specific auditing API in terms of existing API
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.
2022-01-02 15:25:52 +11:00
Nicolas Williams
5e3c0a7ce0 base: Add ret to common svc req elements
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.
2021-12-27 10:16:58 +11:00
Nicolas Williams
5f63215d0d Always perform == or != operation on cmp function result
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
2021-11-24 22:30:44 -05:00
Nicolas Williams
341848a27b base: Fix leak on ENOMEM 2021-10-11 13:58:15 -05:00
Luke Howard
ab3d52e1a2 base: use correct calling convention for log_file()
Use the correct HEIM_CALLCONV calling convention when setting log_file()
callback, otherwise Windows build fails.
2021-08-11 19:09:08 +10:00
Nicolas Williams
e479695c61 base: Make heim_audit_trail() safer when ENOMEM 2020-09-07 22:04:59 -05:00
Nicolas Williams
5e078cdf6e base: Fix logging bug 2020-09-07 22:04:59 -05:00
Nicolas Williams
4772674e57 base: Make *_log_msg() use contextual log dest 2020-09-07 22:04:59 -05:00
Nicolas Williams
e2264e8374 base: Make log facility opaque, ref-counted 2020-09-07 22:04:59 -05:00
Nicolas Williams
7208217410 base: Make log reopen option thread-safe 2020-09-07 22:04:59 -05:00
Andrew Bartlett
1a65611f61 Check some error returns from *asprintf()
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>
2020-05-27 21:54:26 -04:00
Jeffrey Altman
3074561796 lib/base: introduce HEIM_BASE_USE_PATH_TOKENS
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
2020-05-26 11:48:45 -05:00
Jeffrey Altman
afc9ebe08b fix calling conventions
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
2020-05-26 11:48:45 -05:00
Nicolas Williams
01509f553d Move KDC audit functionality to lib/base/ 2020-04-24 16:02:35 -05:00
Daria Phoebe Brashear
c295233648 lib/base log_file should free filename in all cases
once we've used the filename we built, free it before exit
2020-03-06 11:56:03 -06:00
Nicolas Williams
ea90ca8666 Move some infra bits of lib/krb5/ to lib/base/ (2)
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/
2020-03-02 10:56:13 -06:00
Nicolas Williams
b2823cbd74 Move some infra bits of lib/krb5/ to lib/base/ (1)
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/.
2020-03-02 10:56:13 -06:00