New issue
Advanced search Search tips

Issue 845829 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug

Blocking:
issue 839346
issue 839352



Sign in to add a comment

Chromad: D-Bus calls could take a lot of time

Project Member Reported by rsorokin@chromium.org, May 23 2018

Issue description

We have an issue with calling RefreshUserPolicy during first login.
Authpolicyd is executing GetUserStatus call > 10 sec, so user is kicked out of session.

We should decrease number of network calls. Also maybe call not important calls a bit later.

Also AuthPolicyCredentialsManager is created too early, calls GetUserStatus making authpolicyd busy.
 
Blocking: 839346
Please assign to be once you've dealt with the Chrome side. We should also improve authpolicyd performance.
Description: Show this description
Blocking: 839352
Project Member

Comment 5 by bugdroid1@chromium.org, May 29 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/dfbdc8be9b10b7000e2ce542a371c2fa26e22176

commit dfbdc8be9b10b7000e2ce542a371c2fa26e22176
Author: Lutz Justen <ljusten@chromium.org>
Date: Tue May 29 16:44:53 2018

authpolicy: Cache kdc_ip, dc_name and workgroup

Authpolicy is fairly slow since it makes a lot of Samba calls, many of
them are almost always unnecessary. For instance, the Kerberos key
distribution center (KDC) IP, the domain controller (DC) name or the
workgroup are pretty much constants. This CL caches them in memory.

A little complication arises because the KDC IP is fetched in the same
query as the server time, which always changes. Yet, the server time is
only needed for machine password handling, so only enforce up-to-date
server time for that.

If the values ever change, calls to Active Directory might start
failing. The Kerberos ticket might not refresh and policy might fail to
fetch. If that happens, logging out and back in will fix it since
authpolicyd is then restarted.

BUG= chromium:845829 
TEST=Enter 'authpolicy_debug 3' in crosh to enable debug logs.
     Log in, reload policy a few times. Look at /var/log/authpolicy.log.
     The following should only show up twice (for user and for device):
     Executing /usr/bin/net 'ads' 'lookup'
     Executing /usr/bin/net 'ads' 'info'
     Executing /usr/bin/net 'ads' 'workgroup'

Change-Id: I7fe572cc2e8ba63a8dd6026292d657f11e5d46a2
Reviewed-on: https://chromium-review.googlesource.com/1073249
Commit-Ready: Lutz Justen <ljusten@chromium.org>
Tested-by: Lutz Justen <ljusten@chromium.org>
Reviewed-by: Roman Sorokin <rsorokin@chromium.org>

[modify] https://crrev.com/dfbdc8be9b10b7000e2ce542a371c2fa26e22176/authpolicy/samba_interface.h
[modify] https://crrev.com/dfbdc8be9b10b7000e2ce542a371c2fa26e22176/authpolicy/samba_interface.cc

Project Member

Comment 6 by bugdroid1@chromium.org, May 30 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/532319e003e035f4cd376593c4cbaf0a11971d80

commit 532319e003e035f4cd376593c4cbaf0a11971d80
Author: Roman Sorokin <rsorokin@chromium.org>
Date: Wed May 30 09:59:04 2018

Chromad: Wait for policy fetch response if there is no cached policy

This CL fixes the issue that user could not get into the session because
policy fetch takes too long (>10 sec). Now if there is no cached policy
it waits until gets response or D-Bus times out.

BUG= chromium:845829 
TEST=UserActiveDirectoryPolicyManagerTest.*

