Feature #15663

Improve error messages when unlocking a VeraCrypt volume

Added by sajolida 2018-06-18 15:01:14 . Updated 2018-08-12 21:31:13 .

Status:
Resolved
Priority:
Normal
Assignee:
Category:
Target version:
Start date:
2018-06-18
Due date:
% Done:

30%

Feature Branch:
Type of work:
Code
Blueprint:

Starter:
Affected tool:
Deliverable for:
299

Description

During the user testing of the VeraCrypt beta, 4 participants were faced with an error message from either:

  • A mistake while typing the password (2)
  • Not specifying a keyfile (2)

None of them read the error message. Some guessed it’s meaning correctly. Some didn’t even noticed it related it to VeraCrypt.

  • A better formulated error message would definitely help with that.
  • Providing feedback while the unlocking is happening would also help.

Merge requests:
https://gitlab.gnome.org/GNOME/gtk/merge_requests/266 (merged)
https://gitlab.gnome.org/GNOME/gtk/merge_requests/267 (merged)


Files


Subtasks


Related issues

Related to Tails - Feature #15664: Provide feedback during unlocking in GtkPlacesSidebar Resolved 2018-06-18

History

#1 Updated by sajolida 2018-06-18 15:01:59

  • Assignee set to segfault
  • QA Check set to Info Needed

I can design the error messages but first I’d like to know the technical contrains.

#2 Updated by sajolida 2018-06-18 15:06:26

  • related to Feature #15664: Provide feedback during unlocking in GtkPlacesSidebar added

#3 Updated by sajolida 2018-06-18 17:11:37

Adding screenshots!

#4 Updated by segfault 2018-06-19 14:04:46

  • Subject changed from Improve error messages when unlocking a VeraCrypt volume fails to Improve error messages when unlocking a VeraCrypt volume in Disks fails
  • Assignee changed from segfault to sajolida

sajolida wrote:
> Adding screenshots!

Ok, now I get what this is about :) (during the meeting I thought you referred to the error message in the GNOME Shell dialog, which I find easier to understand)

First of all, this error message is a bit different in the Disks/udisks versions we have in Tails and more recent versions.

In Tails:

Error unlocking /dev/loop0: Command line `cryptsetup tcryptOpen "/dev/
loop0" "tcrypt-1793" --readonly --veracrypt  'exited with non-zero exit
status 2: No device header detected with this passphrase.
(udisks-error-quark, 0)

In Fedora 27:

Error unlocking /dev/loop0: Failed to load device's parameters:
Operation not permitted (udisks-error-quark, 0)

In both cases, the first part of the error message - “Error unlocking /dev/loop0” - comes from udisks (udiskslinuxencrypted.c#L505.

I think this could be improved by replacing /dev/loop0 with the name of the file that is attached to the loop device (by using udisks_block_get_preferred_device (block)). But then we should do the same in all other error messages in the handle_unlock() function, which are quite a few.

The next part of the error message differs. The old udisks version we have in Tails doesn’t use libblockdev, but calls cryptsetup directly, so this part includes the full command used to call cryptsetup and the exit status: Command line `cryptsetup tcryptOpen "/dev/ loop0" "tcrypt-1793" --readonly --veracrypt 'exited with non-zero exit status 2: No device header detected with this passphrase..

As a developer I find this error message very useful, and I also find it appropriate when using udisks via the command line, for example via udisksctl. For graphical applications like Disks, I think it would be better to make the important part - No device header detected with this passphrase - more prominent, by putting it in the beginning or making it bold or something. But I don’t see a good solution to solve this for Disks without making the error message less useful for debugging purposes. The way I understand it, Disks generally does not parse strings returned by udisks, but displays them to the user directly. Also, I think we should not spend too much time on this, because we won’t have this part of the error message anymore when Tails is based on Buster.

