Bug 2075560 - Remove the key-based encrypt/decrypt_string functions by theidkamp · Pull Request #7576 · mozilla/application-services

theidkamp · 2026-09-02T18:41:55Z

Stacked on #7573 (base is theidkamp/pr-7573; will be rebased onto main right after #7573 merges, so both land in the same nightly)

What

Bug 2075560: Remove the key-based encrypt_string / decrypt_string namespace functions. With transparent card numbers (#7573) no consumer handles ciphertext, so nothing needs them. create_autofill_key() stays for consumers that manage a static key; for canaries use db_crypto.create_canary / check_canary (format-compatible with canaries created via the removed encrypt_string).

Breaking API changes

encrypt_string(key, ...) / decrypt_string(key, ...) removed. (Android AutofillCrypto.kt & iOS RustAutofill.swift/RustKeychain.swift; Desktop never used these)

Pull Request checklist

Breaking changes: This PR follows our breaking change policy
- This PR follows the breaking change policy:
  - This PR has no breaking API changes, or
  - There are corresponding PRs for our consumer applications that resolve the breaking changes and have been approved
Quality: This PR builds and tests run cleanly
Tests: This PR includes thorough tests or an explanation of why it does not
Changelog: This PR includes a changelog entry in CHANGELOG.md or an explanation of why it does not need one
Dependencies: This PR follows our dependency management guidelines

jo

Looked at ac27ed6#r4103960593, and have some comments which I'd like to submit while looking at the next commits.

I guess the tests testing cc_number_enc will be converted to run against secure fields in the next commit, right?

jo · 2026-09-25T10:56:48Z

+                &record.cc_number_enc,
+                self.encdec.as_ref(),
+                record.guid.as_str(),
+            )?


So if we have any undecryptable credit card, get_local_dupe would return an error - is this expected behaviour?
I know this is how it was before but since we now possibly delay key retrieval - which could fail - this might become more important.

jo · 2026-09-25T11:02:48Z

+        Ok(serde_json::from_str(&cleartext).unwrap_or(Self {
+            cc_number: cleartext,
+            cc_cvv: None,
+        }))


I'm not sure about the fallback here, logins throw if the json is not parseable. And we don't reuse the old cc_number_enc value, so there's no need for this fallback, isn't it?

