This is something that will benefit from a design doc :-)

For existing managed user profiles (which is what matters here), we try to fetch fresh user policy on sign-in. If we have no result within 10 seconds[1], we'll just let the profile initialize and use the cached policy from disk.

It's now possible that afterwards, we will get a response from DMServer indicating HTTP Status code 410. One thing we'll have to decide is whether we want to terminate such a session then and require online sign-in immediately, or let it run with cached policy and just require online sign-in on the next sign-in. 

Another question would be if we want to just wipe the user data and start fresh, or try to re-register policy in an existing user home (the question in the second case would be if we need to do something special in session_manager to accept the new policy).

[1] https://cs.chromium.org/chromium/src/chrome/browser/chromeos/policy/user_policy_manager_factory_chromeos.cc?rcl=550a1c4ab1034d8f0b114cec6e62092b34b9b2c1&l=75

so: +1 to design doc.

Also: yes, I think if we get a 410 post-signin, we should terminate the session, blow away cached policy, and mark the user as requiring an online signin (user should have the flag set stating that they require policy, I think).

We should not wipe all the user data IMO - at most, we should remove policy. Also, the reason I'm suggesting to remove policy is I think it's probably safer than having SessionManager trust us to overwrite policy with a completely new/different blob.

Finally, one thing we probably need to make sure is handled gracefully

1) Device goes offline for some long period of time.
2) In the meantime, DMServer flushes the DMToken.
3) Dasher admin turns off CDM.
4) Device comes back online and user signs in - what should happen here re: management? Should ARC still have a DeviceOwner (I think it should)?

The reason why I thought overwriting may be better is that a "throw away
policy" operation in session manager would essentially not be authorized by
anything, while a "overwrite policy" operation could check the signature of
the new policy.
This would probably only be a benefit if session_manager contained the root
verification key though, which is not the case at the moment.

atwilson via monorail <monorail+v2.613097587@chromium.org> schrieb am Fr.,
3. Aug. 2018, 13:38:

What checks *does* SessionManager do currently? Because maybe overwriting is possible (if, for example, we can just check things like "same signing key" or "same domain and/or gaia id").

Re ARC++: We have no way to remove the DO (other than Unicorn graduation, but I do not want to use that for enterprise/edu).

What is the behavior today if the admin of an active user turns off CDM? Are policies removed? Does the user become unmanaged?

If you turn off CDM today, you remain managed and keep your stale policies.

Anyhow, I *think* we can re-register as long as we keep the same device ID specified in install attributes (session manager will accept the new policy). We're relying on this for offline->online demo mode transition. So the way I'd build this is:

1) Keep policy as-is in session manager.
2) If we get a 410 (or whatever we decide is the new 'omg we deleted yer dmtoken' error), we re-register using the old device ID.
3) Once that succeeds, we overwrite the existing policy.
4) As long as it fails (for whatever reason - network errors, license errors, domain no longer enrolled, etc) we should just keep enforcing the stale policy from #1 and not remove it - this means we never need to worry about transitioning ARC (or the session itself) from managed->unmanaged.

We're supporting re-registration for user sessions only, correct?
What would the 'old device ID' be in this case?

