Nalin Dahyabhai [Fri, 28 Jun 2013 21:12:39 +0000 (17:12 -0400)]
Support PKINIT OpenSSL deferred identity prompting
Add a password to the set of things that we can pass to a PEM password
callback and the function we use for loading PKCS12 bundles. If we're
meant to defer identity prompts, just store the name of the identity
which we're loading. Otherwise, if we're passed a password, use it.
Otherwise, use the prompter callback.
Add a password to the set of things that we can pass to the function
that we use for logging in to PKCS11 tokens, too, but if we're deferring
identity prompts, just return the identity name without doing anything
else. If not, and we're passed a password, use that. Otherwise, try to
use the prompter callback to get one.
The password callback which we usually supply to NSS already gets a
pointer to the pkinit_identity_crypto_context structure, but it needs to
be passed the name of the identity for which it's being called.
If it gets a name, and it's deferring prompting, just add the identity
to the list of deferred identity prompts (the password callback wouldn't
have been called if its result wasn't needed), and either return NULL
(as an indication that we couldn't get a password) or an empty string (a
value which we know is invalid) if that's handier.
Otherwise, check for a password that's been stashed for its use for that
identity, and return a copy of it if one's found. If none of that
works, try to use the prompter callback to ask for the password.
Use the list of deferred identity prompts and warnings, which we have
after calling pkinit_identity_initialize(), to build a list of questions
to supply to responder callbacks.
Before calling pkinit_identity_prompt() to actually load identities that
are protected, save any passwords and PINs which a responder callback
may have supplied.
Because pkinit_client_prep_questions() can be called multiple times, and
we don't want to try to load all of our identities each of those times,
take some steps to ensure that we only call pkinit_identity_initialize()
and pkinit_identity_prompt() once per request.
Nalin Dahyabhai [Fri, 28 Jun 2013 21:12:39 +0000 (17:12 -0400)]
Add support for PKINIT deferring identity prompts
Learn to manage a list of deferred identities, for which we want to
prompt for passwords or PINs, in pkinit_identity_crypto_context
structures, along with their associated token flags. These are opaque
outside of pkinit_crypto_openssl and pkinit_crypto_nss, so both
implementations need to provide wrapper functions that can be called
from elsewhere in the module to populate and query the lists.
Nalin Dahyabhai [Fri, 28 Jun 2013 21:12:39 +0000 (17:12 -0400)]
Split pkinit_identity_initialize into two phases
Split part of pkinit_identity_initialize() into a second piece named
pkinit_identity_prompt(). Have each piece pass a new boolean flag to
crypto_load_certs() to indicate if it should defer prompting for a
password/PIN for client identities that require one. If the flag isn't
set, then crypto_load_certs() should attempt to use a responder-supplied
value, or call the prompter if there isn't one.
If cb->get_string returns 0 with no result in otp_edata, make sure we
set retval to avoid sending an empty OTP hint. If cb->get_string
returns an error code in otp_verify, avoid masking that code.
Add kadmin support for "addprinc -nokey", which creates a principal
with no keys, and "purgekeys -all", which deletes all keys from a
principal. The KDC was modified by #7630 to support principals
without keys.
When we allocate space for an array of key_data structures, make sure
we allocate at least one, so we don't spuriously fail on platforms
where malloc(0) returns NULL. Where we use malloc, use k5calloc
instead. Where we use krb5_db_alloc or realloc, just allocate an
extra entry.
When walking the cache, if we skip a cred because it's a config entry,
make sure to free it. Also free the result of krb5_cc_get_principal.
Based on a patch from Nalin Dahyabhai.
The stacks of certificates and CRLs that we retrieve from CMS objects
include newly-owned references to the certificates and CRLs, so when we
go to free them, we need to remember to free those.
[ghudson@mit.edu: minor formatting change; removed unrelated style fix]
We've never used sigwait() before, and it has some problems on Solaris
10 (a nonconformant prototype by default, and experimentally it didn't
seem to work correctly with _POSIX_PTHREAD_SEMANTICS defined). Use a
pipe instead. Make t_daemon.py less chatty on stdout to avoid filling
the pipe buffer.
Letting calloc() do multiplication helps avoid overflow bugs, so
provide an internal k5calloc() helper which accepts both calloc
arguments, and reimplement k5alloc() in terms of it.
Reorder (and trim) the imports in t_otp.py so that k5test is pulled in
before we try to import pyrad and multiprocessing. Otherwise
success() isn't defined in the case where we decide to skip the entire
test script.
This plugin implements the proposal for providing OTP support by
proxying requests to RADIUS. Details can be found inside the
provided documentation as well as on the project page.
The new library libkrad provides code for the parsing of RADIUS packets
as well as client implementation based around libverto. This library
should be considered unstable.
The previous commit updated the header file references for 1.12 in the
clpreauth and kdcpreauth plugin interface documentation. Add a
parenthetical so that the reference is still useful for prior
releases.
Tom Yu [Fri, 21 Jun 2013 21:58:25 +0000 (17:58 -0400)]
KDC null deref due to referrals [CVE-2013-1417]
An authenticated remote client can cause a KDC to crash by making a
valid TGS-REQ to a KDC serving a realm with a single-component name.
The process_tgs_req() function dereferences a null pointer because an
unusual failure condition causes a helper function to return success.
While attempting to provide cross-realm referrals for host-based
service principals, the find_referral_tgs() function could return a
TGS principal for a zero-length realm name (indicating that the
hostname in the service principal has no known realm associated with
it).
Subsequently, the find_alternate_tgs() function would attempt to
construct a path to this empty-string realm, and return success along
with a null pointer in its output parameter. This happens because
krb5_walk_realm_tree() returns a list of length one when it attempts
to construct a transit path between a single-component realm and the
empty-string realm. This list causes a loop in find_alternate_tgs()
to iterate over zero elements, resulting in the unexpected output of a
null pointer, which process_tgs_req() proceeds to dereference because
there is no error condition.
Add an error condition to find_referral_tgs() when
krb5_get_host_realm() returns an empty realm name. Also add an error
condition to find_alternate_tgs() to handle the length-one output from
krb5_walk_realm_tree().
The vulnerable configuration is not likely to arise in practice.
(Realm names that have a single component are likely to be test
realms.) Releases prior to krb5-1.11 are not vulnerable.
Ben Kaduk [Fri, 28 Jun 2013 16:08:13 +0000 (12:08 -0400)]
Update windows README for VS2012/Windows SDK 8
The Windows SDK 8 dropped the command-line build environment, so
provide instructions for building with the Visual Studio (2012)
command-line build environment.
Ben Kaduk [Wed, 26 Jun 2013 20:30:41 +0000 (16:30 -0400)]
Require Windows Installer 4.5 or higher
As advised by the VC 11 merge modules.
Windows Installer 4.5 requires Windows Server 2008, Windows Vista, Windows XP
with Service Pack 2 (SP2) and later, or Windows Server 2003 with Service
Pack 1 (SP1) and later. I believe we already enforce these restrictions
manually elsewhere, so this should not restrict our set of valid target
machines.
Greg Hudson [Fri, 14 Jun 2013 19:28:06 +0000 (15:28 -0400)]
Add tests for pwqual modules and plugin ordering
Create a test module for the pwqual interface, and script to exercise
the built-in and test modules through kadmin.local. Also create a
test harness to display the order of pwqual modules for the current
configuration, and use it to test the plugin module ordering
guarantees.
Greg Hudson [Fri, 14 Jun 2013 05:33:26 +0000 (01:33 -0400)]
Provide plugin module ordering guarantees
Rewrite the plugin internals so that modules have a well-defined
order--either the order of enable_only tags, or dynamic modules
followed by the built-in modules in order of registration.
Viktor Dukhovni [Tue, 25 Jun 2013 16:27:42 +0000 (12:27 -0400)]
Fix spin loop reading from KDC TCP socket
In the k5_sendto code for reading from a TCP socket, detect
end-of-stream when reading the length. Otherwise we can get stuck in
an infinite loop of poll() and read().
Greg Hudson [Sun, 16 Jun 2013 20:42:00 +0000 (16:42 -0400)]
Fix timing edge cases in t_renew.py
When we are testing maximum renewable lifetimes, the KDC might process
the request at a later time than the request time (typically by no
more than one second). So we need to ask for a ticket lifetime longer
than the maximum renewable lifetime, not equal to it, or we risk
getting a just-barely-renewable ticket instead of a non-renewable one.
Also fix a couple of typos in comments.
Greg Hudson [Thu, 13 Jun 2013 15:06:23 +0000 (11:06 -0400)]
Fix sentinel position in sample u2u server
Print "Server started" after calling listen(), or there wil be a race
where the client tries to connect before there is a listen queue and
gets ECONNREFUSED.
Greg Hudson [Thu, 6 Jun 2013 18:44:30 +0000 (14:44 -0400)]
Refactor KDC renewable ticket handling
Create a new helper to compute the renewable lifetime for AS and TGS
requests. This has some minor behavior differences:
* We only issue a renewable ticket if the renewable lifetime is greater
than the normal ticket lifetime.
* We give RENEWABLE precedence over RENEWABLE-OK in determining the
requested renewable lifetime, instead of sometimes doing the
reverse.
* We use the client's maximum renewable life for TGS requests if we
have looked up its DB entry.
* Instead of rejecting requests for renewable tickets (if the client
or server principal doesn't allow it, or a TGS request's TGT isn't
renewable), issue non-renewable tickets.
Greg Hudson [Sun, 2 Jun 2013 05:22:38 +0000 (01:22 -0400)]
Use KDC clock skew for AS-REQ timestamps
Calculate request timestamps each time we encode a request, and use
the adjusted current time when calculating them, including adjustments
resulting from preauth-required errors early in the AS exchange.
As a side effect, this reverts one of the changes in commit 37b0e55e21926c7875b7176e24e13005920915a6 (#7063); we will once again
use the time adjustment from any ccache we read before the AS
exchange, if we don't have a more specific adjustment from a
preauth-required error.
Greg Hudson [Sun, 2 Jun 2013 04:31:04 +0000 (00:31 -0400)]
Refactor AS-REQ nonce and timestamp handling
Create helper functions to set the request nonce and to set the
request timestamp. Don't bother picking a nonce in
restart_init_creds_loop since we will just pick a new one in
init_creds_step_request. Create a library-internal function to get
the current time with possible adjustment from a preauth-required
error. Only set ctx->request_time in one place (just before encoding
each request). Remove unused parameters from stash_as_reply.
Greg Hudson [Sun, 2 Jun 2013 19:36:40 +0000 (15:36 -0400)]
Fix spurious clock skew caused by preauth delay
Commit 37b0e55e21926c7875b7176e24e13005920915a6 (#7063) prevented
clock skew caused by preauth delay by recording the time of the
initial request. However, it failed to take into account delay
between requests due to prompting during preauthentication. Fix this
by recording the request time for each request.
Greg Hudson [Fri, 31 May 2013 16:47:34 +0000 (12:47 -0400)]
Link dbtest with libkrb5support
In a static build, linking dbtest could fail on platforms where libdb2
depends on krb5support (platforms without a native mkstemp). Reported
by Gilles Espinasse <g.esp@free.fr>.
Greg Hudson [Thu, 30 May 2013 15:39:54 +0000 (11:39 -0400)]
Properly handle use_master in k5_init_creds_get
If we make multiple requests in an initial creds exchange, the
krb5_sendto_kdc call in k5_init_creds_get may flip the use_master
value from 0 to 1 if it detects that the response was from a master
KDC. Don't turn this into a requirement for future requests during
the same exchange, or we may have trouble following AS referrals.
Reported by Sumit Bose.
Greg Hudson [Wed, 29 May 2013 05:20:19 +0000 (01:20 -0400)]
Fix non-builtin crypto module builds
Commit 4b0985f8573840838bcfa8ec1df3dcd39a3dbf15 went a bit too far in
pruning the placeholder makefiles for the openssl and nss crypto
modules. We need enough boilerplate to create OBJS.SH in each
directory.
Greg Hudson [Thu, 23 May 2013 19:33:58 +0000 (15:33 -0400)]
Make empty passwords work via init_creds APIs
In the gak_data value used by krb5_get_as_key_password, separate the
already-known password from the storage we might have allocated to put
it in, so that we no longer use an empty data buffer to determine
whether we know the password. This allows empty passwords to work via
the API.
Remove the kadm5 test which explicitly uses an empty password.
Greg Hudson [Fri, 24 May 2013 17:16:52 +0000 (13:16 -0400)]
Fix rc4 string-to-key on unterminated inputs
The internal UTF-8 to UCS-2 conversion functions did not properly
respect their length arguments, instead assuming that the input string
is terminated with a zero bytes. As a result,
krb5int_arcfour_string_to_key could fail on unterminated inputs. Fix
the underlying support functions to read their inputs only up to the
specified length.
Greg Hudson [Sun, 5 May 2013 01:52:21 +0000 (21:52 -0400)]
Add AES-NI support on Linux
If yasm and cpuid.h are present on a Linux i686 or x64 system, compile
the modified Intel AES-NI assembly sources. In the builtin AES enc
provider, check at runtime whether the CPU supports AES-NI
instructions and use the assembly functions if so.
Add assembly files from the Intel AESNI Sample Library, version 1.2,
which implement AES encryption using AES-NI instructions. Trailing
whitespace was removed.
Greg Hudson [Tue, 7 May 2013 00:23:09 +0000 (20:23 -0400)]
Refactor AES, Camellia builtin enc providers
In preparation for AES-NI support, adjust the block encryption helpers
in the AES enc provider so that the cache is only touched by helpers,
and reframe the CTS operations in terms of CBC operations. Change the
Camellia enc provider as well for consistency.
Greg Hudson [Mon, 6 May 2013 04:43:27 +0000 (00:43 -0400)]
Simplify crypto IOV helpers
Expand the concept of an IOV block state into a cursor which remembers
the IOV set being iterated over, the block size, and both input and
output positions. Eliminate the no-copy inline block getter for now,
but provide helpers to grab contiguous chains of blocks from a cursor.
Also provide an inline helper to sum the total length of an iov chain.
Greg Hudson [Wed, 22 May 2013 05:55:12 +0000 (01:55 -0400)]
Clarify krb5_rd_req documentation
For the user-to-user case, document that callers should pass a server
principal to krb5_rd_req. For the keytab case, more accurately
document which keytab keys are tried against the ticket.
Greg Hudson [Mon, 20 May 2013 15:03:04 +0000 (11:03 -0400)]
Fix transited handling for GSSAPI acceptors
The Acceptor Names project (#6855) extended krb5_rd_req so that it can
accept a "matching principal" in the server parameter. If the
matching principal has an empty realm, rd_req_decoded_opt attempted to
do transited checking with an empty server realm.
To fix this, always reset server to req->ticket->server for future
processing steps if we decrypt the ticket using a keytab.
decrypt_ticket replaces req->ticket->server with the principal name
from the keytab entry, so we know this name is correct.
Based on a bug report and patch from nalin@redhat.com.
Greg Hudson [Tue, 14 May 2013 18:17:05 +0000 (14:17 -0400)]
Try to use best DNS search functions in dnsglue.c
Create macros to declare a DNS search handle, initialize a handle,
perform a search, and destroy a handle. On OS X, use the native
dns_open, dns_search, and dns_free functions, since using the res_*
interfaces may not contact the correct servers. On systems with
res_ninit, use res_nclose if we don't have res_ndestroy.
Greg Hudson [Thu, 16 May 2013 18:21:12 +0000 (14:21 -0400)]
Reduce boilerplate in makefiles
Provide default values in pre.in for PROG_LIBPATH, PROG_RPATH,
SHLIB_DIRS, SHLIB_RDIRS, and STOBJLISTS so that they don't have to be
specified in the common case. Rename KRB5_RUN_ENV and KRB5_RUN_VARS
to RUN_SETUP (already the most commonly used name) and RUN_VARS. Make
sure to use DEFINES for local defines (not DEFS). Remove some other
unnecessary makefile content.
Greg Hudson [Fri, 10 May 2013 18:01:48 +0000 (14:01 -0400)]
Assume mutex locking cannot fail
Locking and unlocking a non-recursive mutex is a simple memory
operation and should not fail on any reasonable platform with correct
usage. A pthread mutex can return EDEADLK on lock or EPERM on unlock,
or EINVAL if the mutex is uninitialized, but all of these conditions
would reflect serious bugs in the calling code.
Change the k5_mutex_lock and k5_mutex_unlock wrappers to return void
and adjust all call sites. Propagate this change through
k5_cc_mutex_lock and k5_cc_mutex_unlock as well.
Greg Hudson [Tue, 14 May 2013 02:59:35 +0000 (22:59 -0400)]
Rename internal Camellia symbols
Symbols from the NTT Camellia sources, used in the builtin crypto
provider, could conflict with symbols from other libraries such as
OpenSSL's libcrypto. Rename those like we rename the Gladman AES
symbols.
Tom Yu [Fri, 3 May 2013 20:26:46 +0000 (16:26 -0400)]
Fix kpasswd UDP ping-pong [CVE-2002-2443]
The kpasswd service provided by kadmind was vulnerable to a UDP
"ping-pong" attack [CVE-2002-2443]. Don't respond to packets unless
they pass some basic validation, and don't respond to our own error
packets.
Some authors use CVE-1999-0103 to refer to the kpasswd UDP ping-pong
attack or UDP ping-pong attacks in general, but there is discussion
leading toward narrowing the definition of CVE-1999-0103 to the echo,
chargen, or other similar built-in inetd services.
Thanks to Vincent Danen for alerting us to this issue.
Some fixes, some use of different APIs which seem to clean things up
better, with the goal of being able to cleanly shut down NSS when we're
done using it.
* Use PK11_FreeSlot() instead of SECMOD_CloseUserDB() to close a
database opened with SECMOD_OpenUserDB().
* Fix a typo and use PK11_DestroyGenericObject() instead of
PK11_DestroyGenericObjects() to destroy one object.
* Use SECMOD_DestroyModule() instead of SECMOD_UnloadUserModule()
to close a module loaded with SECMOD_LoadUserModule().
* crypto_check_for_revocation_information(): don't leak a reference
to the CRL, or to intermediate issuers.
* Don't leak a reference to a PEM private key.
When the PEM module is given an encrypted key, it changes its token
flags to indicate that a password is required (by setting needs-login)
to signal the application that we need to supply a password to decrypt
it. Attempts to load any other items will fail until the flag is
cleared.
If we detect that the flag is set after we've attempted to load a
private key, attempt to "log in" to the "token" using a password. Even
if we fail, the token will reset its needs-login flag, which is
necessary before we can import anything else.
Get better at locating the just-loaded certificate
When loading certificates using the PEM module, use a better method for
finding the just-loaded certificate that will still work if we've
already got a copy of the certificate loaded somewhere else.
In PKINIT NSS crypto code, load certificates first
When using NSS's CMS API to generate signed-data messages, we identify
the key that we want to use for signing by specifying a certificate.
The library then looks up the corresponding private key when it needs to
generate the signature. This lookup fails if a certificate and a its
corresponding private key were loaded key-first, but succeeds if they
were loaded certificate-first (RHBZ#859535). To work around this,
switch to loading the certificate first. (We switch to using different
_pkinit_identity_crypto_file pointers for each instead of reusing just
one, so the diff is messier than it might have been.)
When DEBUG=1, log why we fail to log in to a token
Use PORT_ErrorToName() to let us print an error name instead of an error
code in a couple of debug messages, since in practice we just end up
looking up the code in <secerr.h> anyway.
Nalin Dahyabhai [Thu, 10 Jan 2013 20:39:15 +0000 (15:39 -0500)]
Traverse tokens like we do with OpenSSL for NSS
When PKINIT is built with NSS, change how it traverses tokens to match
the way it's done when built using OpenSSL: ignore slot names (we used
to treat the token label as a possible slot label, too), and either only
look at the token with the specified label, or the first token if a no
token label was specified.
Nalin Dahyabhai [Tue, 18 Dec 2012 22:41:11 +0000 (17:41 -0500)]
Make the text of NSS's prompts look like OpenSSL's
When PKINIT is built with NSS, make the text of prompts that we issue to
the user better match the text we use when we build with OpenSSL: ask
for a pass phrase when we're asking about a hardware token, ask for a
password the rest of the time, and take advantage of translations for
requests for a password.
Nalin Dahyabhai [Mon, 14 Jan 2013 18:57:54 +0000 (13:57 -0500)]
Don't fail if a candidate certificate has no SANs
When we're doing certificate matching and we're asked for the list of
SAN values for a certifiate, and it contains none, don't return an
error, as that will eventually cause the module to just return an error.
Instead, just return an empty list of SAN values so that processing will
continue on to check if other certificates match.
Nalin Dahyabhai [Tue, 26 Feb 2013 22:59:01 +0000 (17:59 -0500)]
Make reassembled PKCS11 names parseable
The reassembled names used "," as a separator between attributes, when
passed-in values use ":". This was due to the original submitter being
confused - they weren't intended to be different.
Fix a typo that caused us to not skip removing "."
When attempting to clean the files out from our temporary directory,
correct the test which was supposed to let us skip over "." and ".." so
that we actually don't try to remove them with remove().
The test, as submitted, included a copy/paste error which caused it to
test PKINIT using unencrypted PKCS12 bundles twice, and to not test a
DIR: location containing unencrypted PEM-formatted keys at all.
Encrypted timestamp and encrypted challenge cannot succeed if the
client has no long-term key matching the request enctypes, so do not
offer them in that case.
Add a new have_client_keys callback to the kdcpreauth interface,
allowing modules to efficiently check whether the client DB entry has
any keys matching the request enctypes.