You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Jim Jagielski <ji...@jaguNET.com> on 2008/06/02 20:02:09 UTC

Re: 2.2.9 status

There are just a handful of useful patches in STATUS lacking
a single vote for inclusion in 2.2.9... Anticipating an
apr/apu release within a day or so, I'd like to get 2.2.9
out pretty much right after that, and with as much goodness
as possible :)

Please review STATUS and prepare for tarball testing in a coupla
days!

Re: SNI in 2.2.9? (Re: 2.2.9 status)

Posted by Kaspar Brand <ht...@velox.ch>.
On Mon, Jun 18, 2008 at 05:27:17PM +0200, I wrote:
> So, to support non-SNI clients "as far as possible", let me propose the
> attached (additional) patch. It corrects the shortcomings of my earlier
> attempt (no longer changing dc->nVerify{Client,Depth} in-place), and
> includes the changes to support SSLCipherSuite, SSLHonorCipherOrder,
> SSLCARevocation{File,Path} and
> SSLOCSP{Enable,DefaultResponder,OverrideResponder} for non-SNI clients.

It turns out that I introduced a regression for SNI clients with this
patch, unfortunately... sorry about that, mea culpa. While the changes
to ssl_callback_SSLVerify and ssl_callback_SSLVerify_CRL in that version
of the patch do the right thing for non-SNI clients, the code will
segfault when trying to verify the peer cert of an SNI client.

