Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix vault import with different RP IDs #42

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

haxtibal
Copy link

@haxtibal haxtibal commented Aug 1, 2024

When a vault contained multiple credential sources with different RP IDs, only the most recently added source could successfully be used after import.

This is because the IdentityVault Import function took the address of a string that resided in the copy created by a range loop. Start address of this copy is always the same for all iterations (iow, range overwrites the previous element copy). We therefore stored the same address &source.RelyingParty multiple times in vault.CredentialSources and eventually virtual-fido behaves as if only the last RP ID was imported.

A simple fix is to not use the range copies, but the range index.

Disclaimer: It's my first experience with golang, there may may be better ways to fix this...

When a vault contained multiple credential sources with different RP
IDs, only the most recently added source could successfully be used
after import.

This is because the IdentityVault Import function took the address of a
string that resided in the copy created by a range loop. Start address
of this copy is always the same for all iterations (iow, range
overwrites the previous element copy). We therefore stored the same
address &source.RelyingParty multiple times in vault.CredentialSources
and eventually virtual-fido behaves as if only the last RP ID was
imported.

The simple fix is to not use the range copies, but the range index.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant