Bug 1337314 - Encrypt card number while normallizing field. r=lchang
authorsteveck-chung <schung@mozilla.com>
Mon, 17 Jul 2017 17:14:54 +0800
changeset 424597 2d15f5dd1966cc693f3a9d7bc8843dddc4360a56
parent 424596 8c1ef3d6ed6dc489ea101b12b38a62883fb04b3e
child 424598 ecff6a69e042a3b3a618f7a83d77728d7aca2d3e
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerslchang
bugs1337314
milestone57.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1337314 - Encrypt card number while normallizing field. r=lchang MozReview-Commit-ID: 9HSpLzJMnoE
browser/extensions/formautofill/FormAutofillParent.jsm
browser/extensions/formautofill/ProfileStorage.jsm
browser/extensions/formautofill/test/browser/head.js
browser/extensions/formautofill/test/unit/test_creditCardRecords.js
browser/extensions/formautofill/test/unit/test_storage_tombstones.js
browser/extensions/formautofill/test/unit/test_transformFields.js
--- a/browser/extensions/formautofill/FormAutofillParent.jsm
+++ b/browser/extensions/formautofill/FormAutofillParent.jsm
@@ -171,17 +171,17 @@ FormAutofillParent.prototype = {
 
   /**
    * Handles the message coming from FormAutofillContent.
    *
    * @param   {string} message.name The name of the message.
    * @param   {object} message.data The data of the message.
    * @param   {nsIFrameMessageManager} message.target Caller's message manager.
    */
-  receiveMessage({name, data, target}) {
+  async receiveMessage({name, data, target}) {
     switch (name) {
       case "FormAutofill:InitStorage": {
         this.profileStorage.initialize();
         break;
       }
       case "FormAutofill:GetRecords": {
         this._getRecords(data, target);
         break;
@@ -190,16 +190,17 @@ FormAutofillParent.prototype = {
         if (data.guid) {
           this.profileStorage.addresses.update(data.guid, data.address);
         } else {
           this.profileStorage.addresses.add(data.address);
         }
         break;
       }
       case "FormAutofill:SaveCreditCard": {
+        await this.profileStorage.creditCards.normalizeCCNumberFields(data.creditcard);
         this.profileStorage.creditCards.add(data.creditcard);
         break;
       }
       case "FormAutofill:RemoveAddresses": {
         data.guids.forEach(guid => this.profileStorage.addresses.remove(guid));
         break;
       }
       case "FormAutofill:OnFormSubmit": {
--- a/browser/extensions/formautofill/ProfileStorage.jsm
+++ b/browser/extensions/formautofill/ProfileStorage.jsm
@@ -111,16 +111,18 @@ Cu.import("resource://gre/modules/Servic
 Cu.import("resource://gre/modules/osfile.jsm");
 
 Cu.import("resource://formautofill/FormAutofillUtils.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "JSONFile",
                                   "resource://gre/modules/JSONFile.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "FormAutofillNameUtils",
                                   "resource://formautofill/FormAutofillNameUtils.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "MasterPassword",
+                                  "resource://formautofill/MasterPassword.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "PhoneNumber",
                                   "resource://formautofill/phonenumberutils/PhoneNumber.jsm");
 
 XPCOMUtils.defineLazyServiceGetter(this, "gUUIDGenerator",
                                    "@mozilla.org/uuid-generator;1",
                                    "nsIUUIDGenerator");
 
 XPCOMUtils.defineLazyGetter(this, "REGION_NAMES", function() {
@@ -1466,36 +1468,19 @@ class CreditCards extends AutofillRecord
       creditCard["cc-family-name"] = nameParts.family;
       hasNewComputedFields = true;
     }
 
     return hasNewComputedFields;
   }
 
   _normalizeFields(creditCard) {
-    // Fields that should not be set by content.
-    delete creditCard["cc-number-encrypted"];
-
-    // Validate and encrypt credit card numbers, and calculate the masked numbers
-    if (creditCard["cc-number"]) {
-      let ccNumber = creditCard["cc-number"].replace(/\s/g, "");
-      delete creditCard["cc-number"];
-
-      if (!/^\d+$/.test(ccNumber)) {
-        throw new Error("Credit card number contains invalid characters.");
-      }
-
-      // TODO: Encrypt cc-number here (bug 1337314).
-      // e.g. creditCard["cc-number-encrypted"] = Encrypt(creditCard["cc-number"]);
-
-      if (ccNumber.length > 4) {
-        creditCard["cc-number"] = "*".repeat(ccNumber.length - 4) + ccNumber.substr(-4);
-      } else {
-        creditCard["cc-number"] = ccNumber;
-      }
+    // Check if cc-number is normalized(normalizeCCNumberFields should be called first).
+    if (!creditCard["cc-number-encrypted"] || !creditCard["cc-number"].includes("*")) {
+      throw new Error("Credit card number needs to be normalized first.");
     }
 
     // Normalize name
     if (creditCard["cc-given-name"] || creditCard["cc-additional-name"] || creditCard["cc-family-name"]) {
       if (!creditCard["cc-name"]) {
         creditCard["cc-name"] = FormAutofillNameUtils.joinNameParts({
           given: creditCard["cc-given-name"],
           middle: creditCard["cc-additional-name"],
@@ -1524,16 +1509,48 @@ class CreditCards extends AutofillRecord
       } else if (expYear < 100) {
         // Enforce 4 digits years.
         creditCard["cc-exp-year"] = expYear + 2000;
       } else {
         creditCard["cc-exp-year"] = expYear;
       }
     }
   }
+
+  /**
+   * Normalize credit card number related field for saving. It should always be
+   * called before adding/updating credit card records.
+   *
+   * @param  {Object} creditCard
+   *         The creditCard record with plaintext number only.
+   */
+  async normalizeCCNumberFields(creditCard) {
+    // Fields that should not be set by content.
+    delete creditCard["cc-number-encrypted"];
+
+    // Validate and encrypt credit card numbers, and calculate the masked numbers
+    if (creditCard["cc-number"]) {
+      let ccNumber = creditCard["cc-number"].replace(/\s/g, "");
+      delete creditCard["cc-number"];
+
+      if (!/^\d+$/.test(ccNumber)) {
+        throw new Error("Credit card number contains invalid characters.");
+      }
+
+      // Based on the information on wiki[1], the shortest valid length should be
+      // 12 digits(Maestro).
+      // [1] https://en.wikipedia.org/wiki/Payment_card_number
+      if (ccNumber.length < 12) {
+        throw new Error("Invalid credit card number because length is under 12 digits.");
+      }
+
+      creditCard["cc-number-encrypted"] = await MasterPassword.encrypt(creditCard["cc-number"]);
+      creditCard["cc-number"] = "*".repeat(ccNumber.length - 4) + ccNumber.substr(-4);
+    }
+  }
 }
 
 function ProfileStorage(path) {
   this._path = path;
   this._initializePromise = null;
   this.INTERNAL_FIELDS = INTERNAL_FIELDS;
 }
 
--- a/browser/extensions/formautofill/test/browser/head.js
+++ b/browser/extensions/formautofill/test/browser/head.js
@@ -150,17 +150,20 @@ function getAddresses() {
 }
 
 function saveAddress(address) {
   Services.cpmm.sendAsyncMessage("FormAutofill:SaveAddress", {address});
   return TestUtils.topicObserved("formautofill-storage-changed");
 }
 
 function saveCreditCard(creditcard) {
-  Services.cpmm.sendAsyncMessage("FormAutofill:SaveCreditCard", {creditcard});
+  let creditcardClone = Object.assign({}, creditcard);
+  Services.cpmm.sendAsyncMessage("FormAutofill:SaveCreditCard", {
+    creditcard: creditcardClone,
+  });
   return TestUtils.topicObserved("formautofill-storage-changed");
 }
 function removeAddresses(guids) {
   Services.cpmm.sendAsyncMessage("FormAutofill:RemoveAddresses", {guids});
   return TestUtils.topicObserved("formautofill-storage-changed");
 }
 
 /**
--- a/browser/extensions/formautofill/test/unit/test_creditCardRecords.js
+++ b/browser/extensions/formautofill/test/unit/test_creditCardRecords.js
@@ -31,16 +31,17 @@ const TEST_CREDIT_CARD_3 = {
 const TEST_CREDIT_CARD_WITH_2_DIGITS_YEAR = {
   "cc-number": "1234123412341234",
   "cc-exp-month": 1,
   "cc-exp-year": 12,
 };
 
 const TEST_CREDIT_CARD_WITH_INVALID_FIELD = {
   "cc-name": "John Doe",
+  "cc-number": "1234123412341234",
   invalidField: "INVALID",
 };
 
 const TEST_CREDIT_CARD_WITH_INVALID_EXPIRY_DATE = {
   "cc-name": "John Doe",
   "cc-number": "1111222233334444",
   "cc-exp-month": 13,
   "cc-exp-year": -3,
@@ -51,35 +52,42 @@ const TEST_CREDIT_CARD_WITH_SPACES_BETWE
   "cc-number": "1111 2222 3333 4444",
 };
 
 const TEST_CREDIT_CARD_WITH_INVALID_NUMBERS = {
   "cc-name": "John Doe",
   "cc-number": "abcdefg",
 };
 
+const TEST_CREDIT_CARD_WITH_SHORT_NUMBERS = {
+  "cc-name": "John Doe",
+  "cc-number": "1234567890",
+};
+
 let prepareTestCreditCards = async function(path) {
   let profileStorage = new ProfileStorage(path);
   await profileStorage.initialize();
 
   let onChanged = TestUtils.topicObserved("formautofill-storage-changed",
                                           (subject, data) => data == "add");
-  do_check_true(profileStorage.creditCards.add(TEST_CREDIT_CARD_1));
+  let encryptedCC_1 = Object.assign({}, TEST_CREDIT_CARD_1);
+  await profileStorage.creditCards.normalizeCCNumberFields(encryptedCC_1);
+  do_check_true(profileStorage.creditCards.add(encryptedCC_1));
   await onChanged;
-  do_check_true(profileStorage.creditCards.add(TEST_CREDIT_CARD_2));
+  let encryptedCC_2 = Object.assign({}, TEST_CREDIT_CARD_2);
+  await profileStorage.creditCards.normalizeCCNumberFields(encryptedCC_2);
+  do_check_true(profileStorage.creditCards.add(encryptedCC_2));
   await profileStorage._saveImmediately();
 };
 
 let reCCNumber = /^(\*+)(.{4})$/;
 
 let do_check_credit_card_matches = (creditCardWithMeta, creditCard) => {
   for (let key in creditCard) {
     if (key == "cc-number") {
-      // check "cc-number-encrypted" after encryption lands (bug 1337314).
-
       let matches = reCCNumber.exec(creditCardWithMeta["cc-number"]);
       do_check_neq(matches, null);
       do_check_eq(creditCardWithMeta["cc-number"].length, creditCard["cc-number"].length);
       do_check_eq(creditCard["cc-number"].endsWith(matches[2]), true);
     } else {
       do_check_eq(creditCardWithMeta[key], creditCard[key]);
     }
   }
@@ -158,17 +166,17 @@ add_task(async function test_getByFilter
   let profileStorage = new ProfileStorage(path);
   await profileStorage.initialize();
 
   let filter = {info: {fieldName: "cc-name"}, searchString: "Tim"};
   let creditCards = profileStorage.creditCards.getByFilter(filter);
   do_check_eq(creditCards.length, 1);
   do_check_credit_card_matches(creditCards[0], TEST_CREDIT_CARD_2);
 
-  // TODO: Uncomment this after decryption lands (bug 1337314).
+  // TODO: Uncomment this after decryption lands (bug 1389413).
   // filter = {info: {fieldName: "cc-number"}, searchString: "11"};
   // creditCards = profileStorage.creditCards.getByFilter(filter);
   // do_check_eq(creditCards.length, 1);
   // do_check_credit_card_matches(creditCards[0], TEST_CREDIT_CARD_2);
 });
 
 add_task(async function test_add() {
   let path = getTempFile(TEST_STORE_FILE_NAME).path;
@@ -186,17 +194,19 @@ add_task(async function test_add() {
 
   do_check_neq(creditCards[0].guid, undefined);
   do_check_eq(creditCards[0].version, 1);
   do_check_neq(creditCards[0].timeCreated, undefined);
   do_check_eq(creditCards[0].timeLastModified, creditCards[0].timeCreated);
   do_check_eq(creditCards[0].timeLastUsed, 0);
   do_check_eq(creditCards[0].timesUsed, 0);
 
-  Assert.throws(() => profileStorage.creditCards.add(TEST_CREDIT_CARD_WITH_INVALID_FIELD),
+  let encryptedCC_invalid = Object.assign({}, TEST_CREDIT_CARD_WITH_INVALID_FIELD);
+  await profileStorage.creditCards.normalizeCCNumberFields(encryptedCC_invalid);
+  Assert.throws(() => profileStorage.creditCards.add(encryptedCC_invalid),
     /"invalidField" is not a valid field\./);
 });
 
 add_task(async function test_update() {
   let path = getTempFile(TEST_STORE_FILE_NAME).path;
   await prepareTestCreditCards(path);
 
   let profileStorage = new ProfileStorage(path);
@@ -205,17 +215,17 @@ add_task(async function test_update() {
   let creditCards = profileStorage.creditCards.getAll();
   let guid = creditCards[1].guid;
   let timeLastModified = creditCards[1].timeLastModified;
 
   let onChanged = TestUtils.topicObserved("formautofill-storage-changed",
                                           (subject, data) => data == "update");
 
   do_check_neq(creditCards[1]["cc-name"], undefined);
-
+  await profileStorage.creditCards.normalizeCCNumberFields(TEST_CREDIT_CARD_3);
   profileStorage.creditCards.update(guid, TEST_CREDIT_CARD_3);
   await onChanged;
   await profileStorage._saveImmediately();
 
   profileStorage = new ProfileStorage(path);
   await profileStorage.initialize();
 
   let creditCard = profileStorage.creditCards.get(guid);
@@ -224,47 +234,61 @@ add_task(async function test_update() {
   do_check_neq(creditCard.timeLastModified, timeLastModified);
   do_check_credit_card_matches(creditCard, TEST_CREDIT_CARD_3);
 
   Assert.throws(
     () => profileStorage.creditCards.update("INVALID_GUID", TEST_CREDIT_CARD_3),
     /No matching record\./
   );
 
+  let encryptedCC_invalid = Object.assign({}, TEST_CREDIT_CARD_WITH_INVALID_FIELD);
+  await profileStorage.creditCards.normalizeCCNumberFields(encryptedCC_invalid);
   Assert.throws(
-    () => profileStorage.creditCards.update(guid, TEST_CREDIT_CARD_WITH_INVALID_FIELD),
+    () => profileStorage.creditCards.update(guid, encryptedCC_invalid),
     /"invalidField" is not a valid field\./
   );
 });
 
 add_task(async function test_validate() {
   let path = getTempFile(TEST_STORE_FILE_NAME).path;
 
   let profileStorage = new ProfileStorage(path);
   await profileStorage.initialize();
 
+  await profileStorage.creditCards.normalizeCCNumberFields(TEST_CREDIT_CARD_WITH_INVALID_EXPIRY_DATE);
   profileStorage.creditCards.add(TEST_CREDIT_CARD_WITH_INVALID_EXPIRY_DATE);
+  await profileStorage.creditCards.normalizeCCNumberFields(TEST_CREDIT_CARD_WITH_2_DIGITS_YEAR);
   profileStorage.creditCards.add(TEST_CREDIT_CARD_WITH_2_DIGITS_YEAR);
+  await profileStorage.creditCards.normalizeCCNumberFields(TEST_CREDIT_CARD_WITH_SPACES_BETWEEN_DIGITS);
   profileStorage.creditCards.add(TEST_CREDIT_CARD_WITH_SPACES_BETWEEN_DIGITS);
 
   let creditCards = profileStorage.creditCards.getAll();
 
   do_check_eq(creditCards[0]["cc-exp-month"], undefined);
   do_check_eq(creditCards[0]["cc-exp-year"], undefined);
 
   do_check_eq(creditCards[1]["cc-exp-month"], TEST_CREDIT_CARD_WITH_2_DIGITS_YEAR["cc-exp-month"]);
   do_check_eq(creditCards[1]["cc-exp-year"],
     parseInt(TEST_CREDIT_CARD_WITH_2_DIGITS_YEAR["cc-exp-year"], 10) + 2000);
 
   do_check_eq(creditCards[2]["cc-number"].length, 16);
-  // TODO: Check the decrypted numbers should not contain spaces after
-  //       decryption lands (bug 1337314).
 
-  Assert.throws(() => profileStorage.creditCards.add(TEST_CREDIT_CARD_WITH_INVALID_NUMBERS),
-    /Credit card number contains invalid characters\./);
+  try {
+    await profileStorage.creditCards.normalizeCCNumberFields(TEST_CREDIT_CARD_WITH_INVALID_NUMBERS);
+    throw new Error("Not receiving invalid characters error");
+  } catch (e) {
+    Assert.equal(e.message, "Credit card number contains invalid characters.");
+  }
+
+  try {
+    await profileStorage.creditCards.normalizeCCNumberFields(TEST_CREDIT_CARD_WITH_SHORT_NUMBERS);
+    throw new Error("Not receiving invalid characters error");
+  } catch (e) {
+    Assert.equal(e.message, "Invalid credit card number because length is under 12 digits.");
+  }
 });
 
 add_task(async function test_notifyUsed() {
   let path = getTempFile(TEST_STORE_FILE_NAME).path;
   await prepareTestCreditCards(path);
 
   let profileStorage = new ProfileStorage(path);
   await profileStorage.initialize();
--- a/browser/extensions/formautofill/test/unit/test_storage_tombstones.js
+++ b/browser/extensions/formautofill/test/unit/test_storage_tombstones.js
@@ -35,20 +35,24 @@ let do_check_tombstone_record = (profile
                    ["guid", "timeLastModified", "deleted"].sort());
 };
 
 // Like add_task, but actually adds 2 - one for addresses and one for cards.
 function add_storage_task(test_function) {
   add_task(async function() {
     let path = getTempFile(TEST_STORE_FILE_NAME).path;
     let profileStorage = new ProfileStorage(path);
+    let testCC1 = Object.assign({}, TEST_CC_1);
     await profileStorage.initialize();
 
     for (let [storage, record] of [[profileStorage.addresses, TEST_ADDRESS_1],
-                                   [profileStorage.creditCards, TEST_CC_1]]) {
+                                   [profileStorage.creditCards, testCC1]]) {
+      if (storage.normalizeCCNumberFields) {
+        await storage.normalizeCCNumberFields(record);
+      }
       await test_function(storage, record);
     }
   });
 }
 
 add_storage_task(async function test_simple_tombstone(storage, record) {
   do_print("check simple tombstone semantics");
 
--- a/browser/extensions/formautofill/test/unit/test_transformFields.js
+++ b/browser/extensions/formautofill/test/unit/test_transformFields.js
@@ -483,68 +483,82 @@ const ADDRESS_NORMALIZE_TESTCASES = [
   },
 ];
 
 const CREDIT_CARD_COMPUTE_TESTCASES = [
   // Empty
   {
     description: "Empty credit card",
     creditCard: {
+      "cc-number": "1234123412341234", // cc-number won't be verified
     },
     expectedResult: {
     },
   },
 
   // Name
   {
     description: "Has \"cc-name\"",
     creditCard: {
       "cc-name": "Timothy John Berners-Lee",
+      "cc-number": "1234123412341234", // cc-number won't be verified
     },
     expectedResult: {
       "cc-name": "Timothy John Berners-Lee",
       "cc-given-name": "Timothy",
       "cc-additional-name": "John",
       "cc-family-name": "Berners-Lee",
     },
   },
 ];
 
 const CREDIT_CARD_NORMALIZE_TESTCASES = [
   // Empty
   {
-    description: "Empty credit card",
+    description: "No normalizable field",
     creditCard: {
+      "cc-number": "1234123412341234", // cc-number won't be verified
     },
     expectedResult: {
     },
   },
 
   // Name
   {
     description: "Has both \"cc-name\" and the split name fields",
     creditCard: {
       "cc-name": "Timothy John Berners-Lee",
       "cc-given-name": "John",
       "cc-family-name": "Doe",
+      "cc-number": "1234123412341234", // cc-number won't be verified
     },
     expectedResult: {
       "cc-name": "Timothy John Berners-Lee",
     },
   },
   {
     description: "Has only the split name fields",
     creditCard: {
       "cc-given-name": "John",
       "cc-family-name": "Doe",
+      "cc-number": "1234123412341234", // cc-number won't be verified
     },
     expectedResult: {
       "cc-name": "John Doe",
     },
   },
+  {
+    description: "Number should be encrypted and masked",
+    creditCard: {
+      "cc-number": "1234123412341234",
+    },
+    expectedResult: {
+      "cc-number": "************1234",
+    },
+  },
 ];
 
 let do_check_record_matches = (expectedRecord, record) => {
   for (let key in expectedRecord) {
     do_check_eq(expectedRecord[key], record[key]);
   }
 };
 
@@ -589,17 +603,21 @@ add_task(async function test_normalizeAd
 });
 
 add_task(async function test_computeCreditCardFields() {
   let path = getTempFile(TEST_STORE_FILE_NAME).path;
 
   let profileStorage = new ProfileStorage(path);
   await profileStorage.initialize();
 
-  CREDIT_CARD_COMPUTE_TESTCASES.forEach(testcase => profileStorage.creditCards.add(testcase.creditCard));
+  for (let testcase of CREDIT_CARD_COMPUTE_TESTCASES) {
+    let encryptedCC = Object.assign({}, testcase.creditCard);
+    await profileStorage.creditCards.normalizeCCNumberFields(encryptedCC);
+    profileStorage.creditCards.add(encryptedCC);
+  }
   await profileStorage._saveImmediately();
 
   profileStorage = new ProfileStorage(path);
   await profileStorage.initialize();
 
   let creditCards = profileStorage.creditCards.getAll();
 
   for (let i in creditCards) {
@@ -609,17 +627,21 @@ add_task(async function test_computeCred
 });
 
 add_task(async function test_normalizeCreditCardFields() {
   let path = getTempFile(TEST_STORE_FILE_NAME).path;
 
   let profileStorage = new ProfileStorage(path);
   await profileStorage.initialize();
 
-  CREDIT_CARD_NORMALIZE_TESTCASES.forEach(testcase => profileStorage.creditCards.add(testcase.creditCard));
+  for (let testcase of CREDIT_CARD_NORMALIZE_TESTCASES) {
+    let encryptedCC = Object.assign({}, testcase.creditCard);
+    await profileStorage.creditCards.normalizeCCNumberFields(encryptedCC);
+    profileStorage.creditCards.add(encryptedCC);
+  }
   await profileStorage._saveImmediately();
 
   profileStorage = new ProfileStorage(path);
   await profileStorage.initialize();
 
   let creditCards = profileStorage.creditCards.getAll();
 
   for (let i in creditCards) {