https://bz.apache.org/bugzilla/show_bug.cgi?id=58025
Bug ID: 58025
Summary: mod_authz_dbd : new dbd-query and AuthzDBDQuery
options
Product: Apache httpd-2
Version: 2.5-HEAD
Hardware: PC
URL: http://mail-archives.apache.org/mod_mbox/httpd-dev/201
504.mbox/%3C304DB914-ED71-4476-B875-F1ABDD1E370C@sharp
.fm%3E
OS: Linux
Status: NEW
Keywords: PatchAvailable
Severity: enhancement
Priority: P2
Component: mod_authz_dbd
Assignee: [email protected]
Reporter: [email protected]
Created attachment 32809
--> https://bz.apache.org/bugzilla/attachment.cgi?id=32809&action=edit
mod_authz_dbd.c.diff patch adding dbd-query / AuthzDBDQuery options
This patch implements the enhancement proposed by Graham Leggett on the
apache-dev list[1]. It is tested and fully working against 2.4.10.
[[
Usage examples:
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”
]]
Although I developed and tested this against the 2.4.10 source
tree (the latest debian jessie version), I checked and this
module as well as mod_dbd.c have not changed since then neither
in the latest 2.4.x nor in trunk. I don't think there will be
an issue running it there. Installing that environment over the
default debian one could be a hazzle for me that I'd prefer
to avoid , but that I could do if needed.
To compile and install the patch (after merging):
apxs2 -i -a -c mod_authz_dbd.c
[1]
http://mail-archives.apache.org/mod_mbox/httpd-dev/201504.mbox/%[email protected]%3E
DETAILED report.
Work done:
- As discussed earlier in this thread, new require dbd-query directive
for supporting query parameters. If the query returns at least one
row, we consider that access is granted.
- It's now possible to use query parameters for dbd-login and
dbd-logout. If no parameter is given, the default value is
%{REMOTE-USER} to keep backwards compatibility.
- While testing the changes for dbd-login, a server misconfiguration
revealed a SIGSEV. If the dbd handle preparation fails due to a
misconfiguration of the database server or the access rights to the
table, the server logs something along these lines:
AH00632: failed to prepare SQL statements: UPDATE command denied to
user 'foo'@'exampleorg' for table 'bar'
As a consequence, the call to dbd_handle(r) returns NULL. However,
in mod_authz_dbd, there was no control for the value of dbd before
its being used in both authz_dbd_login() and authzdbd_group():
ap_dbd_t *dbd = dbd_handle(r);
...
query = apr_hash_get(dbd->prepared, <--- SIGSEV
I added a check to protect against this case.
Open issues not directly related to this patch, but which is good to keep in
mind:
- I wanted to control that the number of bind arguments correspond to
those in the prepared request. However this value is not visible in
apr_dbd_prepared_t *query. Looking at the code in apr_dbd.c, I see
that even though the number of bind arguments are being passed to
apr_dbd_pselect() and apr_dbd_pquery() in nargs, this argument is
discarded before calling the implementation of those functions in
the driver.
At least in the mysql driver, there is no further check that there
is an equivalence between the bind and prepared statement
arguments. In both cases there is just a call to
mysql_stmt_bind_param(statement->stmt, bind);
which expects bind to have the correct number of arguments. I
checked the sqlite3 driver and it has the same issue.
If bind has more arguments, there is no issue. If there is less, we
risk a SIGSEV or unexpected behavior.
To avoid this issue, I wanted to control the number of arguments
when parsing the configuration file and show a configuration error
when necessary. However, as the prepared query goes into opaque
driver implementations, I don't have access to its nargs value. Are
there any ideas on how to get this info? The only hack I could
think of is counting the number of % inside the prepared query and
making sure that we have the corresponding number of bind arguments.
- Would it be worth it to connect this module to mod_socache.c?
Other questions re implementation of the patch that may be modified:
- In function dbd_parse_config, I was unable to decide whether it
would be ok to use cmd->temp_pool instead of cmd->pool in my call to
ap_getword_white()
- I am using ap_expr_parse_cmd() on each argument of the require
dbd-query, dbd-login, and dbd-logout directives. I think this is
akin to building an expression tree for each argument. Is there a
more efficient way to do so?
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