Recent udisks versions use libblockdev, so this part of the error message comes from libblockdev (crypto.c#1900: Failed to load device's parameters: Operation not permitted. The “Operation not permitted” part is the localized string of the error return code returned by libcryptsetup, i.e. EPERM.

I find this part especially unhelpful. It does not indicate the real problem (that an incorrect incorrect passphrase / keyfile was entered), but instead indicates a permission problem, which is pretty confusing. The problem is that this comes from the libcryptsetup function crypt_load, which does a lot of things, and a lot of things can go wrong, and it does not return error messages but only an error status code to indicate what went wrong. We could go through the list of error codes and see if there is something that fits better, and try to convince the cryptsetup developers to change it.

Regarding the last part - (udisks-error-quark) - I think it only indicates that this an error from udisks. I don’t know why this is included in the message.

So, what do you think we should do?

#5 Updated by sajolida 2018-06-22 15:54:55

  • Assignee changed from sajolida to segfault

I bet that most people won’t read the body of the error message if the title doesn’t make sense to them. The body of the error message can of course include information that’s useful for debugging by developers or help desk; even though, as you mentioned already the first part of the body should be more understandable. The debugging detail can come last.

That would be:

  1. Title that makes sense at first sight
  2. Body that starts with the most understandable details
  3. Debugging information

Let’s focus first on the error message outside of Disks, as we’re trying as much as possible to keep people out of Disks. Interestingly, it’s also the one that has the worse title:

  • Unable to access “8.2 GB Encrypted”

Could we change this? Maybe write Wrong passphrase or parameters when unlocking “8.2 GB Encrypted”. Then I wouldn’t be worried so much about the cryptic body. As said in the description of this ticket, having feedback when unlocking until the message appears would also help making sense of the error message.

Replacing /dev/loop0 by the name of the file container would also be great but it’s definitely more important to have it in the title than in the body (if that’s easier).

#6 Updated by segfault 2018-06-26 15:53:57

sajolida wrote:
> I bet that most people won’t read the body of the error message if the title doesn’t make sense to them. The body of the error message can of course include information that’s useful for debugging by developers or help desk; even though, as you mentioned already the first part of the body should be more understandable. The debugging detail can come last.
>
> That would be:
>
> # Title that makes sense at first sight
> # Body that starts with the most understandable details
> # Debugging information

I agree.

> Let’s focus first on the error message outside of Disks, as we’re trying as much as possible to keep people out of Disks. Interestingly, it’s also the one that has the worse title:
>
> * Unable to access “8.2 GB Encrypted”
>
> Could we change this? Maybe write Wrong passphrase or parameters when unlocking “8.2 GB Encrypted”. Then I wouldn’t be worried so much about the cryptic body. As said in the description of this ticket, having feedback when unlocking until the message appears would also help making sense of the error message.

Well, it is easy to change the title, but the problem is that we don’t know why exactly the operation failed, so we should not say that it’s because of a wrong password. Like I said above:

> The problem is that this [the string “Operation not permitted”] comes from the libcryptsetup function crypt_load, which does a lot of things, and a lot of things can go wrong, and it does not return error messages but only an error status code to indicate what went wrong. We could go through the list of error codes and see if there is something that fits better, and try to convince the cryptsetup developers to change it.

> Replacing /dev/loop0 by the name of the file container would also be great but it’s definitely more important to have it in the title than in the body (if that’s easier).

In comparison, replacing /dev/loop0 with the name of the file container is easy.

#7 Updated by segfault 2018-06-26 15:54:13

  • Assignee changed from segfault to sajolida

#8 Updated by segfault 2018-07-04 21:00:55

  • Subject changed from Improve error messages when unlocking a VeraCrypt volume in Disks fails to Improve error messages when unlocking a VeraCrypt volume

#9 Updated by sajolida 2018-07-06 17:56:00

  • Assignee changed from sajolida to segfault

> Well, it is easy to change the title, but the problem is that we don’t know why exactly the operation failed, so we should not say that it’s because of a wrong password.

Ok. If we can be sure that the operation failed because of a wrong set of parameters (and not only because of a wrong password), then I would still be in favor of saying:

Wrong passphrase or parameters when unlocking “8.2 GB Encrypted”.

or

Wrong passphrase or options when unlocking “8.2 GB Encrypted”.

(I’m using “parameters” in the doc but “options” might be a simplier word… or change to “options” in the doc)

If we cannot be sure that the operation failed because of a wrong set of parameters (eg. the volume is unreadable, corrupted, or whatever). Then what about:

Error unlocking “8.2 GB Encrypted”

Which might still be lightly better than “Unable to access”…

#10 Updated by sajolida 2018-07-06 17:57:15

I see now that you are using “options” in other places, which is probably better. I’ll change the doc accordingly. Thanks for improving my terminology :)

#11 Updated by sajolida 2018-07-09 10:15:22

  • QA Check changed from Info Needed to Dev Needed
  • Deliverable for set to 299

We agreed that changing the title was part of SponsorW_2017 but the rest would be too hard.

#12 Updated by intrigeri 2018-07-09 11:00:48

  • Type of work changed from User interface design to Code

#13 Updated by intrigeri 2018-07-09 11:01:05

> We agreed that changing the title was part of SponsorW_2017 but the rest would be too hard.

Is it something that we want improved in GNOME 3.30? If so, please set target version to 3.9.

#14 Updated by segfault 2018-07-23 22:56:29

  • Description updated
  • % Done changed from 0 to 30
  • QA Check deleted (Dev Needed)

See merge requests in the description.

#15 Updated by intrigeri 2018-07-24 00:24:28

  • Status changed from Confirmed to In Progress
  • Target version set to Tails_3.9

#16 Updated by segfault 2018-08-07 20:20:53

  • Description updated

https://gitlab.gnome.org/GNOME/gtk/merge_requests/267 was merged. I don’t know if https://gitlab.gnome.org/GNOME/gtk/merge_requests/266 will also get merged, but asked about it on the merge request.

#17 Updated by segfault 2018-08-08 09:23:14

https://gitlab.gnome.org/GNOME/gtk/merge_requests/266 was merged too!

Now I only have to build a new Debian package for this to ship in Tails 3.9.

#18 Updated by segfault 2018-08-08 09:23:44

  • Description updated

#19 Updated by intrigeri 2018-08-08 09:53:13

> https://gitlab.gnome.org/GNOME/gtk/merge_requests/266 was merged too!

woohoo! \o/

#20 Updated by intrigeri 2018-08-09 10:29:44

  • Priority changed from Normal to High

(Blocks merging feature/14481-TCRYPT-support-beta into devel.)

#21 Updated by segfault 2018-08-12 21:31:13

  • Status changed from In Progress to Resolved
  • Assignee deleted (segfault)
  • Priority changed from High to Normal

segfault wrote:
> Now I only have to build a new Debian package for this to ship in Tails 3.9.

Done on Feature #15521