The reason is simple - for SNI clients, no request_rec has been assigned
at that stage (it doesn't exist yet), so accessing r->server attempts to
dereference the NULL pointer.

The attached version (v5) fixes this issue, and an interdiff to v4 is
also included. Would it be feasible/acceptable to commit this to trunk?

Thanks,
Kaspar

Re: SNI in 2.2.9? (Re: 2.2.9 status)

Posted by Kaspar Brand <ht...@velox.ch>.
Coincidentally, I was about to finish the second part of my analysis
when I saw the "SNI in which release?" message earlier today.

So here it is, part two... Joe, your help in getting SNI into 2.2.10
(possibly) would be very much appreciated. Thanks if you find time for
looking into this.

We're now considering this case:

> (2) non-SNI client connecting to
>     (b) one of the other, also access-restricted vhosts (two or above)

Name-based SSL virtual hosts for "legacy" clients is kind of an oxymoron
- a fact which has always been documented in the mod_ssl FAQ. Trying to
set up such a configuration with 2.2.x will cause the familiar "You
should not use name-based virtual hosts in conjunction with SSL!!"
warning to appear in the error log, but httpd will start and, depending
on the specific setup, even provide a limited way of name-based virtual
hosting (see e.g. http://wiki.cacert.org/wiki/CSRGenerator to see what
"workarounds" people use).

IMO, the primary question is to what extent exactly we should support
legacy/non-SNI clients. In theory, we could just disallow access to all
but the first vhost for non-SNI clients - something I consider too
radical, and it would also break backwards compatibility.

The other strategy (and the one I propose to follow) is to support
non-SNI clients "as far as possible", and taking special care that no
unauthorized access is possible when client authentication comes into play.

Looking again at our list, we basically have to deal with all these
parameters:

mod_ssl directive        Configured in mod_ssl as
                         (modssl_ctx_t*)mctx->...
-------------------------------------------------
SSLCipherSuite           auth.cipher_suite
SSLVerifyDepth           auth.verify_depth
SSLVerifyClient          auth.verify_mode
SSLCACertificateFile     auth.ca_cert_file
SSLCACertificatePath     auth.ca_cert_path
SSLCARevocationFile      crl_file
SSLCARevocationPath      crl_path
SSLOCSPDefaultResponder  ocsp_responder
SSLOCSPEnable            ocsp_enabled
SSLOCSPOverrideResponder ocsp_force_default
SSLCertificateChainFile  cert_chain
SSLCADNRequestFile       pks->ca_name_file
SSLCADNRequestPath       pks->ca_name_path
SSLHonorCipherOrder      sc->cipher_server_pref
SSLProtocol              protocol

SSLCipherSuite and SSLHonorCipherOrder can be supported quite well by
modifications to ssl_hook_Access. A renegotiation will be needed, but
it's a case which is similar to per-directory SSLCipherSuite settings.

SSLProtocol can't be supported in a reasonable way in ssl_hook_Access,
as the handshake has been completed already at that point (on a side
note, I wonder whether SSLv2 should really be supported by mod_ssl any
longer - it has serious design flaws, and SSLv3 was introduced more
than 10 years ago).

The remaining ones actually all apply to client authentication
(verification of the peer's certificate): SSLVerify{Depth,Client},
SSLCACertificate{File,Path}, SSLCARevocation{File,Path},
SSLOCSP{DefaultResponder,Enable,OverrideResponder},
SSLCADNRequest{File,Path}. Unless SSLVerifyClient is set to "require",
I consider a partial/imperfect implementation of these directives for
legacy/non-SNI clients acceptable (when considering access to vhosts two
and above). It's something which was not supported in the past, and for
some of these settings it's simply not possible to reach the same level
of functionality (e.g., legacy clients will *always* run into name
mismatches when accessing a non-default vhost, that's the reason why the
TLS protocol had to be changed, after all).

Changing SSLCACertificate{File,Path} and SSLCADNRequest{File,Path} after
the initial handshake is not possible with current OpenSSL versions,
unfortunately (it would need the SSL_set_cert_store() function, which
was never added to any official release). SSLCARevocation{File,Path} and
SSLOCSP{Enable,DefaultResponder,OverrideResponder}, however, can be
supported by small modifications to ssl_callback_SSLVerify() and
ssl_callback_SSLVerify_CRL().

So, to support non-SNI clients "as far as possible", let me propose the
attached (additional) patch. It corrects the shortcomings of my earlier
attempt (no longer changing dc->nVerify{Client,Depth} in-place), and
includes the changes to support SSLCipherSuite, SSLHonorCipherOrder,
SSLCARevocation{File,Path} and
SSLOCSP{Enable,DefaultResponder,OverrideResponder} for non-SNI clients.

Thanks for any feedback that helps in getting SNI support into 2.2.x
(comments, questions, hints on remaining issues, etc.).

Kaspar

Re: SNI in 2.2.9? (Re: 2.2.9 status)

Posted by Kaspar Brand <ht...@velox.ch>.
Joe Orton wrote:
> A lot of the mod_ssl code will need to be 
> very carefully reviewed since some core assumptions are being broken by 
> supporting SNI.  I would go through each of the config directive which 
> supports vhost context in turn.

So, as promised, I've looked further into it, based on the list of
directives we collected earlier. Here is part one of my findings, i.e.
those covering case 1(b) (I suggest to do it step by step, so I would
follow up on 2(b) afterwards):

> (1) SNI client connecting to
>     (b) one of the other, also access-restricted vhosts (two or above)

In this case, the matching modssl_ctx_t structure (referred to as "mctx"
henceforth) gets assigned to a conn_rec very early in the handshake, in
ssl_callback_ServerNameIndication(). This is the server's first
step in the TLS handshake (parsing the ClientHello), i.e. it happens
before any certificate, cipher, verification mode etc. has been set/sent
to the peer - which also means that virtually all parameters set at that
time in the corresponding OpenSSL structure (SSL *) are applied later on
in the handshake.

Specifically, we have to care about the following parameters:

mod_ssl directive        Configured in mod_ssl as     Applied in OpenSSL
                         (modssl_ctx_t*)mctx->...     based on
                                                      (SSL*)ssl->...
------------------------------------------------------------------------
SSLCipherSuite           auth.cipher_suite            ctx->cipher_list
SSLVerifyDepth           auth.verify_depth            [1]
SSLVerifyClient          auth.verify_mode             verify_mode
SSLCACertificateFile     auth.ca_cert_file            ctx->client_CA [2]
SSLCACertificatePath     auth.ca_cert_path            ctx->client_CA [2]
SSLCARevocationFile      crl_file                     [3]
SSLCARevocationPath      crl_path                     [3]
SSLOCSDefaultResponder   ocsp_responder               [1]
SSLOCSPEnable            ocsp_enabled                 [1]
SSLOCSPOverrideResponder ocsp_force_default           [1]
SSLCertificateChainFile  cert_chain                   ctx->cert_store
SSLCADNRequestFile       pks->ca_name_file            ctx->client_CA [2]
SSLCADNRequestPath       pks->ca_name_path            ctx->client_CA [2]
SSLHonorCipherOrder      sc->cipher_server_pref       options
SSLProtocol              protocol                     method [4]

[1] Enforced by mod_ssl's ssl_callback_SSLVerify(), which gets the
parameter's value either through dc->nVerifyDepth or through
mctx->auth.verify_depth (the latter as a fallback). To make sure it's
the correct mctx, we adjust c->base_server before we leave ssl_find_vhost().

[2] If set, pks->ca_name_file and pks->ca_name_path have preference over
auth.ca_cert_file and auth.ca_cert_path - see
ssl_engine_init.c:ssl_init_ctx_verify(), where the SSL_CTX is set up.

[3] Enforced by mod_ssl's ssl_callback_SSLVerify_CRL, which retrieves
this information through the mctx assigned to the connection (analogous
to [1]).

[4] The first vhost *must* include TLSv1 as a permitted protocol,
otherwise vhosts two and above are not reachable through
ssl_callback_ServerNameIndication (and need to be addressed under case
2(b), i.e. as if SNI is basically disabled - even if we originally
received a TLSv1 ClientHello with an SNI extension).


Now, the consequence of this in reply to your observation

> So *all* vhost-specific config settings which affect the handshake are 
> going to have to be set up manually in the SSL_CTX, otherwise the 
> configuration is not going to be applied correctly.

is that we need to do this in ssl_callback_ServerNameIndication()/
ssl_find_vhost() only for those directives from the table above where
the third column either has no "ctx->..." or no footnote. Or put another
way, it amounts to those members of the SSL structure which are copied
by SSL_new (as documented by openssl/ssl.h) from the SSL_CTX. We can
ignore those which can't be tuned through one of the listed mod_ssl
configuration directives (ctx->read_ahead e.g.), which leaves us with
these two:

SSLVerifyClient          auth.verify_mode             verify_mode
SSLHonorCipherOrder      sc->cipher_server_pref       options

"Luckily" enough, though, that's already the case for the current
ssl_find_vhost() code, which includes these two calls:

for SSLHonorCipherOrder (SSL_OP_CIPHER_SERVER_PREFERENCE):
>         SSL_set_options(ssl, SSL_CTX_get_options(ssl->ctx));

for SSLVerifyClient:
>             SSL_set_verify(ssl, SSL_CTX_get_verify_mode(ssl->ctx),
>                            SSL_CTX_get_verify_callback(ssl->ctx));


To sum up, I still maintain that the implementation currently available
in trunk is correct for SNI clients (i.e. supports all relevant SSL*
directives for name-based SSL virtual hosts, and properly enforces
access control).

Joe, do you agree/disagree with this analysis? If the latter, then
let me know what additional issues you see. If the former, I would
continue my analysis for case 2(b) then. It definitely needs some
additional fine-tuning compared to my latest patch posted on 5 June, but
I think we'll be able to find an acceptable solution (read: compromise).

Thanks for your help!

Kaspar

Re: SNI in 2.2.9? (Re: 2.2.9 status)

Posted by Kaspar Brand <ht...@velox.ch>.
Joe Orton wrote:
> Access control is certainly the most important issue, but e.g. if 
> SSLCertificateChainFile is not supported properly for the named vhost 
> that's also a bug.  Many configs depend on supplying the intermediate 
> certs.

True. I'm using such a configuration on my test host since it's initial
setup (all hosts have at least one intermediate CA), and had verified
earlier that it's indeed working properly for SNI clients - just compare
the chains you get with

openssl s_client -connect sni.velox.ch:443 -servername sni.velox.ch

vs.

openssl s_client -connect sni.velox.ch:443 -servername appendix.velox.ch

>> (SSLCertificateChain is not relevant when verifying peer certs, and
>> apart from this, I didn't see any additional parameters in
>> modssl_ctx_t/modssl_auth_ctx_t which are relevant for access control.)
> 
> I'm think it is relevant.  AFAICT the SSL handshake which takes place in 
> the SNI case is going to be done using the settings from the SSL_CTX 
> from the original vhost not the named vhost, plus those explicitly 
> copied over after the SSL_set_SSL_CTX() call.

Not from what I remember from my work on the patch in January/February,
but I need to look into it again before coming up with a thorough
analysis. Generally speaking, the question is what setting in an SSL
structure (as obtained by SSL_new) is taken from the SSL_CTX at what
time (and whether SSL has its own copy or just takes it from its
underlying SSL_CTX referenced through the ctx pointer member). Depending
on this, some additional tuning after SSL_set_SSL_CTX is necessary - as
it turned out to be the case for verify_mode, e.g.

> SSLCertificateChainFile
> SSLCADNRequest{File,Path}
> SSLHonorCipherOrder
> SSLProtocol
> 
> which I think is an exhaustive list.  SSLProtocol and 
> SSLHonorCipherOrder may be "interesting" because the handshake has 
> already begun at the point it needs to be applied.

Ok, I'll examine these as well - but this of course will take me a few
days, so thanks for staying tuned (and for your replies so far).

Kaspar

Re: SNI in 2.2.9? (Re: 2.2.9 status)

Posted by Joe Orton <jo...@redhat.com>.
On Thu, Jun 05, 2008 at 07:25:30AM +0200, Kaspar Brand wrote:
> Joe Orton wrote:
> > http://svn.apache.org/viewvc?rev=662815&view=rev
> > 
> > Changing the dirconf structure fields in-place seems ugly and may even 
> > be thread-unsafe (not sure).
> 
> Thanks for pointing this out, I wasn't aware of the danger of doing so.
> The same effect can be achieved with the attached, hopefully more
> acceptable patch, however (diff against the current code in trunk).
> 
> > From a quick look I can't see how a reneg would be forced for any of:
> > 
> > 1) SSLCipherSuite changed since original vhost
> > 2) SSLCACeritificate* changed since original vhost (where both 
> > 3) SSLOCSP* changed since original vhost
> 
> To better understand what your primary concern is - can we agree what
> specific case we're considering in this case? I see four of them, actually:
> 
> (1) SNI client connecting to
>     (a) the first, access-restricted vhost
>     (b) one of the other, also access-restricted vhosts (two or above)
> 
> (2) non-SNI client connecting to
>     (a) the first, access-restricted vhost
>     (b) one of the other, also access-restricted vhosts (two or above)
> 
> The issues you raised all belong to (2b), is that correct? From my
> perspective, the patch is working correctly for (1a), (1b) and (2a), but
> the question is  mainly how to handle (2b), i.e. properly enforcing
> access restrictions for "legacy" clients for vhosts two and above.

