You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by co...@apache.org on 2017/06/19 17:01:50 UTC

svn commit: r1799232 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS include/ap_mmn.h include/http_protocol.h server/protocol.c server/request.c

Author: covener
Date: Mon Jun 19 17:01:50 2017
New Revision: 1799232

URL: http://svn.apache.org/viewvc?rev=1799232&view=rev
Log:
Merge https://svn.apache.org/r1796348 from trunk:

  *) SECURITY: CVE-2017-3167 (cve.mitre.org)
     Use of the ap_get_basic_auth_pw() by third-party modules outside of the
     authentication phase may lead to authentication requirements being
     bypassed.
     [Emmanuel Dreyfus <manu netbsd.org>, Jacob Champion, Eric Covener]


Submitted By: Emmanuel Dreyfus <manu netbsd.org>, Jacob Champion, Eric Covener
Reviewed By: covener, ylavic, wrowe


Modified:
    httpd/httpd/branches/2.2.x/CHANGES
    httpd/httpd/branches/2.2.x/STATUS
    httpd/httpd/branches/2.2.x/include/ap_mmn.h
    httpd/httpd/branches/2.2.x/include/http_protocol.h
    httpd/httpd/branches/2.2.x/server/protocol.c
    httpd/httpd/branches/2.2.x/server/request.c

Modified: httpd/httpd/branches/2.2.x/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/CHANGES?rev=1799232&r1=1799231&r2=1799232&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/CHANGES [utf-8] (original)
+++ httpd/httpd/branches/2.2.x/CHANGES [utf-8] Mon Jun 19 17:01:50 2017
@@ -13,6 +13,12 @@ Changes with Apache 2.2.33
      ap_hook_process_connection() during an HTTP request to an HTTPS port.
      [Yann Ylavic]
 
+  *) SECURITY: CVE-2017-3167 (cve.mitre.org)
+     Use of the ap_get_basic_auth_pw() by third-party modules outside of the
+     authentication phase may lead to authentication requirements being
+     bypassed.
+     [Emmanuel Dreyfus <manu netbsd.org>, Jacob Champion, Eric Covener]
+  
   *) Fix HttpProtocolOptions to inherit from global to VirtualHost scope.
      [Joe Orton]
 

Modified: httpd/httpd/branches/2.2.x/STATUS
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/STATUS?rev=1799232&r1=1799231&r2=1799232&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/STATUS (original)
+++ httpd/httpd/branches/2.2.x/STATUS Mon Jun 19 17:01:50 2017
@@ -104,12 +104,6 @@ RELEASE SHOWSTOPPERS:
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
-  *) core: ap_get_basic_auth_pw deprecation
-     trunk patch: https://svn.apache.org/r1796348
-     2.2.x patch: http://people.apache.org/~covener/patches/httpd-2.2.x-ap_get_basic_auth_pw.diff
-                  (remove some_authn_required change, change to strcasecmp, mmn)
-     +1: covener, ylavic, wrowe
-
    *) mod_mime: Fix scanning of quoted-pairs.
       trunk patch: http://svn.apache.org/r1797550
       2.4.x patch: svn merge -c 1797550 ^/httpd/httpd/trunk .

Modified: httpd/httpd/branches/2.2.x/include/ap_mmn.h
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/include/ap_mmn.h?rev=1799232&r1=1799231&r2=1799232&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/include/ap_mmn.h (original)
+++ httpd/httpd/branches/2.2.x/include/ap_mmn.h Mon Jun 19 17:01:50 2017
@@ -167,6 +167,8 @@
  *                      and ap_scan_vchar_obstext()
  *                      Replaced fold boolean with with multiple bit flags
  *                      to ap_[r]getline()
+ * 20051115.43 (2.2.33)  Add ap_get_basic_auth_components() and deprecate
+ *                       ap_get_basic_auth_pw()
  */
 
 #define MODULE_MAGIC_COOKIE 0x41503232UL /* "AP22" */
@@ -174,7 +176,7 @@
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
 #define MODULE_MAGIC_NUMBER_MAJOR 20051115
 #endif
-#define MODULE_MAGIC_NUMBER_MINOR 42                    /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 43                    /* 0...n */
 
 /**
  * Determine if the server's current MODULE_MAGIC_NUMBER is at least a

Modified: httpd/httpd/branches/2.2.x/include/http_protocol.h
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/include/http_protocol.h?rev=1799232&r1=1799231&r2=1799232&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/include/http_protocol.h (original)
+++ httpd/httpd/branches/2.2.x/include/http_protocol.h Mon Jun 19 17:01:50 2017
@@ -486,7 +486,11 @@ AP_DECLARE(void) ap_note_basic_auth_fail
 AP_DECLARE(void) ap_note_digest_auth_failure(request_rec *r);
 
 /**
- * Get the password from the request headers
+ * Get the password from the request headers. This function has multiple side
+ * effects due to its prior use in the old authentication framework.
+ * ap_get_basic_auth_components() should be preferred.
+ *
+ * @deprecated @see ap_get_basic_auth_components
  * @param r The current request
  * @param pw The password as set in the headers
  * @return 0 (OK) if it set the 'pw' argument (and assured
@@ -499,6 +503,25 @@ AP_DECLARE(void) ap_note_digest_auth_fai
  */
 AP_DECLARE(int) ap_get_basic_auth_pw(request_rec *r, const char **pw);
 
