Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@

### Autofill

- Credit cards can store an optional CVV, encrypted like the number and handled transparently: `UpdatableCreditCardFields.cc_cvv` (cleartext in, defaults to none) and `CreditCard.cc_cvv` (decrypted out). A v6 schema upgrade adds the column; existing rows simply have no CVV. The CVV syncs as a new optional `cc-cvv` payload field; records written by older clients round-trip it through the mirror's unknown-fields handling, so their edits do not drop it.

- Added credit card equivalents of the address bulk-import API, for applications migrating a credit card collection into this store: `add_credit_card_with_meta()`, `add_many_credit_cards_with_meta()`, `update_credit_card_with_meta()`, `add_many_credit_card_tombstones()` and `delete_all_credit_cards()`. These take the guid, timestamps and sync change counter from the caller, so a migrated record keeps the identity it already had. `cc_number_enc` is stored exactly as supplied and is not checked against the store's key, matching `add_credit_card()`. ([bug 2068982](https://bugzilla.mozilla.org/show_bug.cgi?id=2068982))
- Timestamps outside the range a JS `Date` can represent are now reported as 0 wherever they enter the store: on the metadata an application supplies to the bulk-import APIs, on every read out of the local database, and on incoming sync payloads. This matches the treatment logins received in [bug 2066257](https://bugzilla.mozilla.org/show_bug.cgi?id=2066257) and covers addresses, credit cards and passports. ([bug 2068982](https://bugzilla.mozilla.org/show_bug.cgi?id=2068982))

Expand Down
3 changes: 3 additions & 0 deletions components/autofill/sql/create_shared_schema.sql
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ CREATE TABLE IF NOT EXISTS credit_cards_data (
cc_number_enc TEXT NOT NULL CHECK(length(cc_number_enc) > 20 OR cc_number_enc == ''),
-- last 4 digits unencrypted. Check no larger than 4 to avoid the full number.
cc_number_last_4 TEXT NOT NULL CHECK(length(cc_number_last_4) <= 4),
-- Encrypted CVV, same encoding and blank-handling as cc_number_enc.
-- Blank means no CVV is stored.
cc_cvv_enc TEXT NOT NULL DEFAULT '' CHECK(length(cc_cvv_enc) > 20 OR cc_cvv_enc == ''),
cc_exp_month INTEGER,
cc_exp_year INTEGER,
cc_type TEXT NOT NULL,
Expand Down
2 changes: 2 additions & 0 deletions components/autofill/src/autofill.udl
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ namespace autofill {
dictionary UpdatableCreditCardFields {
string cc_name;
string cc_number;
string? cc_cvv = null;
i64 cc_exp_month;
i64 cc_exp_year;
string cc_type;
Expand All @@ -38,6 +39,7 @@ dictionary CreditCard {
string guid;
string cc_name;
string cc_number;
string? cc_cvv = null;
string cc_number_last_4;
i64 cc_exp_month;
i64 cc_exp_year;
Expand Down
22 changes: 21 additions & 1 deletion components/autofill/src/db/credit_cards.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/

use crate::db::models::credit_card::{encrypt_str, get_last_4};
use crate::db::models::credit_card::{encrypt_optional_str, encrypt_str, get_last_4};
use crate::db::{
models::{
credit_card::{
Expand Down Expand Up @@ -51,6 +51,7 @@ pub(crate) fn add_credit_card(
guid: Guid::random(),
cc_name: new_credit_card_fields.cc_name,
cc_number_enc: encrypt_number(encdec, &new_credit_card_fields.cc_number)?,
cc_cvv_enc: encrypt_optional_str(encdec, new_credit_card_fields.cc_cvv.as_deref())?,
cc_number_last_4: get_last_4(&new_credit_card_fields.cc_number),
cc_exp_month: new_credit_card_fields.cc_exp_month,
cc_exp_year: new_credit_card_fields.cc_exp_year,
Expand Down Expand Up @@ -174,6 +175,7 @@ fn internal_credit_card_from_meta(
guid: Guid::new(&meta.guid),
cc_name: fields.cc_name,
cc_number_enc: encrypt_number(encdec, &fields.cc_number)?,
cc_cvv_enc: encrypt_optional_str(encdec, fields.cc_cvv.as_deref())?,
cc_number_last_4: get_last_4(&fields.cc_number),
cc_exp_month: fields.cc_exp_month,
cc_exp_year: fields.cc_exp_year,
Expand Down Expand Up @@ -241,6 +243,7 @@ pub(crate) fn add_internal_credit_card(
":guid": card.guid,
":cc_name": card.cc_name,
":cc_number_enc": card.cc_number_enc,
":cc_cvv_enc": card.cc_cvv_enc,
":cc_number_last_4": card.cc_number_last_4,
":cc_exp_month": card.cc_exp_month,
":cc_exp_year": card.cc_exp_year,
Expand Down Expand Up @@ -304,11 +307,13 @@ pub fn update_credit_card(
credit_card: &UpdatableCreditCardFields,
) -> Result<()> {
let cc_number_enc = encrypt_number(encdec, &credit_card.cc_number)?;
let cc_cvv_enc = encrypt_optional_str(encdec, credit_card.cc_cvv.as_deref())?;
let tx = conn.unchecked_transaction()?;
tx.execute(
"UPDATE credit_cards_data
SET cc_name = :cc_name,
cc_number_enc = :cc_number_enc,
cc_cvv_enc = :cc_cvv_enc,
cc_number_last_4 = :cc_number_last_4,
cc_exp_month = :cc_exp_month,
cc_exp_year = :cc_exp_year,
Expand All @@ -319,6 +324,7 @@ pub fn update_credit_card(
rusqlite::named_params! {
":cc_name": credit_card.cc_name,
":cc_number_enc": cc_number_enc,
":cc_cvv_enc": cc_cvv_enc,
":cc_number_last_4": get_last_4(&credit_card.cc_number),
":cc_exp_month": credit_card.cc_exp_month,
":cc_exp_year": credit_card.cc_exp_year,
Expand All @@ -345,6 +351,7 @@ pub(crate) fn update_internal_credit_card(
"UPDATE credit_cards_data
SET cc_name = :cc_name,
cc_number_enc = :cc_number_enc,
cc_cvv_enc = :cc_cvv_enc,
cc_number_last_4 = :cc_number_last_4,
cc_exp_month = :cc_exp_month,
cc_exp_year = :cc_exp_year,
Expand All @@ -359,6 +366,7 @@ pub(crate) fn update_internal_credit_card(
rusqlite::named_params! {
":cc_name": card.cc_name,
":cc_number_enc": card.cc_number_enc,
":cc_cvv_enc": card.cc_cvv_enc,
":cc_number_last_4": card.cc_number_last_4,
":cc_exp_month": card.cc_exp_month,
":cc_exp_year": card.cc_exp_year,
Expand Down Expand Up @@ -481,6 +489,7 @@ pub(crate) mod tests {
// The `credit_cards_data` CHECK constraint requires either an empty
// string or more than 20 characters, real ciphertext being long.
cc_number: "0123456789012345678901234567890".to_string(),
cc_cvv: None,
cc_exp_month: 4,
cc_exp_year: 2030,
cc_type: "visa".to_string(),
Expand Down Expand Up @@ -875,6 +884,7 @@ pub(crate) mod tests {
UpdatableCreditCardFields {
cc_name: "jane doe".to_string(),
cc_number: "XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX".to_string(),
cc_cvv: None,
cc_exp_month: 3,
cc_exp_year: 2022,
cc_type: "visa".to_string(),
Expand Down Expand Up @@ -948,6 +958,7 @@ pub(crate) mod tests {
UpdatableCreditCardFields {
cc_name: "jane doe".to_string(),
cc_number: "YYYYYYYYYYYYYYYYYYYYYYYYYYYYY".to_string(),
cc_cvv: None,
cc_exp_month: 3,
cc_exp_year: 2022,
cc_type: "visa".to_string(),
Expand All @@ -960,6 +971,7 @@ pub(crate) mod tests {
UpdatableCreditCardFields {
cc_name: "john deer".to_string(),
cc_number: "ZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ".to_string(),
cc_cvv: None,
cc_exp_month: 10,
cc_exp_year: 2025,
cc_type: "mastercard".to_string(),
Expand All @@ -973,6 +985,7 @@ pub(crate) mod tests {
UpdatableCreditCardFields {
cc_name: "abraham lincoln".to_string(),
cc_number: "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA".to_string(),
cc_cvv: None,
cc_exp_month: 1,
cc_exp_year: 2024,
cc_type: "amex".to_string(),
Expand Down Expand Up @@ -1016,6 +1029,7 @@ pub(crate) mod tests {
UpdatableCreditCardFields {
cc_name: "john deer".to_string(),
cc_number: "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAA".to_string(),
cc_cvv: None,
cc_exp_month: 10,
cc_exp_year: 2025,
cc_type: "mastercard".to_string(),
Expand All @@ -1030,6 +1044,7 @@ pub(crate) mod tests {
&UpdatableCreditCardFields {
cc_name: expected_cc_name.clone(),
cc_number: "BBBBBBBBBBBBBBBBBBBBBBBBBBBBBB".to_string(),
cc_cvv: None,
cc_type: "mastercard".to_string(),
cc_exp_month: 10,
cc_exp_year: 2025,
Expand Down Expand Up @@ -1113,6 +1128,7 @@ pub(crate) mod tests {
UpdatableCreditCardFields {
cc_name: "john deer".to_string(),
cc_number: "1234567812345678".to_string(),
cc_cvv: None,
cc_exp_month: 10,
cc_exp_year: 2025,
cc_type: "mastercard".to_string(),
Expand All @@ -1129,6 +1145,7 @@ pub(crate) mod tests {
UpdatableCreditCardFields {
cc_name: "john doe".to_string(),
cc_number: "1234123412341234".to_string(),
cc_cvv: None,
cc_exp_month: 5,
cc_exp_year: 2024,
cc_type: "visa".to_string(),
Expand Down Expand Up @@ -1182,6 +1199,7 @@ pub(crate) mod tests {
UpdatableCreditCardFields {
cc_name: "john deer".to_string(),
cc_number: "1234567812345678".to_string(),
cc_cvv: None,
cc_exp_month: 10,
cc_exp_year: 2025,
cc_type: "mastercard".to_string(),
Expand Down Expand Up @@ -1229,6 +1247,7 @@ pub(crate) mod tests {
UpdatableCreditCardFields {
cc_name: "john deer".to_string(),
cc_number: "567812345678123456781".to_string(),
cc_cvv: None,
cc_exp_month: 10,
cc_exp_year: 2025,
cc_type: "mastercard".to_string(),
Expand Down Expand Up @@ -1328,6 +1347,7 @@ pub(crate) mod tests {
UpdatableCreditCardFields {
cc_name: "john doe".to_string(),
cc_number: "WWWWWWWWWWWWWWWWWWWWWWWWWWWWWWW".to_string(),
cc_cvv: None,
cc_exp_month: 5,
cc_exp_year: 2024,
cc_type: "visa".to_string(),
Expand Down
28 changes: 28 additions & 0 deletions components/autofill/src/db/models/credit_card.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use types::Timestamp;
pub struct UpdatableCreditCardFields {
pub cc_name: String,
pub cc_number: String,
pub cc_cvv: Option<String>,
pub cc_exp_month: i64,
pub cc_exp_year: i64,
// Credit card types are a fixed set of strings as defined in the link below
Expand Down Expand Up @@ -78,6 +79,7 @@ pub struct CreditCard {
pub guid: String,
pub cc_name: String,
pub cc_number: String,
pub cc_cvv: Option<String>,
pub cc_number_last_4: String,
pub cc_exp_month: i64,
pub cc_exp_year: i64,
Expand All @@ -103,6 +105,27 @@ pub(crate) fn decrypt_str(encdec: &dyn EncryptorDecryptor, ciphertext: &str) ->
String::from_utf8(cleartext).map_err(|e| Error::CryptoNotUtf8(format!("decrypting: {e}")))
}

pub(crate) fn encrypt_optional_str(
encdec: &dyn EncryptorDecryptor,
cleartext: Option<&str>,
) -> Result<String> {
match cleartext {
Some(value) if !value.is_empty() => encrypt_str(encdec, value),
_ => Ok(String::new()),
}
}

pub(crate) fn decrypt_optional_str(
encdec: &dyn EncryptorDecryptor,
ciphertext: &str,
) -> Result<Option<String>> {
if ciphertext.is_empty() {
Ok(None)
} else {
Ok(Some(decrypt_str(encdec, ciphertext)?))
}
}

// Wow - strings are hard! we need the last 4 chars of a string.
pub(crate) fn get_last_4(v: &str) -> String {
v.chars()
Expand All @@ -125,10 +148,12 @@ impl InternalCreditCard {
} else {
decrypt_str(encdec, &self.cc_number_enc)?
};
let cc_cvv = decrypt_optional_str(encdec, &self.cc_cvv_enc)?;
Ok(CreditCard {
guid: self.guid.to_string(),
cc_name: self.cc_name,
cc_number,
cc_cvv,
cc_number_last_4: self.cc_number_last_4,
cc_exp_month: self.cc_exp_month,
cc_exp_year: self.cc_exp_year,
Expand All @@ -153,6 +178,8 @@ pub struct InternalCreditCard {
pub guid: Guid,
pub cc_name: String,
pub cc_number_enc: String,
/// Empty when no CVV is stored, like a scrubbed number.
pub cc_cvv_enc: String,
pub cc_number_last_4: String,
pub cc_exp_month: i64,
pub cc_exp_year: i64,
Expand All @@ -168,6 +195,7 @@ impl InternalCreditCard {
guid: Guid::from_string(row.get("guid")?),
cc_name: row.get("cc_name")?,
cc_number_enc: row.get("cc_number_enc")?,
cc_cvv_enc: row.get("cc_cvv_enc")?,
cc_number_last_4: row.get("cc_number_last_4")?,
cc_exp_month: row.get("cc_exp_month")?,
cc_exp_year: row.get("cc_exp_year")?,
Expand Down
17 changes: 16 additions & 1 deletion components/autofill/src/db/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ pub const CREDIT_CARD_COMMON_COLS: &str = "
cc_name,
cc_number_enc,
cc_number_last_4,
cc_cvv_enc,
cc_exp_month,
cc_exp_year,
cc_type,
Expand All @@ -60,6 +61,7 @@ pub const CREDIT_CARD_COMMON_VALS: &str = "
:cc_name,
:cc_number_enc,
:cc_number_last_4,
:cc_cvv_enc,
:cc_exp_month,
:cc_exp_year,
:cc_type,
Expand Down Expand Up @@ -108,7 +110,7 @@ pub struct AutofillConnectionInitializer;

impl ConnectionInitializer for AutofillConnectionInitializer {
const NAME: &'static str = "autofill db";
const END_VERSION: u32 = 5;
const END_VERSION: u32 = 6;

fn prepare(&self, conn: &Connection, _db_empty: bool) -> Result<()> {
define_functions(conn)?;
Expand Down Expand Up @@ -139,6 +141,7 @@ impl ConnectionInitializer for AutofillConnectionInitializer {
2 => upgrade_from_v2(db),
3 => upgrade_from_v3(db),
4 => upgrade_from_v4(db),
5 => upgrade_from_v5(db),
_ => Err(Error::IncompatibleVersion(version)),
}
}
Expand Down Expand Up @@ -279,6 +282,16 @@ fn upgrade_from_v4(db: &Connection) -> Result<()> {
Ok(())
}

fn upgrade_from_v5(db: &Connection) -> Result<()> {
// v5 -> v6 adds the encrypted CVV column. Existing rows keep an empty
// value, meaning no CVV is stored.
db.execute_batch(
"ALTER TABLE credit_cards_data ADD COLUMN cc_cvv_enc TEXT NOT NULL DEFAULT ''
CHECK(length(cc_cvv_enc) > 20 OR cc_cvv_enc == '');",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we have this?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the same check we already have for cc_number_enc, to make sure the value is encrypted.

)?;
Ok(())
}

pub fn create_empty_sync_temp_tables(db: &Connection) -> Result<()> {
debug!("Initializing sync temp tables");
db.execute_batch(CREATE_SYNC_TEMP_TABLES_SQL)?;
Expand Down Expand Up @@ -352,6 +365,8 @@ mod tests {
assert_eq!(cc.guid, "A");
assert_eq!(cc.cc_name, "Jane Doe");
assert_eq!(cc.cc_number_enc, "012345678901234567890");
// The v6 upgrade adds the CVV column; pre-existing rows have none.
assert_eq!(cc.cc_cvv_enc, "");
assert_eq!(cc.cc_number_last_4, "1234");
assert_eq!(cc.cc_exp_month, 1);
assert_eq!(cc.cc_exp_year, 2020);
Expand Down
Loading