I'm concerned mostly about 1 (b) but I guess we'd have to consider an 
access control bypass in any of these four cases to be a security issue 
once we start claiming this as a supported configuration.

> > I would go through each of the config directive which 
> > supports vhost context in turn.  What about SSLCertificateChainFile?  
> > What about CRLs?  etc etc.
> 
> If we agree that the remaining problem is about enforcing access
> control, then I would consider these directives relevant:

Access control is certainly the most important issue, but e.g. if 
SSLCertificateChainFile is not supported properly for the named vhost 
that's also a bug.  Many configs depend on supplying the intermediate 
certs.

> Directive                  accessed as... (assuming that mctx is
>                            pointing to current modssl_ctx_t)
> 
> SSLCipherSuite             mctx->auth.cipher_suite
> SSLVerifyDepth             mctx->auth.verify_depth
> SSLVerifyClient            mctx->auth.verify_mode
> SSLCACertificateFile       mctx->auth.ca_cert_file
> SSLCACertificatePath       mctx->auth.ca_cert_path
> SSLCARevocationFile        mctx->crl_file
> SSLCARevocationPath        mctx->crl_path
> SSLOCSDefaultResponder     mctx->ocsp_responder
> SSLOCSPEnable              mctx->ocsp_enabled
> SSLOCSPOverrideResponder   mctx->ocsp_force_default
> 
> Do you see others that should be added to this list?
>
> (SSLCertificateChain is not relevant when verifying peer certs, and
> apart from this, I didn't see any additional parameters in
> modssl_ctx_t/modssl_auth_ctx_t which are relevant for access control.)