+#define AP_GET_BASIC_AUTH_PW_NOTE "AP_GET_BASIC_AUTH_PW_NOTE"
+
+/**
+ * Get the username and/or password from the request's Basic authentication
+ * headers. Unlike ap_get_basic_auth_pw(), calling this function has no side
+ * effects on the passed request_rec.
+ *
+ * @param r The current request
+ * @param username If not NULL, set to the username sent by the client
+ * @param password If not NULL, set to the password sent by the client
+ * @return APR_SUCCESS if the credentials were successfully parsed and returned;
+ *         APR_EINVAL if there was no authentication header sent or if the
+ *         client was not using the Basic authentication scheme. username and
+ *         password are unchanged on failure.
+ */
+AP_DECLARE(apr_status_t) ap_get_basic_auth_components(const request_rec *r,
+                                                      const char **username,
+                                                      const char **password);
+
 /**
  * parse_uri: break apart the uri
  * @warning Side Effects: 

Modified: httpd/httpd/branches/2.2.x/server/protocol.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/server/protocol.c?rev=1799232&r1=1799231&r2=1799232&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/server/protocol.c (original)
+++ httpd/httpd/branches/2.2.x/server/protocol.c Mon Jun 19 17:01:50 2017
@@ -1594,6 +1594,7 @@ AP_DECLARE(int) ap_get_basic_auth_pw(req
 
     t = ap_pbase64decode(r->pool, auth_line);
     r->user = ap_getword_nulls (r->pool, &t, ':');
+    apr_table_setn(r->notes, AP_GET_BASIC_AUTH_PW_NOTE, "1");
     r->ap_auth_type = "Basic";
 
     *pw = t;
@@ -1601,6 +1602,53 @@ AP_DECLARE(int) ap_get_basic_auth_pw(req
     return OK;
 }
 
+AP_DECLARE(apr_status_t) ap_get_basic_auth_components(const request_rec *r,
+                                                      const char **username,
+                                                      const char **password)
+{
+    const char *auth_header;
+    const char *credentials;
+    const char *decoded;
+    const char *user;
+
+    auth_header = (PROXYREQ_PROXY == r->proxyreq) ? "Proxy-Authorization"
+                                                  : "Authorization";
+    credentials = apr_table_get(r->headers_in, auth_header);
+
+    if (!credentials) {
+        /* No auth header. */
+        return APR_EINVAL;
+    }
+
+    if (strcasecmp(ap_getword(r->pool, &credentials, ' '), "Basic")) {
+        /* These aren't Basic credentials. */
+        return APR_EINVAL;
+    }
+
+    while (*credentials == ' ' || *credentials == '\t') {
+        credentials++;
+    }
+
+    /* XXX Our base64 decoding functions don't actually error out if the string
+     * we give it isn't base64; they'll just silently stop and hand us whatever
+     * they've parsed up to that point.
+     *
+     * Since this function is supposed to be a drop-in replacement for the
+     * deprecated ap_get_basic_auth_pw(), don't fix this for 2.4.x.
+     */
+    decoded = ap_pbase64decode(r->pool, credentials);
+    user = ap_getword_nulls(r->pool, &decoded, ':');
+
+    if (username) {
+        *username = user;
+    }
+    if (password) {
+        *password = decoded;
+    }
+
+    return APR_SUCCESS;
+}
+
 struct content_length_ctx {
     int data_sent;  /* true if the C-L filter has already sent at
                      * least one bucket on to the next output filter

Modified: httpd/httpd/branches/2.2.x/server/request.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/server/request.c?rev=1799232&r1=1799231&r2=1799232&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/server/request.c (original)
+++ httpd/httpd/branches/2.2.x/server/request.c Mon Jun 19 17:01:50 2017
@@ -179,6 +179,14 @@ AP_DECLARE(int) ap_process_request_inter
         r->ap_auth_type = r->prev->ap_auth_type;
     }
     else {
+        /* A module using a confusing API (ap_get_basic_auth_pw) caused
+        ** r->user to be filled out prior to check_authn hook. We treat
+        ** it is inadvertent.
+        */
+        if (r->user && apr_table_get(r->notes, AP_GET_BASIC_AUTH_PW_NOTE)) { 
+            r->user = NULL;
+        }
+
         switch (ap_satisfies(r)) {
         case SATISFY_ALL:
         case SATISFY_NOSPEC: