After discussing this with the author I realized I had misread the patch.
The new code moves the check in question from before the "if (!SC->bEnabled)" to later
in the sequence:
(check used to be here)
/*
* We decline operation in various situations...
*/
if (!sc->bEnabled)
return DECLINED;
if (ap_ctx_get(r->connection->client->ctx, "ssl") == NULL)
return DECLINED;
if (!(dc->nOptions & SSL_OPT_FAKEBASICAUTH))
return DECLINED;
if (r->connection->user)
return DECLINED;
if ((clientdn = (char *)ap_ctx_get(r->connection->client->ctx, "ssl::client::dn"))
== NULL)
{
/*
* Make sure the user is not able to fake the client certificate
* based authentication by just entering an X.509 Subject DN
* ("/XX=YYY/XX=YYY/..") as the username and "password" as the
* password.
*/
if ((cpAL = ap_table_get(r->headers_in, "Authorization")) != NULL) {
.
.
.
This fixes the problem where the check fails the second time through on a subrequest
or internal redirect and catches a spoof attempt in the situation when there is no
client certificate DN.
My only question is: Can a user still "spoof" a FakeBasicAuth request when one of the
other four previous "DECLINED" conditions are true?
Another way to approach the problem might be to keep the check where it was, but
enforce it only when (ap_is_initial_req(r)) is true. The spoof can only be attempted
on the initial request - not on any subrequests or internal redirects and will catch
spoof attempts for all of the "DECLINED" conditions.
Anyone with more experience with this code care to comment?
Rick Barry
Compaq Computer Corporation Compaq Secure Web Server Project Team
110 Spit Brook Road OpenVMS System Software Group
Nashua, NH 03062 Business Critical Server Group
(603) 884-0634
-----Original Message-----
From: Barry, Richard
Sent: Wednesday, April 24, 2002 10:42 AM
To: '[EMAIL PROTECTED]'
Subject: RE: [BugDB] Client Authentication BUG with FakeBasicAuth
(PR#695)
This submission is missing a conditional expression before line 1161.
What test is performed prior to executing the DN/password check in the
new code?
Rick Barry
Compaq Computer Corporation Compaq Secure Web Server Project Team
110 Spit Brook Road OpenVMS System Software Group
Nashua, NH 03062 Business Critical Server Group
(603) 884-0634
-----Original Message-----
From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED]]
Sent: Wednesday, April 17, 2002 6:54 AM
To: [EMAIL PROTECTED]
Cc: [EMAIL PROTECTED]
Subject: [BugDB] Client Authentication BUG with FakeBasicAuth (PR#695)
Full_Name: Sergio Rabellino
Version: 2.8.8
OS: Solaris 7
Submission from: (NULL) (130.192.239.73)
The "if" in ssl_engine_kernel.c at line 1130 to check against DN/password
authorization
directly form a client, break also the internal redirect done by apache under
some conditions, as the directory indexing ...
So if you use client auth, with fake basic auth and require an index, you get a
301 followed by a 403 (Forbidden)...
Below i've attached a diff patch to correct this behaviour; i've tested it on my
hosts
and all things should be fine now.
Thanks to Nick Miles for pinpointing me to the solution.
Bye.
---snip
1130,1147d1129
< * Make sure the user is not able to fake the client certificate
< * based authentication by just entering an X.509 Subject DN
< * ("/XX=YYY/XX=YYY/..") as the username and "password" as the
< * password.
< */
< if ((cpAL = ap_table_get(r->headers_in, "Authorization")) != NULL) {
< if (strcEQ(ap_getword(r->pool, &cpAL, ' '), "Basic")) {
< while (*cpAL == ' ' || *cpAL == '\t')
< cpAL++;
< cpAL = ap_pbase64decode(r->pool, cpAL);
< cpUN = ap_getword_nulls(r->pool, &cpAL, ':');
< cpPW = cpAL;
< if (cpUN[0] == '/' && strEQ(cpPW, "password"))
< return FORBIDDEN;
< }
< }
<
< /*
1158a1141,1161
> {
> /*
> * Make sure the user is not able to fake the client certificate
> * based authentication by just entering an X.509 Subject DN
> * ("/XX=YYY/XX=YYY/..") as the username and "password" as the
> * password.
> */
> if ((cpAL = ap_table_get(r->headers_in, "Authorization")) != NULL) {
> if (strcEQ(ap_getword(r->pool, &cpAL, ' '), "Basic")) {
> while (*cpAL == ' ' || *cpAL == '\t')
> cpAL++;
> cpAL = ap_pbase64decode(r->pool, cpAL);
> cpUN = ap_getword_nulls(r->pool, &cpAL, ':');
> cpPW = cpAL;
> if (cpUN[0] == '/' && strEQ(cpPW, "password"))
> {
> ssl_log(r->server, SSL_LOG_INFO, "WARNING: Old mod_ssl
breakthrough solicited (FakeBasicAuth by DN) !");
> return FORBIDDEN;
> }
> }
> }
1159a1163
> }
1160a1165
>
--snip
______________________________________________________________________
Apache Interface to OpenSSL (mod_ssl) www.modssl.org
User Support Mailing List [EMAIL PROTECTED]
Automated List Manager [EMAIL PROTECTED]
______________________________________________________________________
Apache Interface to OpenSSL (mod_ssl) www.modssl.org
User Support Mailing List [EMAIL PROTECTED]
Automated List Manager [EMAIL PROTECTED]
