Re: DH params and multiple certificates in one VHost

2014-04-21 Thread Kaspar Brand
On 19.04.2014 09:37, Falco Schwarz wrote:
 I successfully tested your attached patch with the latest 1.0.2
 branch. The DH temp key now has the bit length of the used RSA key,
 regardless of SSLCertificate[Key]File order.

Thanks for testing. Committed to trunk with r1588851 and proposed for
backport to 2.4.x.

Kaspar


Re: SSL_CTX_get_{first,next}_certificate

2014-04-21 Thread Kaspar Brand
On 05.02.2014 14:05, Kaspar Brand wrote:
 On 03.02.2014 12:21, Dr Stephen Henson wrote:
 On 02/02/2014 13:45, Kaspar Brand wrote:
 Yes, this sounds like a useful extension - not only for the issue at
 hand (i.e. SSL_CONF and stapling initialisation), but as a general
 mechanism for retrieving all certificates of an SSL_CTX.


 Added now. The API is slightly different, but easy enough to use.
 
 I have adapted the stapling init code in trunk to switch to this
 mechanism with r1564760 (just committed). Reviews appreciated, would
 afterwards propose for backport.

Steve, I just noticed that using SSL_CTX_set_current_cert became
broken with [1] and [2], respectively - SSL_CTX_get0_certificate
may now return bogus pointers (and we segfault when trying to
dereference them, worst case). Patch for ssl_cert.c attached.

Kaspar


[1] 
https://git.openssl.org/gitweb/?p=openssl.git;a=commitdiff;h=358d352aa244b4f2ef655bccff6658d92d5ce03c

