After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 774818 - Implement LUKS read-write actions NOT requiring a passphrase
Implement LUKS read-write actions NOT requiring a passphrase
Status: RESOLVED FIXED
Product: gparted
Classification: Other
Component: application
GIT HEAD
Other Linux
: Normal enhancement
: ---
Assigned To: Mike Fleetwood
gparted maintainers alias
Depends on:
Blocks:
 
 
Reported: 2016-11-21 22:07 UTC by Mike Fleetwood
Modified: 2017-02-14 18:33 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
No passphrase LUKS read-write support (draft 1) (268.19 KB, patch)
2016-11-21 22:29 UTC, Mike Fleetwood
none Details | Review
No passphrase LUKS read-write support (v1) (159.12 KB, patch)
2016-12-17 11:52 UTC, Mike Fleetwood
none Details | Review
No passphrase LUKS read-write support (v2) (161.37 KB, patch)
2017-01-11 09:41 UTC, Mike Fleetwood
none Details | Review
No passphrase LUKS read-write support (v3) (165.41 KB, patch)
2017-01-12 14:35 UTC, Mike Fleetwood
none Details | Review
Improve error message in check_repair_filesystem (v1) (1.21 KB, patch)
2017-02-07 18:47 UTC, Mike Fleetwood
none Details | Review

Description Mike Fleetwood 2016-11-21 22:07:45 UTC
This bug is to track adding support for LUKS read-write actions NOT
requiring a passphrase.

Reference:
Bug 627701 - option to encrypt new partitions (using LUKS)
Comment 1 Mike Fleetwood 2016-11-21 22:29:41 UTC
Created attachment 340477 [details] [review]
No passphrase LUKS read-write support (draft 1)

Hi Curtis,

Here is rough draft 1.

The functionality is complete against the the target of read-write
actions not requiring a passphrase.  However the code is somewhat rough
and the patches towards the end have just been thrown together.  Look
but don't review.

I would like you to give it a bit of a test.

What do you think about new menu items:
  Initialize Encryption / Clear Encryption
  Open Encryption Volume / Close Encryption Volume
They seem the right way to keep as analogus of format and mount/unmount for the
file systems, optionally within LUKS.

Also without being able to enter LUKS passphrases yet GParted can't
open or close any encryption volumes.  This leads to:
1) Move, delete can only be performed when LUKS is closed
2) Resize can only be performed when LUKS is open,
   plus requires libparted >= 3.2 and kernel >= 3.6 for online partition
   resizing because GParted can't close LUKS yet because it can't
   re-open it.
But it do resizing of file systems and LUKS given the above requirements
are met.  (Not yet enforced in the code)!

This patchset is way too big so I plan to separate out much of the
refactoring.

Thanks,
Mike
Comment 2 Curtis Gedak 2016-11-22 19:08:07 UTC
Hi Mike,

I've started to look at this patch set.  Over 6600 lines!  That's a lot of coding and refactoring.  :-)

Before even testing the patch set, the open and close encryption volume menu items make sense to me.  The initialize and clear encryption menu items are not as clear to me, at least at first glance.  I assume these are to "format the partition with encryption", and to "wipe out the partition" respectively.  I'll figure it out as I test the patch set.

Curtis
Comment 3 Mike Fleetwood 2016-11-22 20:36:43 UTC
Hi Curtis,

As this patchset is actions not requiring a passphrase (actually not
changing the busy state of the LUKS encryption mapping at all) the menu
options don't do anything, other than change their name depending on
whether the LUKS mapping is already open or closed.  They are there so
you can see what I am planning.  Will probably remove them from the
final patchset as they aren't part of the set of actions not requiring a
passphrase.

Mike
Comment 4 Curtis Gedak 2016-11-23 21:59:19 UTC
Hi Mike,

From testing I re-discovered that there are a lot of things to
consider when working with encryption.

Following are the details and results from testing patch set (d1) from
comment #1.

Test Machine Description:

    VMWare Virtual Machine with Ubuntu 15.10
    Test device /dev/sdb used an MSDOS partition table.
    Test partition /dev/sdb2 formatted with ext4.

Commands Used in Environment Setup:

    sudo cryptsetup luksFormat /dev/sdb2 -c aes -s 256 -h sha256
    sudo cryptsetup luksOpen /dev/sdb2 sdb2_crypt
    sudo mkfs.ext4 /dev/mapper/sdb2_crypt
    sudo mount /dev/mapper/sdb2_crypt /mnt
    sudo umount /mnt
    sudo cryptsetup luksClose /dev/sdb2_crypt


Testing Results Follow:

                       | Encrypted    | Encrypted    | Encrypted
		       | partition    | partition    | partition
		       | not luksOpen | luksOpen     | luksOpen
Action                 | not mounted  | not mounted  | mounted
-----------------------+--------------+--------------+----------------
New                    |  disabled    |  disabled    |  disabled
Delete                 | Works        |  disabled    |  disabled
Resize/Move            | Move only    | Resize only  | Grow only [6]
-----------------------+--------------+--------------+----------------
Copy                   |  disabled    | Works        |  disabled
Paste                  |  disabled    | FAIL [2]     |  disabled
-----------------------+--------------+--------------+----------------
Init/Clear Encryption  |  disabled    |  disabled    |  disabled
Format to >            | Works [1]    | Works [3]    |  disabled
-----------------------+--------------+--------------+----------------
Open/Close Encrypt Vol |  disabled    |  disabled    |  disabled
Mount/Unmount          |  disabled    |  disabled    | Unmounts
-----------------------+--------------+--------------+----------------
Name partition         |  disabled    |  disabled    |  disabled
Manage Flags           | Works        | Works        | Works
Check                  |  disabled    | Works [4]    |  disabled
Label File System      |  disabled    | Works [5]    |  disabled
New UUID               |  disabled    | Works        |  disabled
-----------------------+--------------+--------------+----------------
Information            | Works        | Works        | Works
-----------------------+--------------+--------------+----------------

[1] Formats partition with no encryption.
    When password handling available, should either prompt for
    password, or be disabled.

[2] Paste created preview that was 2 MiB smaller than original
    partition.  The operation fails when applied.

[3] Formats partition and maintains encryption.  This is as expected.

[4] Check also tries to maximize both encrypted area within partition
    and file system within ecnrypted partition.  This is as expected.

[5] Labelled file system, but did not open up Label column in GParted
    display beside File System column.  Instead there is an empty
    Mount Point column displayed.  The empty Mount Point column
    appears to be displayed if a luksOpen encrypted partition exists
    on the disk.
    The Label and Mount Point columns should display only if there is
    data in these columns.

[6] Grew the encrypted ext4 file system and partition.
    When password handling available, should also be able to move and
    shrink.


In terms of the menu names, it would be nice to have something shorter
than:

    Open Encryption Volume / Close Encryption Volume

Perhaps:

    Open Encrypted Volume / Close Encrypted Volume

Or:

    Open Encryption / Close Encryption

Having said that, the term "Volume" does help differentiate this
encrypted area as being different than a partition or a disk.  Maybe
the long name is best.


For the other two, specifically,

    Initialize Encryption / Clear Encryption

I wonder if we might consider something different like:

    Encrypt Volume / Destroy Encrypted Volume

Ideally we want users to be clear on the difference between formatting
and encrypting.

Curtis
Comment 5 Mike Fleetwood 2016-11-25 15:33:44 UTC
Hi Curtis,

Thanks for testing.


Comments on testing findings where needed
-----------------------------------------

[1] Formatting over the top of closed LUKS.

Yes when LUKS password handling is added and encrypted partitions can be
created and opened then this will be disallowed.  The user will then
have to use "Clear Encryption" menu item to allow the partition to be
formatted with an unencrypted file system.  Equivalent also applies to
pasting into existing partition with closed LUKS.

[2] I assume you were pasting into a new partition (from open LUKS).

[2.1] Pasted partition is 2 MiB smaller than the original.

The LUKS format reserves 2 MiB at the start of the partition for the
header and key slots.  This makes the encryption mapping block device
(and therefore the file system) 2 MiB smaller than the partition.  Copy/
paste works as the file system level (actually the block device directly
containing the file system, in this case the encryption mapping).
Therefore this is expected.

[2.2] Applying copy/paste of encrypted into a new partition fails

Bug; will fix.
(Must have always been testing with pasting into existing partitions)!

[5] Labelled encrypted file system not shown

Bug; will fix.
(I was always testing on a disk with existing plain partitions with
mount points and labels so they were always shown in the main window).

[6] Grow only of mounted encrypted ext4 file system

Remember that a mounted ext3/4 file system can only be grown, not
shrunk, hence the restriction.  Yes when opening/closing LUKS is
implemented moving of open LUKS will be implemented.


Menu item names
---------------

Using "Encrypted" only saves a single character so doen't seem worth it.
Lets go with:
    Open Encryption / Close Encryption

The initialize / clear menu item is harder.  There is no way with LUKS
to encrypt a partition keeping the existing data without taking a full
copy of the data and restoring it.  Therefore the single "initialize"
word needs to convey that any existing content is lost.  I don't think
"Encrypt Volume" says that.  I prefer something like:
    Format Encryption
    Create Encryption
    Initialize Encryption
