Migration from CredentialV3 to CredentialV5 seems to be impossible when the Passkey was backed up in the past

[zUnixorn]

I did this

• Have a CredentialV3 Passkey that was already backed up and therefore has backup_state set to true, stored for a user as their 2FA.
• Migrate CredentialV3 to CredentialV5 with Credential::from(other: CredentialV3)
• Use start_passkey_authentication() and finish_passkey_authentication() to authenticate the user

I expected the following

When allow_backup_eligible_upgrade is allowed finish_passkey_authentication() should return an AuthenticationResult to make it possible to actually upgrade backup_eligible

What actually happened

finish_passkey_authentication() returns the CredentialMayNotBeHardwareBound error, which makes it seemingly impossible to migrate those credentials.

Version (and git commit)

webauthn-rs 0.5.2 (tag 0.5.2)

Operating System / Version

Linux 6.15.9-zen1-1-zen

Any other comments

Is this the intended behavior or am I missing something here? Or is there some other way to update the credential before calling finish_passkey_authentication()?

Since the CredentialV3 didn't save the value of backup_eligible when it was originally registered, it is set to false in the conversion to CredentialV5. When it could have been true for that specific Passkey, when it was setup up originally.

This causes backup_state to be true, because the key had backup_eligible set to true internally and it got backed up. But the backup_eligible value on the server-side doesn't match since it never existed before the migration to CredentialV5 and is just assumed to be false

[Firstyear]

Okay, I think I see where this confusion came from.

When we did that migration code, it was in 2022 when no multi-device credentials exists, so we just set that to "false". But now you are doing this migration many years later, and multi-device credentials do exist. We didn't really count on someone using that older version for so long!

So that means our assumption is wrong, and needs to be updated.

I think the correct thing is on migration:

• backup_eligible should be true
• backup_state is false

This way start/finish passkey work as they define allow_synchronised_authenticators as true, so then you should be good to go.

Remember if you do this, you need to check the authentication_result.needs_update(), and then if that's true, call Passkey.update_credential(authentication_result).

Does that work for you?

[Firstyear]

Can you test with the PR #511 ?

If there are more issues we can resolve them, and then once that's all sorted we will do a release so that you can proceed in vaultwarden.

[zUnixorn]

First of all, thanks you for the explanation and your help.

I did Implement your suggestion from #511 already outside of this crate in a branch. A user with the mentioned problem already confirmed that this fixed their problem.

Unfortunately this breaks all hardware authenticators that report having backup_eligible as false, like a my yubikey for example. For those keys the CredentialBackupElligibilityInconsistent error is thrown. So with backup_eligible defaulting to false migrated Passkeys don't work but yubikeys do, and with backup_eligible defaulting to true, migrated Paskkeys work, but yubikeys don't.

The only fix I could think of as of now, would be to update backup_eligible to the value that's reported by the authenticator after it's used for the first time after the migration, since the true value wasn't captured when the authenticator was registered. But maybe there is a better way to handle this.

[tessus]

For those keys the CredentialBackupElligibilityInconsistent error is thrown. So with backup_eligible defaulting to false migrated Passkeys don't work but yubikeys do, and with backup_eligible defaulting to true, migrated Paskkeys work, but yubikeys don't.

Maybe a dumb question, but in that case why not migrate the passkeys with backup_eligible set to true, and HW keys with backup_eligible set to false?

[Firstyear]

Because you don't know which is which.

[tessus]

Ha, ok. That sucks. I thought that info is available. Variables have types like int and float, so I thought keys (hw, soft, ...) would have something similar (or a field in a struct, but you know what I mean).

[Firstyear]

First of all, thanks you for the explanation and your help.

I did Implement your suggestion from #511 already outside of this crate in a branch. A user with the mentioned problem already confirmed that this fixed their problem.

Unfortunately this breaks all hardware authenticators that report having backup_eligible as false, like a my yubikey for example. For those keys the CredentialBackupElligibilityInconsistent error is thrown. So with backup_eligible defaulting to false migrated Passkeys don't work but yubikeys do, and with backup_eligible defaulting to true, migrated Paskkeys work, but yubikeys don't.

Can you test #513 for me? This allows the backup eligibility to change at authentication time.

The only fix I could think of as of now, would be to update backup_eligible to the value that's reported by the authenticator after it's used for the first time after the migration, since the true value wasn't captured when the authenticator was registered. But maybe there is a better way to handle this.

[Firstyear]

Ha, ok. That sucks. I thought that info is available. Variables have types like int and float, so I thought keys (hw, soft, ...) would have something similar (or a field in a struct, but you know what I mean).

It's only available if you request attestation at registration, and attestation prevents a number of classes of devices from registering. So without that, you have no info, you just know "somekind of authenticator exists" and you have some wishy-washy thoughts and prayers maybe signals about what it is, and often they are wrong.

[tessus]

I guess the industry and market once again messed up big time.

e.g. you can be happy, if a chain of HDMI devices is able to negotiate EDID/HDCP/HDMI. If I plugin in a FireTV stick into my A/V Receiver, the picture and audio drops every few seconds. If I plug it it into the TV, it works, but then I don't have audio unless I use an HDMI audio return channel, which in turns requires me to use use CEC on all HDMI devices, which then screws up a bunch of other stuff. Long story short: I understand your pain.

[zUnixorn]

Can you test #513 for me? This allows the backup eligibility to change at authentication time.

I did try it but unfortunately the crate doesn't seem to compile. I get the same error as seen here in the PR's action.