On Wed, May 29, 2013 at 11:37:14AM -0400, Matthew Steele wrote:
> Oops, yes, RUN_ALL semantics are desired; the misleading API description is
> my fault, sorry.  (I confess I never really understood why RUN_ALL hooks
> accept both OK and DECLINED values, but then don't actually treat them any
> differently.)  So probably we should update the doc comments.  I'd be happy
> to draft new versions for those if that would be helpful, or not, as you
> prefer.

OK vs DECLINED has always confused me too ;)

How does this look?  I've specified the behaviour for OK and DONE as 
return codes for both the callbacks; since the caller doesn't pay 
attention to errors I've left behaviour undefined for any other values.

(Really attached this time)

Regards, Joe
Index: modules/ssl/mod_ssl.c
===================================================================
--- modules/ssl/mod_ssl.c       (revision 1487480)
+++ modules/ssl/mod_ssl.c       (working copy)
@@ -285,18 +285,6 @@
     AP_END_CMD
 };
 
-/* Implement 'modssl_run_npn_advertise_protos_hook'. */
-APR_IMPLEMENT_OPTIONAL_HOOK_RUN_ALL(
-    modssl, AP, int, npn_advertise_protos_hook,
-    (conn_rec *connection, apr_array_header_t *protos),
-    (connection, protos), OK, DECLINED)
-
-/* Implement 'modssl_run_npn_proto_negotiated_hook'. */
-APR_IMPLEMENT_OPTIONAL_HOOK_RUN_ALL(
-    modssl, AP, int, npn_proto_negotiated_hook,
-    (conn_rec *connection, const char *proto_name, apr_size_t proto_name_len),
-    (connection, proto_name, proto_name_len), OK, DECLINED)
-
 /*
  *  the various processing hooks
  */
@@ -444,6 +432,37 @@
     return 1;
 }
 
+static int modssl_register_npn(conn_rec *c, 
+                               ssl_npn_advertise_protos advertisefn,
+                               ssl_npn_proto_negotiated negotiatedfn)
+{
+#ifdef HAVE_TLS_NPN
+    SSLConnRec *sslconn = myConnConfig(c);
+
+    if (!sslconn) {
+        return DECLINED;
+    }
+
+    if (!sslconn->npn_advertfns) {
+        sslconn->npn_advertfns = 
+            apr_array_make(c->pool, 5, sizeof(ssl_npn_advertise_protos));
+        sslconn->npn_negofns = 
+            apr_array_make(c->pool, 5, sizeof(ssl_npn_proto_negotiated));
+    }
+
+    if (advertisefn)
+        APR_ARRAY_PUSH(sslconn->npn_advertfns, ssl_npn_advertise_protos) =
+            advertisefn;
+    if (negotiatedfn)
+        APR_ARRAY_PUSH(sslconn->npn_negofns, ssl_npn_proto_negotiated) =
+            negotiatedfn;
+
+    return OK;
+#else
+    return DECLINED;
+#endif
+}
+
 int ssl_init_ssl_connection(conn_rec *c, request_rec *r)
 {
     SSLSrvConfigRec *sc;
@@ -615,6 +634,7 @@
 
     APR_REGISTER_OPTIONAL_FN(ssl_proxy_enable);
     APR_REGISTER_OPTIONAL_FN(ssl_engine_disable);
+    APR_REGISTER_OPTIONAL_FN(modssl_register_npn);
 
     ap_register_auth_provider(p, AUTHZ_PROVIDER_GROUP, "ssl",
                               AUTHZ_PROVIDER_VERSION,
Index: modules/ssl/mod_ssl.h
===================================================================
--- modules/ssl/mod_ssl.h       (revision 1487480)
+++ modules/ssl/mod_ssl.h       (working copy)
@@ -63,26 +63,40 @@
 
 APR_DECLARE_OPTIONAL_FN(int, ssl_engine_disable, (conn_rec *));
 
-/** The npn_advertise_protos optional hook allows other modules to add entries
- * to the list of protocol names advertised by the server during the Next
- * Protocol Negotiation (NPN) portion of the SSL handshake.  The hook callee is
- * given the connection and an APR array; it should push one or more char*'s
- * pointing to null-terminated strings (such as "http/1.1" or "spdy/2") onto
- * the array and return OK, or do nothing and return DECLINED. */
-APR_DECLARE_EXTERNAL_HOOK(modssl, AP, int, npn_advertise_protos_hook,
-                          (conn_rec *connection, apr_array_header_t *protos))
+/** The npn_advertise_protos callback allows another modules to add
+ * entries to the list of protocol names advertised by the server
+ * during the Next Protocol Negotiation (NPN) portion of the SSL
+ * handshake.  The callback is given the connection and an APR array;
+ * it should push one or more char*'s pointing to NUL-terminated
+ * strings (such as "http/1.1" or "spdy/2") onto the array and return
+ * OK.  To prevent further processing of (other modules') callbacks,
+ * return DONE. */
+typedef int (*ssl_npn_advertise_protos)(conn_rec *connection, 
+                                        apr_array_header_t *protos);
 
-/** The npn_proto_negotiated optional hook allows other modules to discover the
- * name of the protocol that was chosen during the Next Protocol Negotiation
- * (NPN) portion of the SSL handshake.  Note that this may be the empty string
- * (in which case modules should probably assume HTTP), or it may be a protocol
- * that was never even advertised by the server.  The hook callee is given the
- * connection, a non-null-terminated string containing the protocol name, and
- * the length of the string; it should do something appropriate (i.e. insert or
- * remove filters) and return OK, or do nothing and return DECLINED. */
-APR_DECLARE_EXTERNAL_HOOK(modssl, AP, int, npn_proto_negotiated_hook,
-                          (conn_rec *connection, const char *proto_name,
-                           apr_size_t proto_name_len))
+/** The npn_proto_negotiated callback allows other modules to discover
+ * the name of the protocol that was chosen during the Next Protocol
+ * Negotiation (NPN) portion of the SSL handshake.  Note that this may
+ * be the empty string (in which case modules should probably assume
+ * HTTP), or it may be a protocol that was never even advertised by
+ * the server.  The callback is given the connection, a
+ * non-NUL-terminated string containing the protocol name, and the
+ * length of the string; it should do something appropriate
+ * (i.e. insert or remove filters) and return OK.  To prevent further
+ * processing of (other modules') callbacks, return DONE. */
+typedef int (*ssl_npn_proto_negotiated)(conn_rec *connection, 
+                                        const char *proto_name,
+                                        apr_size_t proto_name_len);
 
+/* An optional function which can be used to register a pair of
+ * callbacks for NPN handling.  This optional function should be
+ * invoked from a pre_connection hook which runs *after* mod_ssl.c's
+ * pre_connection hook.  The function returns OK if the callbacks are
+ * register, or DECLINED otherwise (for example if mod_ssl does not
+ * support NPN).  */
+APR_DECLARE_OPTIONAL_FN(int, modssl_register_npn, (conn_rec *conn, 
+                                                   ssl_npn_advertise_protos 
advertisefn,
+                                                   ssl_npn_proto_negotiated 
negotiatedfn));
+
 #endif /* __MOD_SSL_H__ */
 /** @} */
Index: modules/ssl/ssl_engine_io.c
===================================================================
--- modules/ssl/ssl_engine_io.c (revision 1487480)
+++ modules/ssl/ssl_engine_io.c (working copy)
@@ -1422,16 +1422,27 @@
      * which protocol was decided upon and inform other modules by calling
      * npn_proto_negotiated_hook. */
     if (!inctx->npn_finished) {
+        SSLConnRec *sslconn = myConnConfig(f->c);
         const unsigned char *next_proto = NULL;
         unsigned next_proto_len = 0;
+        int n;
 
-        SSL_get0_next_proto_negotiated(
-            inctx->ssl, &next_proto, &next_proto_len);
-        ap_log_cerror(APLOG_MARK, APLOG_DEBUG, APR_SUCCESS, f->c,
-                      APLOGNO(02306) "SSL NPN negotiated protocol: '%*s'",
-                      next_proto_len, (const char*)next_proto);
-        modssl_run_npn_proto_negotiated_hook(
-            f->c, (const char*)next_proto, next_proto_len);
+        if (sslconn->npn_negofns) {
+            SSL_get0_next_proto_negotiated(
+                inctx->ssl, &next_proto, &next_proto_len);
+            ap_log_cerror(APLOG_MARK, APLOG_DEBUG, APR_SUCCESS, f->c,
+                          APLOGNO(02306) "SSL NPN negotiated protocol: '%*s'",
+                          next_proto_len, (const char*)next_proto);
+            
+            for (n = 0; n < sslconn->npn_negofns->nelts; n++) {
+                ssl_npn_proto_negotiated fn = 
+                    APR_ARRAY_IDX(sslconn->npn_negofns, n, 
ssl_npn_proto_negotiated);
+                
+                if (fn(f->c, (const char *)next_proto, next_proto_len) == DONE)
+                    break;
+            }
+        }
+            
         inctx->npn_finished = 1;
     }
 #endif
Index: modules/ssl/ssl_engine_kernel.c
===================================================================
--- modules/ssl/ssl_engine_kernel.c     (revision 1487480)
+++ modules/ssl/ssl_engine_kernel.c     (working copy)
@@ -2186,6 +2186,7 @@
                                      unsigned int *size_out, void *arg)
 {
     conn_rec *c = (conn_rec*)SSL_get_app_data(ssl);
+    SSLConnRec *sslconn = myConnConfig(c);
     apr_array_header_t *protos;
     int num_protos;
     unsigned int size;
@@ -2196,16 +2197,22 @@
     *data_out = NULL;
     *size_out = 0;
 
-    /* If the connection object is not available, then there's nothing for us
-     * to do. */
-    if (c == NULL) {
+    /* If the connection object is not available, or there are no NPN
+     * hooks registered, then there's nothing for us to do. */
+    if (c == NULL || sslconn->npn_advertfns == NULL) {
         return SSL_TLSEXT_ERR_OK;
     }
 
     /* Invoke our npn_advertise_protos hook, giving other modules a chance to
      * add alternate protocol names to advertise. */
-    protos = apr_array_make(c->pool, 0, sizeof(char*));
-    modssl_run_npn_advertise_protos_hook(c, protos);
+    protos = apr_array_make(c->pool, 0, sizeof(char *));
+    for (i = 0; i < sslconn->npn_advertfns->nelts; i++) {
+        ssl_npn_advertise_protos fn = 
+            APR_ARRAY_IDX(sslconn->npn_advertfns, i, ssl_npn_advertise_protos);
+        
+        if (fn(c, protos) == DONE)
+            break;
+    }
     num_protos = protos->nelts;
 
     /* We now have a list of null-terminated strings; we need to concatenate
Index: modules/ssl/ssl_private.h
===================================================================
--- modules/ssl/ssl_private.h   (revision 1487480)
+++ modules/ssl/ssl_private.h   (working copy)
@@ -496,6 +496,10 @@
                      * connection */
     } reneg_state;
 
+    /* Poor man's inter-module optional hooks for NPN. */
+    apr_array_header_t *npn_advertfns; /* list of ssl_npn_advertise_protos 
callbacks */
+    apr_array_header_t *npn_negofns; /* list of ssl_npn_proto_negotiated 
callbacks. */
+
     server_rec *server;
 } SSLConnRec;
 

Reply via email to