Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Date: Tue, 18 Feb 2003 10:16:55 +0100 (CET)
From: Dirk-Willem van Gulik <dirkx@webweaving.org>
To: dev@httpd.apache.org
Subject: Re: round 2 of mod_authn_mysql
In-Reply-To: <20030218042458.M47348@cyan.com>
Message-ID: <20030218095907.U82095-100000@foem.leiden.webweaving.org>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII


> I have put the version(0.0.3) with these and a couple over small changes on
> http://open.cyanworlds.com

Compiles and Works for me gov.

Few minor nits below. Feel free to ignore. Nothing major.

mysql_init can return a NULL;  are we sure that mysql_close is thread safe
? And I'd make the psprintf's into 'psnprintf's with a, say 1-2k limit as
some of the fields may be under potential malicious http-wire or .htaccess
control (note the NAME_LEN and a few others in mysql.h or mysql_com.h);
mysql does little checking afaik and simply barfs/cores.

#define MYSQL_MAX_QUERY_STRING (1024)
...
    if (conf->rec.isactive_field) {
        query = apr_psnprintf(r->pool, MYSQL_MAX_QUERY_STRING,
		"SELECT %s FROM %s WHERE %s='%s' AND %s!=0 LIMIT 0,1",
                                         conf->rec.password_field, conf->rec.mysql_table,
                                         conf->rec.username_field, esc_user, conf->rec.isactive_field);

this also has another issue; a local user could cause apache to create a
-lot- of connections to the database with rogue .htaccess files. Not sure
that that is -really- an issue. But given that you've very nicely mutexed
all the connects; a simple counter may help. Though file descriptors would
run out early I'd imagine. But then again; I could imagine this not being
an issue at all. If you where -really- paranoid you could do another
sanity check on (m/A-Z\-_0-9/ && len<NAME_LENGTH) on the conf->rec.*
fields. Also - mysql - is quite happy with 0x01 and \n's and stuff like
UTF8 in its '' fields ??

init_authn_mysql
	tmpnam()	-> no error trapping

Trusted Solaris barfed on this without it being clear that this was the
cause of my problem. It also makes a file in some random location; did
apr_ not have a nice version of it which has some more control for the
admin over where ? Or was that never written ?

Dw.