Re: SNI in 2.2.9? (Re: 2.2.9 status)
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 Index: httpd-trunk/modules/ssl/ssl_engine_kernel.c === --- httpd-trunk/modules/ssl/ssl_engine_kernel.c (revision 671932) +++ httpd-trunk/modules/ssl/ssl_engine_kernel.c (working copy) @@ -327,32 +327,35 @@ int ssl_hook_Access(request_rec *r) * because it's the servers choice to select a cipher from the ones the * client supports. So as long as the current cipher is still in the new * cipher suite we're happy. Because we can assume we would have * selected it again even when other (better) ciphers exists now in the * new cipher suite. This approach is fine because the user explicitly * has to enable this via ``SSLOptions +OptRenegotiate''. So we do no * implicit optimizations. */ -if (dc-szCipherSuite) { +if (dc-szCipherSuite || (r-server != r-connection-base_server)) { /* remember old state */ if (dc-nOptions SSL_OPT_OPTRENEGOTIATE) { cipher = SSL_get_current_cipher(ssl); } else { cipher_list_old = (STACK_OF(SSL_CIPHER) *)SSL_get_ciphers(ssl); if (cipher_list_old) { cipher_list_old = sk_SSL_CIPHER_dup(cipher_list_old); } } /* configure new state */ -if (!modssl_set_cipher_list(ssl, dc-szCipherSuite)) { +if ((dc-szCipherSuite + !modssl_set_cipher_list(ssl, dc-szCipherSuite)) || +(sc-server-auth.cipher_suite + !modssl_set_cipher_list(ssl, sc-server-auth.cipher_suite))) { ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, Unable to reconfigure (per-directory) permitted SSL ciphers); ssl_log_ssl_error(APLOG_MARK, APLOG_ERR, r-server); if (cipher_list_old) { sk_SSL_CIPHER_free(cipher_list_old); } @@ -408,18 +411,23 @@ int ssl_hook_Access(request_rec *r) } } /* cleanup */ if (cipher_list_old) { sk_SSL_CIPHER_free(cipher_list_old); } -/* tracing */ if (renegotiate) { +#ifdef SSL_OP_CIPHER_SERVER_PREFERENCE +if (sc-cipher_server_pref == TRUE) { +SSL_set_options(ssl, SSL_OP_CIPHER_SERVER_PREFERENCE); +} +#endif +/* tracing */ ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, Reconfigured cipher suite will force renegotiation); } } /* * override of SSLVerifyDepth * @@ -427,29 +435,26 @@ int ssl_hook_Access(request_rec *r) * function and not by OpenSSL internally (and our function is aware of * both the per-server and per-directory contexts). So we cannot ask * OpenSSL about the currently verify depth. Instead we remember it in our * ap_ctx attached to the SSL* of OpenSSL. We've to force the * renegotiation if the reconfigured/new verify depth is less than the * currently active/remembered verify depth (because this means more * restriction on the certificate chain). */ -if ((sc-server-auth.verify_depth != UNSET) -(dc-nVerifyDepth == UNSET)) { -/* apply per-vhost setting, if per-directory config is not set */ -dc-nVerifyDepth = sc-server-auth.verify_depth; -} -if (dc-nVerifyDepth != UNSET) { +if ((dc-nVerifyDepth != UNSET) || +(sc-server-auth.verify_depth != UNSET)) { /* XXX: doesnt look like sslconn-verify_depth is actually used */ if (!(n = sslconn-verify_depth)) { sslconn-verify_depth = n = sc-server-auth.verify_depth; } /* determine whether a renegotiation has to be forced */ -if (dc-nVerifyDepth n) { +if ((dc-nVerifyDepth
Re: SNI in 2.2.9? (Re: 2.2.9 status)
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 directiveConfigured 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 SSLOCSPEnableocsp_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 Index: httpd-trunk/modules/ssl/ssl_engine_kernel.c === --- httpd-trunk/modules/ssl/ssl_engine_kernel.c (revision 669140) +++ httpd-trunk/modules/ssl/ssl_engine_kernel.c (working copy) @@ -327,32 +327,35 @@ int ssl_hook_Access(request_rec *r) * because it's the servers choice to select a cipher from the ones the * client supports. So as long as the current cipher is still in the new *
Re: SNI in 2.2.9? (Re: 2.2.9 status)
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 directiveConfigured in mod_ssl as Applied in OpenSSL (modssl_ctx_t*)mctx-... based on (SSL*)ssl-... SSLCipherSuite auth.cipher_suitectx-cipher_list SSLVerifyDepth auth.verify_depth[1] SSLVerifyClient auth.verify_mode verify_mode SSLCACertificateFile auth.ca_cert_filectx-client_CA [2] SSLCACertificatePath auth.ca_cert_pathctx-client_CA [2] SSLCARevocationFile crl_file [3] SSLCARevocationPath crl_path [3] SSLOCSDefaultResponder ocsp_responder [1] SSLOCSPEnableocsp_enabled [1] SSLOCSPOverrideResponder ocsp_force_default [1] SSLCertificateChainFile cert_chain ctx-cert_store SSLCADNRequestFile pks-ca_name_filectx-client_CA [2] SSLCADNRequestPath pks-ca_name_pathctx-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
Re: SNI in 2.2.9? (Re: 2.2.9 status)
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)
[EMAIL PROTECTED] wrote: SNI in 2.2.9? (Re: 2.2.9 status) 61495 by: Kaspar Brand 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), [technical stuff snipped] 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. Yes, ditto. Although I'm totally unfamiliar with the code, SNI is a big deal for users, because it makes SSL available for all the small time virtual hosts. If I had a vote I'd say do it! Making SSL available for small sites puts a big pressure on the browsers to work on their secure interfaces, which they lack right now. Are there any roadblocks that can be cleared away by external wannabe-helpfuls? iang Thanks, Kaspar smime.p7s Description: S/MIME Cryptographic Signature
Re: SNI in 2.2.9? (Re: 2.2.9 status)
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=662815view=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
Re: SNI in 2.2.9? (Re: 2.2.9 status)
Den Wednesday 04 June 2008 14:06:22 skrev Ian G: [EMAIL PROTECTED] wrote: SNI in 2.2.9? (Re: 2.2.9 status) 61495 by: Kaspar Brand FYI. SNI is in Mandriva Linux 2008.1. -- Regards // Oden Eriksson
Re: SNI in 2.2.9? (Re: 2.2.9 status)
Oden Eriksson wrote: Den Wednesday 04 June 2008 14:06:22 skrev Ian G: [EMAIL PROTECTED] wrote: SNI in 2.2.9? (Re: 2.2.9 status) 61495 by: Kaspar Brand FYI. SNI is in Mandriva Linux 2008.1. Then you should pull it out ASAP, as noted by others the patch currently in trunk is broken in several ways, with possible security configuration implications: http://mail-archives.apache.org/mod_mbox/httpd-dev/200806.mbox/[EMAIL PROTECTED] Thanks, -Paul
Re: SNI in 2.2.9? (Re: 2.2.9 status)
Joe Orton wrote: http://svn.apache.org/viewvc?rev=662815view=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 SSLVerifyClientmctx-auth.verify_mode SSLCACertificateFile mctx-auth.ca_cert_file SSLCACertificatePath mctx-auth.ca_cert_path SSLCARevocationFilemctx-crl_file SSLCARevocationPathmctx-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 Index: httpd-trunk/modules/ssl/ssl_engine_kernel.c === --- httpd-trunk/modules/ssl/ssl_engine_kernel.c (revision 663447) +++ httpd-trunk/modules/ssl/ssl_engine_kernel.c (working copy) @@ -432,19 +432,16 @@ int ssl_hook_Access(request_rec *r) * currently active/remembered verify depth (because this means more * restriction on the certificate chain). */ -if ((sc-server-auth.verify_depth != UNSET) -(dc-nVerifyDepth == UNSET)) { -/* apply per-vhost setting, if per-directory config is not set */ -dc-nVerifyDepth = sc-server-auth.verify_depth; -} -if (dc-nVerifyDepth != UNSET) { +if ((dc-nVerifyDepth != UNSET) || +(sc-server-auth.verify_depth != UNSET)) { /* XXX: doesnt look like sslconn-verify_depth is actually used */ if (!(n = sslconn-verify_depth)) { sslconn-verify_depth = n = sc-server-auth.verify_depth; } /* determine whether a renegotiation has to be forced */ -if (dc-nVerifyDepth n) { +if ((dc-nVerifyDepth n) || +(sc-server-auth.verify_depth n)) { renegotiate = TRUE; ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, Reduced client verification depth will force @@ -466,23 +463,22 @@ int ssl_hook_Access(request_rec *r) * verification but at least skip the I/O-intensive renegotation * handshake. */ -if ((sc-server-auth.verify_mode != SSL_CVERIFY_UNSET) -(dc-nVerifyClient == SSL_CVERIFY_UNSET)) { -/* apply per-vhost setting, if per-directory config is not set */ -dc-nVerifyClient = sc-server-auth.verify_mode; -} -if (dc-nVerifyClient != SSL_CVERIFY_UNSET) { +if ((dc-nVerifyClient != SSL_CVERIFY_UNSET) || +(sc-server-auth.verify_mode != SSL_CVERIFY_UNSET)) { /* remember old state */ verify_old = SSL_get_verify_mode(ssl); /* configure new state */ verify = SSL_VERIFY_NONE; -if (dc-nVerifyClient == SSL_CVERIFY_REQUIRE) { +if ((dc-nVerifyClient == SSL_CVERIFY_REQUIRE) || +(sc-server-auth.verify_mode == SSL_CVERIFY_REQUIRE)) { verify |= SSL_VERIFY_PEER_STRICT; } if ((dc-nVerifyClient == SSL_CVERIFY_OPTIONAL) || -(dc-nVerifyClient == SSL_CVERIFY_OPTIONAL_NO_CA)) +(dc-nVerifyClient == SSL_CVERIFY_OPTIONAL_NO_CA) || +(sc-server-auth.verify_mode