jo · 2026-09-25T11:04:53Z

        Ok(InternalCreditCard {
            guid: p.id,
            cc_name: p.entry.cc_name,
            cc_number_enc,


wait, why does InternalCreditCard still use cc_number_enc?

jo · 2026-09-25T11:05:49Z

            id: self.guid,
            entry: PayloadEntry {
                cc_name: self.cc_name,
                cc_number,


Do we need cc_cvv here?

jo · 2026-09-25T11:08:21Z

+        cc_number: cleartext,
+        ..Default::default()
+    }
+    .encrypt(&static_key_encryptor(&key)?, "<no guid>")


Maybe I do not understand this, this seems wrong. Isn't this part of the old API for cc_number_enc, which this commit does not touch?

jo · 2026-09-25T11:15:50Z

+        let cleartext = serde_json::to_string(self)
+            .map_err(|e| Error::EncryptionFailed(format!("{e} (encrypting {guid})")))?;
+        let cipherbytes = encdec
+            .encrypt(cleartext.into_bytes())
+            .map_err(|e| Error::EncryptionFailed(format!("{e} (encrypting {guid})")))?;
+        String::from_utf8(cipherbytes)
+            .map_err(|e| Error::EncryptionFailed(format!("{e} (encrypting {guid}: data not utf8)")))


This masks all errors, maybe you could align with logins and propagate the underlying error?

jo · 2026-09-25T12:24:41Z

The encrypted SecureCreditCardFields blob should go into a new field secFields, analog to logins, instead of re-using the current cc_number_enc field. A migration should not change a field's type, but only add new fields, or deprecate old ones.

jo · 2026-09-25T13:40:49Z

Okay, so here's the new plan, as discussed on slack:

cc_number_enc will be made transparent, ideally directly in Bug 2075563 - Integrate EncryptorDecryptor into the autofill database #7573 as the second commit. Then the consumer will only see cc_number and the component will handle the encoding and decoding internally.
a new PR that adds cc_cvv_enc with a migration, and handles encryption in the same way as cc_number_enc.

We can close this branch.

With transparent card numbers no consumer handles ciphertext, so the namespace functions that took a key have no callers left. create_autofill_key() stays for consumers that manage a static key.

mergify · 2026-09-28T13:13:49Z

⚠️ The sha of the head commit of this PR conflicts with #7573. Mergify cannot evaluate rules on this PR. Once #7573 is merged or closed, Mergify will resume processing this PR. ⚠️

theidkamp · 2026-09-28T13:14:11Z

Per review discussion: the removal moved into #7573 as its fourth commit, so consumers need exactly one parallel patch. Closing - the CVV work continues in #7631.

theidkamp changed the title ~~FXCM-2280: Implement Migration for Encrypted Autofill Storage~~ FXCM-2279 & FXCM-2280: Versioned secure-fields blob for credit-card numbers + migration Sep 10, 2026

theidkamp mentioned this pull request Sep 10, 2026

FXCM-2279: Introduce EncryptedFields Abstraction for Autofill Storage #7575

Closed

5 tasks

theidkamp force-pushed the fxcm-2280-migration branch 3 times, most recently from 00784a9 to 3c5b714 Compare September 14, 2026 13:46

theidkamp changed the base branch from main to db-crypto September 14, 2026 13:50

theidkamp force-pushed the fxcm-2280-migration branch 10 times, most recently from 2d1e2e5 to afc4fce Compare September 18, 2026 13:08

theidkamp force-pushed the fxcm-2280-migration branch from afc4fce to 88e764d Compare September 22, 2026 13:54

theidkamp changed the title ~~FXCM-2279 & FXCM-2280: Versioned secure-fields blob for credit-card numbers + migration~~ FXCM-2279/2280/2282: Secure-fields blob for card numbers, v6 migration, store-owned string crypto Sep 22, 2026

theidkamp marked this pull request as ready for review September 22, 2026 13:59

theidkamp changed the base branch from db-crypto to main September 23, 2026 08:36

theidkamp requested review from DimiDL and jo September 24, 2026 11:26

theidkamp force-pushed the fxcm-2280-migration branch from 88e764d to 84c493a Compare September 25, 2026 08:30

jo mentioned this pull request Sep 25, 2026

Bug 2075563 - Integrate EncryptorDecryptor into the autofill database #7573

Open

5 tasks

theidkamp changed the base branch from main to fxcm-2281-integrate-encryptor September 25, 2026 10:02

theidkamp changed the base branch from fxcm-2281-integrate-encryptor to theidkamp/pr-7573 September 25, 2026 10:04

jo reviewed Sep 25, 2026

View reviewed changes

theidkamp force-pushed the fxcm-2280-migration branch from 84c493a to 659baa2 Compare September 25, 2026 11:24

theidkamp force-pushed the theidkamp/pr-7573 branch from b6d4ceb to 1405cb1 Compare September 25, 2026 11:24

theidkamp force-pushed the fxcm-2280-migration branch from 659baa2 to a41d977 Compare September 25, 2026 11:31

theidkamp force-pushed the theidkamp/pr-7573 branch from 1405cb1 to 835f79e Compare September 25, 2026 11:31

theidkamp changed the title ~~FXCM-2279/2280/2282: Secure-fields blob for card numbers, v6 migration, store-owned string crypto~~ Bug 2075567, 2075564, 2075560 - Secure-fields blob for card numbers, v6 migration, store-owned string crypto Sep 25, 2026

theidkamp force-pushed the fxcm-2280-migration branch from 6c4a511 to 67b2c07 Compare September 25, 2026 15:50

theidkamp changed the base branch from theidkamp/pr-7573 to main September 25, 2026 15:51

theidkamp changed the base branch from main to theidkamp/pr-7573 September 25, 2026 15:51

theidkamp changed the title ~~Bug 2075567, 2075564, 2075560 - Secure-fields blob for card numbers, v6 migration, store-owned string crypto~~ Bug 2075560, 2075563 - Remove the key-based string crypto; add a --profile option to autofill-utils Sep 25, 2026

theidkamp marked this pull request as draft September 25, 2026 15:55

theidkamp force-pushed the fxcm-2280-migration branch from 67b2c07 to 78b935d Compare September 28, 2026 10:43

theidkamp force-pushed the theidkamp/pr-7573 branch from 2e22c13 to 373d3d7 Compare September 28, 2026 10:43

theidkamp force-pushed the fxcm-2280-migration branch from 78b935d to f0084db Compare September 28, 2026 11:13

theidkamp force-pushed the theidkamp/pr-7573 branch from 373d3d7 to e16fb6b Compare September 28, 2026 11:13

theidkamp force-pushed the fxcm-2280-migration branch from f0084db to 5410677 Compare September 28, 2026 11:59

theidkamp force-pushed the theidkamp/pr-7573 branch from e16fb6b to cec147e Compare September 28, 2026 11:59

theidkamp force-pushed the fxcm-2280-migration branch from 5410677 to f18e557 Compare September 28, 2026 12:16

theidkamp force-pushed the theidkamp/pr-7573 branch from cec147e to e893ad6 Compare September 28, 2026 12:16

Bug 2075560 - Remove the key-based encrypt/decrypt_string functions

6156f4a

With transparent card numbers no consumer handles ciphertext, so the namespace functions that took a key have no callers left. create_autofill_key() stays for consumers that manage a static key.

theidkamp force-pushed the fxcm-2280-migration branch from f18e557 to 6156f4a Compare September 28, 2026 12:24

theidkamp changed the title ~~Bug 2075560, 2075563 - Remove the key-based string crypto; add a --profile option to autofill-utils~~ Bug 2075560 - Remove the key-based encrypt/decrypt_string functions Sep 28, 2026

theidkamp changed the base branch from theidkamp/pr-7573 to main September 28, 2026 12:25

theidkamp changed the base branch from main to theidkamp/pr-7573 September 28, 2026 12:25

theidkamp mentioned this pull request Sep 28, 2026

Bug 2076154 - Store an optional CVV encrypted alongside the card number #7631

Open

4 of 5 tasks

theidkamp merged commit 6156f4a into mozilla:theidkamp/pr-7573 Sep 28, 2026
13 checks passed

theidkamp deleted the fxcm-2280-migration branch September 28, 2026 13:14

Navigation Menu

Bug 2075560 - Remove the key-based encrypt/decrypt_string functions - #7576

Bug 2075560 - Remove the key-based encrypt/decrypt_string functions#7576
theidkamp merged 1 commit into
mozilla:theidkamp/pr-7573from
theidkamp:fxcm-2280-migration

theidkamp commented Sep 2, 2026 •

edited

Loading

Uh oh!

jo left a comment

Uh oh!

jo Sep 25, 2026

Uh oh!

jo Sep 25, 2026

Uh oh!

jo Sep 25, 2026

Uh oh!

jo Sep 25, 2026

Uh oh!

jo Sep 25, 2026

Uh oh!

jo Sep 25, 2026

Uh oh!

jo commented Sep 25, 2026

Uh oh!

jo commented Sep 25, 2026 •

edited

Loading

Uh oh!

Uh oh!

mergify Bot commented Sep 28, 2026

Uh oh!

theidkamp commented Sep 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

theidkamp commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Breaking API changes

Pull Request checklist

Uh oh!

jo left a comment

Choose a reason for hiding this comment

Uh oh!

jo Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

jo Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

jo Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

jo Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

jo Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

jo Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

jo commented Sep 25, 2026

Uh oh!

jo commented Sep 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

mergify Bot commented Sep 28, 2026

Uh oh!

theidkamp commented Sep 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

theidkamp commented Sep 2, 2026 •

edited

Loading

jo commented Sep 25, 2026 •

edited

Loading