Retry the auto-enrollment check when the server asks for a smaller modulus.

BUG=chromium-os:28062
TEST=Send more bits with --enterprise-enrollment-initial-modulus than the server expects. AE still succeeds.


Review URL: http://codereview.chromium.org/9835021

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@128475 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 30 insertions, 16 deletions

diff --git a/chrome/browser/policy/auto_enrollment_client.cc b/chrome/browser/policy/auto_enrollment_client.cc
index 1a32d79..d0ffb6a 100644
--- a/chrome/browser/policy/auto_enrollment_client.cc
+++ b/chrome/browser/policy/auto_enrollment_client.cc
@@ -80,6 +80,7 @@ AutoEnrollmentClient::AutoEnrollmentClient(const base::Closure& callback,
       device_id_(guid::GenerateGUID()),
       power_initial_(power_initial),
       power_limit_(power_limit),
+      requests_sent_(0),
       device_management_service_(service),
       local_state_(local_state) {
   DCHECK_LE(power_initial_, power_limit_);
@@ -217,7 +218,7 @@ void AutoEnrollmentClient::SendRequest(int power) {
     return;
   }
 
-  last_power_used_ = power;
+  requests_sent_++;
 
   // Only power-of-2 moduli are supported for now. These are computed by taking
   // the lower |power| bits of the hash.
@@ -254,28 +255,27 @@ void AutoEnrollmentClient::OnRequestCompletion(
   if (enrollment_response.has_expected_modulus()) {
     // Server is asking us to retry with a different modulus.
     int64 modulus = enrollment_response.expected_modulus();
-    int64 last_modulus_used = GG_INT64_C(1) << last_power_used_;
     int power = NextPowerOf2(modulus);
     if ((GG_INT64_C(1) << power) != modulus) {
       LOG(WARNING) << "Auto enrollment: the server didn't ask for a power-of-2 "
                    << "modulus. Using the closest power-of-2 instead "
                    << "(" << modulus << " vs 2^" << power << ")";
     }
-    if (modulus < last_modulus_used) {
-      LOG(ERROR) << "Auto enrollment error: the server asked for a smaller "
-                 << "modulus than we used.";
-    } else if (modulus == last_modulus_used) {
-      LOG(ERROR) << "Auto enrollment error: the server asked for the same "
-                 << "modulus that was already used.";
-    } else if (last_power_used_ != power_initial_) {
+    if (requests_sent_ >= 2) {
       LOG(ERROR) << "Auto enrollment error: already retried with an updated "
-                 << "modulus but the server asked for a new one.";
+                 << "modulus but the server asked for a new one again: "
+                 << power;
     } else if (power > power_limit_) {
       LOG(ERROR) << "Auto enrollment error: the server asked for a larger "
                  << "modulus than the client accepts (" << power << " vs "
                  << power_limit_ << ").";
     } else {
-      // Retry with a larger modulus.
+      // Retry at most once with the modulus that the server requested.
+      if (power <= power_initial_) {
+        LOG(WARNING) << "Auto enrollment: the server asked to use a modulus ("
+                     << power << ") that isn't larger than the first used ("
+                     << power_initial_ << "). Retrying anyway.";
+      }
       SendRequest(power);
       // Don't invoke the callback yet.
       return;
diff --git a/chrome/browser/policy/auto_enrollment_client.h b/chrome/browser/policy/auto_enrollment_client.h
index f100d5c8..3b7e41e 100644
--- a/chrome/browser/policy/auto_enrollment_client.h
+++ b/chrome/browser/policy/auto_enrollment_client.h
@@ -124,9 +124,9 @@ class AutoEnrollmentClient {
   // a retry response from the server.
   int power_limit_;
 
-  // Modulus used in the last request sent to the server.
-  // Used to determine if the server is asking for the same modulus.
-  int last_power_used_;
+  // Number of requests sent to the server so far.
+  // Used to determine if the server keeps asking for different moduli.
+  int requests_sent_;
 
   // Used to communicate with the device management service.
   scoped_ptr<DeviceManagementService> device_management_service_;
diff --git a/chrome/browser/policy/auto_enrollment_client_unittest.cc b/chrome/browser/policy/auto_enrollment_client_unittest.cc
index 59fe5da..2cfe944 100644
--- a/chrome/browser/policy/auto_enrollment_client_unittest.cc
+++ b/chrome/browser/policy/auto_enrollment_client_unittest.cc
@@ -174,14 +174,28 @@ TEST_F(AutoEnrollmentClientTest, AskForMoreThenEvenMore) {
 }
 
 TEST_F(AutoEnrollmentClientTest, AskForLess) {
+  InSequence sequence;
   ServerWillReply(8, false, false);
+  ServerWillReply(-1, true, true);
   client_->Start();
-  EXPECT_FALSE(client_->should_auto_enroll());
+  EXPECT_TRUE(client_->should_auto_enroll());
   EXPECT_EQ(1, completion_callback_count_);
-  VerifyCachedResult(false, 8);
+  VerifyCachedResult(true, 8);
 }
 
 TEST_F(AutoEnrollmentClientTest, AskForSame) {
+  InSequence sequence;
+  ServerWillReply(16, false, false);
+  ServerWillReply(-1, true, true);
+  client_->Start();
+  EXPECT_TRUE(client_->should_auto_enroll());
+  EXPECT_EQ(1, completion_callback_count_);
+  VerifyCachedResult(true, 8);
+}
+
+TEST_F(AutoEnrollmentClientTest, AskForSameTwice) {
+  InSequence sequence;
+  ServerWillReply(16, false, false);
   ServerWillReply(16, false, false);
   client_->Start();
   EXPECT_FALSE(client_->should_auto_enroll());