Bug 1301469 - Handle exceptions thrown by `PushCrypto.decodeMsg` as decryption errors. r=mt draft
authorKit Cambridge <kit@yakshaving.ninja>
Wed, 05 Oct 2016 08:56:55 -0700
changeset 421775 b21f15cc9ac12ff11496d4f63ee531c021bd7a29
parent 419886 344920af45b92da6d4f5b84738e1c7a3fb582461
child 421776 046e8c93f8e15f338498fdbdaa47c86aabc02c41
child 421848 8f1b047b6f01c89a852aefbb1349a608f1178ab8
push id31596
push userbmo:kcambridge@mozilla.com
push dateThu, 06 Oct 2016 21:44:46 +0000
reviewersmt
bugs1301469
milestone52.0a1
Bug 1301469 - Handle exceptions thrown by `PushCrypto.decodeMsg` as decryption errors. r=mt Previously, errors thrown by `decodeMsg` and `getCryptoParams` would bubble up to the catch handler in `receivedPushMessage`, causing us to report "ACK_NOT_DELIVERED" instead of "ACK_DECRYPTION_ERROR" in the ack sent to the server. MozReview-Commit-ID: FZFzYdebQGy
dom/push/PushService.jsm
dom/push/PushServiceAndroidGCM.jsm
dom/push/PushServiceHttp2.jsm
dom/push/PushServiceWebSocket.jsm
--- a/dom/push/PushService.jsm
+++ b/dom/push/PushService.jsm
@@ -7,22 +7,24 @@
 
 const Cc = Components.classes;
 const Ci = Components.interfaces;
 const Cu = Components.utils;
 const Cr = Components.results;
 
 Cu.import("resource://gre/modules/AppConstants.jsm");
 Cu.import("resource://gre/modules/Preferences.jsm");
