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 ...
We define the meaning of the various log levels in the man page
for krb5_openlog(3). If logging configured and levels are not
specified, we change the default levels to 0-3 which should exclude
debugging messages which are generally only desired in exceptional
circumstances.
We also go through the KDC and adjust the levels to be appropriate.
This commit adds support for proof of posession to the kx509 protocol by
using PKCS#10 CSRs.
This allows conveyance of extReq CSR attributes requesting desired
Certificate Extensions.
This commit adds support for kx509 in libkrb5, and revamps the KDC's
kx509 service (fixing bugs, adding features).
Of note is that kx509 is attempted optimistically by the client, with
the certificate and private key stored in the ccache, and optionally in
an external PEM or DER file.
NOTE: We do not optimistically use kx509 in krb5_cc_store_cred() if the
ccache is a MEMORY ccache so we don't generate a key when
accepting a GSS context with a delegated credential.
kx509 protocol issues to be fixed in an upcoming commit:
- no proof of possession (this is mostly not too bad, but we'll want to
fix it by using CSRs)
- no algorithm agility (only plain RSA is supported)
- very limited (no way to request any options in regards to the
requested cert)
- error codes are not very useful
Things we're adding in this commit:
- libkrb5 kx509 client
- automatic kx509 usage hooked in via krb5_cc_store_cred() of start TGT
- per-realm templates on the KDC side
- per-realm issuer certificates
- send error messages on the KDC side
(this is essential to avoid client-side timeouts on error)
- authenticate as many error messages
- add a protocol probe feature so we can avoid generating a
keypair if the service is not enabled
(once we add support for ECC algorithms we won't need this
anymore; the issue is that RSA keygen is slow)
- support for different types of client principals, not just username:
- host-based service and domain-based service, each with its own
template set per-{realm, service} or per-service
(the idea is to support issuance of server certificates too, not
just client/user certs)
- more complete support for SAN types
- tests (including that PKINIT->kx509->PKINIT works, which makes it
possible to have "delegation" of PKIX credentials by just delegating
Kerberos credentials)
- document the protocol in lib/krb5/kx509.c
Future work:
- add option for longer-ticket-lifetime service certs
- add support for ECDSA, and some day for ed25519 and ed448
- reuse private key when running kinit
(this will require rethinking how we trigger optimistic kx509
usage)
- HDB lookup for:
- optional revocation check (not strictly necessary)
- adding to certificates those SANs listed in HDB
- hostname aliases (dNSName SANs)
- rfc822Name (email)
- XMPP SANs
- id-pkinit-san (a user could have aliases too)
- support username wild-card A RRs, ala OSKT/krb5_admin
i.e., if a host/f.q.d.n principal asks for a certificate for
some service at some-label.f.q.d.n, then issue it
(this is not needed at OSKT sites because OSKT already
supports keying such service principals, which means kx509
will issue certificates for them, however, it would be nice
to be able to have this independent of OSKT)
(a better way to do this would be to integrate more of OSKT
into Heimdal proper)
- a kx509 command, or heimtools kx509 subcommand for explicitly
attempting use of the kx509 protocol (as opposed to implicit, as is
done in kinit via krb5_cc_store_cred() magic right now)
Issues:
- optimistically trying kx509 on start realm TGT store -> timeout issues!
- newer KDCs will return errors because of this commit; older ones
will not, which causes timouts
- need a separate timeout setting for kx509 for optimistic case
- need a [realm] config item and DNS SRV RR lookup for whether a
realm is expected to support kx509 service
In order to support certain use cases, we implement a mechanism to
allow wildcard principals to be defined and for the KDC to issue
tickets for said principals by deriving a key for them from a
cluster master entry in the HDB.
The way that this works is we defined an entry of the form:
WELLKNOWN/DERIVED-KEY/KRB5-CRYPTO-PRFPLUS/<hostname>@REALM
When reading from the Kerberos DB, if we can't find an entry for
what looks like a hostbased principal, then we will attempt to
search for a principal of the above form chopping name components
off the front as we search.
If we find an entry, then we derive keys for it by using
krb5_crypto_prfplus() with the entry's key and the principal name
of the request.
* Anonymous pkinit responses from the KDC where the name
type is not well-known (as issued by 7.5 KDCs and earlier)
are accepted by the client. There is no need for the client
to strictly enforce the name type.
* With historical_anon_pkinit = true, the kinit(1) client's
"--anonymous" option only performs anon pkinit, and does
not require an '@' prefix for the realm argument.
* With historical_anon_realm = true, the KDC issues anon
pkinit tickets with the legacy pre-7.0 "real" realm.
