Improved error reporting in cryptotoken

BUG=416998

Review URL: https://codereview.chromium.org/607913003

Cr-Commit-Position: refs/heads/master@{#297036}

chrome/browser/resources/cryptotoken/enroller.js

@@ -18,11 +18,11 @@
  */
 function handleWebEnrollRequest(sender, request, sendResponse) {
   var sentResponse = false;
-  var closeable;
+  var closeable = null;
 
-  function sendErrorResponse(u2fCode) {
+  function sendErrorResponse(error) {
     var response = makeWebErrorResponse(request,
-        mapErrorCodeToGnubbyCodeType(u2fCode, false /* forSign */));
+        mapErrorCodeToGnubbyCodeType(error.errorCode, false /* forSign */));
     sendResponseOnce(sentResponse, closeable, response, sendResponse);
   }
 
@@ -41,10 +41,16 @@ function handleWebEnrollRequest(sender, request, sendResponse) {
     sendResponseOnce(sentResponse, closeable, response, sendResponse);
   }
 
-  closeable =
-      validateAndBeginEnrollRequest(
+  var enroller =
+      validateEnrollRequest(
           sender, request, 'enrollChallenges', 'signData',
           sendErrorResponse, sendSuccessResponse);
+  if (enroller) {
+    var registerRequests = request['enrollChallenges'];
+    var signRequests = getSignRequestsFromEnrollRequest(request, 'signData');
+    closeable = /** @type {Closeable} */ (enroller);
+    enroller.doEnroll(registerRequests, signRequests, request['appId']);
+  }
   return closeable;
 }
 
@@ -58,10 +64,11 @@ function handleWebEnrollRequest(sender, request, sendResponse) {
  */
 function handleU2fEnrollRequest(sender, request, sendResponse) {
   var sentResponse = false;
-  var closeable;
+  var closeable = null;
 
-  function sendErrorResponse(u2fCode) {
-    var response = makeU2fErrorResponse(request, u2fCode);
+  function sendErrorResponse(error) {
+    var response = makeU2fErrorResponse(request, error.errorCode,
+        error.errorMessage);
     sendResponseOnce(sentResponse, closeable, response, sendResponse);
   }
 
@@ -80,67 +87,57 @@ function handleU2fEnrollRequest(sender, request, sendResponse) {
     sendResponseOnce(sentResponse, closeable, response, sendResponse);
   }
 
-  closeable =
-      validateAndBeginEnrollRequest(
+  var enroller =
+      validateEnrollRequest(
           sender, request, 'registerRequests', 'signRequests',
           sendErrorResponse, sendSuccessResponse, 'registeredKeys');
+  if (enroller) {
+    var registerRequests = request['registerRequests'];
+    var signRequests = getSignRequestsFromEnrollRequest(request,
+        'signRequests', 'registeredKeys');
+    closeable = /** @type {Closeable} */ (enroller);
+    enroller.doEnroll(registerRequests, signRequests, request['appId']);
+  }
   return closeable;
 }
 
 /**
- * Validates an enroll request using the given parameters, and, if valid, begins
- * handling the enroll request. (The enroll request may be modified as a result
- * of handling it.)
+ * Validates an enroll request using the given parameters.
  * @param {MessageSender} sender The sender of the message.
  * @param {Object} request The web page's enroll request.
  * @param {string} enrollChallengesName The name of the enroll challenges value
  *     in the request.
  * @param {string} signChallengesName The name of the sign challenges value in
  *     the request.
- * @param {function(ErrorCodes)} errorCb Error callback.
+ * @param {function(U2fError)} errorCb Error callback.
  * @param {function(string, string, (string|undefined))} successCb Success
  *     callback.
  * @param {string=} opt_registeredKeysName The name of the registered keys
  *     value in the request.
- * @return {Closeable} Request handler that should be closed when the browser
- *     message channel is closed.
+ * @return {Enroller} Enroller object representing the request, if the request
+ *     is valid, or null if the request is invalid.
  */
-function validateAndBeginEnrollRequest(sender, request,
+function validateEnrollRequest(sender, request,
     enrollChallengesName, signChallengesName, errorCb, successCb,
     opt_registeredKeysName) {
   var origin = getOriginFromUrl(/** @type {string} */ (sender.url));
   if (!origin) {
-    errorCb(ErrorCodes.BAD_REQUEST);
+    errorCb({errorCode: ErrorCodes.BAD_REQUEST});
     return null;
   }
 
   if (!isValidEnrollRequest(request, enrollChallengesName,
       signChallengesName, opt_registeredKeysName)) {
-    errorCb(ErrorCodes.BAD_REQUEST);
+    errorCb({errorCode: ErrorCodes.BAD_REQUEST});
     return null;
   }
 
-  var enrollChallenges = request[enrollChallengesName];
-  var signChallenges;
-  if (opt_registeredKeysName &&
-      request.hasOwnProperty(opt_registeredKeysName)) {
-    // Convert registered keys to sign challenges by adding a challenge value.
-    signChallenges = request[opt_registeredKeysName];
-    for (var i = 0; i < signChallenges.length; i++) {
-      // The actual value doesn't matter, as long as it's a string.
-      signChallenges[i]['challenge'] = '';
-    }
-  } else {
-    signChallenges = request[signChallengesName];
-  }
-  var logMsgUrl = request['logMsgUrl'];
-
   var timer = createTimerForRequest(
       FACTORY_REGISTRY.getCountdownFactory(), request);
+  var logMsgUrl = request['logMsgUrl'];
   var enroller = new Enroller(timer, origin, errorCb, successCb,
       sender.tlsChannelId, logMsgUrl);
-  enroller.doEnroll(enrollChallenges, signChallenges, request['appId']);
-  return /** @type {Closeable} */ (enroller);
+  return enroller;
 }
 
 /**
@@ -271,12 +268,40 @@ function makeEnrollResponseData(enrollChallenge, u2fVersion, enrollDataName,
   return responseData;
 }
 
+/**
+ * Gets the expanded sign challenges from an enroll request, potentially by
+ * modifying the request to contain a challenge value where one was omitted.
+ * (For enrolling, the server isn't interested in the value of a signature,
+ * only whether the presented key handle is already enrolled.)
+ * @param {Object} request The request.
+ * @param {string} signChallengesName The name of the sign challenges value in
+ *     the request.
+ * @param {string=} opt_registeredKeysName The name of the registered keys
+ *     value in the request.
+ * @return {Array.<SignChallenge>}
+ */
+function getSignRequestsFromEnrollRequest(request, signChallengesName,
+    opt_registeredKeysName) {
+  var signChallenges;
+  if (opt_registeredKeysName &&
+      request.hasOwnProperty(opt_registeredKeysName)) {
+    // Convert registered keys to sign challenges by adding a challenge value.
+    signChallenges = request[opt_registeredKeysName];
+    for (var i = 0; i < signChallenges.length; i++) {
+      // The actual value doesn't matter, as long as it's a string.
+      signChallenges[i]['challenge'] = '';
+    }
+  } else {
+    signChallenges = request[signChallengesName];
+  }
+  return signChallenges;
+}
+
 /**
  * Creates a new object to track enrolling with a gnubby.
  * @param {!Countdown} timer Timer for enroll request.
  * @param {string} origin The origin making the request.
- * @param {function(ErrorCodes)} errorCb Called upon enroll failure with an
- *     error code.
+ * @param {function(U2fError)} errorCb Called upon enroll failure.
  * @param {function(string, string, (string|undefined))} successCb Called upon
  *     enroll success with the version of the succeeding gnubby, the enroll
  *     data, and optionally the browser data associated with the enrollment.
@@ -291,7 +316,7 @@ function Enroller(timer, origin, errorCb, successCb, opt_tlsChannelId,
   this.timer_ = timer;
   /** @private {string} */
   this.origin_ = origin;
-  /** @private {function(ErrorCodes)} */
+  /** @private {function(U2fError)} */
   this.errorCb_ = errorCb;
   /** @private {function(string, string, (string|undefined))} */
   this.successCb_ = successCb;
@@ -358,7 +383,7 @@ Enroller.prototype.doEnroll = function(enrollChallenges, signChallenges,
   // Sanity check
   if (!enrollAppIds.length) {
     console.warn(UTIL_fmt('empty enroll app ids?'));
-    this.notifyError_(ErrorCodes.BAD_REQUEST);
+    this.notifyError_({errorCode: ErrorCodes.BAD_REQUEST});
     return;
   }
   var self = this;
@@ -371,10 +396,10 @@ Enroller.prototype.doEnroll = function(enrollChallenges, signChallenges,
             (self.helperComplete_.bind(self));
         self.handler_.run(helperComplete);
       } else {
-        self.notifyError_(ErrorCodes.OTHER_ERROR);
+        self.notifyError_({errorCode: ErrorCodes.OTHER_ERROR});
       }
     } else {
-      self.notifyError_(ErrorCodes.BAD_REQUEST);
+      self.notifyError_({errorCode: ErrorCodes.BAD_REQUEST});
     }
   });
 };
@@ -483,7 +508,7 @@ Enroller.prototype.checkAppIds_ = function(enrollAppIds, signChallenges, cb) {
  */
 Enroller.prototype.originChecked_ = function(appIds, cb, result) {
   if (!result) {
-    this.notifyError_(ErrorCodes.BAD_REQUEST);
+    this.notifyError_({errorCode: ErrorCodes.BAD_REQUEST});
     return;
   }
   /** @private {!AppIdChecker} */
@@ -505,16 +530,16 @@ Enroller.prototype.close = function() {
 };
 
 /**
- * Notifies the caller with the error code.
- * @param {ErrorCodes} code Error code
+ * Notifies the caller with the error.
+ * @param {U2fError} error Error.
  * @private
  */
-Enroller.prototype.notifyError_ = function(code) {
+Enroller.prototype.notifyError_ = function(error) {
   if (this.done_)
     return;
   this.close();
   this.done_ = true;
-  this.errorCb_(code);
+  this.errorCb_(error);
 };
 
 /**
@@ -540,9 +565,9 @@ Enroller.prototype.notifySuccess_ =
  */
 Enroller.prototype.helperComplete_ = function(reply) {
   if (reply.code) {
-    var reportedError = mapDeviceStatusCodeToErrorCode(reply.code);
+    var reportedError = mapDeviceStatusCodeToU2fError(reply.code);
     console.log(UTIL_fmt('helper reported ' + reply.code.toString(16) +
-        ', returning ' + reportedError));
+        ', returning ' + reportedError.errorCode));
     this.notifyError_(reportedError);
   } else {
     console.log(UTIL_fmt('Gnubby enrollment succeeded!!!!!'));

chrome/browser/resources/cryptotoken/errorcodes.js

@@ -3,7 +3,7 @@
 // found in the LICENSE file.
 
 /**
- * @fileoverview Error codes reported by top-level request handlers.
+ * @fileoverview Errors reported by top-level request handlers.
  */
 'use strict';
 
@@ -20,3 +20,12 @@ var ErrorCodes = {
   'DEVICE_INELIGIBLE': 4,
   'TIMEOUT': 5
 };
+
+/**
+ * An error object for responses
+ * @typedef {{
+ *   errorCode: ErrorCodes,
+ *   errorMessage: (?string|undefined)
+ * }}
+ */
+var U2fError;

chrome/browser/resources/cryptotoken/manifest.json

 {
   "name": "CryptoTokenExtension",
   "description": "CryptoToken Component Extension",
-  "version": "0.8.63",
+  "version": "0.8.64",
   "key": "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAq7zRobvA+AVlvNqkHSSVhh1sEWsHSqz4oR/XptkDe/Cz3+gW9ZGumZ20NCHjaac8j1iiesdigp8B1LJsd/2WWv2Dbnto4f8GrQ5MVphKyQ9WJHwejEHN2K4vzrTcwaXqv5BSTXwxlxS/mXCmXskTfryKTLuYrcHEWK8fCHb+0gvr8b/kvsi75A1aMmb6nUnFJvETmCkOCPNX5CHTdy634Ts/x0fLhRuPlahk63rdf7agxQv5viVjQFk+tbgv6aa9kdSd11Js/RZ9yZjrFgHOBWgP4jTBqud4+HUglrzu8qynFipyNRLCZsaxhm+NItTyNgesxLdxZcwOz56KD1Q4IQIDAQAB",
   "manifest_version": 2,
   "permissions": [

chrome/browser/resources/cryptotoken/signer.js

@@ -23,10 +23,10 @@ function handleWebSignRequest(sender, request, sendResponse) {
   var sentResponse = false;
   var queuedSignRequest;
 
-  function sendErrorResponse(u2fCode) {
+  function sendErrorResponse(error) {
     sendResponseOnce(sentResponse, queuedSignRequest,
         makeWebErrorResponse(request,
-            mapErrorCodeToGnubbyCodeType(u2fCode, true /* forSign */)),
+            mapErrorCodeToGnubbyCodeType(error.errorCode, true /* forSign */)),
         sendResponse);
   }
 
@@ -57,9 +57,10 @@ function handleU2fSignRequest(sender, request, sendResponse) {
   var sentResponse = false;
   var queuedSignRequest;
 
-  function sendErrorResponse(u2fCode) {
+  function sendErrorResponse(error) {
     sendResponseOnce(sentResponse, queuedSignRequest,
-        makeU2fErrorResponse(request, u2fCode), sendResponse);
+        makeU2fErrorResponse(request, error.errorCode, error.errorMessage),
+        sendResponse);
   }
 
   function sendSuccessResponse(challenge, info, browserData) {
@@ -123,7 +124,7 @@ function addSignatureAndBrowserDataToResponseData(responseData, signatureData,
  * @param {Object} request The web page's sign request.
  * @param {string} signChallengesName The name of the sign challenges value in
  *     the request.
- * @param {function(ErrorCodes)} errorCb Error callback.
+ * @param {function(U2fError)} errorCb Error callback.
  * @param {function(SignChallenge, string, string)} successCb Success callback.
  * @return {Closeable} Request handler that should be closed when the browser
  *     message channel is closed.
@@ -132,14 +133,14 @@ function validateAndEnqueueSignRequest(sender, request,
     signChallengesName, errorCb, successCb) {
   var origin = getOriginFromUrl(/** @type {string} */ (sender.url));
   if (!origin) {
-    errorCb(ErrorCodes.BAD_REQUEST);
+    errorCb({errorCode: ErrorCodes.BAD_REQUEST});
     return null;
   }
   // More closure type inference fail.
   var nonNullOrigin = /** @type {string} */ (origin);
 
   if (!isValidSignRequest(request, signChallengesName)) {
-    errorCb(ErrorCodes.BAD_REQUEST);
+    errorCb({errorCode: ErrorCodes.BAD_REQUEST});
     return null;
   }
 
@@ -155,7 +156,7 @@ function validateAndEnqueueSignRequest(sender, request,
   // Sanity check
   if (!appId) {
     console.warn(UTIL_fmt('empty sign appId?'));
-    errorCb(ErrorCodes.BAD_REQUEST);
+    errorCb({errorCode: ErrorCodes.BAD_REQUEST});
     return null;
   }
   var timer = createTimerForRequest(
@@ -197,7 +198,7 @@ function isValidSignRequest(request, signChallengesName) {
  * @param {!Array.<SignChallenge>} signChallenges The sign challenges.
  * @param {Countdown} timer Timeout timer
  * @param {string} origin Signature origin
- * @param {function(ErrorCodes)} errorCb Error callback
+ * @param {function(U2fError)} errorCb Error callback
  * @param {function(SignChallenge, string, string)} successCb Success callback
  * @param {string|undefined} opt_appId The app id for the entire request.
  * @param {string|undefined} opt_tlsChannelId TLS Channel Id
@@ -213,7 +214,7 @@ function QueuedSignRequest(signChallenges, timer, origin, errorCb,
   this.timer_ = timer;
   /** @private {string} */
   this.origin_ = origin;
-  /** @private {function(ErrorCodes)} */
+  /** @private {function(U2fError)} */
   this.errorCb_ = errorCb;
   /** @private {function(SignChallenge, string, string)} */
   this.successCb_ = successCb;
@@ -261,18 +262,18 @@ QueuedSignRequest.prototype.begin = function(token) {
       this.tlsChannelId_, this.logMsgUrl_);
   if (!this.signer_.setChallenges(this.signChallenges_, this.appId_)) {
     token.complete();
-    this.errorCb_(ErrorCodes.BAD_REQUEST);
+    this.errorCb_({errorCode: ErrorCodes.BAD_REQUEST});
   }
 };
 
 /**
  * Called when this request's signer fails.
- * @param {ErrorCodes} code The failure code reported by the signer.
+ * @param {U2fError} error The failure reported by the signer.
  * @private
  */
-QueuedSignRequest.prototype.signerFailed_ = function(code) {
+QueuedSignRequest.prototype.signerFailed_ = function(error) {
   this.token_.complete();
-  this.errorCb_(code);
+  this.errorCb_(error);
 };
 
 /**
@@ -292,7 +293,7 @@ QueuedSignRequest.prototype.signerSucceeded_ =
  * Creates an object to track signing with a gnubby.
  * @param {Countdown} timer Timer for sign request.
  * @param {string} origin The origin making the request.
- * @param {function(ErrorCodes)} errorCb Called when the sign operation fails.
+ * @param {function(U2fError)} errorCb Called when the sign operation fails.
  * @param {function(SignChallenge, string, string)} successCb Called when the
  *     sign operation succeeds.
  * @param {string=} opt_tlsChannelId the TLS channel ID, if any, of the origin
@@ -306,7 +307,7 @@ function Signer(timer, origin, errorCb, successCb,
   this.timer_ = timer;
   /** @private {string} */
   this.origin_ = origin;
-  /** @private {function(ErrorCodes)} */
+  /** @private {function(U2fError)} */
   this.errorCb_ = errorCb;
   /** @private {function(SignChallenge, string, string)} */
   this.successCb_ = successCb;
@@ -362,7 +363,11 @@ Signer.prototype.checkAppIds_ = function() {
     appIds = UTIL_unionArrays([this.appId_], appIds);
   }
   if (!appIds || !appIds.length) {
-    this.notifyError_(ErrorCodes.BAD_REQUEST);
+    var error = {
+      errorCode: ErrorCodes.BAD_REQUEST,
+      errorMessage: 'missing appId'
+    };
+    this.notifyError_(error);
     return;
   }
   FACTORY_REGISTRY.getOriginChecker().canClaimAppIds(this.origin_, appIds)
@@ -379,7 +384,11 @@ Signer.prototype.checkAppIds_ = function() {
  */
 Signer.prototype.originChecked_ = function(appIds, result) {
   if (!result) {
-    this.notifyError_(ErrorCodes.BAD_REQUEST);
+    var error = {
+      errorCode: ErrorCodes.BAD_REQUEST,
+      errorMessage: 'bad appId'
+    };
+    this.notifyError_(error);
     return;
   }
   /** @private {!AppIdChecker} */
@@ -398,11 +407,15 @@ Signer.prototype.originChecked_ = function(appIds, result) {
  */
 Signer.prototype.appIdChecked_ = function(result) {
   if (!result) {
-    this.notifyError_(ErrorCodes.BAD_REQUEST);
+    var error = {
+      errorCode: ErrorCodes.BAD_REQUEST,
+      errorMessage: 'bad appId'
+    };
+    this.notifyError_(error);
     return;
   }
   if (!this.doSign_()) {
-    this.notifyError_(ErrorCodes.BAD_REQUEST);
+    this.notifyError_({errorCode: ErrorCodes.BAD_REQUEST});
     return;
   }
 };
@@ -463,16 +476,16 @@ Signer.prototype.close = function() {
 };
 
 /**
- * Notifies the caller of error with the given error code.
- * @param {ErrorCodes} code Error code
+ * Notifies the caller of error.
+ * @param {U2fError} error Error.
  * @private
  */
-Signer.prototype.notifyError_ = function(code) {
+Signer.prototype.notifyError_ = function(error) {
   if (this.done_)
     return;
   this.close();
   this.done_ = true;
-  this.errorCb_(code);
+  this.errorCb_(error);
 };
 
 /**
@@ -498,15 +511,15 @@ Signer.prototype.notifySuccess_ = function(challenge, info, browserData) {
  */
 Signer.prototype.helperComplete_ = function(helperReply, opt_source) {
   if (helperReply.type != 'sign_helper_reply') {
-    this.notifyError_(ErrorCodes.OTHER_ERROR);
+    this.notifyError_({errorCode: ErrorCodes.OTHER_ERROR});
     return;
   }
   var reply = /** @type {SignHelperReply} */ (helperReply);
 
   if (reply.code) {
-    var reportedError = mapDeviceStatusCodeToErrorCode(reply.code);
+    var reportedError = mapDeviceStatusCodeToU2fError(reply.code);
     console.log(UTIL_fmt('helper reported ' + reply.code.toString(16) +
-        ', returning ' + reportedError));
+        ', returning ' + reportedError.errorCode));
     this.notifyError_(reportedError);
   } else {
     if (this.logMsgUrl_ && opt_source) {

chrome/browser/resources/cryptotoken/webrequest.js

@@ -228,24 +228,27 @@ function mapErrorCodeToGnubbyCodeType(errorCode, forSign) {
 }
 
 /**
- * Maps a helper's error code from the DeviceStatusCodes namespace to the
- * ErrorCodes namespace.
+ * Maps a helper's error code from the DeviceStatusCodes namespace to a
+ * U2fError.
  * @param {number} code Error code from DeviceStatusCodes namespace.
- * @return {ErrorCodes} A ErrorCodes error code.
+ * @return {U2fError} An error.
  */
-function mapDeviceStatusCodeToErrorCode(code) {
-  var reportedError = ErrorCodes.OTHER_ERROR;
+function mapDeviceStatusCodeToU2fError(code) {
   switch (code) {
     case DeviceStatusCodes.WRONG_DATA_STATUS:
-      reportedError = ErrorCodes.DEVICE_INELIGIBLE;
-      break;
+      return {errorCode: ErrorCodes.DEVICE_INELIGIBLE};
 
     case DeviceStatusCodes.TIMEOUT_STATUS:
     case DeviceStatusCodes.WAIT_TOUCH_STATUS:
-      reportedError = ErrorCodes.TIMEOUT;
-      break;
+      return {errorCode: ErrorCodes.TIMEOUT};
+
+    default:
+      var reportedError = {
+        errorCode: ErrorCodes.OTHER_ERROR,
+        errorMessage: 'device status code: ' + code.toString(16)
+      };
+      return reportedError;
   }
-  return reportedError;
 }
 
 /**