I'm think it is relevant.  AFAICT the SSL handshake which takes place in 
the SNI case is going to be done using the settings from the SSL_CTX 
from the original vhost not the named vhost, plus those explicitly 
copied over after the SSL_set_SSL_CTX() call.

So *all* vhost-specific config settings which affect the handshake are 
going to have to be set up manually in the SSL_CTX, otherwise the 
configuration is not going to be applied correctly.  That means in 
addition:

SSLCertificateChainFile
SSLCADNRequest{File,Path}
SSLHonorCipherOrder
SSLProtocol

which I think is an exhaustive list.  SSLProtocol and 
SSLHonorCipherOrder may be "interesting" because the handshake has 
already begun at the point it needs to be applied.

joe

Re: SNI in 2.2.9? (Re: 2.2.9 status)

Posted by Kaspar Brand <ht...@velox.ch>.
Joe Orton wrote:
> http://svn.apache.org/viewvc?rev=662815&view=rev
> 
> Changing the dirconf structure fields in-place seems ugly and may even 
> be thread-unsafe (not sure).

Thanks for pointing this out, I wasn't aware of the danger of doing so.
The same effect can be achieved with the attached, hopefully more
acceptable patch, however (diff against the current code in trunk).

> From a quick look I can't see how a reneg would be forced for any of:
> 
> 1) SSLCipherSuite changed since original vhost
> 2) SSLCACeritificate* changed since original vhost (where both 
> 3) SSLOCSP* changed since original vhost