-Cu.import("resource://gre/modules/Promise.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/Timer.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
-const {PushCrypto} = Cu.import("resource://gre/modules/PushCrypto.jsm");
+const {
+  PushCrypto,
+  getCryptoParams,
+} = Cu.import("resource://gre/modules/PushCrypto.jsm");
 const {PushDB} = Cu.import("resource://gre/modules/PushDB.jsm");
 
 const CONNECTION_PROTOCOLS = (function() {
   if ('android' != AppConstants.MOZ_WIDGET_TOOLKIT) {
     const {PushServiceWebSocket} = Cu.import("resource://gre/modules/PushServiceWebSocket.jsm");
     const {PushServiceHttp2} = Cu.import("resource://gre/modules/PushServiceHttp2.jsm");
     return [PushServiceWebSocket, PushServiceHttp2];
   } else {
@@ -777,59 +779,56 @@ this.PushService = {
    * quota for the associated push registration. If the quota is exceeded,
    * the registration and message will be dropped, and the worker will not
    * be notified.
    *
    * @param {String} keyID The push registration ID.
    * @param {String} messageID The message ID, used to report service worker
    *  delivery failures. For Web Push messages, this is the version. If empty,
    *  failures will not be reported.
+   * @param {Object} headers The encryption headers.
    * @param {ArrayBuffer|Uint8Array} data The encrypted message data.
-   * @param {Object} cryptoParams The message encryption settings.
    * @param {Function} updateFunc A function that receives the existing
    *  registration record as its argument, and returns a new record. If the
    *  function returns `null` or `undefined`, the record will not be updated.
    *  `PushServiceWebSocket` uses this to drop incoming updates with older
    *  versions.
    * @returns {Promise} Resolves with an `nsIPushErrorReporter` ack status
    *  code, indicating whether the message was delivered successfully.
    */
-  receivedPushMessage(keyID, messageID, data, cryptoParams, updateFunc) {
+  receivedPushMessage(keyID, messageID, headers, data, updateFunc) {
     console.debug("receivedPushMessage()");
     Services.telemetry.getHistogramById("PUSH_API_NOTIFICATION_RECEIVED").add();
 
     return this._updateRecordAfterPush(keyID, updateFunc).then(record => {
-      if (!record) {
-        throw new Error("Ignoring update for key ID " + keyID);
-      }
       if (record.quotaApplies()) {
         // Update quota after the delay, at which point
         // we check for visible notifications.
         let timeoutID = setTimeout(_ =>
           {
             this._updateQuota(keyID);
             if (!this._updateQuotaTimeouts.delete(timeoutID)) {
               console.debug("receivedPushMessage: quota update timeout missing?");
             }
           }, prefs.get("quotaUpdateDelay"));
         this._updateQuotaTimeouts.add(timeoutID);
       }
-      return this._decryptAndNotifyApp(record, messageID, data, cryptoParams);
+      return this._decryptAndNotifyApp(record, messageID, headers, data);
     }).catch(error => {
       console.error("receivedPushMessage: Error notifying app", error);
       return Ci.nsIPushErrorReporter.ACK_NOT_DELIVERED;
     });
   },
 
   /**
    * Updates a registration record after receiving a push message.
    *
    * @param {String} keyID The push registration ID.
    * @param {Function} updateFunc The function passed to `receivedPushMessage`.
-   * @returns {Promise} Resolves with the updated record, or `null` if the
+   * @returns {Promise} Resolves with the updated record, or rejects if the
    *  record was not updated.
    */
   _updateRecordAfterPush(keyID, updateFunc) {
     return this.getByKeyID(keyID).then(record => {
       if (!record) {
         this._recordDidNotNotify(kDROP_NOTIFICATION_REASON_KEY_NOT_FOUND);
         throw new Error("No record for key ID " + keyID);
       }
@@ -857,59 +856,64 @@ this.PushService = {
           if (newRecord.isExpired()) {
             return null;
           }
           newRecord.receivedPush(lastVisit);
           return newRecord;
         });
       });
     }).then(record => {
-      if (record) {
-        gPushNotifier.notifySubscriptionModified(record.scope,
-                                                 record.principal);
+      if (!record) {
+        throw new Error("Ignoring update for key ID " + keyID);
       }
+      gPushNotifier.notifySubscriptionModified(record.scope,
+                                               record.principal);
       return record;
     });
   },
 
   /**
    * Decrypts a message. Will resolve with null if cryptoParams is falsy.
    *
    * @param {PushRecord} record The receiving registration.
+   * @param {Object} headers The encryption headers.
    * @param {ArrayBuffer|Uint8Array} data The encrypted message data.
-   * @param {Object} cryptoParams The message encryption settings.
+
    * @returns {Promise} Resolves with the decrypted message.
    */
-  _decryptMessage(data, record, cryptoParams) {
-    if (!cryptoParams) {
-      return Promise.resolve(null);
-    }
-    return PushCrypto.decodeMsg(
-      data,
-      record.p256dhPrivateKey,
-      record.p256dhPublicKey,
-      cryptoParams.dh,
-      cryptoParams.salt,
-      cryptoParams.rs,
-      record.authenticationSecret,
-      cryptoParams.padSize
-    );
+  _decryptMessage(record, headers, data) {
+    return Promise.resolve().then(_ => {
+      let cryptoParams = getCryptoParams(headers);
+      if (!cryptoParams) {
+        return null;
+      }
+      return PushCrypto.decodeMsg(
+        data,
+        record.p256dhPrivateKey,
+        record.p256dhPublicKey,
+        cryptoParams.dh,
+        cryptoParams.salt,
+        cryptoParams.rs,
+        record.authenticationSecret,
+        cryptoParams.padSize
+      );
+    });
   },
 
   /**
    * Decrypts an incoming message and notifies the associated service worker.
    *
    * @param {PushRecord} record The receiving registration.
    * @param {String} messageID The message ID.
+   * @param {Object} headers The encryption headers.
    * @param {ArrayBuffer|Uint8Array} data The encrypted message data.
-   * @param {Object} cryptoParams The message encryption settings.
    * @returns {Promise} Resolves with an ack status code.
    */
-  _decryptAndNotifyApp(record, messageID, data, cryptoParams) {
-    return this._decryptMessage(data, record, cryptoParams)
+  _decryptAndNotifyApp(record, messageID, headers, data) {
+    return this._decryptMessage(record, headers, data)
       .then(
         message => this._notifyApp(record, messageID, message),
         error => {
           let message = gDOMBundle.formatStringFromName(
             "PushMessageDecryptionFailure", [record.scope, String(error)], 2);
           gPushNotifier.notifyError(record.scope, record.principal, message,
                                     Ci.nsIScriptError.errorFlag);
           return Ci.nsIPushErrorReporter.ACK_DECRYPTION_ERROR;
--- a/dom/push/PushServiceAndroidGCM.jsm
+++ b/dom/push/PushServiceAndroidGCM.jsm
@@ -7,20 +7,17 @@
 
 const Cc = Components.classes;
 const Ci = Components.interfaces;
 const Cu = Components.utils;
 const Cr = Components.results;
 
 const {PushDB} = Cu.import("resource://gre/modules/PushDB.jsm");
 const {PushRecord} = Cu.import("resource://gre/modules/PushRecord.jsm");
-const {
-  PushCrypto,
-  getCryptoParams,
-} = Cu.import("resource://gre/modules/PushCrypto.jsm");
+const {PushCrypto} = Cu.import("resource://gre/modules/PushCrypto.jsm");
 Cu.import("resource://gre/modules/Messaging.jsm"); /*global: Messaging */
 Cu.import("resource://gre/modules/Services.jsm"); /*global: Services */
 Cu.import("resource://gre/modules/Preferences.jsm"); /*global: Preferences */
 Cu.import("resource://gre/modules/Promise.jsm"); /*global: Promise */
 Cu.import("resource://gre/modules/XPCOMUtils.jsm"); /*global: XPCOMUtils */
 
 const Log = Cu.import("resource://gre/modules/AndroidLog.jsm", {}).AndroidLog.bind("Push");
 
@@ -102,46 +99,45 @@ this.PushServiceAndroidGCM = {
     }
     if (!data) {
       console.error("No data from Java!  Dropping message.");
       return;
     }
     data = JSON.parse(data);
     console.debug("ReceivedPushMessage with data", data);
 
-    let { message, cryptoParams } = this._messageAndCryptoParams(data);
+    let { headers, message } = this._messageAndHeaders(data);
 
-    console.debug("Delivering message to main PushService:", message, cryptoParams);
+    console.debug("Delivering message to main PushService:", message, headers);
     this._mainPushService.receivedPushMessage(
-      data.channelID, "", message, cryptoParams, (record) => {
+      data.channelID, "", headers, message, (record) => {
         // Always update the stored record.
         return record;
       });
   },
 
-  _messageAndCryptoParams(data) {
+  _messageAndHeaders(data) {
     // Default is no data (and no encryption).
     let message = null;
-    let cryptoParams = null;
+    let headers = null;
 
     if (data.message && data.enc && (data.enckey || data.cryptokey)) {
-      let headers = {
+      headers = {
         encryption_key: data.enckey,
         crypto_key: data.cryptokey,
         encryption: data.enc,
         encoding: data.con,
       };
-      cryptoParams = getCryptoParams(headers);
       // Ciphertext is (urlsafe) Base 64 encoded.
       message = ChromeUtils.base64URLDecode(data.message, {
         // The Push server may append padding.
         padding: "ignore",
       });
     }
-    return { message, cryptoParams };
+    return { headers, message };
   },
 
   _configure: function(serverURL, debug) {
     return Messaging.sendRequestForResult({
       type: "PushServiceAndroidGCM:Configure",
       endpoint: serverURL.spec,
       debug: debug,
     });
--- a/dom/push/PushServiceHttp2.jsm
+++ b/dom/push/PushServiceHttp2.jsm
@@ -18,17 +18,16 @@ Cu.import("resource://gre/modules/NetUti
 Cu.import("resource://gre/modules/IndexedDBHelper.jsm");
 Cu.import("resource://gre/modules/Timer.jsm");
 Cu.import("resource://gre/modules/Preferences.jsm");
 Cu.import("resource://gre/modules/Promise.jsm");
 
 const {
   PushCrypto,
   concatArray,
-  getCryptoParams,
 } = Cu.import("resource://gre/modules/PushCrypto.jsm");
 
 this.EXPORTED_SYMBOLS = ["PushServiceHttp2"];
 
 XPCOMUtils.defineLazyGetter(this, "console", () => {
   let {ConsoleAPI} = Cu.import("resource://gre/modules/Console.jsm", {});
   return new ConsoleAPI({
     maxLogLevelPref: "dom.push.loglevel",
@@ -152,23 +151,22 @@ PushChannelListener.prototype = {
         this._mainListener &&
         this._mainListener._pushService) {
       let headers = {
         encryption_key: getHeaderField(aRequest, "Encryption-Key"),
         crypto_key: getHeaderField(aRequest, "Crypto-Key"),
         encryption: getHeaderField(aRequest, "Encryption"),
         encoding: getHeaderField(aRequest, "Content-Encoding"),
       };
-      let cryptoParams = getCryptoParams(headers);
       let msg = concatArray(this._message);
 
       this._mainListener._pushService._pushChannelOnStop(this._mainListener.uri,
                                                          this._ackUri,
-                                                         msg,
-                                                         cryptoParams);
+                                                         headers,
+                                                         msg);
     }
   }
 };
 
 function getHeaderField(aRequest, name) {
   try {
     return aRequest.getRequestHeader(name);
   } catch(e) {
@@ -779,21 +777,21 @@ this.PushServiceHttp2 = {
   },
 
   removeListenerPendingRetry: function(aListener) {
     if (!this._listenersPendingRetry.remove(aListener)) {
       console.debug("removeListenerPendingRetry: listener not in list?");
     }
   },
 
-  _pushChannelOnStop: function(aUri, aAckUri, aMessage, cryptoParams) {
+  _pushChannelOnStop: function(aUri, aAckUri, aHeaders, aMessage) {
     console.debug("pushChannelOnStop()");
 
     this._mainPushService.receivedPushMessage(
-      aUri, "", aMessage, cryptoParams, record => {
+      aUri, "", aHeaders, aMessage, record => {
         // Always update the stored record.
         return record;
       }
     )
     .then(_ => this._ackMsgRecv(aAckUri))
     .catch(err => {
       console.error("pushChannelOnStop: Error receiving message",
         err);
--- a/dom/push/PushServiceWebSocket.jsm
+++ b/dom/push/PushServiceWebSocket.jsm
@@ -14,20 +14,17 @@ Cu.import("resource://gre/modules/AppCon
 Cu.import("resource://gre/modules/Preferences.jsm");
 Cu.import("resource://gre/modules/Promise.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/Timer.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
 const {PushDB} = Cu.import("resource://gre/modules/PushDB.jsm");
 const {PushRecord} = Cu.import("resource://gre/modules/PushRecord.jsm");
-const {
-  PushCrypto,
-  getCryptoParams,
-} = Cu.import("resource://gre/modules/PushCrypto.jsm");
+const {PushCrypto} = Cu.import("resource://gre/modules/PushCrypto.jsm");
 
 if (AppConstants.MOZ_B2G) {
   XPCOMUtils.defineLazyServiceGetter(this, "gPowerManagerService",
                                      "@mozilla.org/power/powermanagerservice;1",
                                      "nsIPowerManagerService");
 }
 
 const kPUSHWSDB_DB_NAME = "pushapi";
@@ -733,32 +730,27 @@ this.PushServiceWebSocket = {
       promise = this._mainPushService.receivedPushMessage(
         update.channelID,
         update.version,
         null,
         null,
         updateRecord
       );
     } else {
-      let params = getCryptoParams(update.headers);
-      if (params) {
-        let message = ChromeUtils.base64URLDecode(update.data, {
-          // The Push server may append padding.
-          padding: "ignore",
-        });
-        promise = this._mainPushService.receivedPushMessage(
-          update.channelID,
-          update.version,
-          message,
-          params,
-          updateRecord
-        );
-      } else {
-        promise = Promise.reject(new Error("Invalid crypto headers"));
-      }
+      let message = ChromeUtils.base64URLDecode(update.data, {
+        // The Push server may append padding.
+        padding: "ignore",
+      });
+      promise = this._mainPushService.receivedPushMessage(
+        update.channelID,
+        update.version,
+        update.headers,
+        message,
+        updateRecord
+      );
     }
     promise.then(status => {
       this._sendAck(update.channelID, update.version, status);
     }, err => {
       console.error("handleDataUpdate: Error delivering message", update, err);
       this._sendAck(update.channelID, update.version,
         Ci.nsIPushErrorReporter.ACK_DECRYPTION_ERROR);
     }).catch(err => {