I really like "Destroy".  It really says everything will be lost.
(However it will use erase_filesystem_signatures() to implement so there
will be a sub-detail saying "clear old file system signatures in
/dev/PTN").  So for now I'll use:
    Initialize Encryption / Destroy Encryption

Anyway as mentioned above these menu items won't be in this patchset.
They will appear later when entering of passphrases are supported.


What next
---------

I am going to create a separate refactoring patchset and shrink this one
down before submitting any more code.


Mike
Comment 6 Curtis Gedak 2016-11-25 16:15:02 UTC
Hi Mike,

Great progress on this enhancement.  It is far from an easy task with the current structure of the program.

Further comments to your comments follow.

> [2] I assume you were pasting into a new partition (from open
>     LUKS).

Yes, that is exactly what I did.  I did not test pasting into an existing encrypted partition.  That would be a good test to try.


> [2.1] Pasted partition is 2 MiB smaller than the original.

> The LUKS format reserves 2 MiB at the start of the partition
> for the header and key slots.  This makes the encryption
> mapping block device (and therefore the file system) 2 MiB
> smaller than the partition.  Copy/ paste works as the file
> system level (actually the block device directly containing the
> file system, in this case the encryption mapping).  Therefore
> this is expected.

I had anticipated that the copied partition would be the same size as the original because I *assumed* that the new copied partition would be the same (for example encrypted and containing an ext4 file system).

We should be sure our users know or are warned what the copied partition will look like (e.g., encrypted or not).  Otherwise it might be a surprise to copy an encrypted partition to a new partition to discover that the encryption is missing from the new partition.  At least those are my thoughts at the moment.

I look forward to your next patch set.  No rush on my part, especially with the holiday season approaching.

Curtis
Comment 7 Mike Fleetwood 2016-11-25 18:09:24 UTC
Hi Curtis,

As you have seen copying encrypted file systems as currently implemented
is somewhat non-obvious, but for good reasons.

The choices for copying are either (or both):
1) copy the partition containing closed LUKS,
2) copy the encrypted data inside an open LUKS.

I didn't wan't to implement the first option because that also
duplicates the LUKS header containing the UUID, passphrase and master
encryption key.  From a security point of view having additional copies
of encrypted data with the same master key is an extra risk; but it all
depend on what what is going to happen with that copy.  The Cryptsetup
FAQ [1] talks about how to make a backup at the file system level and
block level, preferring file system level with separate encryption if
needed.  It strongly recommends separate encryption if the copy is
removable or going offsite [2].  Also in the case of cloning the data,
cloning the LUKS container is strongly discouraged [3].

Therefore copying was implemented at the level of encrypted file system
inside the LUKS container.  However as this patchset doesn't implement
creation of encrypted partitions it can only create an unencrypted copy
of the data.

Given these security concerns I will disallow creating new partitions
containing unencrypted copies of encrypted data until GParted can offer
the choice of creating encrypted copies, or not, in the copy dialog
where the user gets to make the choice.

The user can still copy an encrypted file system into another already
open LUKS partition to keep their privacy, or copy into a plain
partition to remove encryption.

Mike


[1] The cryptsetup FAQ, Backup and data Recovery section
https://gitlab.com/cryptsetup/cryptsetup/wikis/FrequentlyAskedQuestions

[2] 6.7 Does a backup compromise security?
"If you do network-backup or tape-backup, I strongly recommend to go the
filesystem backup path with independent encryption, as you typically
cannot reliably delete data in these scenarios, especially in a cloud
setting."

[3] 6.15 Can I clone a LUKS container?
"You can, but it breaks security, because the cloned container has the
same header and hence the same master key.  You cannot change the master
key on a LUKS container, even if you change the passphrase(s), the
master key stays the same. That means whoever has access to one of the
clones can decrypt them all, completely bypassing the passphrases.

The right way to do this is to first luksFormat the target container,
then to clone the contents of the source container, with both containers
mapped, i.e. decrypted.  You can clone the decrypted contents of a LUKS
container in binary mode, although you may run into secondary issues
with GUIDs in filesystems, partition tables, RAID-components and the
like. These are just the normal problems binary cloning causes.

Note that if you need to ship (e.g.) cloned LUKS containers with a
default passphrase, that is fine as long as each container was
individually created (and hence has its own master key). In this case,
changing the default passphrase will make it secure again."
Comment 8 Curtis Gedak 2016-11-25 18:36:17 UTC
Hi Mike,

Thanks for the background information.  I can see that you've looked deeply into the issue.

> Given these security concerns I will disallow creating new
> partitions containing unencrypted copies of encrypted data
> until GParted can offer the choice of creating encrypted
> copies, or not, in the copy dialog where the user gets to make
> the choice.

This sounds like a reasonable compromise at this time.

