[Win] Add option to reauthenticate the OS user before revealing passwords.

chrome/browser/password_manager/password_manager_util_win.cc

+// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// windows.h must be first otherwise Win8 SDK breaks.
+#include <windows.h>
+#include <ntsecapi.h>
+#include <wincred.h>
+
+// SECURITY_WIN32 must be defined in order to get
+// EXTENDED_NAME_FORMAT enumeration.
+#define SECURITY_WIN32 1
+#include <security.h>
+#undef SECURITY_WIN32
+
+#include "base/strings/utf_string_conversions.h"
+#include "grit/chromium_strings.h"
+#include "grit/generated_resources.h"
+#include "ui/base/l10n/l10n_util.h"
+
+#define PASSWORD_MANAGER_MAX_PASSWORD_TRIES 3

[Garrett Casto, 2013/10/23 20:39:33]

Use a static constant here instead
(http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Preprocessor_M...)

[Will Harris, 2013/10/25 16:01:42]

Done.

+
+namespace password_manager_util {
+
+bool AuthenticateUser() {
+  bool retval = false;
+  CREDUI_INFO cui = {};
+  WCHAR username[CREDUI_MAX_USERNAME_LENGTH+1] = {};
+  WCHAR displayname[CREDUI_MAX_USERNAME_LENGTH+1] = {};
+  WCHAR password[CREDUI_MAX_PASSWORD_LENGTH+1] = {};
+  DWORD username_length = CREDUI_MAX_USERNAME_LENGTH;
+  std::wstring product_name =
+      UTF16ToWide(l10n_util::GetStringUTF16(IDS_PRODUCT_NAME));
+  std::wstring password_prompt =
+      UTF16ToWide(l10n_util::GetStringUTF16(
+                  IDS_PASSWORDS_PAGE_AUTHENTICATION_PROMPT));
+  HANDLE handle = INVALID_HANDLE_VALUE;
+  int tries = 0;
+  bool use_displayname = false;
+  bool use_principalname = false;
+  DWORD logon_result = 0;
+
+  // On a domain, we obtain the User Principal Name
+  // for domain authentication.
+  if (GetUserNameEx(NameUserPrincipal, username, &username_length)) {

[cpu_(ooo_6.6-7.5), 2013/10/23 19:54:43]

I don't think we need to do SecureZeroMemory for username or displayname. I
understand it does not hurt but it keeps me on my toes when reading the code.

[Will Harris, 2013/10/25 16:01:42]

Done.

+    use_principalname = true;
+  } else {
+    username_length = CREDUI_MAX_USERNAME_LENGTH;

[Garrett Casto, 2013/10/23 20:39:33]

Do you actually need this? The API for GetUserNameEx() seems to say that
username_length will only be modified if it returns true
(http://msdn.microsoft.com/en-us/library/windows/desktop/ms724435(v=vs.85).aspx).

[Will Harris, 2013/10/25 16:01:42]

both APIs have lpnSize as an in/out parameter so we need to reset the parameter
to the length otherwise the call will fail if the length supplied is longer,
unless I'm misunderstanding what you're saying here?

+    // Otherwise, we're a workstation, use the plain local username.
+    if (!GetUserName(username, &username_length)) {
+      DLOG(ERROR) << "Unable to obtain username " << GetLastError();
+      return false;
+    } else {
+      // As we are on a workstation, it's possible the user
+      // has no password, so check here.
+      logon_result = LogonUser(username,
+                               L".",
+                               L"",
+                               LOGON32_LOGON_NETWORK,
+                               LOGON32_PROVIDER_DEFAULT,
+                               &handle);
+      // Windows XP and above return ERROR_ACCOUNT_RESTRICTION for
+      // when the password is blank, so check for that here.
+      // See http://support.microsoft.com/kb/303846.
+      if (logon_result || GetLastError() == ERROR_ACCOUNT_RESTRICTION) {
+        SecureZeroMemory(username, sizeof(username));
+        if (logon_result)
+          CloseHandle(handle);
+        return true;
+      }
+    }
+  }
+
+  // Try and obtain a friendly display name.
+  username_length = CREDUI_MAX_USERNAME_LENGTH;
+  if (GetUserNameEx(NameDisplay, displayname, &username_length)) {
+    use_displayname = true;
+  }
+
+  cui.cbSize = sizeof(CREDUI_INFO);
+  cui.hwndParent = NULL;
+  cui.pszMessageText = password_prompt.c_str();
+  cui.pszCaptionText = product_name.c_str();
+
+  cui.hbmBanner = NULL;
+  BOOL save_password = FALSE;
+  DWORD credErr = NO_ERROR;
+  DWORD flags = CREDUI_FLAGS_GENERIC_CREDENTIALS |
+                CREDUI_FLAGS_EXCLUDE_CERTIFICATES |
+                CREDUI_FLAGS_KEEP_USERNAME |
+                CREDUI_FLAGS_ALWAYS_SHOW_UI |
+                CREDUI_FLAGS_DO_NOT_PERSIST;
+
+  do {
+    SecureZeroMemory(password, sizeof(password));
+    tries++;
+
+   // TODO(wfh) Make sure we support smart cards here.

[Garrett Casto, 2013/10/23 20:39:33]

indentation

[Will Harris, 2013/10/25 16:01:42]

Done.

+    credErr = CredUIPromptForCredentials(
+                  &cui,
+                  product_name.c_str(),
+                  NULL,
+                  0,
+                  use_displayname ? displayname : username,
+                  CREDUI_MAX_USERNAME_LENGTH+1,
+                  password,
+                  CREDUI_MAX_PASSWORD_LENGTH+1,
+                  &save_password,
+                  flags | (tries > 1 ? CREDUI_FLAGS_INCORRECT_PASSWORD : 0));
+
+    if (credErr == NO_ERROR) {
+      logon_result = LogonUser(username,
+                               use_principalname ? NULL : L".",
+                               password,
+                               LOGON32_LOGON_NETWORK,
+                               LOGON32_PROVIDER_DEFAULT,
+                               &handle);
+      if (logon_result) {
+          retval = true;

[Garrett Casto, 2013/10/23 20:39:33]

indentation

[Will Harris, 2013/10/25 16:01:42]

Done.

+        CloseHandle(handle);
+      } else {
+        if (GetLastError() == ERROR_ACCOUNT_RESTRICTION &&
+            wcslen(password) == 0) {
+          // Password is blank, so permit.

[Garrett Casto, 2013/10/23 20:39:33]

You have already checked local users for blank passwords so this must only be to
check for users with domain authentication. Is there any reason that this can't
also be checked ahead of time? Something like

if (GetUserNameEx(...)) {
  ...
} else if (!GetUserName(...)) {
  ...
}

// Check for empty password
logon_result = LogonUser(...)

[Will Harris, 2013/10/25 16:01:42]

I only wanted to perform the blank password check against local SAM credentials
because it's far more less likely in the standalone workstation case that
account lockout restrictions are enabled.   It's also very unlikely that a blank
password is set on a domain account - in fact I'm not sure that's even possible
since a blank domain account password would by default prevent network logins
and all sorts of havoc would ensue.

I now have some code that checks for blank password from the SAM in a superior
way, I'll upload this for review.  Once that code is in, this code will only
remain in case the blank password check reports a false negative for some reason
and to prevent locking users out of their passwords.

+          retval = true;
+        } else {
+          DLOG(WARNING) << "Unable to authenticate " << GetLastError();
+        }
+      }
+    }
+  } while (credErr == NO_ERROR &&
+           (retval == false && tries < PASSWORD_MANAGER_MAX_PASSWORD_TRIES));
+
+  SecureZeroMemory(displayname, sizeof(displayname));
+  SecureZeroMemory(username, sizeof(username));
+  SecureZeroMemory(password, sizeof(password));

[cpu_(ooo_6.6-7.5), 2013/10/23 19:54:43]

lets move 136 to right after LogonUser  line 117.

[Will Harris, 2013/10/25 16:01:42]

can't because need wcslen(password)

+
+  return retval;
+}
+
+}  // namespace password_manager_util