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.)
With a last little bit of help from Justin, this should cause the
appropriate amount of tumolt and turmoil if our client has 'gone away'
on us, sparing us of further processing (and potential 'renegotiations'
with a non-existant client.)
Appears we forgot to check the possibility of errors coming from the
write brigade passed down from the content generator through the body
and protocol filters.
Rename the many flavors of filter_ctx (pRec, fctx etc) to filter_ctx,
wbio to bio_out, BIO new and free to create and destroy (to match OpenSSL),
refactor the bio write code to stash errors in outctx->rc,
fix the blocking read at EOF if we have some data to return,
and preempt the nonblock read on GETLINE if we have the newline already.
I don't see how *len > wanted, but leave the check there.
Stick a comment in there as a 'Waldo was here' so that if I ever see this
again, I realize that I've actually thought about it and didn't think >
was necessary.
Suggestions by Justin, implemention by Will. Rename away all bogisity,
especially eliminating all of 'our' capitalized identifiers that were
easily confused with library symbols; go with APR_STATUS_IS_EOF() just
in case there is a platform result; fix a bogus *len = 0; reassignment
and fold the two flavors of input context tracking into one.
Jeff Trawick [Fri, 25 Oct 2002 21:44:28 +0000 (21:44 +0000)]
mod_ssl uses free() inappropriately in several places, to free
memory which has been previously allocated inside OpenSSL.
Such memory should be freed with OPENSSL_free(), not with free().
Jim Jagielski [Tue, 22 Oct 2002 23:18:14 +0000 (23:18 +0000)]
2 silly bugs. First of all, make the code match the error log
(and allow 8192 to be valid). Secondly, this missplaced else
made the size part (8192) non-optional for shm:
PR:
Obtained from:
Submitted by:
Reviewed by:
Ryan Bloom [Fri, 11 Oct 2002 15:29:22 +0000 (15:29 +0000)]
Fix a compile of compiler warnings. I don't know how these slipped past.
Also, uncomment a line of code that the last commit should have uncommented.
Randall found this line and the fix, but I forgot to uncomment this line
along with the fix.
Ryan Bloom [Mon, 30 Sep 2002 23:43:18 +0000 (23:43 +0000)]
Call out the success cases when we don't get APR_SUCCESS back from the
network write. All other status codes result in c->aborted being set,
which allows the logs to note that the connection was aborted. Previous
to this patch, if the network cable was unplugged on the client, the server
would get APR_ETIMEUP, but we wouldn't note that the connection was
aborted.
Add a filter_init function to the filters so that a filter can execute
arbitrary code before the handlers are invoked.
This resolves an issue with incorrect 304s on If-Modified-Since mod_include
requests since ap_meets_conditions() is not aware that this is a dynamic
request and it is not possible to satisfy 304 for these requests (unless
xbithack full is on, of course). When mod_include runs as a filter, it is
too late to set any flag since the handler is responsible for calling
ap_meets_conditions(), which it should do before generating any data.
If a module doesn't need to run such arbitrary code, it can just pass NULL
as the argument and all is well.
These emits occur mainline, outside of the pphrase_callback, so we never
opened readtty or writetty. But they are absolute failures, nothing the
user could do to deal with them. They are logged in the ssl vhost's error
log.
In this case, I forgot my SSLCertificateKeyFile, so the server never
tried the callback. writetty wasn't initialized, so we segfaulted.
This segfault is due to misconfig, not to the dialog with the user.
This is the easiest fix (easier to read, too), but we shouldn't need
to worry too much that the release is tagged. If we retag, fine, then
grab it, but it only addresses a config problem.
Doug MacEachern [Tue, 11 Jun 2002 03:45:54 +0000 (03:45 +0000)]
in case there is actually a cert chain in the cache, we should be
using the value of SSL_get_peer_certificate(ssl) to verify as it will
have been removed from the chain before it was put in the cache.
Doug MacEachern [Tue, 11 Jun 2002 03:19:27 +0000 (03:19 +0000)]
PR:
Obtained from:
Submitted by:
Reviewed by:
allow POST method over SSL when per-directory client cert
authentication is used with 'SSLOptions +OptRenegotiate' enabled
and a client cert was found in the ssl session cache.
Doug MacEachern [Tue, 11 Jun 2002 03:12:33 +0000 (03:12 +0000)]
PR:
Obtained from:
Submitted by:
Reviewed by:
'SSLOptions +OptRengotiate' will use client cert in from the ssl
session cache when there is no cert chain in the cache. prior to
the fix this situation would result in a FORBIDDEN response and
error message "Cannot find peer certificate chain"
Cliff Woolley [Thu, 6 Jun 2002 18:27:42 +0000 (18:27 +0000)]
Update Geoff's email address. PS: Geoff still volunteers to answer any
questions about shmcb:
"Feel free to buzz me on shmcb matters to as/when you like - my time
may be limited right now but I will certainly reply as best I can to
anything that comes up."
The only remaining question ... are nested or strictly unnested locks
expected by OpenSSL? Right now I've left it as _DEFAULT for the platform
preference. Very simple code really - the server_rec was superfluous.
Cliff Woolley [Thu, 30 May 2002 22:39:08 +0000 (22:39 +0000)]
This definitely gets the award for least useful error message of the month.
Not only should it just say "can't do that on win32," which is after all
the bottom line, it was spitting out openssl error messages which were
totally useless. Eg:
[30/May/2002 17:31:17 05760] [error] Init: PassPhraseDialog BuiltIn not
supported in server private key from file
F:/Apache/Apache2/conf/ssl/secure.key (OpenSSL library error follows)
[30/May/2002 17:31:17 05760] [error] OpenSSL: error:0D084069:asn1
encoding routines:d2i_ASN1_SET:bad tag
[30/May/2002 17:31:17 05760] [error] OpenSSL: error:0D09D082:asn1
encoding routines:d2i_RSAPrivateKey:parsing
[30/May/2002 17:31:17 05760] [error] OpenSSL: error:0D09B00D:asn1
encoding routines:d2i_PrivateKey:ASN1 lib
Which is essentially saying "OpenSSL couldn't read your private key because
it was encrypted, and we can't get the passphrase the way you asked us to
on this platform."
Brought to my attention by the inquiry of: Chris Hsiang <chsiang@ivivos.com>
Cliff Woolley [Sat, 25 May 2002 20:10:55 +0000 (20:10 +0000)]
Fix the rest of the apr_pool_userdata_setn() bogosity w.r.t. DSO modules.
It's totally unsafe to use apr_pool_userdata_setn() in the post_config
phase of a module, since on some platforms when the DSO gets reloaded
between phases, the data segment will be at a different address on the
second phase and the userdata_get() call will fail.
Doug MacEachern [Fri, 17 May 2002 18:21:12 +0000 (18:21 +0000)]
prevent possible segv in ssl_init_CheckServers if s->addrs is NULL.
for example: <VirtualHost *:>, for which the core only spits out a warning:
Name or service not known: Cannot resolve host name *: --- ignoring!
Change mod_ssl from using ssl_log() to ap_log_error().
The issue is that ssl_log doesn't handle apr_status_t result codes. This
leads to a number of places (esp. with mutexes) where the error codes get
lost. Rather than extending ssl_log further, since mod_ssl is part of
our core, migrate to ap_log_error. This means that mod_ssl no longer
does its own logging.
Most uses of SSL_ADD_ERRNO are now mapped correctly to apr_status_t values
(mainly because the APIs that used to return errnos are now APRized and
have apr_status_t codes available).
SSL_LOG_TRACE and SSL_LOG_DEBUG were mapped to the APLOG_DEBUG values.
mod_ssl prints out a LOT of debugging information, so mod_ssl with LogLevel
Debug may not be a good idea - perhaps mod_ssl should be less chatty.
Numerous printf type collisions were also resolved.
(The ssl logging code itself will be removed in a subsequent commit.)
This has been discussed on dev@httpd, but the fact that there isn't
much to review besides the mindless changes, I'm going to commit now
and rely on CTR if I screwed up anything on the translation.
Stop using SSL_ADD_SSLERR option in ssl_log() and replace with new
ssl_log_ssl_error() function that wraps ap_log_error instead.
This begins the migration from ssl_log() -> ap_log_error(). Divorcing
ourselves from the SSL_ADD_SSLERR option is required to make the next
pass easier.
Jeff Trawick [Thu, 9 May 2002 10:53:28 +0000 (10:53 +0000)]
Fix a mod_ssl build problem on OS/390.
This is admittedly rather ugly code to come up with a unique 4-byte
identifier for the thread. Since our threads are pthreads and a pthread
maps 1:1 to a TCB, the address of the TCB is sufficient. Yes, every
TCB sees a different piece of real storage mapped to the first page,
so the code does make sense.
Paul J. Reder [Wed, 1 May 2002 19:28:52 +0000 (19:28 +0000)]
Fix a case where an invalid pass phrase is entered and an
error message is given, but the prompt is not shown again.
This left the user in an ambiguous state.
SHMCB should not have been using apr_rmm -- it was doing so incorrectly,
for one thing. But it just plain doesn't need it. Rip it out to avoid
segfaulting.
Touch these files so that their datestamps are newer than the corresponding
.y and .l files. These must be kept newer than those at all times to avoid
introducing a dependency on flex and yacc.
PR:
Obtained from:
Submitted by:
Reviewed by:
ssl_io_input_read now returns APR_EOF if ssl_io_hook_read returns 0
bytes for a reason other than SSL_ERROR_WANT_READ. this should
prevent a possible endless loop.
Get the HTTP-on-HTTPS hint to come through again. We're in AP_MODE_GETLINE
at this point, so the \r\n\r\n just confuses the http input filter.
One concern: this patch is only correct as long as we only ever call this
function while in AP_MODE_GETLINE. Ideally we would account for the mode
and return the newlines if not in GETLINE mode, but at the moment it doesn't
seem to matter.
PR:
Obtained from:
Submitted by:
Reviewed by:
avoid the error_log message: [error] mod_ssl: Certificate Verification: Error ...
if SSLProxyVerify is not configured or set to "none".
the verify callback does not happen in the server context when
SSLVerify is not configured or set to "none".
PR:
Obtained from:
Submitted by:
Reviewed by: Ryan Bloom
ap_remove_output_filter no longer works for connection filters.
change logic in the case of "HTTP spoken on HTTPS port" to disable the
ssl filters rather than attempt to remove the filters.