Subject: [BugDB] Client Authentication BUG with FakeBasicAuth (PR#695)

From: [email-masked]
Date: Wed, 17 Apr 2002 12:54:22 +0200 (MET DST)

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-masked]
Automated List Manager                            [email-masked]

---

From: [email-masked]
Date: Wed, 24 Apr 2002 16:43:07 +0200 (MET DST)

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-masked] [mailto:[email-masked]]
Sent: Wednesday, April 17, 2002 6:54 AM
To: [email-masked]
Cc: [email-masked]
Subject: [BugDB] Client Authentication BUG with FakeBasicAuth (PR#695)

Bye.

---snip
1130,1147d1129
1159a1163
1160a1165
--snip

---

From: [email-masked]
Date: Tue, 7 May 2002 15:38:05 +0200 (MET DST)

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

-----Original Message-----
From: Barry, Richard 
Sent: Wednesday, April 24, 2002 10:42 AM
To: [email-masked]'
Subject: RE: [BugDB] Client Authentication BUG with FakeBasicAuth
(PR#695)

Rick Barry

-----Original Message-----
From: [email-masked] [mailto:[email-masked]]
Sent: Wednesday, April 17, 2002 6:54 AM
To: [email-masked]
Cc: [email-masked]
Subject: [BugDB] Client Authentication BUG with FakeBasicAuth (PR#695)

Bye.

---snip
1130,1147d1129
1159a1163
1160a1165
--snip