subject:"svn commit\: r1679032 \- in \/httpd\/httpd\/trunk\: CHANGES docs\/manual\/mod\/mod_ssl.xml modules\/ssl\/mod_ssl.c modules\/ssl\/ssl_engine_config.c modules\/ssl\/ssl_private.h modules\/ssl\/ssl_util

Re: svn commit: r1679032 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_ssl.xml modules/ssl/mod_ssl.c modules/ssl/ssl_engine_config.c modules/ssl/ssl_private.h modules/ssl/ssl_util_stapling.c

2015-05-13 Thread Jeff Trawick


On 05/12/2015 04:50 PM, Jeff Trawick wrote:

On 05/12/2015 03:32 PM, Yann Ylavic wrote:

On Tue, May 12, 2015 at 8:59 PM, traw...@apache.org wrote:

Author: trawick
Date: Tue May 12 18:59:29 2015
New Revision: 1679032

URL: http://svn.apache.org/r1679032
Log:
mod_ssl OCSP Stapling: Don't block initial handshakes while refreshing
the OCSP response for a different certificate.  mod_ssl has an 
additional

global mutex, ssl-stapling-refresh.


[]

Modified: httpd/httpd/trunk/modules/ssl/ssl_util_stapling.c
URL: 
http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_util_stapling.c?rev=1679032r1=1679031r2=1679032view=diff
== 


--- httpd/httpd/trunk/modules/ssl/ssl_util_stapling.c (original)
+++ httpd/httpd/trunk/modules/ssl/ssl_util_stapling.c Tue May 12 
18:59:29 2015

[]

+
+static int get_and_check_cached_response(server_rec *s, 
modssl_ctx_t *mctx,
+ OCSP_RESPONSE **rsp, BOOL 
*ok,
+ certinfo *cinf, apr_pool_t 
*p)

+{
+int rv;
+
+/* Check to see if we already have a response for this 
certificate */

+rv = stapling_get_cached_response(s, rsp, ok, cinf, p);
+if (rv == FALSE) {
+return SSL_TLSEXT_ERR_ALERT_FATAL;
+}
+
+if (*rsp) {
+/* see if response is acceptable */
+ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(01953)
+ stapling_cb: retrieved cached response);
+rv = stapling_check_response(s, mctx, cinf, *rsp, NULL);
+if (rv == SSL_TLSEXT_ERR_ALERT_FATAL) {
+OCSP_RESPONSE_free(*rsp);
+return SSL_TLSEXT_ERR_ALERT_FATAL;
+}
+else if (rv == SSL_TLSEXT_ERR_NOACK) {
+/* Error in response. If this error was not present 
when it was

+ * stored (i.e. response no longer valid) then it can be
+ * renewed straight away.
+ *
+ * If the error *was* present at the time it was stored 
then we
+ * don't renew the response straight away; we just wait 
for the

+ * cached response to expire.
+ */
+if (ok) {

if (*ok) ?
Or maybe 'ok' shouldn't be a pointer (not updated here)?


Thanks a bunch!  I'll sort it out tomorrow 


r1679192


and make sure I'm testing more paths.


TBD :)

Thanks again!




+ OCSP_RESPONSE_free(*rsp);
+*rsp = NULL;
+}
+else if (!mctx-stapling_return_errors) {
+OCSP_RESPONSE_free(*rsp);
+return SSL_TLSEXT_ERR_NOACK;
+}
+}
+}
+return 0;
+}
+

Regards,
Yann.

Re: svn commit: r1679032 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_ssl.xml modules/ssl/mod_ssl.c modules/ssl/ssl_engine_config.c modules/ssl/ssl_private.h modules/ssl/ssl_util_stapling.c

2015-05-13 Thread Yann Ylavic

On Wed, May 13, 2015 at 2:34 PM, Jeff Trawick traw...@gmail.com wrote:

 Thanks again!

You're welcome ;)

WDYT of the following?
(cosmetic only, but helps read/reuse-ability a bit)

