Joe Orton [Tue, 25 Nov 2003 13:10:09 +0000 (13:10 +0000)]
* modules/ssl/ssl_engine_vars.c (ssl_var_lookup): Only call
ssl_var_lookup_ssl for a real SSL connection; fix lookup of "HTTPS"
for non-SSL connections.
(ssl_var_log_handler_x): Give results for non-SSL connections too;
e.g. %{HTTPS}x does the right thing.
Joe Orton [Tue, 25 Nov 2003 12:35:45 +0000 (12:35 +0000)]
* modules/ssl/ssl_engine_init.c (ssl_init_proxy_certs): Fail early
(rather than segfault later) if a client cert is configured which is
missing either the certificate or private key.
Simplify includes - we always (in HTTPD 2.1 forward) are looking
for the openssl/foo.h headers explicitly. Fix the abs.dsp build
to define HAVE_OPENSSL instead of USE_SSL so the correct headers
are included upfront.
Cliff Woolley [Tue, 12 Aug 2003 20:58:53 +0000 (20:58 +0000)]
Make mod_ssl consistent with itself when you have a halfass install of
openssl-engine (ie, you're missing the headers). ssl_cmd_SSLCryptoDevice()
is thrown away by the preprocessor if you're missing the header, so the
call to it should have the same condition applied. otherwise, mod_ssl
will fail to link.
Sander Striker [Thu, 7 Aug 2003 23:57:11 +0000 (23:57 +0000)]
Add an error msg when encountering a spoofed identity. If this would
have been here in the first place. Makes issues like these be found
easier in the future.
The fix is to make mod_ssl's check_user_id hook stop tripping
over it's own checks in case of a subrequest. That is, it
should DECLINE in case of a subrequest.
Although we initialize mc->pid in the child init phase,
we haven't initialized it before initially performing
our ssl_rand_seed() in the parent/postconfig phase.
Joe Orton [Mon, 21 Jul 2003 12:02:40 +0000 (12:02 +0000)]
Prevent segfaults after SSL renegotiation failures.
* modules/ssl/ssl_engine_kernel.c (ssl_hook_Access): Set aborted flag
after renegotiation failure.
* modules/ssl/ssl_engine_io.c (ssl_filter_write, ssl_io_filter_output):
Don't dereference BIOs in filter_ctx when filter_ctx->pssl is NULL.
(ssl_filter_io_shutdown): Set aborted flag on abortive shutdown.
PR: 21370
Submitted by: Hartmut Keil <Hartmut.Keil@adnovum.ch>
Cleaned up by: Jeff Trawick, Joe Orton
SECURITY [CAN-2003-0192]: Fixed a bug whereby certain sequences
of per-directory renegotiations and the SSLCipherSuite directive
being used to upgrade from a weak ciphersuite to a strong one
could result in the weak ciphersuite being used in place of the
strong one. [Ben Laurie]
Reaction to Jeff Trawick's observations that we are double-initializing
dynalinked OpenSSL Engines and Configs. Move the library teardown code
so that it is torn down in the proper order, corresponding to when the
library itself was initialized. And leave a little reminder that some
memory diagnostics would be good if OpenSSL is built for malloc debugging.
Jeff Trawick [Sat, 7 Jun 2003 19:50:01 +0000 (19:50 +0000)]
Unix: Handle permissions settings for flock-based mutexes in
unixd_set_global|proc_mutex_perms(). Allow the functions to be
called for any type of mutex.
This resolves a fatal problem with mod_rewrite on systems where
APR uses flock-based mutex.
It simplifies mod_ssl as well, which had special logic to perform
the chown(). It fixed an init error with mod_ssl on systems where
flock is used when the user had no SSLMutex directive.
The Unix MPMs continue to call unixd_set_global|proc_mutex_perms()
only for SysV sems. There is no permission problem with flock-based
accept mutexes since the child init logic for the MPMs is done
prior to switching identity.
The right patch (thanks to Eric for identifying the wrong patch) to move
SSL_library_init() into the register hooks phase. OpenSSL_add_ssl_algorithms
devolves to SSL_library_init, which is the same for most toolkits (and would
be accomodated in ssl_toolkit_config.h if not.)
OpenSSL_add_all_algorithms is simply an alias for SSL_load_library.
Note that the entire schema of what-we-load-how follows from
OpenSSL 0.9.7's own apps/ example applications. More review
is greatly desired, but that's where I believed I should
start looking for the 'correct' order of operations.
Provide a far more useful explanation when SSLCryptoDevice fails to
find a device. Still would be nice to implement dynamic:{options}
but this gets us to display the usual, builtin devices.
We now load builtin engines up front, in the pre_config phase, because
this and any other config cmd processor must have an already valid
library config. So loading builtin engines becomes redundant in this
cmd handler.
Solve a pretty horrific bug in SSLCryptoDevice and other places where
the config cmd processors should be examining the SSL context. We must
initialize the SSL library before we can actually obtain any useful
information from the SSL library.
Based on list discussion between myself and Geoff, it seems prudent
to check for both the existence of the openssl/engine.h header file
and some 'expected function' such as ENGINE_init() (better suggestions
are welcome.) Also clear up some confusion; so long as we have
ENGINE_load_builtin_engines() we should attempt to preload those.
This patch protects all ENGINE-based code within the tests for the
engine header and function, and changes a version test into a
function test.
The patch below reverts the prior commit to eliminate SSL_set_state().
Some additional work or research is required in order to pass the
perl-framework regressions, but I don't have the cycles and don't
care to leave the broken code in cvs HEAD.
REVERTING: wrowe 2003/05/19 08:13:19
Modified: modules/ssl config.m4 ssl_engine_io.c ssl_engine_kernel.c
ssl_toolkit_compat.h
Log:
Drop SSL_set_state() in favor of a proper SSL_renegotiate() to begin
rehandshaking the SSL connection, vis-a-vis ApacheSSL.
Assure that we block on the read BIO when we invoke the read BIO for both
first-use cases (via ssl_io_input_add_filter) and when we are writing and
need response from the client (via ssl_io_filter_output). Both of these
cases are always blocking. [
PR: 19242
Submitted by: David Deaves <David.Deaves@dd.id.au>, William Rowe
Solve SSL-C breakage introduced in mod_ssl.h rev 1.129 and
ssl_engine_kernel.c rev 1.88. SSL* is not const under SSL-C.
I've confirmed Jeff's comment that the original patch doesn't harm
earlier OpenSSL versions which declared no arguments at all.
I suspect now that we could fold
#define MODSSL_BIO_CB_ARG_TYPE const char
#define MODSSL_CRYPTO_CB_ARG_TYPE const char
#define MODSSL_INFO_CB_ARG_TYPE const SSL*
into a single MODSSL_CB_ARG_CONST define, but this works for now.
Reapply the fix *intended* by rev 1.79 in a safer manner. Prior to
all assignments and the final SSL_free(), free ssl_conn->client_cert
to avoid leaks of this refcounted X509*. Prereleasing refcounted
objects is unsafe programming; fix applied to both branches.
EVP_PKEY_free() is refcounted on OpenSSL, but NOT under RSA SSL-C.
Eliminate a number of test failures by conditionally reverting rev 1.79
pubkey handling in ssl_engine_kernel.c, except under OpenSSL.
Also revert a rev 1.79 bogisity for all toolkits; it's entirely bogus
to release a refcount after setting aside the results in a persistant
structure, in this case sslconn->client_cert from SSL_get_peer_certificate()
mustn't be freed while sslconn is still in play. The proper patch (not
written yet) is to invoke the X509_free(sslconn->client_cert) when we
cleanup the sslconn structure.
A cosmetic change to 1.79 - a real X509 *cert is in play, don't use
that same variable to retrieve/release the quick lookup and discard
of the peercert.
Jeff Trawick [Fri, 4 Apr 2003 03:57:10 +0000 (03:57 +0000)]
Fix a compile failure with recent OpenSSL and picky compilers
(e.g., OpenSSL 0.9.7a and xlc_r on AIX).
The OpenSSL info callback field changed recently from a generic
function pointer to a specific one, and ssl_callback_LogTracingState
wasn't quite right.
Introduce a number of SSLC hints to mod_ssl, including the following
type overrides;
MODSSL_CLIENT_CERT_CB_ARG_TYPE
MODSSL_PCHAR_CAST (for a host of non-void/const sslc values)
modssl_read_bio_cb_fn (for several callbacks with same prototypes)
Declare callback functions appropriately.
And protect us from indetermineant toolkits with
#error "Unrecognized SSL Toolkit!"
Jim Jagielski [Mon, 31 Mar 2003 14:38:51 +0000 (14:38 +0000)]
Match what we do with the ssl_scache_dbm
chown junk, which we know is safe and works, and more directly
handles the issue with chown (agreed that a macro is needed
eventually)
Fix mod_ssl.dsp and abs.dsp to use also the openssl-0.9.7-defines for
NO_MD5, NO_IDEA and NO_MDC2 (won't compile otherwise with 0.9.7+ and
restricted crypto algorithms)
Jim Jagielski [Sat, 29 Mar 2003 02:18:43 +0000 (02:18 +0000)]
Because SSL's child init is run *after* we change uid/gid. So we need to ensure that file-based
locks have the correct perms so that the child process
can access them
Fix a serious bug where the 'next' generation of the server would open
a brand new mutex. This patch creates a single mutex in the first config
phase that survives for the life of the server (server->process->pool).
Now one server generation to the next will respect the same mutex between
one another, while the previous generation is still mopping up.
Allow any mutex to accept a 'filename' ... and always root it to the
server root unless we are using posixsem, which can't handle big paths.
This reorganization should make the code much more readable because
all of the common code is at the beginning and end of the function,
simplifing the long conditional test case block.
This patch allows SSLMutex default:logs/ssl_mutex syntax. It also
removes the mod_ssl historical '.pid' suffixes - that isn't how Apache2
specifies files.
Fix PR 17864, and also fixes a SEGV problem when SHMHT was used.
The porting of the code from mod_ssl 1.3.x was still incomplete, and depended
upon a complete implentation of apr_shm (hence pieces of code was #if 0'ed out).
After discussions at length on dev@apr/httpd, it is determined that
the older .dbg format symbols are not worth the interference with
generating complete .pdb symbolic debugging databases.
This patch further eliminates pdbtype:sept flags that interfere with
deciphering local symbols and type information.
Jim Jagielski [Sun, 23 Feb 2003 17:12:43 +0000 (17:12 +0000)]
Right now SSLMutex is bogus. It just uses APR_LOCK_DEFAULT no
matter what. We now allow for the full range of APR mutex
locking mechanims to be used, while maintaining backwards
compatibility.
PR: 8122
Obtained from:
Submitted by:
Reviewed by: William Rowe
After consultations on the APR list, it was decided that /map files are
fairly redundant when you retain rich .pdb debugging symbol files. We
have rarely used them, and generally .dbg and .pdb files prove much more
useful for the cases we have.
While eliminating /map files, we are also shrinking the size of the .dbg
files by stripping 'private' symbol information. Really this means less
rich diagnostics from Dr. Watson on NT or Win9x when they query the .dbg
symbols in creating a DrWatson log file. But it's more than compensated
for on newer OS'es where Dr. Watson will query the .pdb symbols, on all
Win32 flavors when WinDbg is used with the .pdb symbols, and the fact that
the distribution of binary symbols will use less bandwidth when less
information is duplicated from the .pdb format into the .dbg files.
foo.dbgmark turned out to be the same 8.3 name as foo.dbg itself, which
was badness. Twist this puppy to .dbr, the only name I could invent that
doesn't look like any database file extension I recall.
*) Introduce debugging symbols for Win32 release builds, both .pdb
and .dbg files (older debuggers and Dr. Watson-type utilities
on WinNT or Win9x don't support the newer .pdb flavor.)
[Allen Edwards, William Rowe]
Catch up with the changes to apr/build/win32ver.awk and name all loadable
httpd modules as .so, internally. Credit to Mladen Turk for identifing
the issue.
After introducing tests in the cmds, we lose the absolute authority
of the CRYPTO_malloc_init() which must happen the moment we load the
module and prior to *any* ssl library fn invocation.
Moved the CRYPTO_malloc_init() into the ssl_register_hooks() function,
the absolute first call made into any loaded module.
After some productive feedback and no negative feedback, introduce
SSLEngine upgrade so that we can begin and continue to support these
facilities. This makes it simpler to keep this effort (while we have
no known clients that support Connection: upgrade at this time), and
begin refactoring more of SSL into smaller and tighter (and then optional)
components.
After some productive feedback and no negative feedback, introduce
SSLEngine upgrade so that we can begin and continue to support these
facilities. This makes it simpler to keep this effort (while we have
no known clients that support Connection: upgrade at this time), and
begin refactoring more of SSL into smaller and tighter (and then optional)
components.
Submitted by: Ryan Bloom
Reviewed by: William Rowe, Joe Orton
Rule one of winsock and other one-offs (even unix EINTR) ... blocking
isn't necessarily blocking. Should not have changed this in the prior
commit, and adding the same retry to the -1/EAGAIN|EINTR case.
errno? EINTR? what planet was this code on :-? Normalize the
ssl_io_filter_connect code to follow the filter read and write.
Notice that it's buck ugly, but we will extract an rc first from
the input BIO if it was written, and then try the output bio if
it was APR_SUCCESS, during _connect processing.
Merge the last of the 'filtering' functions into ssl_engine_io.c, merge
ssl_abort into what was ssl_hook_CloseConnection, clean out a bunch of
now-static or private headers from mod_ssl.h, and final fix a very small
but potent segfault if ->pssl is destroyed within our read loop.
Actually, the APR_ECONNABORTED (EOS-only brigade) is the direction we
are contemplating for the next release, not the prior behavior
(which was APR_SUCCESS for c->aborted.)