Re: Supporting "SSL:" in the expression parser via mod_ssl

[Rainer Jung]

Option 3) tested and committed to trunk in r1707152, tested and proposed 
to backport for 2.4 now. Thanks for the feedback!


Regards,

Rainer

Am 06.10.2015 um 12:12 schrieb Plüm, Rüdiger, Vodafone Group:

+1

Regards

Rüdiger


-Original Message-
From: Yann Ylavic [mailto:ylavic@gmail.com]
Sent: Dienstag, 6. Oktober 2015 12:07
To: dev@httpd.apache.org
Subject: Re: Supporting "SSL:" in the expression parser via mod_ssl

On Tue, Oct 6, 2015 at 11:55 AM, Rainer Jung <rainer.j...@kippdata.de>
wrote:


I will commit 3) but are very open to other opinions before suggesting a
backport.


+1

Regards,
Yann.

Re: Supporting "SSL:" in the expression parser via mod_ssl

[Rainer Jung]

Am 06.10.2015 um 00:44 schrieb Stefan Fritsch:

On Wednesday 30 September 2015 23:26:30, Rainer Jung wrote:

I noticed that currently the expression parser in 2.4/trunk does not
support the SSL:VARIABLE lookups that mod_rewrite supports.

The expression parser uses ":" as an alternative function call
syntax, so HTTP:VARIABLE is the same as HTTP(VARIABLE) which in
turn executes http(VARIABLE). The same is true for ENV:VARIABLE
which maps to env(VARIABLE) etc.

We already do support the syntax SSL_VARIABLE to look up SSL
variables, because mod_ssl registers in the expression parser for
variables whose name starts with "SSL_". But mod_ssl does not
register for a function named SSL (or ssl), so the SSL: notation
does not work.

Is that just an omission, or was that intentional? If not
intentional, I would apply the following (yet untested) simple
patch to trunk mod_ssl and propose for backport. It registers the
SSL (and ssl) function in the expression parser:


I think that was just an omission. +1 to your patch.


I tested it an it would work, but one implementation detail remains to 
discuss. The %{SSL:variable} syntax goes back to mod_rewrite. There 
"variable" is the full name of the mod_ssl variable, i.e.


   %{SSL:SSL_PROTOCOL}

mod_rewrite looked up varname by calling ssl_var_lookup() which itself 
resolves lots of variables, even many non-ssl ones. In the ssl case it 
quickly falls through and then strips the "SSL_" prefix and calls 
ssl_var_lookup_ssl().


For 2.4 and the nes %{SSL:variable} impl in the expr parser we have 
three options:


1) Support only variable name with the "SSL_" prefix already stripped 
and call ssl_var_lookup_ssl().


Example: %{SSL:PROTOCOL}

There's less redundancy in the notation, but it is incompatible with 
mod_rewrite's use of %{SSL:variable}. Therefore I'd say it is too 
confusing and would be -1 to this option.


2) Demand including the "SSL_" prefix in the variable name, strip it 
when resolving the value and call ssl_var_lookup_ssl() for the shortened 
name. Throw error if the variable name does not start with "SSL_".


Example: %{SSL:SSL_PROTOCOL}

This is compatible with mod_rewrite, but only as long as you really use 
a variable whose name begins with "SSL_". I guess mostly this will be 
the case, but people might have used e.g. %{SSL:HTTPS}, which would now 
throw an error.


3) Support any mod_ssl variable. Demand using the original full name and 
call ssl_var_lookup() with the full name.


Example: %{SSL:SSL_PROTOCOL}

This is the most compatible and complete solution. The only drawback is 
the use of ssl_var_lookup() which adds a bit of overhead for the general 
case and itself seems to be a candidate for long term replacement by 
ap_expr. At that point in the future it would bite us to let ap_expr 
base its SSL: implementation on a function, that could be replaced by 
ap_expr itself (and ssl_var_lookup_ssl()). Nevertheless I would vote for 
3) as the solution to chose.


I will commit 3) but are very open to other opinions before suggesting a 
backport.


Regards,

Rainer

Re: Supporting "SSL:" in the expression parser via mod_ssl

[Yann Ylavic]

On Tue, Oct 6, 2015 at 11:55 AM, Rainer Jung  wrote:
>
> I will commit 3) but are very open to other opinions before suggesting a
> backport.

+1

Regards,
Yann.

RE: Supporting "SSL:" in the expression parser via mod_ssl

[Plüm , Rüdiger , Vodafone Group]

+1 

Regards

Rüdiger

> -Original Message-
> From: Yann Ylavic [mailto:ylavic@gmail.com]
> Sent: Dienstag, 6. Oktober 2015 12:07
> To: dev@httpd.apache.org
> Subject: Re: Supporting "SSL:" in the expression parser via mod_ssl
> 
> On Tue, Oct 6, 2015 at 11:55 AM, Rainer Jung <rainer.j...@kippdata.de>
> wrote:
> >
> > I will commit 3) but are very open to other opinions before suggesting a
> > backport.
> 
> +1
> 
> Regards,
> Yann.

Re: Supporting "SSL:" in the expression parser via mod_ssl

[Stefan Fritsch]

On Wednesday 30 September 2015 23:26:30, Rainer Jung wrote:
> I noticed that currently the expression parser in 2.4/trunk does not
> support the SSL:VARIABLE lookups that mod_rewrite supports.
> 
> The expression parser uses ":" as an alternative function call
> syntax, so HTTP:VARIABLE is the same as HTTP(VARIABLE) which in
> turn executes http(VARIABLE). The same is true for ENV:VARIABLE
> which maps to env(VARIABLE) etc.
> 
> We already do support the syntax SSL_VARIABLE to look up SSL
> variables, because mod_ssl registers in the expression parser for
> variables whose name starts with "SSL_". But mod_ssl does not
> register for a function named SSL (or ssl), so the SSL: notation
> does not work.
> 
> Is that just an omission, or was that intentional? If not
> intentional, I would apply the following (yet untested) simple
> patch to trunk mod_ssl and propose for backport. It registers the
> SSL (and ssl) function in the expression parser:

I think that was just an omission. +1 to your patch.

> 
> Index: modules/ssl/ssl_engine_vars.c
> ===
> --- modules/ssl/ssl_engine_vars.c   (revision 1706155)
> +++ modules/ssl/ssl_engine_vars.c   (working copy)
> @@ -149,6 +149,15 @@
>   return sslconn ? ssl_var_lookup_ssl(ctx->p, ctx->c, ctx->r,
> var) : NULL;
>   }
> 
> +static const char *expr_func_fn(ap_expr_eval_ctx_t *ctx, const void
> *data, +const char *arg)
> +{
> +char *var = (char *)arg;
> +SSLConnRec *sslconn = myConnConfig(ctx->c);
> +
> +return (var && sslconn) ? ssl_var_lookup_ssl(ctx->p, ctx->c,
> ctx->r, var) : NULL;
> +}
> +
>   static int ssl_expr_lookup(ap_expr_lookup_parms *parms)
>   {
>   switch (parms->type) {
> @@ -163,6 +172,15 @@
>   return OK;
>   }
>   break;
> +case AP_EXPR_FUNC_STRING:
> +/* Function SSL() is implemented by us.
> + */
> +if (strcEQ(parms->name, "SSL")) {
> +*parms->func = expr_func_fn;
> +*parms->data = NULL;
> +return OK;
> +}
> +break;
>   case AP_EXPR_FUNC_LIST:
>   if (strcEQ(parms->name, "PeerExtList")) {
>   *parms->func = expr_peer_ext_list_fn;
> 
> 
> Regards,
> 
> Rainer