TOTP: OTP settings not updated if in invalid format [DATA LOSS]

[Nothing4You]

Expected Behavior

TOTP secrets are persistently stored.

Current Behavior / Steps to Reproduce

1. Start KeePassXC
2. Unlock DB
3. Select record
4. Set up TOTP
5. Enter secret
6. Quit KeePassXC (only tested with autosave on every change enabled)
7. Repeat steps 1-4
8. TOTP secret is gone

Context

This used to work just fine in the past, I don't know which version broke it.
I'm experiencing this both on windows and macos.
fwiw this totp secret has been in my db for a long time before it disappeared, surviving many saves and restarts.

Debug Info

KeePassXC - Version 2.4.1
Revision: 7bafe65

Qt 5.12.2
Debugging mode is disabled.

Operating system: Windows 10 (10.0)
CPU architecture: x86_64
Kernel: winnt 10.0.17763

Enabled extensions:

• Auto-Type
• Browser Integration
• SSH Agent
• KeeShare (signed and unsigned sharing)
• YubiKey

Cryptographic libraries:
libgcrypt 1.8.4

[droidmonkey]

Can you create a new entry and restart KeePassXC to confirm it sticks? What happens if you don't quit KeePassXC but instead just setup totp again? I am not experiencing this on either OS. Where are you saving your database?

[Nothing4You]

After setting it up it'll work until restart, setup will still show the secret, show will show the correct code. DB is stored on local ssd.
I'll test with a new entry later.

[droidmonkey]

It is likely that the database file is not being saved or is not saving where you think it is.

[Nothing4You]

Since I'm using the option to remember the last used DB I highly doubt it's not where I think it is.
Using a new entry works fine.

I have just figured out the following:
On the new entry it'll add TOTP Seed and TOTP Settings attributes.
On my old entry I only have a field named otp containing 30;6.
Even after setting up a seed again I don't get the correct attributes including the secret.

[droidmonkey]

Ahhh ok. So those are from another plugin in the past. Just delete the old settings they are corrupt. Otp should look like a url not a 30;6.

[Nothing4You]

I just tested and it worked fine after deleting this record.
This should surely be fixed though, as it currently may result in loss of secrets.

[droidmonkey]

Unfortunately the fix will likely come at the expense of interoperability. The fact we support the 4 different totp "standards" developed for keepass is crazy as is.

[Nothing4You]

At the very least KeePassXC should detect and warn when it can't ensure the secret is correctly saved.
There could also be a popup asking whether it should be overwritten using the KeePassXC format.

[childnode]

At the very least KeePassXC should detect and warn when it can't ensure the secret is correctly saved.

I agree, at least because this has changed somewhere along 2.4.0 (probably #1167 or #2284)

Additionally the "otp" Attribute is parsed differently to other clients, see
https://github.com/google/google-authenticator/wiki/Key-Uri-Format
typically the defaults are &digits=6&period=30 but KeePassXC uses &digits=1&period=1

This is naggy even for users switching from or with keeweb!
Because their totp implementation uses the otp field with the URL format too by default which is saved as otpauth://totp/default?secret=<secret> without any additional parameters.

last but not least: if you copy the value from the otp field but do not delelete it and use the new TOTP setup dialog for custom 30;6 setup, neither TOTP Seed nor TOTP Settings are created, because some check decides to "not save anything because there is already an OTP saved"...

in the end: the setup dialog should give a damn good hint that entering a key-URI in the Key / secret field is bad too the other way! Or just should parse it ..

[droidmonkey]

I am totally in favor of eliminating the stupid 30;6 and totp seed fields... Otp string is the preferred route. How many people will lose interoperability though.