[2] 
https://git.openssl.org/gitweb/?p=openssl.git;a=commitdiff;h=c3f5d3d93ac81c2866a739f1981d948e6aba1fde
diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c
index 830490e..aaa6e0a 100644
--- a/ssl/ssl_cert.c
+++ b/ssl/ssl_cert.c
@@ -664,7 +664,7 @@ int ssl_cert_set_current(CERT *c, long op)
return 0;
for (i = idx; i  SSL_PKEY_NUM; i++)
{
-   CERT_PKEY *cpk = c-key + i;
+   CERT_PKEY *cpk = c-pkeys + i;
if (cpk-x509  cpk-privatekey)
{
c-key = cpk;


Re: svn commit: r1587607 - in /httpd/httpd/trunk: ./ include/ modules/ssl/

2014-04-21 Thread Kaspar Brand
On 15.04.2014 17:25, traw...@apache.org wrote:
 Author: trawick
 Date: Tue Apr 15 15:25:03 2014
 New Revision: 1587607
 
 URL: http://svn.apache.org/r1587607
 Log:
 mod_ssl: Add hooks to allow other modules to perform processing at
 several stages of initialization and connection handling.  See
 mod_ssl_openssl.h.
 
 This is enough to allow implementation of Certificate Transparency
 outside of mod_ssl.
 

[...]

 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=1587607r1=1587606r2=1587607view=diff
 ==
 --- httpd/httpd/trunk/modules/ssl/ssl_engine_io.c (original)
 +++ httpd/httpd/trunk/modules/ssl/ssl_engine_io.c Tue Apr 15 15:25:03 2014
 @@ -29,8 +29,13 @@
  -- Unknown*/
  #include ssl_private.h
  #include mod_ssl.h
 +#include mod_ssl_openssl.h
  #include apr_date.h
  
 +APR_IMPLEMENT_OPTIONAL_HOOK_RUN_ALL(ssl, SSL, int, proxy_post_handshake,
 +(conn_rec *c,SSL *ssl),
 +(c,ssl),OK,DECLINED);
 +
  /*  _
  **
  **  I/O Hooks
 @@ -1119,6 +1124,8 @@ static apr_status_t ssl_io_filter_handsh
  const char *hostname_note = apr_table_get(c-notes,
proxy-request-hostname);
  BOOL proxy_ssl_check_peer_ok = TRUE;
 +int post_handshake_rc;
 +
  sc = mySrvConfig(server);
  
  #ifdef HAVE_TLSEXT
 @@ -1208,11 +1215,17 @@ static apr_status_t ssl_io_filter_handsh
  }
  }
  
 +if (proxy_ssl_check_peer_ok == TRUE) {
 +/* another chance to fail */
 +post_handshake_rc = ssl_run_proxy_post_handshake(c, 
 filter_ctx-pssl);
 +}
 +
  if (cert) {
  X509_free(cert);
  }
  
 -if (proxy_ssl_check_peer_ok != TRUE) {
 +if (proxy_ssl_check_peer_ok != TRUE
 +|| (post_handshake_rc != OK  post_handshake_rc != DECLINED)) {
  /* ensure that the SSL structures etc are freed, etc: */
  ssl_filter_io_shutdown(filter_ctx, c, 1);
  apr_table_setn(c-notes, SSL_connect_rv, err);

GCC warns about 'post_handshake_rc' may be used uninitialized in this
function... which would be true for the proxy_ssl_check_peer_ok !=
TRUE case (line 1228), I think.

Kaspar


Re: svn commit: r1587607 - in /httpd/httpd/trunk: ./ include/ modules/ssl/

2014-04-21 Thread Jeff Trawick
On Mon, Apr 21, 2014 at 4:01 AM, Kaspar Brand httpd-dev.2...@velox.chwrote:

 On 15.04.2014 17:25, traw...@apache.org wrote:
  Author: trawick
  Date: Tue Apr 15 15:25:03 2014
  New Revision: 1587607
 
  URL: http://svn.apache.org/r1587607
  Log:
  mod_ssl: Add hooks to allow other modules to perform processing at
  several stages of initialization and connection handling.  See
  mod_ssl_openssl.h.
 
  This is enough to allow implementation of Certificate Transparency
  outside of mod_ssl.
 

 [...]

  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=1587607r1=1587606r2=1587607view=diff
 
 ==
  --- httpd/httpd/trunk/modules/ssl/ssl_engine_io.c (original)
  +++ httpd/httpd/trunk/modules/ssl/ssl_engine_io.c Tue Apr 15 15:25:03
 2014
  @@ -29,8 +29,13 @@
   -- Unknown*/
   #include ssl_private.h
   #include mod_ssl.h
  +#include mod_ssl_openssl.h
   #include apr_date.h
 
  +APR_IMPLEMENT_OPTIONAL_HOOK_RUN_ALL(ssl, SSL, int, proxy_post_handshake,
  +(conn_rec *c,SSL *ssl),
  +(c,ssl),OK,DECLINED);
  +
   /*  _
   **
   **  I/O Hooks
  @@ -1119,6 +1124,8 @@ static apr_status_t ssl_io_filter_handsh
   const char *hostname_note = apr_table_get(c-notes,
 
  proxy-request-hostname);
   BOOL proxy_ssl_check_peer_ok = TRUE;
  +int post_handshake_rc;
  +
   sc = mySrvConfig(server);
 
   #ifdef HAVE_TLSEXT
  @@ -1208,11 +1215,17 @@ static apr_status_t ssl_io_filter_handsh
   }
   }
 
  +if (proxy_ssl_check_peer_ok == TRUE) {
  +/* another chance to fail */
  +post_handshake_rc = ssl_run_proxy_post_handshake(c,
 filter_ctx-pssl);
  +}
  +
   if (cert) {
   X509_free(cert);
   }
 
  -if (proxy_ssl_check_peer_ok != TRUE) {
  +if (proxy_ssl_check_peer_ok != TRUE
  +|| (post_handshake_rc != OK  post_handshake_rc !=
 DECLINED)) {
   /* ensure that the SSL structures etc are freed, etc: */
   ssl_filter_io_shutdown(filter_ctx, c, 1);
   apr_table_setn(c-notes, SSL_connect_rv, err);

 GCC warns about 'post_handshake_rc' may be used uninitialized in this
 function... which would be true for the proxy_ssl_check_peer_ok !=
 TRUE case (line 1228), I think.

 Kaspar


Fixed in r1588868.

FWIW, post_handshake_rc wouldn't be referenced in the case where it was
uninitialized (proxy_ssl_check_peer_ok != TRUE), but it is nice to keep
more levels of gcc quiet.  (My gcc 4.8.1 from Ubuntu doesn't say anything.)

Thanks!

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


Re: mod_cache thundering herd bug

2014-04-21 Thread Graham Leggett
On 19 Apr 2014, at 10:26 PM, Eric Covener cove...@gmail.com wrote:

 Graham -- related subject brought up either in Denver or in the bug.
 It seems that when we serve a stale file while the cache is locked,
 the age headers are small instead of large. I got totally lost trying
 to track down the issue, maybe it makes sense to you?  It's almost as
 if they time of the revalidation is somehow updated early and the
 delta in the stale cache hits is based off of that.

All thundering herd does is after letting the first conditional request 
through, it serves stale data (RFC willing) until that conditional request 
comes back or a specific maximum time is reached, whichever comes first.

The most valuable piece of information in this process is the reason 
variable, which describes the reason why something wasn't eligible for caching. 
In httpd v2.4 the X-Cache-Detail header will give this to you, in httpd v2.2 
you'll need to log at DEBUG level to get this:

ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
cache: %s not cached. Reason: %s, r-unparsed_uri,
reason);

The questions to answer are:

- Is there stale content to serve? No stale content, no thundering herd 
protection.
- If stale content is being deleted, identify why that is. This is likely to be 
unrelated to thundering herd, but rather in other parts of mod_cache.

Regards,
Graham
--



SSLUserName - mod_auth_user

2014-04-21 Thread Graham Leggett
Hi all,

Right now, we have the SSLUserName directive, which takes an arbitrary SSL 
variable and turns it into a username for the benefit of the request. This has 
the downside that only SSL variables (and some CGI variables) are usable as 
usernames, and it combines with FakeBasicAuth to create undesirable side 
effects.

What would be cleaner is if we deprecate SSLUserName and create a 
mod_auth_user.c module that declares AuthType User, and then offers a AuthUser 
directive that sets the user based on an arbitrary expression from ap_expr.h. 
This will make client certificates easier to work with, and provide options for 
authentication that aren't based purely on logins, such as tokens in URLs, etc.

Thoughts?

Regards,
Graham
--



Re: SSLUserName - mod_auth_user

2014-04-21 Thread Eric Covener
On Mon, Apr 21, 2014 at 7:38 AM, Graham Leggett minf...@sharp.fm wrote:
 Hi all,

 Right now, we have the SSLUserName directive, which takes an arbitrary SSL 
 variable and turns it into a username for the benefit of the request. This 
 has the downside that only SSL variables (and some CGI variables) are usable 
 as usernames, and it combines with FakeBasicAuth to create undesirable side 
 effects.

 What would be cleaner is if we deprecate SSLUserName and create a 
 mod_auth_user.c module that declares AuthType User, and then offers a 
 AuthUser directive that sets the user based on an arbitrary expression from 
 ap_expr.h. This will make client certificates easier to work with, and 
 provide options for authentication that aren't based purely on logins, such 
 as tokens in URLs, etc.

I have a working module that just does the certificate in lieue of
basic auth based on ap_expr:
  https://github.com/covener/apache-modules/blob/master/mod_authn_cert.c

IMO the problem with doing that in a new authtype is that people
immediately want N authtypes in order which is much harder to.


Re: SSLUserName - mod_auth_user

2014-04-21 Thread Tim Bannister
On 21 Apr 2014, at 12:38, Graham Leggett minf...@sharp.fm wrote:

 Hi all,
 
 Right now, we have the SSLUserName directive, which takes an arbitrary SSL 
 variable and turns it into a username for the benefit of the request. This 
 has the downside that only SSL variables (and some CGI variables) are usable 
 as usernames, and it combines with FakeBasicAuth to create undesirable side 
 effects.
 
 What would be cleaner is if we deprecate SSLUserName and create a 
 mod_auth_user.c module that declares AuthType User, and then offers a 
 AuthUser directive that sets the user based on an arbitrary expression from 
 ap_expr.h. This will make client certificates easier to work with, and 
 provide options for authentication that aren't based purely on logins, such 
 as tokens in URLs, etc.

What string should httpd return to mean “no user found”? Users are going to 
want this.
I suggest  (empty string).

PS. I'd be tempted to call it AuthType Expr.


-- 
Tim Bannister - is...@jellybaby.net



Re: mod_cache thundering herd bug

2014-04-21 Thread Jim Riggs
On 21 Apr 2014, at 06:38, Graham Leggett minf...@sharp.fm wrote:

 On 19 Apr 2014, at 10:26 PM, Eric Covener cove...@gmail.com wrote:
 
 Graham -- related subject brought up either in Denver or in the bug.
 It seems that when we serve a stale file while the cache is locked,
 the age headers are small instead of large. I got totally lost trying
 to track down the issue, maybe it makes sense to you?  It's almost as
 if they time of the revalidation is somehow updated early and the
 delta in the stale cache hits is based off of that.
 
 All thundering herd does is after letting the first conditional request 
 through, it serves stale data (RFC willing) until that conditional request 
 comes back or a specific maximum time is reached, whichever comes first.
 
 The most valuable piece of information in this process is the reason 
 variable, which describes the reason why something wasn't eligible for 
 caching. In httpd v2.4 the X-Cache-Detail header will give this to you, in 
 httpd v2.2 you'll need to log at DEBUG level to get this:
 
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
cache: %s not cached. Reason: %s, r-unparsed_uri,
reason);
 
 The questions to answer are:
 
 - Is there stale content to serve? No stale content, no thundering herd 
 protection.
 - If stale content is being deleted, identify why that is. This is likely to 
 be unrelated to thundering herd, but rather in other parts of mod_cache.



Covener - Are you talking about my comments in #16 on the ticket? 
(https://issues.apache.org/bugzilla/show_bug.cgi?id=50317#c16)

If so, do either you or Graham have thoughts on the Age header getting returned 
with stale content? In my testing, when stale content is getting returned, no 
Age header is set which appears to be a violation of HTTP 1.1.



Re: mod_cache thundering herd bug

2014-04-21 Thread Eric Covener
 Covener - Are you talking about my comments in #16 on the ticket? 
 (https://issues.apache.org/bugzilla/show_bug.cgi?id=50317#c16)

 If so, do either you or Graham have thoughts on the Age header getting 
 returned with stale content? In my testing, when stale content is getting 
 returned, no Age header is set which appears to be a violation of HTTP 1.1.


yes, I think it's not that it's unset, but that the calculation
somehow uses the revalidation-in-progress check time as the basis.

-- 
Eric Covener
cove...@gmail.com


Re: Problem of URL in bugzilla

2014-04-21 Thread Rainer Jung
On 21.04.2014 23:15, Mark Thomas wrote:
 On 20/04/2014 20:11, Mark Thomas wrote:
 On 20/04/2014 18:51, Rainer Jung wrote:
 CCing Mark our Bugzilla (and much more) champion, hoping he knows more
 or at least needs the info.

 @Mark: I think the transform svn revision to link feature is a
 Bugzilla global one, not specific to httpd. It seems partially broken
 after the recent update, see below for details.

 The svn linking is custom code. I'll take a look.
 
 (note I'm not subscribed to dev@httpd.a.o so this is going to bounce)
 
 This is fixed.
 
 I needed to port the change made in the 4.4.4 fix to our custom code for
 svn links.
 
 Enjoy.

Thanks a bunch Mark, looks great.

Regards,

Rainer

 On 20.04.2014 19:36, Christophe JAILLET wrote:
 Hi,

 When browsing https://issues.apache.org/bugzilla/show_bug.cgi?id=56371,
 in the top most frame, we can see:
 ASF Bugzilla – Bug 56371 Please merge r1576233 into the next
 httpd-2.4.10 version, the fix is missing in httpd-2.4.9

 But below, we only have:
 Bug 56371 - Please merge 0 into the next httpd-2.4.10 version, the fix
 is missing in httpd-2.4.9 (edit)

 link to r1576233 has been turned into 0.


 In the comment below, we have the same issue, the link to r1576233 is
 turned into 0.


 same kind of issue can be seen in the 2nd comment of
 https://issues.apache.org/bugzilla/show_bug.cgi?id=56235



 I think it is a bug in bugzilla. We are using version 4.4.4 which has
 been released just 2 days ago.
 This release is just a day after 4.4.3. It is a quick fix because of URL
 broken in comments. That is to say, more or less what is described above.
 So I guess, that the issue is in bugzilla.


 However, before posting a bug report, I wanted to have your feedback.
 Is there some custom code in our installation that turns rxx into a
 link to svn repository that should be updated to match changes in 4.4.4
 release?

 Best regards,

 CJ

 

-- 
kippdata
informationstechnologie GmbH   Tel: 0228 98549 -0
Bornheimer Str. 33aFax: 0228 98549 -50
53111 Bonn www.kippdata.de

HRB 8018 Amtsgericht Bonn / USt.-IdNr. DE 196 457 417
Geschäftsführer: Dr. Thomas Höfer, Rainer Jung, Sven Maurmann