Curtis
Comment 9 Mike Fleetwood 2016-12-17 11:52:42 UTC
Created attachment 342116 [details] [review]
No passphrase LUKS read-write support (v1)

Hi Curtis,

Here is patchset v1 for this.  Issues identified in comment #4 and #5
above are resolved.

Also tested moving a closed LUKS partition, cancelling in the middle and
having it roll back.  Confirmed both the partition and encrypted mapping
content match before and after by comparing md5 checksums.


Couple of questions:
Q1) Is patch [P18/34] "Display LUKS copy capability (#774818)" a good
    idea or not?
Q2) In the File System Support dialog, LUKS shrink support is: Cross +
    Tick.  Says that LUKS can't be shrunk when closed but only when
    open.  This is not covered in the legend at the moment.  Consider
    displaying differently as Space + Tick or Dash + Tick.  What ever is
    decided the legend probably needs updating.


Here's suggested fragments for the NEWS:

Release Notes
-------------

  This release adds partial read-write support for LUKS encrypted file
  systems.  GParted can't create, open or close LUKS encryption volumes;  however it can copy, resize and manipulate file systems inside open
  LUKS volumes and move closed LUKS volumes.  (Resizing requires Linux
  kernel >= 3.6 and libparted >= 3.2 for online partition resizing).
  [More text mentioning other patches here.]

  REMINDER:
  You are strongly advised to backup you data before editing partitions
  as a failure can lead to data loss.  This is especially true for
  encrypted data where all of the data can become permanently
  inaccessible after a failure.  Please refer to the Cryptsetup FAQ for
  backup and recovery advice of encrypted data.

  The Cryptsetup FAQ
  https://gitlab.com/cryptsetup/cryptsetup/wikis/FrequentlyAskedQuestions

Dependencies (new/updated)
--------------------------
* cryptsetup for manipulation of LUKS encryption volumes.


Thanks,
Mike
Comment 10 Curtis Gedak 2017-01-10 20:55:02 UTC
Hi Mike,

Great work on this patch set.  It is a good improvement from the
previous patch set.  Thanks also for the suggested NEWS announcement.

In addition thanks for the link to the cryptsetup FAQ.  That helped me
understand several of the encryption issues and design choices.


So far I have tested the new patch set, but have only done a very
brief code comparison with the previous patch set.  I still need to do
a comprehensive code review.

Following are answers to questions and test results.


ANSWERS TO QUESTIONS
====================

> Q1) Is patch [P18/34] "Display LUKS copy capability (#774818)" a
>     good idea or not?

A1) I think it is a good idea to show that LUKS can be copied (even it
    if is only file system copying, not partition copying).  On the
    web site we can put a reference number beside this ability and
    then outline this difference in copying ability.


> Q2) In the File System Support dialog, LUKS shrink support is: Cross
>     + Tick.  Says that LUKS can't be shrunk when closed but only
>     when open.  This is not covered in the legend at the moment.
>     Consider displaying differently as Space + Tick or Dash + Tick.
>     What ever is decided the legend probably needs updating.

A2) Let me answer a question with another question.

    Q2a) How close are you to implementing LUKS password support?

    If soon, then we might skip over the issue of modifying the
    legend, and jump ahead to implement LUKS password support before
    the next GParted release.

    If not soon, then showing a Cross + Tick isn't a bad temporary
    solution.  It should raise questions in the users mind.  We can
    add the Cross + Tick to the legend.  On the web site we can
    outline the difference between copying a partition versus a file
    system, and the need for encryption to be open to copy the file
    system.


TEST RESULTS
============

Overall, patch set (v1) works as I would expect.  From a testing
perspective there is nothing to change other than updating the legend.

Following are the details and results from testing patch set (v1) from
comment #9.

Test Machine Description:

    VMWare Virtual Machine with Ubuntu 16.10
    Test device /dev/sdb used an MSDOS partition table.
    Test partition /dev/sdb2 encrypted and formatted with ext4.

Commands Used in Environment Setup:

    sudo cryptsetup luksFormat /dev/sdb2 -c aes -s 256 -h sha256
    sudo cryptsetup luksOpen /dev/sdb2 sdb2_crypt
    sudo mkfs.ext4 /dev/mapper/sdb2_crypt
    sudo mount /dev/mapper/sdb2_crypt /mnt
    sudo umount /mnt
    sudo cryptsetup luksClose /dev/mapper/sdb2_crypt

Testing Results Follow:

                       | Encrypted    | Encrypted    | Encrypted
		       | partition    | partition    | partition
		       | not luksOpen | luksOpen     | luksOpen