Change-Id: I96db7e065564b8249b7eb0fa1f151ac5d2f4f27a
Reviewed-on: https://chromium-review.googlesource.com/1070372
Reviewed-by: Drew Wilson <atwilson@chromium.org>
Reviewed-by: Lutz Justen <ljusten@chromium.org>
Commit-Queue: Roman Sorokin <rsorokin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562781}
[modify] https://crrev.com/532319e003e035f4cd376593c4cbaf0a11971d80/chrome/browser/chromeos/policy/active_directory_policy_manager.cc
[modify] https://crrev.com/532319e003e035f4cd376593c4cbaf0a11971d80/chrome/browser/chromeos/policy/active_directory_policy_manager.h
[modify] https://crrev.com/532319e003e035f4cd376593c4cbaf0a11971d80/chrome/browser/chromeos/policy/active_directory_policy_manager_unittest.cc
[modify] https://crrev.com/532319e003e035f4cd376593c4cbaf0a11971d80/chrome/browser/chromeos/policy/component_active_directory_policy_browsertest.cc
[modify] https://crrev.com/532319e003e035f4cd376593c4cbaf0a11971d80/chrome/browser/chromeos/policy/user_policy_manager_factory_chromeos.cc

Project Member

Comment 7 by bugdroid1@chromium.org, May 30 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/3eb8f95166edf9f2709a82785bc29c60d15e4e56

commit 3eb8f95166edf9f2709a82785bc29c60d15e4e56
Author: Lutz Justen <ljusten@chromium.org>
Date: Wed May 30 19:50:37 2018

authpolicy: Delay initial password age check

Waits for the D-Bus object to be registered before the initial password
age check is performed, so that it doesn't slow down D-Bus object setup.
This fixes the issue where Chrome can't connect to the D-Bus object
because the password age check takes too long.

Also simplifies task runner handling (no need to pass around some
globally accessible object.

BUG= chromium:845829 
TEST=Check /var/log/authpolicy.log. The logs should appear in the
     following order:
     authpolicyd starting
     authpolicyd started
     Read configuration file '/var/lib/authpolicyd/config.dat'
     Running scheduled machine password age check

Change-Id: Ia252e7b6f3c7d90801ef113449aad9c7e79a6f97
Reviewed-on: https://chromium-review.googlesource.com/1073371
Commit-Ready: Lutz Justen <ljusten@chromium.org>
Tested-by: Lutz Justen <ljusten@chromium.org>
Reviewed-by: Roman Sorokin <rsorokin@chromium.org>

[modify] https://crrev.com/3eb8f95166edf9f2709a82785bc29c60d15e4e56/authpolicy/authpolicy.h
[modify] https://crrev.com/3eb8f95166edf9f2709a82785bc29c60d15e4e56/authpolicy/samba_interface.cc
[modify] https://crrev.com/3eb8f95166edf9f2709a82785bc29c60d15e4e56/authpolicy/tgt_manager.cc
[modify] https://crrev.com/3eb8f95166edf9f2709a82785bc29c60d15e4e56/authpolicy/tgt_manager.h
[modify] https://crrev.com/3eb8f95166edf9f2709a82785bc29c60d15e4e56/authpolicy/samba_interface.h
[modify] https://crrev.com/3eb8f95166edf9f2709a82785bc29c60d15e4e56/authpolicy/authpolicy.cc
[modify] https://crrev.com/3eb8f95166edf9f2709a82785bc29c60d15e4e56/authpolicy/authpolicy_main.cc

Status: Fixed (was: Started)
Labels: Merge-Request-68
Requesting merge for CL in #6. 
It fixes the issue: When initial user policy fetch takes > 10 seconds (e.g. if network is slow), user is forced out of session. We have a customer who hit that constantly.
Scope: Very small, for Active Directory management customers only. ~100 devices.
Project Member

Comment 11 by sheriffbot@chromium.org, Jun 8 2018

Labels: -Merge-Request-68 Hotlist-Merge-Review Merge-Review-68
This bug requires manual review: M68 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), kariahda@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-Merge-Review -Merge-Review-68 Merge-Approved-68
Project Member

Comment 13 by bugdroid1@chromium.org, Jun 12 2018

Labels: -merge-approved-68 merge-merged-3440
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bc9fd3793efaace3d5948d63e8ed74b4784d0974