Can't we just re-register with a DeviceRegisterRequest authorized by:
- Authorization: GoogleLogin auth=<auth cookie for Mobile Sync>
(I'm assuming that we can get a that because this will happen in a signed-in state.)

I don't remember having to transfer any kind of device ID in registration for user policy, but I may have forgotten something.

Ah, yeah, sorry. You're right - only gonna discard user dmtokens, so install attributes are irrelevant.

For registration, client_id == device_id. I suspect you'll have to pass the same client_id when re-registering if you want session manager to accept the policy, but again, I don't know what checks session manager does (see comment #4 above).

Point is, I think we should probably try to keep policy around if possible instead of blowing it away as I originally suggested.

[revised comment due to crucial typo in the example flow :) ]

> I don't know what checks session manager does (see comment #4 above).
Not an authoritative answer, but it seems that session_manager only checks if the signature can be verified using the "policy key"[1], which will be the key transported in |new_public_key|[2] during the initial policy fetch[3] or the last key rotation[4].
So as long as DMServer is still using the same signing key for the domain, it should be possible to just re-register and persist the new policy received.
Even if DMServer had a key rotation, but remembers the keys it used to use, the rotation mechanism should be triggered because we upload the |public_key_version| the policy key last known by the client had.

So, I guess the question is whether in the following flow:
(1) Register for user policy with user A -> User_DMToken_1
(2) Fetch policy using User_DMToken_1 -> policy_1, policy_public_key_1
(3) User_DMToken_1 purged
(4) Try fetching policy using User_DMToken_1 -> HTTP 410 or whatever
(5) Register for user policy with user A -> User_DMToken_2
(6) Fetch policy using User_DMToken_2 -> policy_2, policy_public_key_2

Will policy_public_key_1 == policy_public_key_2?
It *seems* that DMServer stores policy keypairs per "customer id" (which *seems* to hint that yes, the key will be the same), but I really have no idea and we should ask dkalin@ :-)

If this is not guaranteed or we explicitly don't want policy_public_key_1 == policy_public_key_2, we could maybe try going another route (after checking back with mnissler@): Bake the root verification public key[5] into session_manager, and accept a policy with policy_public_key_2 if it can be verified using the root verification key[6](*).

> I think we should probably try to keep policy around if possible instead of blowing it away as I originally suggested.
-> Agree!

[1] https://chromium.googlesource.com/chromiumos/platform2/+/46e8d97e70b330115dc00eab0d6a42ab7392aea2/login_manager/policy_service.cc#249
[2] https://cs.chromium.org/chromium/src/components/policy/proto/device_management_backend.proto?rcl=5e09e4f558cec243c4dce1e571d5095fa611d954&l=543
[3] https://chromium.googlesource.com/chromiumos/platform2/+/46e8d97e70b330115dc00eab0d6a42ab7392aea2/login_manager/policy_service.cc#230
[4] https://chromium.googlesource.com/chromiumos/platform2/+/46e8d97e70b330115dc00eab0d6a42ab7392aea2/login_manager/policy_service.cc#225
[5] https://cs.chromium.org/chromium/src/components/policy/core/common/cloud/cloud_policy_constants.cc?rcl=35955aca24d5faf8694baec595006588be411b91&l=88
[6] similar to CloudPolicyValidator::CheckInitialKey: https://cs.chromium.org/chromium/src/components/policy/core/common/cloud/cloud_policy_validator.cc?rcl=35955aca24d5faf8694baec595006588be411b91&l=409

(*) One consequence of this approach would be that if the private key part of the root verification key leaks, an attacker could be generating policies for already registered devices which these devices would accept. Not sure if this would be an issue.

It's a per-domain key so we can always do the rotation - we want to verify that the old public_key_version is passed to the new post-register policy fetch (i.e. CloudPolicyClient::public_key_version_ is left correctly set from the load of the cached policy), and we should be fine.

Triage nag: This Chrome OS bug has an owner but no component. Please add a component so that this can be tracked by the relevant team.

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

commit 3a845098f5f445e54eef1f66581377f7a8a70806
Author: Alexander Hendrich <hendrich@chromium.org>
Date: Wed Oct 17 12:37:24 2018

Handle missing DMServer entry through re-registration

This CL handles the scenario of DMServer reporting a 410 status code
(missing/unknown/purged client ID) by trying background
re-registration. If this fails, the user is marked as requiring an
online sign-in.

DD: go/handle-purged-user-sessions