Action                 | not mounted  | not mounted  | mounted
-----------------------+--------------+--------------+----------------
New                    |  disabled    |  disabled    |  disabled
Delete                 | Works        |  disabled    |  disabled
Resize/Move            | Move & Grow  | Resize only  | Grow only [6]
-----------------------+--------------+--------------+----------------
Copy                   |  disabled    | Works        |  disabled
Paste                  |  disabled    |  Exist only  |  disabled
-----------------------+--------------+--------------+----------------
Init/Clear Encryption  |  missing OK  |  missing OK  |  missing OK
Format to >            | Works [1]    | Works [3]    |  disabled
-----------------------+--------------+--------------+----------------
Open/Close Encrypt Vol |  missing OK  |  missing OK  |  missing OK
Mount/Unmount          |  disabled    |  disabled    | Unmounts
-----------------------+--------------+--------------+----------------
Name partition         |  disabled    |  disabled    |  disabled
Manage Flags           | Works        | Works        | Works
Check                  |  disabled    | Works [4]    |  disabled
Label File System      |  disabled    | Works [5]    |  disabled
New UUID               |  disabled    | Works        |  disabled
-----------------------+--------------+--------------+----------------
Information            | Works        | Works [7]    | Works
-----------------------+--------------+--------------+----------------

[1] Formats partition with no encryption.  This is as expected.
    When password handling available, should either prompt for
    password, or be disabled.

[2] Note - kept this number reference so reference numbers remain
    same as in previous comment #4.

[3] Formats partition and maintains encryption.  This is as expected.

[4] Check also tries to maximize both encrypted area within partition
    and file system within ecnrypted partition.  This is as expected.

[5] See [2].

[6] Grew the encrypted ext4 file system and partition.
    When password handling available, should also be able to move and
    shrink.

[7] Even shows unallocated space if partition grown but not file
    system.  :-)

Curtis
Comment 11 Mike Fleetwood 2017-01-10 21:32:02 UTC
Hi Curtis,


I don't think there is a lot of value in comparing patchset v1 with the
draft 1 from earlier as a lot has changed.  Better just review v1 on its
own merits.
(Draft 1 was thrown together where as v1 is polished and ready for
code review.  Draft 1 contained some refactoring where as that is not in
v1 any more as it has already been committed under
bug 775932 - Refactor mostly applying of operations).


>> Q2) In the File System Support dialog, LUKS shrink support is: Cross
>>     + Tick.  Says that LUKS can't be shrunk when closed but only
>>     when open.  This is not covered in the legend at the moment.
>>     Consider displaying differently as Space + Tick or Dash + Tick.
>>     What ever is decided the legend probably needs updating.
>
> Q2a) How close are you to implementing LUKS password support?

Not even started looking at LUKS password support yet.  I can see it
being as big as this one so it's many months away.  Therefore I'll just
add a small patch which adds Cross + Tick to the legend.


Mike
Comment 12 Mike Fleetwood 2017-01-10 21:38:28 UTC
Also something else to think about: Does the GParted Manual need
updating for this and if so what?
Comment 13 Mike Fleetwood 2017-01-11 09:41:15 UTC
Created attachment 343289 [details] [review]
No passphrase LUKS read-write support (v2)

Hi Curtis,

