Re: check for RAND_egd at configure time

2015-04-17 Thread Kaspar Brand 
On 16.04.2015 22:57, Stefan Sperling wrote:
 On Wed, Apr 15, 2015 at 08:43:04PM +0200, Stefan Sperling wrote:
 LibreSSL does not provide the RAND_egd() function.

 This patch adds a configure check to allow building mod_ssl with LibreSSL.
 
 Updated version following Kaspar Brand's suggestion to move into acinclude.m4.
 
 Index: acinclude.m4
 ===
 --- acinclude.m4  (revision 1673798)
 +++ acinclude.m4  (working copy)
 @@ -598,6 +598,11 @@ AC_DEFUN(APACHE_CHECK_OPENSSL,[
if test x$liberrors != x; then
  AC_MSG_WARN([OpenSSL libraries are unusable])
fi
 +  have_rand_egd=no
 +  AC_CHECK_LIB(crypto, RAND_egd, [have_rand_egd=yes])
 +  if test $have_rand_egd = yes; then
 +AC_DEFINE([HAVE_RAND_EGD], [1], [Define if RAND_egd exists.])
 +  fi
  else
AC_MSG_WARN([OpenSSL version is too old])
  fi

I was actually thinking about

Index: acinclude.m4
===
--- acinclude.m4(revision 1673835)
+++ acinclude.m4(working copy)
@@ -594,7 +594,7 @@
   liberrors=
   AC_CHECK_HEADERS([openssl/engine.h])
   AC_CHECK_FUNCS([SSLeay_version SSL_CTX_new], [], [liberrors=yes])
-  AC_CHECK_FUNCS([ENGINE_init ENGINE_load_builtin_engines])
+  AC_CHECK_FUNCS([ENGINE_init ENGINE_load_builtin_engines RAND_egd])
   if test x$liberrors != x; then
 AC_MSG_WARN([OpenSSL libraries are unusable])
   fi

... or does that not fit in this case?

Kaspar

Re: svn commit: r1585609 - /httpd/httpd/trunk/modules/aaa/mod_authz_core.c

2015-04-17 Thread Yann Ylavic 
On Tue, Apr 8, 2014 at 12:38 AM,  bre...@apache.org wrote:
 Author: breser
 Date: Mon Apr  7 22:38:53 2014
 New Revision: 1585609

 URL: http://svn.apache.org/r1585609
 Log:
 Allow Require expr to work when the expression is quoted.

 For example as appears in our documentation:
 Require expr %{TIME_HOUR} -ge 9  %{TIME_HOUR} -le 17

 PR: 56235

 Modified:
 httpd/httpd/trunk/modules/aaa/mod_authz_core.c

 Modified: httpd/httpd/trunk/modules/aaa/mod_authz_core.c
 URL: 
 http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/aaa/mod_authz_core.c?rev=1585609r1=1585608r2=1585609view=diff
 ==
 --- httpd/httpd/trunk/modules/aaa/mod_authz_core.c (original)
 +++ httpd/httpd/trunk/modules/aaa/mod_authz_core.c Mon Apr  7 22:38:53 2014
 @@ -1062,6 +1062,16 @@ static const char *expr_parse_config(cmd
  const char *expr_err = NULL;
  struct require_expr_info *info = apr_pcalloc(cmd-pool, sizeof(*info));

 +/* if the expression happens to be surrounded by quotes, skip them */
 +if (require_line[0] == '') {
 +apr_size_t len = strlen(require_line);
 +
 +if (require_line[len-1] == '')
 +require_line = apr_pstrndup(cmd-temp_pool,
 +require_line + 1,
 +len - 2);
 +}
 +
  apr_pool_userdata_setn(info, REQUIRE_EXPR_NOTE, apr_pool_cleanup_null,
cmd-temp_pool);
  info-expr = ap_expr_parse_cmd(cmd, require_line, 0, expr_err,



This fix does not handle inner quotes (ie. \-escaping), nor other
providers (than core_authz_expr) which handle exprs or multiple
trailing words (eg. mod_authnz_ldap,
mod_authz_{dbd,groupfile,host,user,...}).

How about handling the case in the Require directive parser itself
(add_authz_provider)?

Something like:

Index: modules/aaa/mod_authz_core.c
===
--- modules/aaa/mod_authz_core.c(revision 1674046)
+++ modules/aaa/mod_authz_core.c(working copy)
@@ -393,7 +393,13 @@ static const char *add_authz_provider(cmd_parms *c
 section-negate = 1;
 }

-section-provider_args = args;
+if (args  (args[0] == '' || args[0] == '\'')
+  (args[strlen(args) - 1] == args[0])) {
+section-provider_args = ap_getword_conf(cmd-pool, args);
+}
+else {
+section-provider_args = args;
+}

 /* lookup and cache the actual provider now */
 section-provider = ap_lookup_provider(AUTHZ_PROVIDER_GROUP,
@@ -425,7 +431,7 @@ static const char *add_authz_provider(cmd_parms *c
AUTHZ_PROVIDER_NAME_NOTE,
apr_pool_cleanup_null,
cmd-temp_pool);
-err = section-provider-parse_require_line(cmd, args,
+err = section-provider-parse_require_line(cmd,
section-provider_args,
   section-provider_parsed_args);

 if (err)
@@ -1069,16 +1075,6 @@ static const char *expr_parse_config(cmd_parms *cm
 const char *expr_err = NULL;
 struct require_expr_info *info = apr_pcalloc(cmd-pool, sizeof(*info));

-/* if the expression happens to be surrounded by quotes, skip them */
-if (require_line[0] == '') {
-apr_size_t len = strlen(require_line);
-
-if (require_line[len-1] == '')
-require_line = apr_pstrndup(cmd-temp_pool,
-require_line + 1,
-len - 2);
-}
-
 apr_pool_userdata_setn(info, REQUIRE_EXPR_NOTE, apr_pool_cleanup_null,
   cmd-temp_pool);
 info-expr = ap_expr_parse_cmd(cmd, require_line, 0, expr_err,
--

(the second hunk is a revert of this commit).

Re: Extending mod_authz_dbd

2015-04-17 Thread Jose Kahan 
Hello Graham,

Thanks for your feedback. It all makes sense to me. 
Following your reasoning, the only directive that will have i
to stay as is for backwards compatibility will be require 
dbd-group, as it expects either no parameter or the value 
the query will return.

I had initially thought of only using one directive and
extracting the variables from the AuthzDBDQuery arguments, 
substituting them for %s and then preparing the request. Your
proposal is much simpler so I'll go with it.

I'm already looking at the code. Will keep you (and the list)
updated as soon as I have something to share.

Regards,

-jose

On Wed, Apr 15, 2015 at 11:17:04AM +0200, Graham Leggett wrote:
 
 The limitations we’d have to work with is that all the queries are prepared 
 statements, and are reused for multiple requests. At the same time any query 
 that is interpreted purely as a string would need to be protected against SQL 
 injection.
 
   Require dbd-login %{REQUEST_URI} %{REQUEST_METHOD} %{REMOTE_USER}
   AuthzDBDQuery UPDATE authn SET uri = %s, method = %s WHERE user = %s”
 
   Require dbd-logout %{TIME} %{REMOTE_USER}
   AuthzDBDQuery UPDATE authn SET logout_time = %s WHERE user = %s”
 
 To be backwards compatible, Require dbd-login” on it’s own would imply 
 Require dbd-login %{REMOTER_USER}”.
 
 One possible approach to support completely generic queries might be as 
 follows:
 
   Require dbd-query %{REQUEST_URI} %{REMOTE_USER}
   AuthzDBDQuery “SELECT count(user) FROM authn WHERE uri=%s AND user = %s”
 
 The bit above where you’d limit the requests to GET or POST you’d probably do 
 with Limit or LimitExcept.

Re: svn commit: r1585609 - /httpd/httpd/trunk/modules/aaa/mod_authz_core.c

2015-04-17 Thread Yann Ylavic 
On Fri, Apr 17, 2015 at 11:54 AM, Yann Ylavic ylavic@gmail.com wrote:

 How about handling the case in the Require directive parser itself
 (add_authz_provider)?

Or even maybe in the expr parser itself:

Index: server/util_expr_eval.c
===
--- server/util_expr_eval.c(revision 1674046)
+++ server/util_expr_eval.c(working copy)
@@ -455,6 +455,10 @@ AP_DECLARE(ap_expr_info_t*) ap_expr_parse_cmd_mi(c
 info-line_number = cmd-directive-line_num;
 info-flags = flags;
 info-module_index = module_index;
+if (expr  (expr[0] == '' || expr[0] == '\'')  (expr[1] != '\0')
+  (expr[strlen(expr) - 1] == expr[0])) {
+expr = ap_getword_conf(cmd-temp_pool, expr);
+}
 *err = ap_expr_parse(cmd-pool, cmd-temp_pool, info, expr, lookup_fn);

 if (*err)
--

ap_getword_conf() and badly quoted strings

2015-04-17 Thread Yann Ylavic 
Hi,

currently ap_getword_conf() considers a word is quoted when (and only
when) it starts with a quote, regardless of this word ending (or not)
with the same quote.

That is, eg., ap_getword_conf(\) ==  or
ap_getword_conf(\whatever \\\badly\\\ quoted) == whatever
\badly\ quoted.

I wonder if it should not return the (first) word as-is in this case,
hence including the leading quote and up to the first space (ie.
restart normal parsing from the beginning of the given line):

Index: server/util.c
===
--- server/util.c(revision 1674046)
+++ server/util.c(working copy)
@@ -780,7 +780,7 @@ AP_DECLARE(char *) ap_getword_conf_nc(apr_pool_t *

 AP_DECLARE(char *) ap_getword_conf(apr_pool_t *p, const char **line)
 {
-const char *str = *line, *strend;
+const char *str = *line, *strend = NULL;
 char *res;
 char quote;

@@ -803,12 +803,16 @@ AP_DECLARE(char *) ap_getword_conf(apr_pool_t *p,
 ++strend;
 }
 }
-res = substring_conf(p, str + 1, strend - str - 1, quote);

-if (*strend == quote)
+if (*strend) {
+res = substring_conf(p, str + 1, strend - str - 1, quote);
 ++strend;
+}
+else {
+strend = NULL;
+}
 }
-else {
+if (!strend) {
 strend = str;
 while (*strend  !apr_isspace(*strend))
 ++strend;
--

With this, ap_getword_conf(\) == \ and
ap_getword_conf(\whatever \\\badly\\\ quoted) == \whatever =
\\\badly\\\ = quoted, which may raise syntax errors/typos the
administrator could be interested in.

Is that more of a fix or a compatibility issue?

Regards,
Yann.

Re: [PATCH] mod_log_config: Allow logging using errorlog provider

2015-04-17 Thread Jan Kaluža 

On 04/07/2015 11:47 AM, Jan Kaluža wrote:

Hi,

we have ap_errorlog_provider in the trunk for some time. I was thinking
about extending it to mod_log_config, so CustomLog/TransferLog would
work with any module providing error_log logging ability like mod_syslog
or mod_journald.

Attached patch does that by introducing CustomLog
errorlog_provider_name:arg syntax, which should be backward compatible
with the current syntax used for CustomLog.


Committed in r1674261.


The patch changes ap_default_log_writer_init to detect this syntax, find
the provider and initialize it. It also changes ap_default_log_writer to
detect initialized provider and use it to log the message

I would like to see that feature in the trunk, but since this changes
the syntax little bit, I will wait a bit for other opinions if any.

Regards,
Jan Kaluza


Regards,
Jan Kaluza

Re: [users@httpd] Weird connection issues with mod_proxy_wstunnel

2015-04-17 Thread Yann Ylavic 
Follow up to [1] based on users@ experience...

On Fri, Apr 17, 2015 at 3:38 PM, Marc Hörsken i...@marc-hoersken.de wrote:

 I just figured out the configuration issue causing my problem.

 Original configuration mod_proxy_wstunnel with SwampDragon:

 ProxyPass /data/ ws://127.0.0.1:9001/data/
 ProxyPassReverse /data/ ws://127.0.0.1:9001/data/
 ProxyPass /settings.js http://127.0.0.1:9001/settings.js
 ProxyPassReverse /settings.js http://127.0.0.1:9001/settings.js
 ProxyPreserveHost On
 ProxyRequests Off
 ProxyVia On

 The following configuration for mod_proxy_wstunnel with SwampDragon works 
 fine:

 ProxyPassMatch /data/(\d+)/(\w+)/websocket 
 ws://127.0.0.1:9001/data/$1/$2/websocket
 ProxyPass /data/info http://127.0.0.1:9001/data/info
 ProxyPassReverse /data/info http://127.0.0.1:9001/data/info
 ProxyPass /settings.js http://127.0.0.1:9001/settings.js
 ProxyPassReverse /settings.js http://127.0.0.1:9001/settings.js
 ProxyPreserveHost On
 ProxyRequests Off
 ProxyVia On

 The issue was caused by the fact that /data/info is requested before a 
 WebSocket-upgrade
 request to /data/(\d+)/(\w+)/websocket is performed. Because /data/info was 
 redirected to
 the WebSocket-server using the same rule as /data/(\d+)/(\w+)/websocket 
 before,
 mod_proxy_wstunnel continued to also forward all HTTP-traffic to the 
 WebSocket-server.


As already proposed in [1], shouldn't mod_proxy_wstunnel should
DECLINE[D] the request if it is not an Upgrade: WebSocket one?

This could possibly simplify the configuration above to:
  ProxyPass / ws://127.0.0.1:9001/
  ProxyPass / http://127.0.0.1:9001/

The patch would be something like:

Index: modules/proxy/mod_proxy_wstunnel.c
===
--- modules/proxy/mod_proxy_wstunnel.c(revision 1665828)
+++ modules/proxy/mod_proxy_wstunnel.c(working copy)
@@ -305,7 +320,7 @@ static int proxy_wstunnel_handler(request_rec *r,
 int status;
 char server_portstr[32];
 proxy_conn_rec *backend = NULL;
-char *scheme;
+const char *scheme, *upgrade;
 int retry;
 conn_rec *c = r-connection;
 apr_pool_t *p = r-pool;
@@ -324,6 +339,25 @@ static int proxy_wstunnel_handler(request_rec *r,
 return DECLINED;
 }

+upgrade = apr_table_get(r-headers_in, Connection);
+if (upgrade) {
+if (strcasecmp(upgrade, Upgrade) != 0) {
+upgrade = NULL;
+}
+else {
+upgrade = apr_table_get(r-headers_in, Upgrade);
+if (upgrade  strcasecmp(upgrade, WebSocket) != 0) {
+upgrade = NULL;
+}
+}
+}
+if (!upgrade) {
+ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
+  APLOGNO() %s: declining URL %s (not WebSocket),
+  scheme, url);
+return DECLINED;
+}
+
 uri = apr_palloc(p, sizeof(*uri));
 ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02451)
serving URL %s, url);

--

[1] 
https://mail-archives.apache.org/mod_mbox/httpd-dev/201503.mbox/%3CCAKQ1sVPq-9cqS4zKvfwuC5Hi1mXZ=pkfuxrqsv2lexsnrzd...@mail.gmail.com%3E

Re: svn commit: r1585609 - /httpd/httpd/trunk/modules/aaa/mod_authz_core.c

2015-04-17 Thread Christophe JAILLET 

Hi,

I would +1 for this solution which is, IMHO, much better.
However, changing the parser itself would require checking the potential 
impact as it is used in many places.


CJ

Le 17/04/2015 13:15, Yann Ylavic a écrit :

On Fri, Apr 17, 2015 at 11:54 AM, Yann Ylavic ylavic@gmail.com wrote:

How about handling the case in the Require directive parser itself
(add_authz_provider)?

Or even maybe in the expr parser itself:

Index: server/util_expr_eval.c
===
--- server/util_expr_eval.c(revision 1674046)
+++ server/util_expr_eval.c(working copy)
@@ -455,6 +455,10 @@ AP_DECLARE(ap_expr_info_t*) ap_expr_parse_cmd_mi(c
  info-line_number = cmd-directive-line_num;
  info-flags = flags;
  info-module_index = module_index;
+if (expr  (expr[0] == '' || expr[0] == '\'')  (expr[1] != '\0')
+  (expr[strlen(expr) - 1] == expr[0])) {
+expr = ap_getword_conf(cmd-temp_pool, expr);
+}
  *err = ap_expr_parse(cmd-pool, cmd-temp_pool, info, expr, lookup_fn);

  if (*err)
--

Re: check for RAND_egd at configure time

2015-04-17 Thread Stefan Sperling 
On Fri, Apr 17, 2015 at 07:02:21AM +0200, Kaspar Brand wrote:
 I was actually thinking about
 
 Index: acinclude.m4
 ===
 --- acinclude.m4(revision 1673835)
 +++ acinclude.m4(working copy)
 @@ -594,7 +594,7 @@
liberrors=
AC_CHECK_HEADERS([openssl/engine.h])
AC_CHECK_FUNCS([SSLeay_version SSL_CTX_new], [], [liberrors=yes])
 -  AC_CHECK_FUNCS([ENGINE_init ENGINE_load_builtin_engines])
 +  AC_CHECK_FUNCS([ENGINE_init ENGINE_load_builtin_engines RAND_egd])
if test x$liberrors != x; then
  AC_MSG_WARN([OpenSSL libraries are unusable])
fi
 
 ... or does that not fit in this case?
 
 Kaspar

Thanks, this work fine. Tested on OpenBSD and Debian.

Index: acinclude.m4
===
--- acinclude.m4(revision 1673798)
+++ acinclude.m4(working copy)
@@ -594,7 +594,7 @@ AC_DEFUN(APACHE_CHECK_OPENSSL,[
   liberrors=
   AC_CHECK_HEADERS([openssl/engine.h])
   AC_CHECK_FUNCS([SSLeay_version SSL_CTX_new], [], [liberrors=yes])
-  AC_CHECK_FUNCS([ENGINE_init ENGINE_load_builtin_engines])
+  AC_CHECK_FUNCS([ENGINE_init ENGINE_load_builtin_engines RAND_egd])
   if test x$liberrors != x; then
 AC_MSG_WARN([OpenSSL libraries are unusable])
   fi
Index: modules/ssl/ssl_engine_rand.c
===
--- modules/ssl/ssl_engine_rand.c   (revision 1673798)
+++ modules/ssl/ssl_engine_rand.c   (working copy)
@@ -86,6 +86,7 @@ int ssl_rand_seed(server_rec *s, apr_pool_t *p, ss
 nDone += ssl_rand_feedfp(p, fp, pRandSeed-nBytes);
 ssl_util_ppclose(s, p, fp);
 }
+#ifdef HAVE_RAND_EGD
 else if (pRandSeed-nSrc == SSL_RSSRC_EGD) {
 /*
  * seed in contents provided by the external
@@ -95,6 +96,7 @@ int ssl_rand_seed(server_rec *s, apr_pool_t *p, ss
 continue;
 nDone += n;
 }
+#endif
 else if (pRandSeed-nSrc == SSL_RSSRC_BUILTIN) {
 struct {
 time_t t;

Does Apache httpd server dynamically generate just-in-time (JIT) compiled code?

2015-04-17 Thread Yue Chen 
Hi,

In some OS's, the network stack would compile packet filters to the native
code, like the Berkeley Packet Filter (BPF).

Would Apache httpd server also do this? In httpd, what's the usage of
function ``ap_core_input_filter'', and does this function generate JIT
code?  If it does, will the generated code contain code pointers to its
original static code in ``ap_core_input_filter''?

I go through the source code, but cannot get much sense on what it does and
how it works.

Best thanks and regards,
Yue

Re: Does Apache httpd server dynamically generate just-in-time (JIT) compiled code?

2015-04-17 Thread Reindl Harald 



Am 17.04.2015 um 23:29 schrieb Yue Chen:

Hi,

In some OS's, the network stack would compile packet filters to the
native code, like the Berkeley Packet Filter (BPF)


apache and packet filter are completly different things at completly 
different layers




signature.asc
Description: OpenPGP digital signature

Question about some examples in the doc

2015-04-17 Thread Christophe JAILLET 

Hi,

looking at comment 
http://httpd.apache.org/docs/current/en/mod/mod_authn_core.html#comment_751,

I think that what is proposed is not enough and that turning:
   Order deny,allow
   Allow from all
into
   Require all granted

is not correct.


Require all granted would bypass the Require valid-user, wouldn't it?

So I think that the best fix would be just to remove the 2 
Order...Allow... lines from the example, just as in the other example 
above it.


Correct?




The same way, in 
http://httpd.apache.org/docs/current/en/mod/mod_info.html#security,

I think that the example:
Location /server-info
SetHandler server-info
Order allow,deny
# Allow access from server itself
Allow from 127.0.0.1
# Additionally, allow access from local workstation
Allow from 192.168.1.17
/Location

should be turned in:
Location /server-info
SetHandler server-info
# Allow access from server itself
Require ip 127.0.0.1
# Additionally, allow access from local workstation
Require ip 192.168.1.17
/Location

or
Location /server-info
SetHandler server-info
# Allow access from server itself or from a local workstation
Require ip 127.0.0.1 192.168.1.17
/Location

Correct?

CJ

Re: ap_getword_conf() and badly quoted strings

2015-04-17 Thread William A Rowe Jr 
I think in trunk we should properly bail if the same quote char does not
occur as termination.

I don't think we should second-guess the admin's intent.




On Fri, Apr 17, 2015 at 6:43 AM, Yann Ylavic ylavic@gmail.com wrote:

 Hi,

 currently ap_getword_conf() considers a word is quoted when (and only
 when) it starts with a quote, regardless of this word ending (or not)
 with the same quote.

 That is, eg., ap_getword_conf(\) ==  or
 ap_getword_conf(\whatever \\\badly\\\ quoted) == whatever
 \badly\ quoted.

 I wonder if it should not return the (first) word as-is in this case,
 hence including the leading quote and up to the first space (ie.
 restart normal parsing from the beginning of the given line):

 Index: server/util.c
 ===
 --- server/util.c(revision 1674046)
 +++ server/util.c(working copy)
 @@ -780,7 +780,7 @@ AP_DECLARE(char *) ap_getword_conf_nc(apr_pool_t *

  AP_DECLARE(char *) ap_getword_conf(apr_pool_t *p, const char **line)
  {
 -const char *str = *line, *strend;
 +const char *str = *line, *strend = NULL;
  char *res;
  char quote;

 @@ -803,12 +803,16 @@ AP_DECLARE(char *) ap_getword_conf(apr_pool_t *p,
  ++strend;
  }
  }
 -res = substring_conf(p, str + 1, strend - str - 1, quote);

 -if (*strend == quote)
 +if (*strend) {
 +res = substring_conf(p, str + 1, strend - str - 1, quote);
  ++strend;
 +}
 +else {
 +strend = NULL;
 +}
  }
 -else {
 +if (!strend) {
  strend = str;
  while (*strend  !apr_isspace(*strend))
  ++strend;
 --

 With this, ap_getword_conf(\) == \ and
 ap_getword_conf(\whatever \\\badly\\\ quoted) == \whatever =
 \\\badly\\\ = quoted, which may raise syntax errors/typos the
 administrator could be interested in.

 Is that more of a fix or a compatibility issue?

 Regards,
 Yann.