Prevent the ability to scrape for valid usernames

- return the same error message when either bind or the username search
   fails to find a user
 - config variables should use hypen and not underscore

conf/default.conf.php

@@ -648,7 +648,7 @@ return array(
   'ldap.search-first'         => false,
 
   // The attribute to search for if you have to search for a user
-  'ldap.username_attribute' => '',
+  'ldap.username-attribute' => '',
 
   // The attribute(s) to be regarded as 'real name'.
   // If more then one attribute is supplied the values of the attributes in

src/__phutil_library_map__.php

@@ -710,6 +710,7 @@ phutil_register_library_map(array(
     'PhabricatorLDAPLoginController' => 'applications/auth/controller/PhabricatorLDAPLoginController.php',
     'PhabricatorLDAPProvider' => 'applications/auth/ldap/PhabricatorLDAPProvider.php',
     'PhabricatorLDAPRegistrationController' => 'applications/auth/controller/PhabricatorLDAPRegistrationController.php',
+    'PhabricatorLDAPUnknownUserException' => 'applications/auth/ldap/PhabricatorLDAPUnknownUserException.php',
     'PhabricatorLDAPUnlinkController' => 'applications/auth/controller/PhabricatorLDAPUnlinkController.php',
     'PhabricatorLintEngine' => 'infrastructure/lint/PhabricatorLintEngine.php',
     'PhabricatorLiskDAO' => 'infrastructure/storage/lisk/PhabricatorLiskDAO.php',
@@ -1727,6 +1728,7 @@ phutil_register_library_map(array(
     'PhabricatorJavelinLinter' => 'ArcanistLinter',
     'PhabricatorLDAPLoginController' => 'PhabricatorAuthController',
     'PhabricatorLDAPRegistrationController' => 'PhabricatorAuthController',
+    'PhabricatorLDAPUnknownUserException' => 'Exception',
     'PhabricatorLDAPUnlinkController' => 'PhabricatorAuthController',
     'PhabricatorLintEngine' => 'PhutilLintEngine',
     'PhabricatorLiskDAO' => 'LiskDAO',

src/applications/auth/ldap/PhabricatorLDAPProvider.php

@@ -17,6 +17,10 @@
  */
 
 final class PhabricatorLDAPProvider {
+  // http://www.php.net/manual/en/function.ldap-errno.php#20665 states
+  // that the number could be 31 or 49, in testing it has always been 49
+  const LDAP_INVALID_CREDENTIALS = 49;
+
   private $userData;
   private $connection;
 
@@ -47,7 +51,7 @@ final class PhabricatorLDAPProvider {
   }
 
   public function getUsernameAttribute() {
-    return PhabricatorEnv::getEnvConfig('ldap.username_attribute');
+    return PhabricatorEnv::getEnvConfig('ldap.username-attribute');
   }
 
   public function getLDAPVersion() {
@@ -116,14 +120,28 @@ final class PhabricatorLDAPProvider {
     return $this->userData;
   }
 
+  private function invalidLDAPUserErrorMessage($errno, $errmsg) {
+    return "LDAP Error #".$errno.": ".$errmsg;
+  }
+
   public function auth($username, PhutilOpaqueEnvelope $password) {
     if (strlen(trim($username)) == 0) {
       throw new Exception('Username can not be empty');
     }
 
     if (PhabricatorEnv::getEnvConfig('ldap.search-first')) {
+      // To protect against people phishing for accounts we catch the
+      // exception and present the default exception that would be presented
+      // in the case of a failed bind.
+      try {
         $user = $this->getUser($this->getUsernameAttribute(), $username);
-      $username = $user[($this->getSearchAttribute())][0];
+        $username = $user[$this->getSearchAttribute()][0];
+      } catch (PhabricatorLDAPUnknownUserException $e) {
+        throw new Exception(
+          $this->invalidLDAPUserErrorMessage(
+            self::LDAP_INVALID_CREDENTIALS,
+            ldap_err2str(self::LDAP_INVALID_CREDENTIALS)));
+      }
     }
 
     $conn = $this->getConnection();
@@ -149,7 +167,9 @@ final class PhabricatorLDAPProvider {
 
     if (!$result) {
       throw new Exception(
-        "LDAP Error #".ldap_errno($conn).": ".ldap_error($conn));
+        $this->invalidLDAPUserErrorMessage(
+          ldap_errno($conn),
+          ldap_error($conn)));
     }
 
     $this->userData = $this->getUser($this->getSearchAttribute(), $username);
@@ -183,7 +203,7 @@ final class PhabricatorLDAPProvider {
     }
 
     if ($entries['count'] == 0) {
-      throw new Exception('Could not find user');
+      throw new PhabricatorLDAPUnknownUserException('Could not find user');
     }
 
     return $entries[0];

src/applications/auth/ldap/PhabricatorLDAPUnknownUserException.php

@@ -0,0 +1,20 @@
+<?php
+
+/*
+ * Copyright 2012 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+final class PhabricatorLDAPUnknownUserException extends Exception{
+}