Re: svn commit: r1729208 - in /httpd/httpd/trunk/modules: proxy/proxy_util.c ssl/ssl_engine_io.c

[Stefan Eissing]

> Am 10.02.2016 um 20:09 schrieb Ruediger Pluem :
> 
> 
> 
> On 02/08/2016 05:50 PM, ic...@apache.org wrote:
>> [...]#ifdef HAVE_TLSEXT
>> +#ifdef HAVE_TLS_ALPN
>> +alpn_note = apr_table_get(c->notes, "proxy-request-alpn-protos");
>> +if (alpn_note) {
>> +char *protos, *s, *p, *last;
>> +apr_size_t len;
>> +
>> +s = protos = apr_pcalloc(c->pool, strlen(alpn_note)+1);
>> +p = apr_pstrdup(c->pool, alpn_note);
>> +while ((p = apr_strtok(p, ", ", ))) {
>> +len = last - p - (*last? 1 : 0); 
>> +if (len > 255) {
>> +ap_log_cerror(APLOG_MARK, APLOG_ERR, 0, c, APLOGNO()
>> +  "ALPN proxy protocol identifier too long: 
>> %s",
>> +  p);
>> +ssl_log_ssl_error(SSLLOG_MARK, APLOG_ERR, server);
>> +return APR_EGENERAL;
>> +}
>> +*s++ = (unsigned char)len;
>> +while (len--) {
>> +*s++ = *p++;
>> +}
>> +p = last;
> 
> Why not p = NULL as it should be for subsequent calls of apr_strtok?

Matter of taste. If code is more readable to everyone using NULL, that is fine 
with me.

Changed in r1729782.

Stefan

Re: svn commit: r1729208 - in /httpd/httpd/trunk/modules: proxy/proxy_util.c ssl/ssl_engine_io.c

[Ruediger Pluem]

On 02/08/2016 05:50 PM, ic...@apache.org wrote:
> Author: icing
> Date: Mon Feb  8 16:50:07 2016
> New Revision: 1729208
> 
> URL: http://svn.apache.org/viewvc?rev=1729208=rev
> Log:
> let proxy handler forward ALPN protocol strings for ssl proxy connections
> 
> Modified:
> httpd/httpd/trunk/modules/proxy/proxy_util.c
> httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
> 

> Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
> URL: 
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_io.c?rev=1729208=1729207=1729208=diff
> ==
> --- httpd/httpd/trunk/modules/ssl/ssl_engine_io.c (original)
> +++ httpd/httpd/trunk/modules/ssl/ssl_engine_io.c Mon Feb  8 16:50:07 2016
> @@ -1146,12 +1146,48 @@ static apr_status_t ssl_io_filter_handsh
>  #endif
>  const char *hostname_note = apr_table_get(c->notes,
>"proxy-request-hostname");
> +const char *alpn_note;
>  BOOL proxy_ssl_check_peer_ok = TRUE;
>  int post_handshake_rc = OK;
>  
>  sc = mySrvConfig(server);
>  
>  #ifdef HAVE_TLSEXT
> +#ifdef HAVE_TLS_ALPN
> +alpn_note = apr_table_get(c->notes, "proxy-request-alpn-protos");
> +if (alpn_note) {
> +char *protos, *s, *p, *last;
> +apr_size_t len;
> +
> +s = protos = apr_pcalloc(c->pool, strlen(alpn_note)+1);
> +p = apr_pstrdup(c->pool, alpn_note);
> +while ((p = apr_strtok(p, ", ", ))) {
> +len = last - p - (*last? 1 : 0); 
> +if (len > 255) {
> +ap_log_cerror(APLOG_MARK, APLOG_ERR, 0, c, APLOGNO()
> +  "ALPN proxy protocol identifier too long: 
> %s",
> +  p);
> +ssl_log_ssl_error(SSLLOG_MARK, APLOG_ERR, server);
> +return APR_EGENERAL;
> +}
> +*s++ = (unsigned char)len;
> +while (len--) {
> +*s++ = *p++;
> +}
> +p = last;

Why not p = NULL as it should be for subsequent calls of apr_strtok?

> +}
> +ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c, 
> +  "setting alpn protos from '%s', protolen=%d", 
> +  alpn_note, (int)(s - protos));
> +if (protos != s && SSL_set_alpn_protos(filter_ctx->pssl, 
> +   (unsigned char *)protos, 
> +   s - protos)) {
> +ap_log_cerror(APLOG_MARK, APLOG_WARNING, 0, c, APLOGNO()
> +  "error setting alpn protos from '%s'", 
> alpn_note);
> +ssl_log_ssl_error(SSLLOG_MARK, APLOG_WARNING, server);
> +}
> +}
> +#endif /* defined HAVE_TLS_ALPN */
>  /*
>   * Enable SNI for backend requests. Make sure we don't do it for
>   * pure SSLv3 connections, and also prevent IP addresses

Regards

Rüdiger