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

In New DB Wizard, choosing Advanced settings changes transform rounds to 1 round #2806

Closed
sts10 opened this issue Mar 20, 2019 · 5 comments
Closed
Milestone

Comments

@sts10
Copy link
Contributor

sts10 commented Mar 20, 2019

Expected Behavior

In the New Database Wizard, at the "Encryption Settings" step, if users do NOT go into "Advanced Settings", Decryption Time is set to a default of 1.0 seconds. I assume a benchmark test is performed immediately after the user clicks "Continue". That's great.

I would expect that if a curious user clicks "Advanced Settings", that that 1.0 second default would persist into the Advanced Settings menu.

Current Behavior

NOTE: I believe it only happens on a first run through the Wizard. I could reproduce when I uninstalled and re-installed KeePassXC 2.4.0 (which I did via snap).

  1. Get to Encryption Settings
  2. Click "Advanced Settings"
  3. Default number of transform rounds is now 1 transform round (not a 1-second benchmark -- just 1 round).
  4. Don't change anything, just click continue (think a curious but inexperienced user freaking out a little)
  5. Database is created with only 1 transform round of security.

Screenshot_20190319_224332

Possible Solution

  1. Perform the 1-second benchmark when user clicks "Advanced Settings".
  2. Make the result of that benchmark the default number of transform rounds in the Advanced Settings menu.

OR

Hard-code a default number of transform rounds for the Advanced Settings menu. Something conservatively low, but acceptable. My guess would be 5? 10? 15? Here I defer to experts, but 1 feels low?

Steps to Reproduce

  1. Launch the New Database Wizard for the first time (after a fresh install of 2.4.0 -- remove and reinstall if necessary)
  2. Get to Encryption Settings
  3. Click "Advanced Settings"
  4. Default number of transform rounds is now 1 round (not 1 second benchmark -- just 1 round).
  5. Don't change anything, just click continue (think a curious but inexperienced user)
  6. User gets a database with 1 transform round of security.

Context

As noted, curious-but-inexperienced user may end up with a 1-round database.

I'll separately note that this may only be an issue with Snap installations of KeePassXC (see debug info below).

Debug Info

KeePassXC - Version 2.4.0
Revision: c51752d
Distribution: Snap

Libraries:

  • Qt 5.9.5
  • libgcrypt 1.8.1

Operating system: Ubuntu Core 18
CPU architecture: x86_64
Kernel: linux 4.18.0-16-generic

Enabled extensions:

  • Auto-Type
  • Browser Integration
  • SSH Agent
  • KeeShare (signed and unsigned sharing)
  • YubiKey
@sts10 sts10 added the bug label Mar 20, 2019
@sts10 sts10 changed the title In DB Wizard, choosing Advanced settings changes transform rounds to 1 In New DB Wizard, choosing Advanced settings changes transform rounds to 1 round Mar 20, 2019
@droidmonkey droidmonkey added this to the v2.4.1 milestone Mar 20, 2019
@phoerious
Copy link
Member

phoerious commented Mar 26, 2019

The actual settings are calibrated when you click continue. If you click continue first and then go back, the settings will be there. This is a bit of a lazy approach, but it prevents unnecessary key transformations which cost time. You also don't necessarily want it to recalibrate when you click Advanced, because that kills your previous settings. I'd be fine with a higher "default" value, though.

@droidmonkey
Copy link
Member

We should tell the user that we will perform a 1-second calibration automatically.

@phoerious
Copy link
Member

phoerious commented Mar 26, 2019

I'm not so sure about that. The dialogue is confusing enough as is which is why I stripped it of everything the average Joe does not need to know.

@droidmonkey
Copy link
Member

But we are talking about the advanced dialog, right? Avg Joe already stepped into the matrix at this point.

@phoerious
Copy link
Member

phoerious commented Mar 26, 2019

But then we can only do it first time advanced settings are opened. Everything else would make them entirely useless if recalibration overwrites any settings. I think whatever solution we try to build here would be rather complicated for both us and the user. I'd simply increase the default value to something like 10 or 15 rounds.

@droidmonkey droidmonkey modified the milestones: v2.4.1, v2.4.2 Apr 12, 2019
droidmonkey added a commit that referenced this issue May 31, 2019
droidmonkey added a commit that referenced this issue May 31, 2019
droidmonkey added a commit that referenced this issue May 31, 2019
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

No branches or pull requests

3 participants