commit bc9fd3793efaace3d5948d63e8ed74b4784d0974
Author: Roman Sorokin <rsorokin@chromium.org>
Date: Tue Jun 12 10:14:38 2018

Chromad: Wait for policy fetch response if there is no cached policy

This CL fixes the issue that user could not get into the session because
policy fetch takes too long (>10 sec). Now if there is no cached policy
it waits until gets response or D-Bus times out.

BUG= chromium:845829 
TEST=UserActiveDirectoryPolicyManagerTest.*

Change-Id: I96db7e065564b8249b7eb0fa1f151ac5d2f4f27a
Reviewed-on: https://chromium-review.googlesource.com/1070372
Reviewed-by: Drew Wilson <atwilson@chromium.org>
Reviewed-by: Lutz Justen <ljusten@chromium.org>
Commit-Queue: Roman Sorokin <rsorokin@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#562781}(cherry picked from commit 532319e003e035f4cd376593c4cbaf0a11971d80)
Reviewed-on: https://chromium-review.googlesource.com/1096957
Reviewed-by: Roman Sorokin <rsorokin@chromium.org>
Cr-Commit-Position: refs/branch-heads/3440@{#297}
Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}
[modify] https://crrev.com/bc9fd3793efaace3d5948d63e8ed74b4784d0974/chrome/browser/chromeos/policy/active_directory_policy_manager.cc
[modify] https://crrev.com/bc9fd3793efaace3d5948d63e8ed74b4784d0974/chrome/browser/chromeos/policy/active_directory_policy_manager.h
[modify] https://crrev.com/bc9fd3793efaace3d5948d63e8ed74b4784d0974/chrome/browser/chromeos/policy/active_directory_policy_manager_unittest.cc
[modify] https://crrev.com/bc9fd3793efaace3d5948d63e8ed74b4784d0974/chrome/browser/chromeos/policy/component_active_directory_policy_browsertest.cc
[modify] https://crrev.com/bc9fd3793efaace3d5948d63e8ed74b4784d0974/chrome/browser/chromeos/policy/user_policy_manager_factory_chromeos.cc

Project Member

Comment 14 by bugdroid1@chromium.org, Jun 14 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7121bff49902876c083257b502e5a882f06218f1

commit 7121bff49902876c083257b502e5a882f06218f1
Author: Roman Sorokin <rsorokin@chromium.org>
Date: Thu Jun 14 08:48:26 2018

Chromad: GetUserKerberosFiles on Chrome startup

Fixes regression introduced in CL:1068184

Also added a browsertest to verify behaviour on user kerberos files
changed D-Bus signal

BUG= chromium:845829 
TEST=ExistingUserControllerActiveDirectoryTest.UserKerberosFilesChangedSignalTriggersFileUpdate

Change-Id: Ieeb800f85210c094f34c93a71b0e7502ceae3114
Reviewed-on: https://chromium-review.googlesource.com/1089060
Reviewed-by: Alexander Alekseev <alemate@chromium.org>
Commit-Queue: Roman Sorokin <rsorokin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567195}
[modify] https://crrev.com/7121bff49902876c083257b502e5a882f06218f1/chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.cc
[modify] https://crrev.com/7121bff49902876c083257b502e5a882f06218f1/chrome/browser/chromeos/login/existing_user_controller_browsertest.cc
[modify] https://crrev.com/7121bff49902876c083257b502e5a882f06218f1/chromeos/dbus/fake_auth_policy_client.cc
[modify] https://crrev.com/7121bff49902876c083257b502e5a882f06218f1/chromeos/dbus/fake_auth_policy_client.h

Status: Verified (was: Fixed)
Verified fixed, no issues with the first and subsequent logins on Chromad. Also checked when ephemeral mode is enabled.

Chrome OS: 10718.27.0
Chrome: 68.0.3440.31
Device: Reks

Chrome OS: 10798.0.0
Chrome: 69.0.3464.0
Device: Robo

Sign in to add a comment