Here's patchset v2.  The only difference is the addition of the patch
   Add "Available online only" to the Supported Actions legend (#774818)
towards the end of the set.

Mike
Comment 14 Curtis Gedak 2017-01-11 16:18:58 UTC
Hi Mike,

Thanks for the updated patch set.  I will review v2 with an eye towards committing it to the git repository.

> Does the GParted Manual need updating for this and if so what?

With the GParted Manual I have tried to keep the document maintenance requirements low by not including all the different idiosyncrasies involved with each file system.

That being said, I did include tips on how to shrink NTFS [1] due to the volume of questions we received in this area.

[1] http://gparted.org/display-doc.php?name=help-manual#gparted-resize-partition

We could consider adding another tip or note for resizing LUKS encrypted partitions.  Or we could make a post in the GParted forum detailing the steps.

What do you think?

Curtis
Comment 15 Mike Fleetwood 2017-01-11 18:20:46 UTC
Hi Curtis,


Scanning through the GParted Manual we could consider adding this to the
end of the existing warning in the Introduction:

  <!> Editing partitions has the potential to cause LOSS of DATA.
      ...
      Your are advised to BACKUP your DATA before using the /gparted/
      application.
[New]               This is especially true for encrypted data where all
      of the data can become permanently inaccessible after a failure.
      Please refer to the Cryptseutp FAQ for backup and recovery advice
      of encrypted data.

      The Cryptsetup FAQ
      https://gitlab.com/cryptsetup/cryptsetup/wikis/FrequentlyAskedQuestions


As for the rest of the Manual, LUKS could be considered special enough
to have a few hints for various operations such as these:

  (i) LUKS encrypted partitions can only be moved when the encryption
      mapping is closed.

  (i) LUKS encrypted partitions can only be resize when the encryption
      mapping is open.

As you say we want to keep the GParted Manual focused on using GParted
and not become a series of HOWTOs.

Even less sure about adding a new page into the Manual, titled, and with
sub-sections:

  Managing LUKS
  * List active LUKS Mappings
  * Stopping LUKS Mappings
  * Starting LUKS Mappings
  For further information please refer to The Cryptsetup FAQ

And and links from the hints to the relevant stopping and starting LUKS
sub-sections.


Mike
Comment 16 Curtis Gedak 2017-01-11 18:59:16 UTC
Hi Mike,

GPARTED MANUAL COMMENTS
=======================

Thanks for the GParted Manual suggestions.

I am in favour of the first two manual enhancement suggestions.
Specifically adding the CryptSetup stuff to the "Editing partitions
has the potential to cause LOSS of DATA" caution message.  Also adding
the hints for moving and resizing.

I am not in favour of adding a "Managing LUKS" section because I think
it sets a precedent for us to maintain more and more file system
specific details.  These file system specific details belong more with
the file system and less with GParted, at least in my opinion.


CODE REVIEW
===========

The code for patch set v2 in comment #13 looks good to me.

I will change one minor typo in the following commit message:

P 24/35 Implement resize/move operation of encrypted file systems (#774818)
  CHANGE FROM:
encrypted file system within to be resize in the right order for both
                                        ^^
  TO:
encrypted file system within to be resized in the right order for both
                                         ^


REGRESSION TESTING
==================

In effort to check if prior functionality still works, I ran a series
of tests in different VMs.  In my tests, things such as moving and
resizing ext4 and ntfs worked as expected.

However I did encounter one strange anomaly with growing and shrinking
a deactivated LVM2 PV wherein sometimes the VG was activated after
the operation.

Note that the anomaly exists with the latest patch set v2, and with
the official GParted 0.27.0 release, so the anomaly is not introduced
with this patch set.

Distros Tested:
- Fedora   24
- openSUSE 42.1

   A) Inconsistent behaviour where growing or shrinking a deactivated
      LVM2 PV will also have the side effect of activating the VG.

      TESTS

      - Grow to 520 MiB a deactivated 512 MiB LMV2 PV works as expected,
      but a following shrink LVM2 PV to original size has the side
      effect of also activating the VG.

      - Shrinking to a different size does not activate the VG.

      - Shrinking a deactivated LVM2 PV to a smaller size works as
      expected, and a following grow LVM2 PV to original size works as
      expected (no side effect of activating the VG).

      - Growing to 528 MiB a deactivated 512 MiB LVM2 PV has the side
      effect of activating the VG.

Unfortunately I have been unable to come up with 100% consistent
behaviour.  To me this implies that something other than just GParted
is activating the VG.

I tried running similar tests in kubuntu 16.04 and saw different
behaviour.  More specifically each grow or shrink of a deactivated
LVM2 PV seems to have the side effect of activating the VG.

At the moment I don't think there is much we can do about this odd
behaviour.  Further the odd behaviour is not introduced by the patch
set.


Curtis
Comment 17 Mike Fleetwood 2017-01-12 14:35:55 UTC
Created attachment 343367 [details] [review]
No passphrase LUKS read-write support (v3)

Hi Curtis,

Here's patchset v3.  The primary difference is the addition of this
patch towards the end of the set:
  Add LUKS notes to the GParted Manual (#774818)

Also moves this patch a couple of steps earlier.  No functional
difference.
  Use virtual get_filessytem_string() in remaining operation
  descriptions (#774818)

Mike
Comment 18 Mike Fleetwood 2017-01-12 15:21:01 UTC
Regression: Resizing LVM PV (sometimes) starts the VG

I've re-produced this behaviour too with GParted 0.27.0 on Fedora 24.

My initial gut reaction is that there some new udev rule which is
restarting LVM VGs when the partition is resized (recreated by
libparted).

Infact just running "partprobe" also causes an inactive VG to be
started.
Comment 19 Curtis Gedak 2017-01-12 17:27:53 UTC
Hi Mike,

Thanks for confirming the odd VG activation behaviour.  At first I
thought I was imagining things, so it's good to know I'm not losing
my sanity.  :-)


The code for patch set v3 in comment #17 looks good to me.

If you are in agreement, I will make the following three changes and
commit patch set v3.


A)  Fix minor typo in the following commit message.

P 24/35 Implement resize/move operation of encrypted file systems (#774818)
    CHANGE FROM:
encrypted file system within to be resize in the right order for both
                                        ^^
    TO:
encrypted file system within to be resized in the right order for both
                                         ^

B)  Update date for GParted Manual change.

P 32/36 Add LUKS notes to the GParted Manual
    CHANGE FROM:
  <!ENTITY appversion "0.28.0">
  <!ENTITY manrevision "1.10">
  <!ENTITY date "December 2016">
                 ^^^^^^^^^^^^^
    TO:
  <!ENTITY appversion "0.28.0">
  <!ENTITY manrevision "1.10">
  <!ENTITY date "January 2017">
                 ^^^^^^^^^^^^

C)  Update the link for The Cryptsetup FAQ.

