Add support for Credential Exchange Format with passkeys - #13343
Add support for Credential Exchange Format with passkeys#13343varjolintu wants to merge 5 commits into
Conversation
varjolintu
added
feature: Passkeys
pr: new feature
There was a problem hiding this comment.
I'm excited to see more adoption of credential exchange, I've done a light read through and found a few gotcha's.
One thing I would recommend (if your CI supports rust) would be to test your implementation against https://github.com/bitwarden/credential-exchange/. We've ensured that it is as close to the spec as possible while implementing it. It's what Bitwarden and 1Password both use in production, so if this implementation successfully round-trips with that library, it should reduce the chances of interoperability issues with those providers.
| {"title", entry->title()}, | ||
| {"credentials", credentials}}; | ||
|
|
||
| entryObject["items"] = items; |
There was a problem hiding this comment.
Question: Am I correct in understanding that this will create an Account entry for each entry in a user's KeepassXC instance? The goal of this data type wasn't regarding their account for a specific website, but the account in their password manager/credential provider. I'm not really sure how well that translate's to KeepassXC's data model though.
There was a problem hiding this comment.
I was thinking about this too, and it probably needs some adjusting.
| {"username", passkeyUsername}, | ||
| {"userDisplayName", QString()}, // KeePassXC does not store this | ||
| {"userHandle", userHandle}, | ||
| {"key", privateKey}, |
There was a problem hiding this comment.
Suggestion: Given the entry attribute's name of EntryAttributes::KPEX_PASSKEY_PRIVATE_KEY_PEM We've had issues in previous interoperability tests with PEM, they're not all equal. Which is why the spec defines PKCS#8 ASN.1 DER. Sometimes the PEM format uses that, sometimes not. One thing PEM usually always includes is the guards though, and those are (unfortunately implicitly) excluded from the format.
There was a problem hiding this comment.
This is something I also put to my TODO list. For example, when doing import from Bitwarden the format differs from our entry attribute's value. I need to make sure this is compatible.
Thanks for the tip! I didn't knew this exists. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #13343 +/- ##
===========================================
- Coverage 64.45% 64.44% -0.01%
===========================================
Files 378 378
Lines 39797 39797
===========================================
- Hits 25649 25646 -3
- Misses 14148 14151 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| entry->setEmitModified(false); | ||
| entry->setUuid(QUuid::createUuid()); | ||
|
|
||
| // TODO: How to handle titles if there are multiple items? |
There was a problem hiding this comment.
In general the CXF data model is designed with the following potential mappings for different providers. Use this as you wish, it's not really prescriptive in any way.
- CXF_Account: A user's account in a credential provider, may contain one or many entries and or entry organizational constructs. A credential provider may permit Multiple user accounts to be signed in at the same time, hence a CXF output was modelled to contain multiple accounts.
- CXF_Collection: A construct used to organize entries in an account. They may be shared across accounts if that's supported by the provider, so it also contains a pointer to the owning account in case the current account is not the owner. This may be a first class construct, as in all entries are always organized into at least one of these, or it may be an additional construct where entries may be in a organizational construct or not. This is why they are only pointers in CXF and all items are flattened out under accounts. Examples of these constructs across different providers can be: Vaults, Folders, Collections, etc. I don't know if such a construct exists in KeePassXC.
- CXF_Items: A collection of credentials that are associated to the same scope or context. For a login for a website for example, it may contain a totp, password and/or a passkey. Not all credential providers support the same combinations of credentials, so one CXF_item may be translated into multiple KeePass entries if there are credentials that can't be combined into one. As an example if a CXF_Item has multiple passkeys in its credentials, as far as I understand a KeePassXC entry, you would create one entry per passkey credential in that item, and clone the item's metadata for all of those entries. As you can see I my understanding is that a cxf_item would be a Entry here.
- CXF_Credentials: This would be a dedicated credential that may be a field (or many) on an entry.
Now that I write all this out, I'm thinking there might be a need for an implementation considerations section or more details in the format overview. Would you find that sort of information helpful in the spec?
|
@varjolintu Is it possible to hook it up with the APIs https://developer.apple.com/documentation/authenticationservices/ascredentialexportmanager and https://developer.apple.com/documentation/authenticationservices/ascredentialimportmanager supported since Mac OS 26? |
Yes, but we need to have support with Windows and Linux (+ FreeBSD etc.) too. Sticking to one API is not enough :( |
|
I am not aware of support in the other OS's yet. Why not supporting Mac OS first and add the other ones once there is built in support. |
It's possible, but it would be preferred to have support for multiple OS's at once. |
|
Are there any exporters supporting macOS? For example 1Password only supports export/import with CXF on iOS |
|
Apple supports import/export through its passwords app on Mac OS. There is also support in Dashlane: https://support.dashlane.com/hc/en-us/articles/32907864739474-Export-your-Dashlane-data-using-the-Credential-Exchange-Protocol-CXP |
4 participants
Implements the Credentials Exchange Format for passkeys. This change doesn't implement anything related to the Credential Exchange Protocol, file opening, adding entries to database etc. Support for other credential types could be added later.
The specification: https://fidoalliance.org/specs/cx/cxf-v1.0-ps-errata-20260309.pdf
Two new classes are created. CredentialExchangeReader expects the raw JSON data from a file that has been already decrypted, and returns a list of entries. CredentialExchangeWriter writes raw JSON data from entries.
Limitations:
Other types we could already support with this change:
basic-auth-> Normal entrynote-> Entry with a notessh-key-> Entry with SSH keytotp-> Entry with TOTPTesting strategy
Automatic tests added. Maybe the raw JSON data could be used as files.
Type of change