patch - fix logic flaw, make seahorse-tool's encrypt command work

Bug #1394582 reported by Vlad Orlov
32
This bug affects 4 people
Affects Status Importance Assigned to Milestone
libcryptui
Fix Released
Medium
libcryptui (Debian)
Fix Released
Unknown
libcryptui (Ubuntu)
Triaged
Medium
Unassigned
Xenial
Triaged
Medium
Unassigned

Bug Description

[Impact]

The prompt recipients dialog that's called when you run seahorse-tool with --encrypt argument is broken.

Clicking on OK button does nothing, and encryption of the chosen file does not happen.

The prompt recipients dialog has been broken in this commit:

https://github.com/GNOME/libcryptui/commit/cd74aa6bf810a5ce0935d2ec89d6db64dbbde24d#diff-f0ea8a1eef5386b0149314d2a1743e85L202

The attached debdiffs fix the logic there and makes seahorse-tool's encrypt command work again. Therefore, seahorse plugins for both Nautilus and Nemo should start working as well. Clicking on OK button works, and the file gets encrypted as it should.

So the fix should be ported to the listed stable releases. In 14.04 things haven't been broken yet.

[Test Case]

Steps to reproduce:

1. Run "seahorse-tool --encrypt some_file".
2. In the dialog that appears, choose the recipient by clicking on a checkbox.
3. Click on OK button.
4. The dialog just closes and the file doesn't get encrypted as it should.

[Regression Potential]

No regressions have been found during several months of testing. I consider the regression risk to be very low here.

Revision history for this message
Vlad Orlov (monsta) wrote :
Revision history for this message
Ubuntu Foundations Team Bug Bot (crichton) wrote :

The attachment "This patch fixes the logic in the prompt recipients dialog." seems to be a patch. If it isn't, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are a member of the ~ubuntu-reviewers, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issues please contact him.]

tags: added: patch
Vlad Orlov (monsta)
tags: added: utopic
Revision history for this message
Launchpad Janitor (janitor) wrote :

Status changed to 'Confirmed' because the bug affects multiple users.

Changed in libcryptui (Ubuntu):
status: New → Confirmed
Vlad Orlov (monsta)
tags: added: vivid
Changed in libcryptui (Debian):
status: Unknown → New
Changed in libcryptui (Ubuntu):
importance: Undecided → High
Revision history for this message
Tim Lunn (darkxst) wrote :

The fix in the attached patch is incorrect, it is clearly breaking the api:
@symmetric: Variable in which to store if symmetric encryption is requested.
          * Set to NULL to disable symmetric encryption.

possibly the logic error is actually in seahorse-nautilus, passing NULL to cryptui_prompt_recipients_with_symmetric when it appears to want symmetric encyption.

Revision history for this message
David Beswick (dlbeswick) wrote :

I don't think that's the case, Tim.

The version of seahorse-nautilus in utopic, 3.8.0, is calling "cryptui_prompt_recipients". This cryptui method consists of the following call:

 return cryptui_prompt_recipients_with_symmetric (keyset, title, signer, NULL);

So it's cryptui that is itself supplying the NULL value.

Further, the function comment you cite specifies that NULL is a valid input when symmetric encryption is not desired. The comment around the patch states "Ask for recipients if symmetric encryption was not requested or if the user didn't give a symmetric passphrase." It seems both appropriate and the intention of the code (by the comment) to pop up a key chooser dialog if the caller requests that symmetric encryption should not be used. However, if the caller ever passes a NULL then no dialog will be popped up.

It does seem like a logic error to me.

Finally, I will note that the latest version of seahorse-nautilus that I looked at in bzr does call cryptui_prompt_recipients_with_symmetric, does supply a non-null value for "symmetric", and does seem to work fine. However, it seems to me that callers to libcryptui's "cryptui_prompt_recipients" won't get the results from the function that they would expect.

Revision history for this message
Vlad Orlov (monsta) wrote :

Still not considered, despite the "High" importance?

Revision history for this message
Vlad Orlov (monsta) wrote :
Revision history for this message
Vlad Orlov (monsta) wrote :
Revision history for this message
Vlad Orlov (monsta) wrote :

[Impact]

The prompt recipients dialog that's called when you run seahorse-tool with --encrypt argument is broken.
Clicking on OK button does nothing, encryption of the chosen file does not happen.

The attached debdiffs fix the problem - clicking on OK button works, the file gets encrypted as it should.

So the fix should be ported to the listed stable releases.

[Test Case]

Steps to reproduce:

1. Run "seahorse-tool --encrypt some_file".
2. In the dialog that appears, choose the recipient by clicking on a checkbox.
3. Click on OK button.
4. The dialog just closes and the file doesn't get encrypted as it should.

