Mailing List Archive

it would have been nice if he had separated these out into separate
patches, oy.

Brian

--=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=--
brian@organic.com brian@hyperreal.com http://www.[hyperreal,organic].com/

On Mon, 21 Aug 1995, Rob Hartill wrote:

[snip]

> I put the BIG patch file in httpd/incoming/EdwardMoy_patches
> [rob]

Okay... now how about giving us mere mortals permission to read it?

--/ Alexei Kosut <akosut@nueva.pvt.k12.ca.us> /--------/ Lefler on IRC
----------------------------/ <http://www.nueva.pvt.k12.ca.us/~akosut/>
The viewpoints expressed above are entirely false, and in no way
represent Alexei Kosut nor any other person or entity. /--------------

From: Rob Hartill <hartill@hyperreal.com>
Date: Mon, 21 Aug 1995 16:20:01 -0700 (PDT)

> I've made some useful patches to Apache 0.8.8, which I hope you will
> included in a future release. The changes are summarized below. Then the
> context diff is attached.

I like some of these, I don't like others (quelle surprise!)... see
below, positive comments first.

Could he break it into pieces? It would be easier to deal with that
way...

> The is_url() routing in util.c now requires that some text exist before
> the colon for it to return true.

Useful fix.

> Created a new module, mod_auth_pw.c, that uses the standard
> getpwname() and getgrname() function calls. With NIS configured
> correctly, this accesses the central NIS databases. Two new
> commands are defined, AuthPWUser and AuthPWGroup.

If this can access NIS maps *other* than the real password and group
maps (which are bad things to use for reasons we've gone over
repeatedly), it would be nice to have --- NB NCSA 1.5b3 supports this
functionality, though the details may be slightly different.

> Patched http_request.c to allow .htaccess to override central config files
> if OVERRIDE_CONF is defined. (In a recent netnews message, a user of
> Apache 0.6.5 converted to 0.8.8 and his .htaccess file stopped working. I
> believe defining OVERRIDE_CONF will make it work as he expects).

If we're incompatible here, then at least by my criteria, that's just
a bug, and we should fix it and be done. I dislike compile-time
options.

> The "allow/deny from ip_address" form now can take a bit mask:
> "allow/deny from ip_address:bit_mask", where bit_mask is either
> octet style (aaa.bbb.ccc.ddd) or hexidecimal (0xnnnnnnnn).
> Actually, the bit mask is always used internally not. So if
> 1.2.3 is defined, a bit mask of 0xffffff00 is automatically used.
> 1.2.3:0xfffffc00 uses a different bit mask. (Note: the bit mask
> 255.255 corresponds to 0xffff0000, while the bit mask 0xffff is
> 0x0000ffff.)

Unobjectionable, but it is a new feature, and we're trying to hold off
on those for the moment...

> All functions and globals in modules (except the module structure
> itself) are declared static to avoid name conflicts and to insure
> modularity.

A useful cleanup...

> Modified all of the authentication modules, along with
> http_config.c, to allow multiple means of authentication. Thus,
> if one method of user authentication fails, it is passed to
> another authentication modules (e.g., one can use the
> mod_auth_pw.c authentication and, in addition, assign separate
> user/passwords using one of the other auth modules. Also, one
> can, for instance, require password authentication using
> mod_auth_pw.c and define one's own require groups using one of
> the other auth modules.)

Some background --- in the distributed code, it certainly *ought* to
work to combine, say, an ordinary password file with a dbm group file,
or vice versa --- a module which hasn't been configured to operate in
a particular directory DECLINEs, and then the others get a crack, very
much as described for this change. (The only reason that I don't say
it *does* work is that I have never tried this specific thing, and my
cautious nature has the better of me).

However, trying to mix a DBM password file and an ordinary password
file in the same directory does not work --- if a DBM password file is
configured, mod_auth_dbm assumes that it has jurisdiction in this
particular directory; it either authorizes the client or rejects him
definitively, and no other modules get a crack.

What this patch does, as I understand it, is that if a particular
check_user module (say) doesn't find the client's name in whatever
database it queries, then it DECLINEs in a special, magic way, such
that if all other modules also decline, it reports an auth failure,
rather than a server error; this allows you to, say, have a NIS map
and an ordinary password file in the same directory, and to have it
fall back on the latter if it doesn't find the client in the former.

This is an interesting idea, but the implementation is a bit of a
botch. In particular, calling note_basic_auth_failure() from inside
the check_auth routines is The Wrong Thing To Do --- while basic auth
is the only style of authentication which is handled by the server for
the moment, that won't be the case forever, and returning a Basic auth
www-authenticate line when it's really Digest auth that's configured is
obviously incorrect.

This may be pointing to a weakness in the design, in that one might
want a cleaner separation between authentication types at the protocol
level (we could then have a generic note_auth_failure() routine, which
would dispatch on auth_type). However, that would mean that new
authentication types at the protocol-level would be difficult to
prototype in modules, which wouldn't be an ideal situation either.

At any rate, this certainly needs to be thought through (for one
thing, is this feature useful enough to be worth the hair?)...

> Further, in http_request.c, overrides now always apply to the directory
> trees, so the .htacess as well as central config files can protect the
> whole directory tree.

This is one item which I don't understand --- .htaccess files
do apply to stuff in subdirectories already. (And in any case, I
don't see anything in the patch files themselves which does this sort
of thing --- the closest is that he reenabled wildcard <Directory>
sections incorrectly, by sticking back the code I had commented out
for a reason, without fixing the reason for commenting it out.
Sigh...).

> util.c and util_script.c were modified to use pointers and
> registers rather than arrays for efficiency.

Hmmm... Whenever I see something which claims to be a speed
improvement, my first question is whether the gains in performance
have been measured. (If they have, then the second question is
whether the gains are worth the trouble).

Now, I certainly haven't benchmarked these. However, I have looked at
my share of gcc output, --- with the optimizer on full, it *routinely*
transforms array-based code into pointer arithmetic. So, these
changes aren't likely to speed the code up much, simply because the
generated code probably isn't much different. (In any case, last time
I profiled a server, it wasn't spending enough time any of these
routines anyway to add up to a problem worth fixing).

In my view, this sort of thing is far more likely to inadvertently
introduce a bug than it is to yield a significant performance gain, so
as long as this code works, even if it isn't terribly dashing or
stylish, I'd be inclined not to mess with it.

> The getparent() routine in util.c has built-in squashing of slashes (much
> like no2slash.

Ooops... getparents() is called in contexts where squashing multiple
slashes is incorrect behavior. Not recommended.

rst