Index: modules/ssl/ssl_util_stapling.c
===
--- modules/ssl/ssl_util_stapling.c(revision 1679195)
+++ modules/ssl/ssl_util_stapling.c(working copy)
@@ -250,13 +250,11 @@ static BOOL stapling_cache_response(server_rec *s,

 i2d_OCSP_RESPONSE(rsp, p);

-if (mc-stapling_cache-flags  AP_SOCACHE_FLAG_NOTMPSAFE)
-stapling_cache_mutex_on(s);
+stapling_cache_mutex_on(s);
 rv = mc-stapling_cache-store(mc-stapling_cache_context, s,
cinf-idx, sizeof(cinf-idx),
expiry, resp_der, stored_len, pool);
-if (mc-stapling_cache-flags  AP_SOCACHE_FLAG_NOTMPSAFE)
-stapling_cache_mutex_off(s);
+stapling_cache_mutex_off(s);
 if (rv != APR_SUCCESS) {
 ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(01929)
  stapling_cache_response: OCSP response session
store error!);
@@ -277,13 +275,11 @@ static BOOL stapling_get_cached_response(server_re
 const unsigned char *p;
 unsigned int resp_derlen = MAX_STAPLING_DER;

-if (mc-stapling_cache-flags  AP_SOCACHE_FLAG_NOTMPSAFE)
-stapling_cache_mutex_on(s);
+stapling_cache_mutex_on(s);
 rv = mc-stapling_cache-retrieve(mc-stapling_cache_context, s,
   cinf-idx, sizeof(cinf-idx),
   resp_der, resp_derlen, pool);
-if (mc-stapling_cache-flags  AP_SOCACHE_FLAG_NOTMPSAFE)
-stapling_cache_mutex_off(s);
+stapling_cache_mutex_off(s);
 if (rv != APR_SUCCESS) {
 ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(01930)
  stapling_get_cached_response: cache miss);
@@ -623,8 +619,11 @@ static int stapling_cache_mutex_on(server_rec *s)
 {
 SSLModConfigRec *mc = myModConfig(s);

-return stapling_mutex_on(s, mc-stapling_cache_mutex,
- SSL_STAPLING_CACHE_MUTEX_TYPE);
+if (mc-stapling_cache-flags  AP_SOCACHE_FLAG_NOTMPSAFE) {
+return stapling_mutex_on(s, mc-stapling_cache_mutex,
+ SSL_STAPLING_CACHE_MUTEX_TYPE);
+}
+return TRUE;
 }

 static int stapling_cache_mutex_off(server_rec *s)
@@ -631,8 +630,11 @@ static int stapling_cache_mutex_off(server_rec *s)
 {
 SSLModConfigRec *mc = myModConfig(s);

-return stapling_mutex_off(s, mc-stapling_cache_mutex,
-  SSL_STAPLING_CACHE_MUTEX_TYPE);
+if (mc-stapling_cache-flags  AP_SOCACHE_FLAG_NOTMPSAFE) {
+return stapling_mutex_off(s, mc-stapling_cache_mutex,
+  SSL_STAPLING_CACHE_MUTEX_TYPE);
+}
+return TRUE;
 }

 static int stapling_refresh_mutex_on(server_rec *s)
--

Re: svn commit: r1679032 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_ssl.xml modules/ssl/mod_ssl.c modules/ssl/ssl_engine_config.c modules/ssl/ssl_private.h modules/ssl/ssl_util_stapling.c

2015-05-13 Thread Jeff Trawick


On 05/13/2015 08:59 AM, Yann Ylavic wrote:

On Wed, May 13, 2015 at 2:34 PM, Jeff Trawick traw...@gmail.com wrote:

Thanks again!

You're welcome ;)

WDYT of the following?
(cosmetic only, but helps read/reuse-ability a bit)

Index: modules/ssl/ssl_util_stapling.c
===
--- modules/ssl/ssl_util_stapling.c(revision 1679195)
+++ modules/ssl/ssl_util_stapling.c(working copy)
@@ -250,13 +250,11 @@ static BOOL stapling_cache_response(server_rec *s,

  i2d_OCSP_RESPONSE(rsp, p);

-if (mc-stapling_cache-flags  AP_SOCACHE_FLAG_NOTMPSAFE)
-stapling_cache_mutex_on(s);
+stapling_cache_mutex_on(s);
  rv = mc-stapling_cache-store(mc-stapling_cache_context, s,
 cinf-idx, sizeof(cinf-idx),
 expiry, resp_der, stored_len, pool);
-if (mc-stapling_cache-flags  AP_SOCACHE_FLAG_NOTMPSAFE)
-stapling_cache_mutex_off(s);
+stapling_cache_mutex_off(s);


At the moment I very slightly prefer seeing the reminder that there 
isn't always a mutex, but I won't care before long.  I prefer that this 
matches the implementation of the session cache mutex on where the 
socache flag is checked, but if it makes you happy and you change the 
session cache equivalent to match then go for it :)



  if (rv != APR_SUCCESS) {
  ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(01929)
   stapling_cache_response: OCSP response session
store error!);
@@ -277,13 +275,11 @@ static BOOL stapling_get_cached_response(server_re
  const unsigned char *p;
  unsigned int resp_derlen = MAX_STAPLING_DER;

-if (mc-stapling_cache-flags  AP_SOCACHE_FLAG_NOTMPSAFE)
-stapling_cache_mutex_on(s);
+stapling_cache_mutex_on(s);
  rv = mc-stapling_cache-retrieve(mc-stapling_cache_context, s,
cinf-idx, sizeof(cinf-idx),
resp_der, resp_derlen, pool);
-if (mc-stapling_cache-flags  AP_SOCACHE_FLAG_NOTMPSAFE)
-stapling_cache_mutex_off(s);
+stapling_cache_mutex_off(s);
  if (rv != APR_SUCCESS) {
  ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(01930)
   stapling_get_cached_response: cache miss);
@@ -623,8 +619,11 @@ static int stapling_cache_mutex_on(server_rec *s)
  {
  SSLModConfigRec *mc = myModConfig(s);

-return stapling_mutex_on(s, mc-stapling_cache_mutex,
- SSL_STAPLING_CACHE_MUTEX_TYPE);
+if (mc-stapling_cache-flags  AP_SOCACHE_FLAG_NOTMPSAFE) {
+return stapling_mutex_on(s, mc-stapling_cache_mutex,
+ SSL_STAPLING_CACHE_MUTEX_TYPE);
+}
+return TRUE;
  }

  static int stapling_cache_mutex_off(server_rec *s)
@@ -631,8 +630,11 @@ static int stapling_cache_mutex_off(server_rec *s)
  {
  SSLModConfigRec *mc = myModConfig(s);

-return stapling_mutex_off(s, mc-stapling_cache_mutex,
-  SSL_STAPLING_CACHE_MUTEX_TYPE);
+if (mc-stapling_cache-flags  AP_SOCACHE_FLAG_NOTMPSAFE) {
+return stapling_mutex_off(s, mc-stapling_cache_mutex,
+  SSL_STAPLING_CACHE_MUTEX_TYPE);
+}
+return TRUE;
  }

  static int stapling_refresh_mutex_on(server_rec *s)
--

Re: svn commit: r1679032 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_ssl.xml modules/ssl/mod_ssl.c modules/ssl/ssl_engine_config.c modules/ssl/ssl_private.h modules/ssl/ssl_util_stapling.c

2015-05-13 Thread Yann Ylavic

On Wed, May 13, 2015 at 4:28 PM, Jeff Trawick traw...@gmail.com wrote:
 On 05/13/2015 08:59 AM, Yann Ylavic wrote:

 On Wed, May 13, 2015 at 2:34 PM, Jeff Trawick traw...@gmail.com wrote:

 Thanks again!

 You're welcome ;)

 WDYT of the following?
 (cosmetic only, but helps read/reuse-ability a bit)

 Index: modules/ssl/ssl_util_stapling.c
 ===
 --- modules/ssl/ssl_util_stapling.c(revision 1679195)
 +++ modules/ssl/ssl_util_stapling.c(working copy)
 @@ -250,13 +250,11 @@ static BOOL stapling_cache_response(server_rec *s,

   i2d_OCSP_RESPONSE(rsp, p);

 -if (mc-stapling_cache-flags  AP_SOCACHE_FLAG_NOTMPSAFE)
 -stapling_cache_mutex_on(s);
 +stapling_cache_mutex_on(s);
   rv = mc-stapling_cache-store(mc-stapling_cache_context, s,
  cinf-idx, sizeof(cinf-idx),
  expiry, resp_der, stored_len, pool);
 -if (mc-stapling_cache-flags  AP_SOCACHE_FLAG_NOTMPSAFE)
 -stapling_cache_mutex_off(s);
 +stapling_cache_mutex_off(s);


 At the moment I very slightly prefer seeing the reminder that there isn't
 always a mutex, but I won't care before long.  I prefer that this matches
 the implementation of the session cache mutex on where the socache flag is
 checked, but if it makes you happy and you change the session cache
 equivalent to match then go for it :)

Nope, I have no strong opinion either, let's keep it as is, that makes sense.

Re: svn commit: r1679032 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_ssl.xml modules/ssl/mod_ssl.c modules/ssl/ssl_engine_config.c modules/ssl/ssl_private.h modules/ssl/ssl_util_stapling.c

2015-05-12 Thread Yann Ylavic

On Tue, May 12, 2015 at 8:59 PM,  traw...@apache.org wrote:
 Author: trawick
 Date: Tue May 12 18:59:29 2015
 New Revision: 1679032

 URL: http://svn.apache.org/r1679032
 Log:
 mod_ssl OCSP Stapling: Don't block initial handshakes while refreshing
 the OCSP response for a different certificate.  mod_ssl has an additional
 global mutex, ssl-stapling-refresh.

[]

 Modified: httpd/httpd/trunk/modules/ssl/ssl_util_stapling.c
 URL: 
 http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_util_stapling.c?rev=1679032r1=1679031r2=1679032view=diff
 ==
 --- httpd/httpd/trunk/modules/ssl/ssl_util_stapling.c (original)
 +++ httpd/httpd/trunk/modules/ssl/ssl_util_stapling.c Tue May 12 18:59:29 2015
[]
 +
 +static int get_and_check_cached_response(server_rec *s, modssl_ctx_t *mctx,
 + OCSP_RESPONSE **rsp, BOOL *ok,
 + certinfo *cinf, apr_pool_t *p)
 +{
 +int rv;
 +
 +/* Check to see if we already have a response for this certificate */
 +rv = stapling_get_cached_response(s, rsp, ok, cinf, p);
 +if (rv == FALSE) {
 +return SSL_TLSEXT_ERR_ALERT_FATAL;
 +}
 +
 +if (*rsp) {
 +/* see if response is acceptable */
 +ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(01953)
 + stapling_cb: retrieved cached response);
 +rv = stapling_check_response(s, mctx, cinf, *rsp, NULL);
 +if (rv == SSL_TLSEXT_ERR_ALERT_FATAL) {
 +OCSP_RESPONSE_free(*rsp);
 +return SSL_TLSEXT_ERR_ALERT_FATAL;
 +}
 +else if (rv == SSL_TLSEXT_ERR_NOACK) {
 +/* Error in response. If this error was not present when it was
 + * stored (i.e. response no longer valid) then it can be
 + * renewed straight away.
 + *
 + * If the error *was* present at the time it was stored then we
 + * don't renew the response straight away; we just wait for the
 + * cached response to expire.
 + */
 +if (ok) {

if (*ok) ?
Or maybe 'ok' shouldn't be a pointer (not updated here)?

 +OCSP_RESPONSE_free(*rsp);
 +*rsp = NULL;
 +}
 +else if (!mctx-stapling_return_errors) {
 +OCSP_RESPONSE_free(*rsp);
 +return SSL_TLSEXT_ERR_NOACK;
 +}
 +}
 +}
 +return 0;
 +}
 +

Regards,
Yann.

Re: svn commit: r1679032 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_ssl.xml modules/ssl/mod_ssl.c modules/ssl/ssl_engine_config.c modules/ssl/ssl_private.h modules/ssl/ssl_util_stapling.c

2015-05-12 Thread Jeff Trawick


On 05/12/2015 03:32 PM, Yann Ylavic wrote:

On Tue, May 12, 2015 at 8:59 PM,  traw...@apache.org wrote:

Author: trawick
Date: Tue May 12 18:59:29 2015
New Revision: 1679032

URL: http://svn.apache.org/r1679032
Log:
mod_ssl OCSP Stapling: Don't block initial handshakes while refreshing
the OCSP response for a different certificate.  mod_ssl has an additional
global mutex, ssl-stapling-refresh.


[]

Modified: httpd/httpd/trunk/modules/ssl/ssl_util_stapling.c
URL: 
http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_util_stapling.c?rev=1679032r1=1679031r2=1679032view=diff
==
--- httpd/httpd/trunk/modules/ssl/ssl_util_stapling.c (original)
+++ httpd/httpd/trunk/modules/ssl/ssl_util_stapling.c Tue May 12 18:59:29 2015

[]

+
+static int get_and_check_cached_response(server_rec *s, modssl_ctx_t *mctx,
+ OCSP_RESPONSE **rsp, BOOL *ok,
+ certinfo *cinf, apr_pool_t *p)
+{
+int rv;
+
+/* Check to see if we already have a response for this certificate */
+rv = stapling_get_cached_response(s, rsp, ok, cinf, p);
+if (rv == FALSE) {
+return SSL_TLSEXT_ERR_ALERT_FATAL;
+}
+
+if (*rsp) {
+/* see if response is acceptable */
+ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(01953)
+ stapling_cb: retrieved cached response);
+rv = stapling_check_response(s, mctx, cinf, *rsp, NULL);
+if (rv == SSL_TLSEXT_ERR_ALERT_FATAL) {
+OCSP_RESPONSE_free(*rsp);
+return SSL_TLSEXT_ERR_ALERT_FATAL;
+}
+else if (rv == SSL_TLSEXT_ERR_NOACK) {
+/* Error in response. If this error was not present when it was
+ * stored (i.e. response no longer valid) then it can be
+ * renewed straight away.
+ *
+ * If the error *was* present at the time it was stored then we
+ * don't renew the response straight away; we just wait for the
+ * cached response to expire.
+ */
+if (ok) {

if (*ok) ?
Or maybe 'ok' shouldn't be a pointer (not updated here)?


Thanks a bunch!  I'll sort it out tomorrow and make sure I'm testing 
more paths.





+OCSP_RESPONSE_free(*rsp);
+*rsp = NULL;
+}
+else if (!mctx-stapling_return_errors) {
+OCSP_RESPONSE_free(*rsp);
+return SSL_TLSEXT_ERR_NOACK;
+}
+}
+}
+return 0;
+}
+

Regards,
Yann.

Re: svn commit: r1679032 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_ssl.xml modules/ssl/mod_ssl.c modules/ssl/ssl_engine_config.c modules/ssl/ssl_private.h modules/ssl/ssl_util_stapling.c

Re: svn commit: r1679032 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_ssl.xml modules/ssl/mod_ssl.c modules/ssl/ssl_engine_config.c modules/ssl/ssl_private.h modules/ssl/ssl_util_stapling.c

Re: svn commit: r1679032 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_ssl.xml modules/ssl/mod_ssl.c modules/ssl/ssl_engine_config.c modules/ssl/ssl_private.h modules/ssl/ssl_util_stapling.c

Re: svn commit: r1679032 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_ssl.xml modules/ssl/mod_ssl.c modules/ssl/ssl_engine_config.c modules/ssl/ssl_private.h modules/ssl/ssl_util_stapling.c

Re: svn commit: r1679032 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_ssl.xml modules/ssl/mod_ssl.c modules/ssl/ssl_engine_config.c modules/ssl/ssl_private.h modules/ssl/ssl_util_stapling.c

Re: svn commit: r1679032 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_ssl.xml modules/ssl/mod_ssl.c modules/ssl/ssl_engine_config.c modules/ssl/ssl_private.h modules/ssl/ssl_util_stapling.c

6 matches

Site Navigation

Mail list logo

Footer information