ASF Bugzilla – Full Text Bug Listing
|Summary:||Add TLS Next Protocol Negotiation (NPN) support to mod_ssl|
|Component:||mod_ssl||Assignee:||Apache HTTPD Bugs Mailing List <bugs>|
|Severity:||enhancement||CC:||apache, apache, apache_bugzilla, bjoern, bmcquade, candrews, dma_k, hanno, issues-apache, mnot, nat.guyton, Neustradamus, philantrop, rajnyv, rob.moss, stefan|
Patch for mod_ssl to add NPN hooks
Updated patch for mod_ssl to add NPN hooks
Updated patch for mod_ssl to add NPN hooks
Updated patch for mod_ssl to add NPN hooks
2.4.x patch for adding ALPN+NPN support
Patch update to always announce http/1.1 over NPN
Updated patch introducing ALPN/NPN support for mod_ssl
Updated patch for ALPN/NPN support in mod_ssl
Add ALPN/NPN support to mod_ssl
Enables ALPN+NPN in mod_ssl simultaneously if supported by openssl
Simul. ALPN+NPN support in mod_ssl, verified against mixed, various clients
Description mdsteele 2011-11-18 19:03:51 UTC
Created attachment 27969 [details] Patch for mod_ssl to add NPN hooks OpenSSL 1.0.1 added support for TLS Next Protocol Negotiation (NPN) , a feature which allows client and server to negotiate what protocol should be used over the secure connection. I propose adding hooks into mod_ssl to allow other modules to access this feature. In particular, this would open the door for a module that would support SPDY , a performance-improving protocol that is now supported by (at least) Google Chrome, Amazon Silk, Firefox (targeting FF11), and Strangeloop, but not yet by Apache httpd. (Not coincidentally, I am working on implementing such a module.) The changes needed to mod_ssl are pretty simple; I have a small patch here that adds these hooks. The patch attached below was made against the httpd-2.2.x branch, but of course I would be happy to modify the patch as necessary for other version(s).  NPN is described here: http://technotes.googlecode.com/git/nextprotoneg.html  SPDY is described here: http://dev.chromium.org/spdy/spdy-protocol
Comment 1 Paul Querna 2012-03-12 23:12:06 UTC
Issues: * Patch includes a few inline data declarations that won't compile with a strict c89 compiler. * I'd suggest adding a log level debug with the negotiated protocol.
Comment 2 Paul Querna 2012-03-12 23:15:56 UTC
Also, yes, would prefer a patch rebased against the current trunk -- all new features land there first.
Comment 3 mdsteele 2012-03-20 16:06:29 UTC
Created attachment 28486 [details] Updated patch for mod_ssl to add NPN hooks Thanks for the comments; here's an updated patch. I've rebased the patch against trunk, and I believe I've fixed the C89 issues (tested with gcc -std=c89 -pedantic). I've also added a debug-level logging message for the negotiated protocol, as suggested.
Comment 4 mdsteele 2012-03-27 20:55:18 UTC
Created attachment 28513 [details] Updated patch for mod_ssl to add NPN hooks Some minor tweaks to the patch to better conform to surrounding code style.
Comment 5 Joe Orton 2012-04-10 14:46:02 UTC
Patch looks fine to me, though this: + const char *next_proto = NULL; + unsigned next_proto_len = 0; + SSL_get0_next_proto_negotiated( + inctx->ssl, (const unsigned char**)&next_proto, &next_proto_len); is going to trip up gcc strict-aliasing tests, it should pass next_proto without a cast.
Comment 6 mdsteele 2012-04-10 15:28:27 UTC
Thanks for taking a look. I confess that I am not terribly familiar with the strict-aliasing rules, so I am not sure how best to avoid the problem here. The issue is that SSL_get0_next_proto_negotiated requires an unsigned char**, but apr_pstrmemdup and ssl_run_npn_proto_negotiated_hook each require a (plain, not unsigned) char*. If I have no casts at all, the compiler complains. Is the aliasing issue avoided if I cast to char* when I call apr_pstrmemdup and ssl_run_npn_proto_negotiated_hook, rather than casting to unsigned char** when I call SSL_get0_next_proto_negotiated, as I do now? If so, I'll fix that right away; if not, what should I do instead?
Comment 7 Anders Kaseorg 2012-04-11 22:43:00 UTC
It’s not safe to const char ** to const unsigned char **, but it is safe to cast const unsigned char * to const char * (because a specific exemption in the strict aliasing rules allows any type of object to be accessed through a pointer to a character type). So do this instead: const unsigned char *next_proto = NULL; SSL_get0_next_proto_negotiated(inctx->ssl, &next_proto, &next_proto_len); ssl_run_npn_proto_negotiated_hook(f->c, (const char *)next_proto, next_proto_len);
Comment 8 mdsteele 2012-04-11 23:04:13 UTC
Created attachment 28588 [details] Updated patch for mod_ssl to add NPN hooks Aha, that is good to know, thanks. I've updated the patch as you suggested.
Comment 9 Joe Orton 2012-05-01 13:30:13 UTC
Added in r1332643 - I made a few tweaks: 1) added HAVE_TLS_NPN in ssl_private.h with the OpenSSL version logic 2) tweaked the hook name to use "modssl_" rather than "ssl_" to avoid further polluting the "ssl_" namespace Please shout if I broke something, I only tested compilation. Thanks a lot for the contribution!
Comment 10 Eric Covener 2013-08-27 16:53:22 UTC
Re-opening; this is only in trunk.
Comment 11 Yann Ylavic 2014-04-30 13:31:15 UTC
This is currently behing discussed in https://firstname.lastname@example.org/msg59893.html
Comment 12 Björn Jacke 2014-12-20 00:06:30 UTC
(In reply to Yann Ylavic from comment #11) > This is currently behing discussed in > https://email@example.com/msg59893.html this discussion is dead and it does not take into account that npn is a generic mechanism to be able to switch to alternative protocols not neccessarily limited to be used for spdy support. Also this is part of openssl 1.0.1 - mod_ssl just needs to turn it on. Another aspect why we really want to have NPN support in mod_ssl is: firefox and chrome currently use the SSL optimization "false start" only if the server also supports NPN. So with NPN support in mod_ssl we would get nice the side effect to remove one roundtrip time for ssl handshakes for those clients.
Comment 13 Mark Nottingham 2014-12-20 08:49:44 UTC
NPN has been superseded by ALPN - https://www.imperialviolet.org/2013/03/20/alpn.html
Comment 14 Stefan Eissing 2015-01-08 15:51:46 UTC
With OpenSSL 1.0.1 supporting NPN by default, it would be really great to get this patch into the next 2.4 release. Even though NPN will be replaced by ALPN (one day), there are protocols like SPDY and HTTP2 that will need to support both for some time to allow migration. I cannot speak for the openssl project, but current 1.0.1 release has no ALPN support and the 1.0.2 with support is in beta. To add ALPN support to mod_ssl, it seems to make send to delay that until openssl has a production release that supports it.
Comment 15 Mark Nottingham 2015-01-08 15:55:18 UTC
Stefan, HTTP/2 uses ALPN, not NPN; the only reason some implementations use NPN is for testing while ALPN filtered its way into OpenSSL and other TLS implementations. OpenSSL supports ALPN as of 1.0.2. Browsers are even now starting to drop NPN in their negotiation, because doing both has caused interop problems. NPN is a dead end, as very explicitly stated by AGL and others.
Comment 16 Stefan Eissing 2015-01-09 13:40:54 UTC
Mark, I understand. However OpenSSL 1.0.2 is currently in beta (although according to R$ in production soon) and 1.0.1 does not have ALPN support. I also doubt, giving the history, that everyone will deploy 1.0.2 immediately. So, currently we have the situation where you cannot drop a mod_spdy into a 2.4 server without also replacing mod_ssl. The patch for NPN is in httpd/trunk, but not in 2.4. So, everyone wanting to do ALPN/NPN with a httpd 2.4 needs to replace mod_ssl which is not a good idea, it seems. I today have made a clean patch for the NPN changes in trunk to be applicable to 2.4. I want to enhance this with a second patch that uses ALPN instead of NPN of the underlying openssl supports it (compile time). I think this will allow for an easier transition to newer openssl and introduction of ALPN than having everyone write her own mod_ssl version. My question to httpd developers would be if they want to change the "npn" part of the callback function names to "alpn" - which is worth it imho. I will put this on the mailing list for discussion.
Comment 17 Stefan Eissing 2015-01-19 16:53:42 UTC
Created attachment 32381 [details] 2.4.x patch for adding ALPN+NPN support This patch adds ALPN support to 2.4.x, building on the previous patch attached to this ticket.
Comment 18 Stefan Eissing 2015-01-19 16:58:05 UTC
The ALPN support changes the public functions from the previous patch: - renames from "npn" to "alpn" - adds an optional parameter to the advertise/propose function with the protocol names supplied by the client The patch keeps the NPN support, if the underlying openssl does not support ALPN (version >= 1.0.2). The patch introduces a new mod_ssl config directive: SSLAlpnPreference which is a list of protocol names that determine which protocol gets chosen first, if supported by the client. A patch for httpd/trunk can be made available on request. A patch of mod_spdy to use the new functions also. Let me know.
Comment 19 Stefan Eissing 2015-01-27 09:28:12 UTC
Created attachment 32399 [details] Patch update to always announce http/1.1 over NPN As requested in https://issues.apache.org/bugzilla/show_bug.cgi?id=56028 I updated my patch to always announce "http/1.1" over NPN (ALPN already did it). This means that when no hooks are installed, mod_ssl will always advertise http/1.1 to clients on NPN/ALPN.
Comment 20 Romain Lapoux 2015-02-10 16:27:39 UTC
@Stefan Eissing In your patch, I think you have a bug, why do you have HAVE_ALPN_NPN and HAVE_TLS_ALPN? Only HAVE_TLS_ALPN is defined.
Comment 21 adrhc 2015-02-11 09:40:06 UTC
Hi, I don't understand if the patch: Patch update to always announce http/1.1 over NPN (21.41 KB, patch) 2015-01-27 09:28 UTC, Stefan Eissing applies to httpd-2.2 (more specifically to 2.2.29). The patch starts with these lines: diff -ru gen/httpd-2.4.x/modules/ssl/mod_ssl.c modules/ssl/mod_ssl.c --- gen/httpd-2.4.x/modules/ssl/mod_ssl.c 2015-01-19 16:52:30.000000000 +0100 What do I have to think ? it's for httpd-2.4.x ? If yes than what patch is for httpd-2.2 ?
Comment 22 Stefan Eissing 2015-02-11 09:45:51 UTC
The original patch is from 2011. I think nowadays 2.4 is the target for this. It certainly would be possible to downport it. I updated the ticket to specify 2.4 as target.
Comment 23 Stefan Eissing 2015-02-11 09:50:46 UTC
Created attachment 32452 [details] Updated patch introducing ALPN/NPN support for mod_ssl This version of the patch uses correct ALPN/NPN defines and has been tested against 2.4.10.
Comment 24 Stefan Eissing 2015-02-12 12:49:29 UTC
Created attachment 32463 [details] Updated patch for ALPN/NPN support in mod_ssl Fix in DEBUG log of negotiated protocol.
Comment 25 Yann Ylavic 2015-02-12 14:38:51 UTC
It seems that HAVE_TLS_NPNX is defined instead of HAVE_TLS_NPN in the part of the patch below. Also, probably it shouldn't be defined when OPENSSL_NO_TLSEXT is used (like HAVE_TLS_ALPN). diff -ru gen/httpd-2.4.x/modules/ssl/ssl_private.h modules/ssl/ssl_private.h --- gen/httpd-2.4.x/modules/ssl/ssl_private.h 2015-01-19 16:52:30.000000000 +0100 +++ modules/ssl/ssl_private.h 2015-01-19 15:42:53.908000000 +0100 @@ -176,6 +169,16 @@ [...] +/* Next Protocol Negotiation */ +#if !defined(OPENSSL_NO_NEXTPROTONEG) && defined(OPENSSL_NPN_NEGOTIATED) +#define HAVE_TLS_NPNX +#endif
Comment 26 Stefan Eissing 2015-02-12 16:02:14 UTC
Created attachment 32464 [details] Add ALPN/NPN support to mod_ssl Update: fixed stupid NPNX define type, removed compiler warnings Tested: with 2.4.12 and openssl 1.0.2/1.0.1l on darwin
Comment 27 Stefan Eissing 2015-02-25 12:28:07 UTC
Created attachment 32521 [details] Enables ALPN+NPN in mod_ssl simultaneously if supported by openssl This feature keeps on giving: added support for simultaneous ALPN and NPN support (openssl 1.0.2 onwards), so that old and new clients may negotiate. Necessary as long as there are clients using openssl 1.0.1 versions. No changes to callback API from previous versions.
Comment 28 Stefan Eissing 2015-02-26 09:16:09 UTC
Created attachment 32523 [details] Simul. ALPN+NPN support in mod_ssl, verified against mixed, various clients * Found a bug when testing an NPN only client against ALPN+NPN server with patch from yesterday. * replaced TABs with spaces in patch for proper formatting * verified this patch against httpd 2.4.12 in the following combinations: - OpenSSL 1.0.2 httpd against Firefox 36, Chrome 40 - OpenSSL 1.0.2 httpd against OpenSSL 1.0.2 nghttp2 client - OpenSSL 1.0.2 httpd against OpenSSL 1.0.1 nghttp2 client - OpenSSL 1.0.1 httpd against Firefox 36, Chrome 40 - OpenSSL 1.0.1 httpd against OpenSSL 1.0.2 nghttp2 client - OpenSSL 1.0.1 httpd against OpenSSL 1.0.1 nghttp2 client Sorry, if the update frequency of this patch is making noise.
Comment 29 Kaspar Brand 2015-04-26 08:08:54 UTC
Current status in trunk: the changes in attachment 32523 [details] were committed with r1670397 and r1670434. NPN support was subsequently removed with r1676004. Closing this bug as INVALID (for lack of a better resolution - WONTFIX doesn't really fit), as it relates to NPN, which is now quite unlikely to find its way into a stable release.