Bug:  870616 
Change-Id: I4a4c2370c36f5464752029d755d23221cd064b0e
Reviewed-on: https://chromium-review.googlesource.com/c/1227939
Commit-Queue: Alexander Hendrich <hendrich@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Reviewed-by: Sergey Poromov <poromov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600359}
[modify] https://crrev.com/3a845098f5f445e54eef1f66581377f7a8a70806/chrome/browser/chromeos/login/users/fake_chrome_user_manager.cc
[modify] https://crrev.com/3a845098f5f445e54eef1f66581377f7a8a70806/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc
[modify] https://crrev.com/3a845098f5f445e54eef1f66581377f7a8a70806/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.h
[modify] https://crrev.com/3a845098f5f445e54eef1f66581377f7a8a70806/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos_unittest.cc
[modify] https://crrev.com/3a845098f5f445e54eef1f66581377f7a8a70806/components/policy/core/common/cloud/cloud_policy_client.cc
[modify] https://crrev.com/3a845098f5f445e54eef1f66581377f7a8a70806/components/policy/core/common/cloud/cloud_policy_client.h
[modify] https://crrev.com/3a845098f5f445e54eef1f66581377f7a8a70806/components/policy/core/common/cloud/cloud_policy_client_unittest.cc
[modify] https://crrev.com/3a845098f5f445e54eef1f66581377f7a8a70806/components/policy/proto/device_management_backend.proto

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

commit 2adccbd760c8cc930df3594ea218831cb68abadf
Author: Alexander Hendrich <hendrich@chromium.org>
Date: Tue Oct 23 09:51:33 2018

[UMA] Handle missing DMServer entry through re-registration

This CL adds UMA to the re-registration flow to observe number of
triggered, successful and unsuccessful re-registrations attempts.

DD: go/handle-purged-user-sessions

Bug:  870616 
Change-Id: Ic963d321fb84cac55a61571b49e4a41554872589
Reviewed-on: https://chromium-review.googlesource.com/c/1283026
Commit-Queue: Alexander Hendrich <hendrich@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Sergey Poromov <poromov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601893}
[modify] https://crrev.com/2adccbd760c8cc930df3594ea218831cb68abadf/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc
[modify] https://crrev.com/2adccbd760c8cc930df3594ea218831cb68abadf/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos_unittest.cc
[modify] https://crrev.com/2adccbd760c8cc930df3594ea218831cb68abadf/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/2adccbd760c8cc930df3594ea218831cb68abadf/tools/metrics/histograms/histograms.xml

requesting merge approval for CLs in comments #14 (actual CL) and #15 (UMA stats) to M71. Once we start purging user sessions on the server-side, we want to have as many as possible clients with this re-registration logic, so it would make sense to have this in M71 if possible.

This change impacts a number of areas.  Can you add context regarding testing on ToT/M72 or otherwise prior to a merge?  I'd like to understand the merge risk.  Thanks.

This change has been on ToT for 6 days, but still needs verification from the test team -> setting status to 'Fixed'. I've written some instructions for manual testing in the design doc (go/handle-purged-user-sessions, last section).

This CL will only be triggered when the DMServer sends out a 410 response code (failed policy fetch -> client unknown). Previous behavior was to cancel all scheduled policy updates and remain in that state. With this CL, the client will send a re-registration message to the DMServer and try a new policy fetch if that succeeds.

Your change meets the bar and is auto-approved for M71. Please go ahead and merge the CL to branch 3578 manually. Please contact milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

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

commit ce293652a2e9199cb8f10ba94c8cb2b72c9b0268
Author: Alexander Hendrich <hendrich@chromium.org>
Date: Wed Oct 24 14:10:58 2018

[Merge M71] Handle missing DMServer entry through re-registration

This CL handles the scenario of DMServer reporting a 410 status code
(missing/unknown/purged client ID) by trying background
re-registration. If this fails, the user is marked as requiring an
online sign-in.

DD: go/handle-purged-user-sessions

Bug:  870616 
Change-Id: I4a4c2370c36f5464752029d755d23221cd064b0e
Reviewed-on: https://chromium-review.googlesource.com/c/1227939
Commit-Queue: Alexander Hendrich <hendrich@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Reviewed-by: Sergey Poromov <poromov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600359}
(cherry picked from commit 3a845098f5f445e54eef1f66581377f7a8a70806)

TBR=hendrich@chromium.org, achuith@chromium.org, emaxx@chromium.org,
poromov@chromium.org