To better understand what your primary concern is - can we agree what
specific case we're considering in this case? I see four of them, actually:

(1) SNI client connecting to
    (a) the first, access-restricted vhost
    (b) one of the other, also access-restricted vhosts (two or above)

(2) non-SNI client connecting to
    (a) the first, access-restricted vhost
    (b) one of the other, also access-restricted vhosts (two or above)

The issues you raised all belong to (2b), is that correct? From my
perspective, the patch is working correctly for (1a), (1b) and (2a), but
the question is  mainly how to handle (2b), i.e. properly enforcing
access restrictions for "legacy" clients for vhosts two and above.

> I would go through each of the config directive which 
> supports vhost context in turn.  What about SSLCertificateChainFile?  
> What about CRLs?  etc etc.

If we agree that the remaining problem is about enforcing access
control, then I would consider these directives relevant:

Directive                  accessed as... (assuming that mctx is
                           pointing to current modssl_ctx_t)

SSLCipherSuite             mctx->auth.cipher_suite
SSLVerifyDepth             mctx->auth.verify_depth
SSLVerifyClient            mctx->auth.verify_mode
SSLCACertificateFile       mctx->auth.ca_cert_file
SSLCACertificatePath       mctx->auth.ca_cert_path
SSLCARevocationFile        mctx->crl_file
SSLCARevocationPath        mctx->crl_path
SSLOCSDefaultResponder     mctx->ocsp_responder
SSLOCSPEnable              mctx->ocsp_enabled
SSLOCSPOverrideResponder   mctx->ocsp_force_default