P 32/36 Add LUKS notes to the GParted Manual
    CHANGE FROM:
<ulink type="http" url="http://gitlab.com/cryptsetup/wikis/FrequentlyAskedQuestions">The Cryptsetup FAQ</ulink>
                                                   ^^^
    TO:
<ulink type="http" url="http://gitlab.com/cryptsetup/cryptsetup/wikis/FrequentlyAskedQuestions">The Cryptsetup FAQ</ulink>
                                                    ^^^^^^^^^^^^


Curtis
Comment 20 Curtis Gedak 2017-01-12 17:41:52 UTC
Hi Mike,

'Just found one more change to the GParted Manual.

D)  Change word in GParted Manaul.

P 32/36 Add LUKS notes to the GParted Manual
    CHANGE FROM:
  for backup and recovery advise of encrypted data.
                              ^
    TO:
  for backup and recovery advice of encrypted data.
                              ^


Curtis
Comment 21 Curtis Gedak 2017-01-13 17:01:42 UTC
Hi Mike,

Are you in agreement with the above 4 mentioned changes to patch set v3?

If so then I will commit v3 to the git repository.

Curtis
Comment 22 Mike Fleetwood 2017-01-13 19:26:40 UTC
Hi Curtis,

Yes commit away with your 4 changes.

Mike
Comment 23 Curtis Gedak 2017-01-14 16:01:13 UTC
Patch set v3 in comment #17 has been committed to the git repository.

The relevant git commits can be viewed at the following links:

Display busy status of the file system within LUKS encryption (#774818)
https://git.gnome.org/browse/gparted/commit/?id=2be136b9fdd0a4882ac4438c0f4d4e2d8669297d

Provide virtual Partition::get_filesystem_partition() method (#774818)
https://git.gnome.org/browse/gparted/commit/?id=53fd80e6ca588627d07a248df51fc093b5c6400e

Remove virtual PartitionLUKS::get_filesystem_label() (#774818)
https://git.gnome.org/browse/gparted/commit/?id=aa49b763e4e65da04e9d2059aa00470095bd5af7

Provide virtual Partition::get_filesystem_string() method (#774818)
https://git.gnome.org/browse/gparted/commit/?id=bd6fc67afb772ffa11ac904e3a7130f2253daa66

Implement new UUID operation on encrypted file systems (#774818)
https://git.gnome.org/browse/gparted/commit/?id=3ba7128d55ff9ca3fb990dc73d157862c00c3e4c

Add bug checks into change UUID operation methods (#774818)
https://git.gnome.org/browse/gparted/commit/?id=8984ccaf23917756f8f689a7517e99684da4ac43

Display encrypted path in the calibration step (#774818)
https://git.gnome.org/browse/gparted/commit/?id=8e41a5f712a1620209ffa971383c10973afe5b3f

Implement label operation on encrypted file systems (#774818)
https://git.gnome.org/browse/gparted/commit/?id=ff4ac89ba21a0bcdf3eb937df5360707a83d7380

Add bug checks into label file system operation methods (#774818)
https://git.gnome.org/browse/gparted/commit/?id=49c05646019f76a015afaee32335f53cc4d58f1f

Implement check operation on encrypted file systems (#774818)
https://git.gnome.org/browse/gparted/commit/?id=df05a9785fa9754fa563d2c761891d0bad73e991

Add bug checks into check operation methods (#774818)
https://git.gnome.org/browse/gparted/commit/?id=313c5fcb1ca11b4d5ec4e6902848e6d761fec5f9

Implement mount/umount of encrypted file systems (#774818)
https://git.gnome.org/browse/gparted/commit/?id=112ddef9dfa489b032b9aa13b59cb161c7f33fa7

Extend functions generating encrypted file system string (#774818)
https://git.gnome.org/browse/gparted/commit/?id=88136c96d7dd8576963c2e62eb2e9c85f5bff026

Implement format operation on encrypted file systems (#774818)
https://git.gnome.org/browse/gparted/commit/?id=a568e5365a9872645f4dbac69214c725433d4a72

Add bug checks into format operation methods (#774818)
https://git.gnome.org/browse/gparted/commit/?id=35d57011e912d92ef564870c02b895ca9e7e556d

Preview copy operation of encrypted file systems (#774818)
https://git.gnome.org/browse/gparted/commit/?id=4c70ec3aee7f4b7787eed7ba58322ddc659b0a0f

Implement copy operation of encrypted file systems (#774814)
https://git.gnome.org/browse/gparted/commit/?id=56859e1d6def2072ad41b8faa43098b453876c70

Display LUKS copy capability (#774818)
https://git.gnome.org/browse/gparted/commit/?id=e31fa783a8d98d3e0b49b87f4bcc39d3d199183a

Add bug checks into copy operation methods (#774818)
https://git.gnome.org/browse/gparted/commit/?id=89540fedd84d91580284c166856bf8266b179053

Enable resize/move for encrypted file systems (#774818)
https://git.gnome.org/browse/gparted/commit/?id=e2c70d56390525727744c42b402c3c835b658954

Add specialist clone method PartitionLUKS::clone_as_plain() (#774818)
https://git.gnome.org/browse/gparted/commit/?id=30a0f4506c101db25223219fd85f7ef970920aed

Add Partition object resizing method Partition*::resize() (#774818)
https://git.gnome.org/browse/gparted/commit/?id=08e4ba4ecab2325be6542ff60530408bc245e493

Preview resize/move operation of encrypted file systems (#774818)
https://git.gnome.org/browse/gparted/commit/?id=ee1b2257d2b248f44c48af567d2dcd2af03da420

Implement resize/move operation of encrypted file systems (#774818)
https://git.gnome.org/browse/gparted/commit/?id=828f0d8ab37be5e37b8133dc8dece1cea274fb05

Add bug checks into resize/move operation methods (#774818)
https://git.gnome.org/browse/gparted/commit/?id=a1c140128552edc87df7fcb9b3341039a4415ba5

Implement offline grow of encryption volumes (#774818)
https://git.gnome.org/browse/gparted/commit/?id=e2aff7ba6623c9312c05a532a687c6990ea14c6a

Implement maximize encryption volume as part of check repair operation (#774818)
https://git.gnome.org/browse/gparted/commit/?id=36804b963444e487dfbe23e4b4de985f6c64a7bf

Prevent deletion of open LUKS mappings (#774818)
https://git.gnome.org/browse/gparted/commit/?id=f1e3d42b5604d93dc954f5f218ef489649f11284

Use virtual get_filessytem_string() in remaining operation descriptions (#774818)
https://git.gnome.org/browse/gparted/commit/?id=86597b86720879fec7182f0e2eb86279f8c5a157

Add "Available online only" to the Supported Actions legend (#774818)
https://git.gnome.org/browse/gparted/commit/?id=5cd1f718a139d10037f0bf85a8baff34e8a7b691

Document new requirement on the cryptsetup command (#774818)
https://git.gnome.org/browse/gparted/commit/?id=34185afbf135bf2f168296c0e000010a582e1220

Add LUKS notes to the GParted Manual (#774818)
https://git.gnome.org/browse/gparted/commit/?id=5857b46c5d9918ae811398f2a151fa1512bfdd3a

Remove unused clear_mountpoints parameter from add_mountpoint*()
https://git.gnome.org/browse/gparted/commit/?id=0f76b8f8ffd04e1e0d23b093bc9c05f45712868b

Fix for loop limit in fat16::sanitize_label()
https://git.gnome.org/browse/gparted/commit/?id=fbd39b81e372e4caef37fa594983ffb39a2b4bf1

Replace 2 Win_GParted member variables with local variables
https://git.gnome.org/browse/gparted/commit/?id=786a53b43c628ff1236d8756d4c99d172517ed6c

Create and use general find_extended_partition() function
https://git.gnome.org/browse/gparted/commit/?id=aa98107706405fe0a80d4fd1a1ddb0838518ea7f


Curtis
Comment 24 Mike Fleetwood 2017-02-07 18:47:44 UTC
Created attachment 345133 [details] [review]
Improve error message in check_repair_filesystem (v1)

Hi Curtis,

Here is the patch to correct the error message.

Thanks,
Mike
Comment 25 Curtis Gedak 2017-02-07 19:34:31 UTC
Hi Mike,

Thank you for the updated string patch.

The patch from comment #24 has been committed to the git repository.

The relevant git commit can be viewed at the following link:

https://git.gnome.org/browse/gparted/commit/?id=38857e09c72639f052be15829c1b7f949af944c7

Curtis
Comment 26 Curtis Gedak 2017-02-14 18:33:54 UTC
This enhancement was included in the GParted 0.28.0 release on February 14, 2017.