Change-Id: I4a4c2370c36f5464752029d755d23221cd064b0e
Reviewed-on: https://chromium-review.googlesource.com/c/1297971
Reviewed-by: Alexander Hendrich <hendrich@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#288}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/ce293652a2e9199cb8f10ba94c8cb2b72c9b0268/chrome/browser/chromeos/login/users/fake_chrome_user_manager.cc
[modify] https://crrev.com/ce293652a2e9199cb8f10ba94c8cb2b72c9b0268/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc
[modify] https://crrev.com/ce293652a2e9199cb8f10ba94c8cb2b72c9b0268/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.h
[modify] https://crrev.com/ce293652a2e9199cb8f10ba94c8cb2b72c9b0268/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos_unittest.cc
[modify] https://crrev.com/ce293652a2e9199cb8f10ba94c8cb2b72c9b0268/components/policy/core/common/cloud/cloud_policy_client.cc
[modify] https://crrev.com/ce293652a2e9199cb8f10ba94c8cb2b72c9b0268/components/policy/core/common/cloud/cloud_policy_client.h
[modify] https://crrev.com/ce293652a2e9199cb8f10ba94c8cb2b72c9b0268/components/policy/core/common/cloud/cloud_policy_client_unittest.cc
[modify] https://crrev.com/ce293652a2e9199cb8f10ba94c8cb2b72c9b0268/components/policy/proto/device_management_backend.proto

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

Commit: ce293652a2e9199cb8f10ba94c8cb2b72c9b0268
Author: hendrich@chromium.org
Commiter: hendrich@chromium.org
Date: 2018-10-24 14:10:58 +0000 UTC

[Merge M71] Handle missing DMServer entry through re-registration

This CL handles the scenario of DMServer reporting a 410 status code
(missing/unknown/purged client ID) by trying background
re-registration. If this fails, the user is marked as requiring an
online sign-in.

DD: go/handle-purged-user-sessions

Bug:  870616 
Change-Id: I4a4c2370c36f5464752029d755d23221cd064b0e
Reviewed-on: https://chromium-review.googlesource.com/c/1227939
Commit-Queue: Alexander Hendrich <hendrich@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Reviewed-by: Sergey Poromov <poromov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600359}
(cherry picked from commit 3a845098f5f445e54eef1f66581377f7a8a70806)

TBR=hendrich@chromium.org, achuith@chromium.org, emaxx@chromium.org,
poromov@chromium.org

Change-Id: I4a4c2370c36f5464752029d755d23221cd064b0e
Reviewed-on: https://chromium-review.googlesource.com/c/1297971
Reviewed-by: Alexander Hendrich <hendrich@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#288}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}

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

Commit: 955a28c12a066768aa8c0375853dff82fabfa54b
Author: hendrich@chromium.org
Commiter: hendrich@chromium.org
Date: 2018-10-24 14:22:55 +0000 UTC

[Merge M71][UMA] Handle missing DMServer entry through re-registration

This CL adds UMA to the re-registration flow to observe number of
triggered, successful and unsuccessful re-registrations attempts.

DD: go/handle-purged-user-sessions

(cherry picked from commit 2adccbd760c8cc930df3594ea218831cb68abadf)

Bug:  870616 
Change-Id: Ic963d321fb84cac55a61571b49e4a41554872589
Reviewed-on: https://chromium-review.googlesource.com/c/1283026
Commit-Queue: Alexander Hendrich <hendrich@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Sergey Poromov <poromov@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#601893}
Reviewed-on: https://chromium-review.googlesource.com/c/1298011
Reviewed-by: Alexander Hendrich <hendrich@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#289}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}

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

commit 955a28c12a066768aa8c0375853dff82fabfa54b
Author: Alexander Hendrich <hendrich@chromium.org>
Date: Wed Oct 24 14:22:55 2018

[Merge M71][UMA] Handle missing DMServer entry through re-registration

This CL adds UMA to the re-registration flow to observe number of
triggered, successful and unsuccessful re-registrations attempts.

DD: go/handle-purged-user-sessions

(cherry picked from commit 2adccbd760c8cc930df3594ea218831cb68abadf)

