You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by yl...@apache.org on 2015/06/12 11:07:34 UTC

svn commit: r1685052 - in /httpd/httpd/trunk: CHANGES modules/ssl/ssl_engine_config.c

Author: ylavic
Date: Fri Jun 12 09:07:34 2015
New Revision: 1685052

URL: http://svn.apache.org/r1685052
Log:
mod_ssl: Warn about deprecated SSLCertificateChainFile once at startup,
on first usage only.

Modified:
    httpd/httpd/trunk/CHANGES
    httpd/httpd/trunk/modules/ssl/ssl_engine_config.c

Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1685052&r1=1685051&r2=1685052&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Fri Jun 12 09:07:34 2015
@@ -1,6 +1,9 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.5.0
 
+  *) mod_ssl: Warn about deprecated SSLCertificateChainFile once at startup,
+     on first usage only.  [Yann Ylavic]
+
   *) mod_substitute: Fix configuraton merge order.
      PR 57641 [<Marc.Stern approach.be>]
 

Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_config.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_config.c?rev=1685052&r1=1685051&r2=1685052&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ssl/ssl_engine_config.c (original)
+++ httpd/httpd/trunk/modules/ssl/ssl_engine_config.c Fri Jun 12 09:07:34 2015
@@ -839,13 +839,22 @@ const char *ssl_cmd_SSLCertificateChainF
                                             const char *arg)
 {
     SSLSrvConfigRec *sc = mySrvConfig(cmd->server);
+    void *once = NULL;
     const char *err;
 
-    ap_log_error(APLOG_MARK, APLOG_WARNING|APLOG_STARTUP, 0, NULL,
-                 APLOGNO(02559)
-                 "The SSLCertificateChainFile directive (%s:%d) is deprecated, "
-                 "SSLCertificateFile should be used instead",
-                 cmd->directive->filename, cmd->directive->line_num);
+    apr_pool_userdata_get(&once, "ssl_cmd_SSLCertificateChainFile",
+                          ap_pglobal);
+    if (!once) {
+        ap_log_error(APLOG_MARK, APLOG_WARNING|APLOG_STARTUP, 0, NULL,
+                     APLOGNO(02559)
+                     "The SSLCertificateChainFile directive (%s:%d) is "
+                     "deprecated, SSLCertificateFile should be used instead",
+                     cmd->directive->filename, cmd->directive->line_num);
+
+        apr_pool_userdata_set("ssl_cmd_SSLCertificateChainFile",
+                              apr_pstrdup(ap_pglobal, "1"), NULL,
+                              ap_pglobal);
+    }
 
     if ((err = ssl_cmd_check_file(cmd, &arg))) {
         return err;



Re: svn commit: r1685052 - in /httpd/httpd/trunk: CHANGES modules/ssl/ssl_engine_config.c

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Fri, Jun 12, 2015 at 8:36 AM, Rainer Jung <ra...@kippdata.de>
wrote:

> Am 12.06.2015 um 13:49 schrieb Yann Ylavic:
>
>> Hi Christophe,
>>
>> On Fri, Jun 12, 2015 at 1:26 PM, Christophe JAILLET
>> <ch...@wanadoo.fr> wrote:
>>
>>>
>>> should this warning at startup be an issue, why not just remove it in
>>> 2.4.x
>>> and keep it in trunk?
>>> Having the depreciation written in doc (and in migration note for 2.4 ->
>>> 2.6/3.0 one day) could be enough, no?
>>>
>>
>> Probably, but the (only one) warning per startup can possibly
>> encourage users to look at the doc :)
>> Let's see what others say.
>>
>
> I don't have a strong opinion, but if we want to remove the directive in
> the next version, then it would help to keep the deprecation logging to
> make people aware and reduce the migration complains a bit.
>
> Logging only once instead of for each certificate is IMHO enough and much
> better.
>

Agreed, although this is never a [warn], it's [info] at strongest.


> As to if the current situation in 2.4.14 is a problem or should be
> tolerated (lots of log output for people with huge numbers of certificates)
> I'm quite undecided.
>

Trivial patch, easily applied by the tiny few who would be affected, so I
don't think it has any impact on the 2.4.14 readiness for GA.

Bill

Patch: message received and acknowledged;

--- modules/ssl/ssl_engine_config.c     (revision 8078)
+++ modules/ssl/ssl_engine_config.c     (working copy)
@@ -862,7 +862,7 @@
     SSLSrvConfigRec *sc = mySrvConfig(cmd->server);
     const char *err;

-    ap_log_error(APLOG_MARK, APLOG_WARNING|APLOG_STARTUP, 0, NULL,
+    ap_log_error(APLOG_MARK, APLOG_DEBUG|APLOG_STARTUP, 0, NULL,
                  APLOGNO(02559)
                  "The SSLCertificateChainFile directive (%s:%d) is
deprecated, "
                  "SSLCertificateFile should be used instead",

Re: svn commit: r1685052 - in /httpd/httpd/trunk: CHANGES modules/ssl/ssl_engine_config.c

Posted by Rainer Jung <ra...@kippdata.de>.
Am 12.06.2015 um 13:49 schrieb Yann Ylavic:
> Hi Christophe,
>
> On Fri, Jun 12, 2015 at 1:26 PM, Christophe JAILLET
> <ch...@wanadoo.fr> wrote:
>>
>> should this warning at startup be an issue, why not just remove it in 2.4.x
>> and keep it in trunk?
>> Having the depreciation written in doc (and in migration note for 2.4 ->
>> 2.6/3.0 one day) could be enough, no?
>
> Probably, but the (only one) warning per startup can possibly
> encourage users to look at the doc :)
> Let's see what others say.

I don't have a strong opinion, but if we want to remove the directive in 
the next version, then it would help to keep the deprecation logging to 
make people aware and reduce the migration complains a bit.

Logging only once instead of for each certificate is IMHO enough and 
much better.

As to if the current situation in 2.4.14 is a problem or should be 
tolerated (lots of log output for people with huge numbers of 
certificates) I'm quite undecided.

Regards,

Rainer


Re: svn commit: r1685052 - in /httpd/httpd/trunk: CHANGES modules/ssl/ssl_engine_config.c

Posted by Yann Ylavic <yl...@gmail.com>.
Hi Christophe,

On Fri, Jun 12, 2015 at 1:26 PM, Christophe JAILLET
<ch...@wanadoo.fr> wrote:
>
> should this warning at startup be an issue, why not just remove it in 2.4.x
> and keep it in trunk?
> Having the depreciation written in doc (and in migration note for 2.4 ->
> 2.6/3.0 one day) could be enough, no?

Probably, but the (only one) warning per startup can possibly
encourage users to look at the doc :)
Let's see what others say.

>
>
> BTW, the apr_pstrdup(ap_pglobal, "1") could be just "1".

I'm not sure, if mod_ssl is loaded dynamically, "1" could possibly be
un/reloaded with it (.text section)...

Regards,
Yann.

Re: svn commit: r1685052 - in /httpd/httpd/trunk: CHANGES modules/ssl/ssl_engine_config.c

Posted by Christophe JAILLET <ch...@wanadoo.fr>.
Hi,

should this warning at startup be an issue, why not just remove it in 
2.4.x and keep it in trunk?
Having the depreciation written in doc (and in migration note for 2.4 -> 
2.6/3.0 one day) could be enough, no?


BTW, the apr_pstrdup(ap_pglobal, "1") could be just "1".

CJ

Le 12/06/2015 11:07, ylavic@apache.org a écrit :
> Author: ylavic
> Date: Fri Jun 12 09:07:34 2015
> New Revision: 1685052
>
> URL: http://svn.apache.org/r1685052
> Log:
> mod_ssl: Warn about deprecated SSLCertificateChainFile once at startup,
> on first usage only.
>
> Modified:
>      httpd/httpd/trunk/CHANGES
>      httpd/httpd/trunk/modules/ssl/ssl_engine_config.c
>
> Modified: httpd/httpd/trunk/CHANGES
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1685052&r1=1685051&r2=1685052&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
> +++ httpd/httpd/trunk/CHANGES [utf-8] Fri Jun 12 09:07:34 2015
> @@ -1,6 +1,9 @@
>                                                            -*- coding: utf-8 -*-
>   Changes with Apache 2.5.0
>   
> +  *) mod_ssl: Warn about deprecated SSLCertificateChainFile once at startup,
> +     on first usage only.  [Yann Ylavic]
> +
>     *) mod_substitute: Fix configuraton merge order.
>        PR 57641 [<Marc.Stern approach.be>]
>   
>
> Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_config.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_config.c?rev=1685052&r1=1685051&r2=1685052&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/ssl/ssl_engine_config.c (original)
> +++ httpd/httpd/trunk/modules/ssl/ssl_engine_config.c Fri Jun 12 09:07:34 2015
> @@ -839,13 +839,22 @@ const char *ssl_cmd_SSLCertificateChainF
>                                               const char *arg)
>   {
>       SSLSrvConfigRec *sc = mySrvConfig(cmd->server);
> +    void *once = NULL;
>       const char *err;
>   
> -    ap_log_error(APLOG_MARK, APLOG_WARNING|APLOG_STARTUP, 0, NULL,
> -                 APLOGNO(02559)
> -                 "The SSLCertificateChainFile directive (%s:%d) is deprecated, "
> -                 "SSLCertificateFile should be used instead",
> -                 cmd->directive->filename, cmd->directive->line_num);
> +    apr_pool_userdata_get(&once, "ssl_cmd_SSLCertificateChainFile",
> +                          ap_pglobal);
> +    if (!once) {
> +        ap_log_error(APLOG_MARK, APLOG_WARNING|APLOG_STARTUP, 0, NULL,
> +                     APLOGNO(02559)
> +                     "The SSLCertificateChainFile directive (%s:%d) is "
> +                     "deprecated, SSLCertificateFile should be used instead",
> +                     cmd->directive->filename, cmd->directive->line_num);
> +
> +        apr_pool_userdata_set("ssl_cmd_SSLCertificateChainFile",
> +                              apr_pstrdup(ap_pglobal, "1"), NULL,
> +                              ap_pglobal);
> +    }
>   
>       if ((err = ssl_cmd_check_file(cmd, &arg))) {
>           return err;
>
>
>


Re: svn commit: r1685052 - in /httpd/httpd/trunk: CHANGES modules/ssl/ssl_engine_config.c

Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, Jun 12, 2015 at 2:11 PM, Jeff Trawick <tr...@gmail.com> wrote:
>
> IMHO the  ap_retained_data_get/create APIs make this "nicer" than this older
> pattern.

Ah yes, good catch, I just did not think about it.

>
>     retained = ap_retained_data_get(userdata_key);
>     if (!retained) {
>         retained = ap_retained_data_create(userdata_key, sizeof(*retained))

Will change to use that instead in a follow up.
Unless the warning should be removed completely...

Re: svn commit: r1685052 - in /httpd/httpd/trunk: CHANGES modules/ssl/ssl_engine_config.c

Posted by Jeff Trawick <tr...@gmail.com>.
On Fri, Jun 12, 2015 at 5:07 AM, <yl...@apache.org> wrote:

> Author: ylavic
> Date: Fri Jun 12 09:07:34 2015
> New Revision: 1685052
>
> URL: http://svn.apache.org/r1685052
> Log:
> mod_ssl: Warn about deprecated SSLCertificateChainFile once at startup,
> on first usage only.
>
> Modified:
>     httpd/httpd/trunk/CHANGES
>     httpd/httpd/trunk/modules/ssl/ssl_engine_config.c
>
> Modified: httpd/httpd/trunk/CHANGES
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1685052&r1=1685051&r2=1685052&view=diff
>
> ==============================================================================
> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
> +++ httpd/httpd/trunk/CHANGES [utf-8] Fri Jun 12 09:07:34 2015
> @@ -1,6 +1,9 @@
>                                                           -*- coding:
> utf-8 -*-
>  Changes with Apache 2.5.0
>
> +  *) mod_ssl: Warn about deprecated SSLCertificateChainFile once at
> startup,
> +     on first usage only.  [Yann Ylavic]
> +
>    *) mod_substitute: Fix configuraton merge order.
>       PR 57641 [<Marc.Stern approach.be>]
>
>
> Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_config.c
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_config.c?rev=1685052&r1=1685051&r2=1685052&view=diff
>
> ==============================================================================
> --- httpd/httpd/trunk/modules/ssl/ssl_engine_config.c (original)
> +++ httpd/httpd/trunk/modules/ssl/ssl_engine_config.c Fri Jun 12 09:07:34
> 2015
> @@ -839,13 +839,22 @@ const char *ssl_cmd_SSLCertificateChainF
>                                              const char *arg)
>  {
>      SSLSrvConfigRec *sc = mySrvConfig(cmd->server);
> +    void *once = NULL;
>      const char *err;
>
> -    ap_log_error(APLOG_MARK, APLOG_WARNING|APLOG_STARTUP, 0, NULL,
> -                 APLOGNO(02559)
> -                 "The SSLCertificateChainFile directive (%s:%d) is
> deprecated, "
> -                 "SSLCertificateFile should be used instead",
> -                 cmd->directive->filename, cmd->directive->line_num);
> +    apr_pool_userdata_get(&once, "ssl_cmd_SSLCertificateChainFile",
> +                          ap_pglobal);
> +    if (!once) {
> +        ap_log_error(APLOG_MARK, APLOG_WARNING|APLOG_STARTUP, 0, NULL,
> +                     APLOGNO(02559)
> +                     "The SSLCertificateChainFile directive (%s:%d) is "
> +                     "deprecated, SSLCertificateFile should be used
> instead",
> +                     cmd->directive->filename, cmd->directive->line_num);
> +
> +        apr_pool_userdata_set("ssl_cmd_SSLCertificateChainFile",
> +                              apr_pstrdup(ap_pglobal, "1"), NULL,
> +                              ap_pglobal);
> +    }
>
> IMHO the  ap_retained_data_get/create APIs make this "nicer" than this
older pattern.

    retained = ap_retained_data_get(userdata_key);
    if (!retained) {
        retained = ap_retained_data_create(userdata_key, sizeof(*retained))



     if ((err = ssl_cmd_check_file(cmd, &arg))) {
>          return err;
>
>
>


-- 
Born in Roswell... married an alien...
http://emptyhammock.com/