Do you see others that should be added to this list?
(SSLCertificateChain is not relevant when verifying peer certs, and
apart from this, I didn't see any additional parameters in
modssl_ctx_t/modssl_auth_ctx_t which are relevant for access control.)

Kaspar

Re: SNI in 2.2.9? (Re: 2.2.9 status)

Posted by Joe Orton <jo...@redhat.com>.
On Tue, Jun 03, 2008 at 04:42:07PM +0200, Kaspar Brand wrote:
> So, is there still hope for SNI being added in 2.2.9...? Let me know if
> there's anything else I can do to increase the chances of getting this
> proposal accepted.

http://svn.apache.org/viewvc?rev=662815&view=rev

Changing the dirconf structure fields in-place seems ugly and may even 
be thread-unsafe (not sure).  I still can't see how this handles half 
the cases it needs to, as I've said several times now - SSLVerifyClient 
is only one part of this.  From a quick look I can't see how a reneg 
would be forced for any of:

1) SSLCipherSuite changed since original vhost
2) SSLCACeritificate* changed since original vhost (where both 
3) SSLOCSP* changed since original vhost

but it certainly should be.  A lot of the mod_ssl code will need to be 
very carefully reviewed since some core assumptions are being broken by 
supporting SNI.  I would go through each of the config directive which 
supports vhost context in turn.  What about SSLCertificateChainFile?  
What about CRLs?  etc etc.

It is also a complete cop-out to claim these issues aren't specific to 
SNI since we explicitly don't support any non-SNI configuration in which 
these paths can be triggered.  And for very good reason: *they don't 
work properly*.

joe


SNI in 2.2.9? (Re: 2.2.9 status)

Posted by Kaspar Brand <ht...@velox.ch>.
> There are just a handful of useful patches in STATUS lacking
> a single vote for inclusion in 2.2.9...

While not completely true for the SNI backport proposal (requires more
than a single additional vote), I'd nevertheless like to draw the
attention to that patch.

Looking at the current votes, I think that the -1 no longer applies,
actually (it was added in December '07, before the code was reworked
considerably):

>       Backport version for 2.2.x of updated patch:
>          http://people.apache.org/~fuankg/diffs/httpd-2.2.x-sni.diff
>       +1: fuankg
>       +0: like ssl upgrade of 2.2, perhaps this is a good reason to bring
>           httpd-2.4 to completion?  vhost changes could be disruptive to
>           third party module authors.
>       -1: rpluem: jorton found some problems with the trunk version and they
>                   should be fixed / discussed in trunk before we backport.

The last issue reported by Joe in April
(http://mail-archives.apache.org/mod_mbox/httpd-dev/200804.mbox/%3c20080422155301.GA4164@redhat.com%3e)
can be addressed by the attached patch, if deemed appropriate [1]. All
other problems observed previously are already included in the backport
version.

So, is there still hope for SNI being added in 2.2.9...? Let me know if
there's anything else I can do to increase the chances of getting this
proposal accepted.

Thanks,
Kaspar

[1] The problem is already present in the current 2.2.x branch (it's not
introduced by the SNI patch, in particular): when setting up more than
one SSL-enabled VirtualHost (e.g. by using a wildcard certificate, or a
cert with several subjectAltName entries), only the per-vhost
SSLVerifyClient/SSLVerifyDepth statements set for the *first* vhost are
ever considered.


Re: 2.2.9 status

Posted by "Akins, Brian" <Br...@turner.com>.
On 6/2/08 2:02 PM, "Jim Jagielski" <ji...@jaguNET.com> wrote:
> Please review STATUS and prepare for tarball testing in a coupla
> days!

FWIW, if my vote counts:

*htpasswd: Fix salt generation weakness. PR 31440 +1
* mod_unique_id: Convert request time to seconds before before storing it in
+1


-- 
Brian Akins
Chief Operations Engineer
Turner Digital Media Technologies