Bug:  870616 
Change-Id: Ic963d321fb84cac55a61571b49e4a41554872589
Reviewed-on: https://chromium-review.googlesource.com/c/1283026
Commit-Queue: Alexander Hendrich <hendrich@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Sergey Poromov <poromov@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#601893}
Reviewed-on: https://chromium-review.googlesource.com/c/1298011
Reviewed-by: Alexander Hendrich <hendrich@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#289}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/955a28c12a066768aa8c0375853dff82fabfa54b/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc
[modify] https://crrev.com/955a28c12a066768aa8c0375853dff82fabfa54b/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos_unittest.cc
[modify] https://crrev.com/955a28c12a066768aa8c0375853dff82fabfa54b/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/955a28c12a066768aa8c0375853dff82fabfa54b/tools/metrics/histograms/histograms.xml

Hi hendrich@
Can you provide steps if this requires manual verification. Thanks.!

Manual test using yaps
-Sign in into managed user session.
-Go tho chrome://policy
-Find ‘Client ID’ in the ‘User policies’ box.
-Go to ‘Clients’ tab in YAPS (http://chromium-dm-test.appspot.com/js/app.html#tab=clients)
-Find your client ID and delete that entry
-Reload policies on chrome://policy
-Verify: ‘Status’ should be ‘Policy cache OK’ -> ‘Missing device record’ -> ‘Policy cache OK’
-Verify: Policy fetch should succeed
-Verify: Yaps ‘Clients’ tab should list the client ID again.


Unit tests
$ unit_tests --gtest_filter=UserCloudPolicyManagerChromeOSTest.Reregistration*
[1/2] UserCloudPolicyManagerChromeOSTest.Reregistration
[2/2] UserCloudPolicyManagerChromeOSTest.ReregistrationFails

$ components_unittests --gtest_filter=CloudPolicyClientTest.PolicyReregistration*
[1/2] CloudPolicyClientTest.PolicyReregistration
[2/2] CloudPolicyClientTest.PolicyReregistrationFailsWithNonMatchingDMToken

Followed Steps in #c25 and observed.

1. Client ID is not seen in Yaps (http://chromium-dm-test.appspot.com/js/app.html#tab=clients) after a wait time of ~10 minutes / reload policies / refresh the yaps page.
2. With user login chrome://policy status was Policy cache OK

3. After device Reboot, chrome://policy status changed to "Missing device record" and at the same time client ID is listed in the Yaps page.

4. Reload/Reboot device retains the chrome://policy status as "Missing device record"

5. Remove the client Id listed in Yaps page then reload policies shows status as "Missing device record"


Google Chrome: 71.0.3578.36 Platform: 11151.21.0 kevin

Attached logs and Screenshot, Please take a look. 

Deleted: Screenshot 2018-11-05 at 10.57.26 AM.png 325 KB

	Screenshot 2018-11-05 at 10.57.26 AM.png 325 KB View Download

Deleted: Screenshot 2018-11-05 at 11.03.40 AM.png 325 KB

	Screenshot 2018-11-05 at 11.03.40 AM.png 325 KB View Download

Deleted: debug-logs_20181105-110953.tgz 1.2 MB

debug-logs_20181105-110953.tgz 1.2 MB Download

It's also concerning that the 'Status' under 'Device policies' is also broken (we can only re-register missing user records, not device records).
Did you actually enroll using yaps before? Looking at your chrome://policy in the first screenshot, I suspect that your device is enrolled to DMServer and you added the yaps command line flag later on? Otherwise, the 'Client id' should have also been listed in yaps before (i.e. after successfully enrolling to yaps).

I'll try to verify it myself later today again, but I'm pretty sure your device simply wasn't enrolled to yaps before.

Just manually verified it again on two devices.

Please make sure to unenroll/powerwash the device and then enroll it using yaps (https://sites.google.com/a/google.com/chrome-enterprise-new/faq/using-yaps), thanks!

(The "Clients" tab in Yaps actually lists the client IDs in the column for DM Token at the moment)

Closing the bug as Verified as per #c28. Thanks.!