[Regression Potential]

No regressions have been found during several months of testing.
I consider the regression risk to be very low here.

Vlad Orlov (monsta)
tags: added: wily
Revision history for this message
David Beswick (dlbeswick) wrote :

Hi, this issue persists in wily. I think it warrants attention since two people have gone to the effort of diagnosing the issue, creating a patch and vouching for it. If there's some other way that I can help to get this resolved, then please do let me know.

At the least, upgrading the version of seahorse-nautilus would help. I can say that building and installing git-v1:12dbaa993fa40be1e97f790e390e75f3be1533d2 fixes the issue.

Mathew Hodson (mhodson)
description: updated
Changed in libcryptui (Ubuntu):
status: Confirmed → Triaged
Revision history for this message
Mathew Hodson (mhodson) wrote :

Setting importance to Medium because this impacts a non-core application.

Changed in libcryptui (Ubuntu):
importance: High → Medium
Revision history for this message
Tim Lunn (darkxst) wrote :

I can't reproduce this Xenial and looks like Wily has the same versions, so can anyone confirm it has been fixed there? If not can probably backport the patch that fixed it.
Utopic and Vivid are both EOL now.

The logic fix itself is probably correct, but should be forwarded upstream to GNOME.

Unsubscribing ubuntu-sponsors for now

Changed in libcryptui (Ubuntu):
status: Triaged → Incomplete
Revision history for this message
epictete (p-latreyte) wrote :

The bug still exist in Xenial with Nemo File Manager pull-down menu: when choosing the Encrypt… option your're asked to choose the key to be used but then nothing happens, the file is not encrypted.

The encryption works perfectly in command line.

Revision history for this message
epictete (p-latreyte) wrote :

Exactly the same under Linux Mint 18 Cinnamon 3.0.7 ; with Nemo File Manager pull-down menu: when choosing the Encrypt… option your're asked to choose the key to be used but then nothing happens, the file is not encrypted.

The encryption works perfectly in command line.

Changed in libcryptui (Debian):
status: New → Confirmed
Revision history for this message
Dmitry Nurislamov (nimms) wrote :

I'm experiencing the issue in Linux Mint 18 too.

I installed the package `nemo-seahorse_3.0.0+sarah` which adds
Seahorse integration to Nemo File Manager. It is based on seahorse-nautilus
and uses libcryptui internally.

Signing the files works, but I can't encrypt them. After choosing recipients
and pressing "OK" the window just closes without any messages,
leaving me with nothing. The package contains the tool called `seahorse-tool`,
which the Nemo plug-in internally uses. If I try to start it manually
from the terminal, I get the same result.

Now I'll try to explain why I think the Vlad's fix is correct.

The tool uses the `cryptui_prompt_recipients()` function. As Vlad already said,
the function calls `cryptui_prompt_recipients_with_symmetric()`
with the `NULL` value for its `symmetric` parameter. The problem is that
the imperative code block doesn't work with `symmetric` set to `NULL`.

So look at the definition of the `keys` pointer, which the function returns:
    gchar **keys = NULL;

Now look at the faulty code block:
    if (symmetric != NULL && !*symmetric) {
        /* ... */
        keys = g_new0(gchar*, g_list_length (recipients) + 1);
        for (l = recipients, i = 0; l; l = g_list_next (l), i++)
            keys[i] = g_strdup (l->data);
        /* ... */
    }

When the `symmetric` is set to `NULL`, this block never runs, so the function
returns `NULL`, which the calling program gets and can't do anything.

Let's rewrite the condition to be clearer:
    if (symmetric != NULL && *symmetric == FALSE)

`symmetric == NULL` and `*symmetric == FALSE` should mean the same,
as the comment states:
    @symmetric: Variable in which to store if symmetric encryption is requested.
    Set to NULL to disable symmetric encryption.

The correct condition, which the patch uses, is:
    if (symmetric == NULL || *symmetric == FALSE)

So it *is* a logic flaw. The Vlad's patch fixes it.

Jeremy Bícha (jbicha)
tags: added: xenial yakkety
Changed in libcryptui (Ubuntu):
status: Incomplete → Triaged
Changed in libcryptui (Ubuntu Xenial):
status: New → Triaged
importance: Undecided → Medium
Changed in libcryptui:
importance: Unknown → Medium
status: Unknown → Fix Released
Changed in libcryptui (Debian):
status: Confirmed → Fix Released
Revision history for this message
Dylan Grafmyre (conversica-dylang) wrote :

Nemo users who may still be effected by this bug can replace apt nemo-seahorse with seahorse-nautillus as posted in the nemo specific bug: https://bugs.launchpad.net/linuxmint/+bug/1292302#yui_3_10_3_1_1499975648949_1541

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.